Feature gate DownstreamPlaceholder::unknownCaOutput

This is a part of CA derivations that we forgot to put behind the
experimental feature.

This was caught by @fricklerhandwerk in
https://github.com/NixOS/nix/pull/8369#discussion_r1258133719
This commit is contained in:
John Ericson 2023-07-12 23:33:43 -04:00
parent cafb5e8a17
commit caabc4f648
9 changed files with 57 additions and 31 deletions

View file

@ -1068,14 +1068,15 @@ void EvalState::mkOutputString(
Value & value, Value & value,
const StorePath & drvPath, const StorePath & drvPath,
const std::string outputName, const std::string outputName,
std::optional<StorePath> optOutputPath) std::optional<StorePath> optOutputPath,
const ExperimentalFeatureSettings & xpSettings)
{ {
value.mkString( value.mkString(
optOutputPath optOutputPath
? store->printStorePath(*std::move(optOutputPath)) ? store->printStorePath(*std::move(optOutputPath))
/* Downstream we would substitute this for an actual path once /* Downstream we would substitute this for an actual path once
we build the floating CA derivation */ we build the floating CA derivation */
: DownstreamPlaceholder::unknownCaOutput(drvPath, outputName).render(), : DownstreamPlaceholder::unknownCaOutput(drvPath, outputName, xpSettings).render(),
NixStringContext { NixStringContext {
NixStringContextElem::Built { NixStringContextElem::Built {
.drvPath = drvPath, .drvPath = drvPath,

View file

@ -689,12 +689,15 @@ public:
* be passed if and only if output store object is input-addressed. * be passed if and only if output store object is input-addressed.
* Will be printed to form string if passed, otherwise a placeholder * Will be printed to form string if passed, otherwise a placeholder
* will be used (see `DownstreamPlaceholder`). * will be used (see `DownstreamPlaceholder`).
*
* @param xpSettings Stop-gap to avoid globals during unit tests.
*/ */
void mkOutputString( void mkOutputString(
Value & value, Value & value,
const StorePath & drvPath, const StorePath & drvPath,
const std::string outputName, const std::string outputName,
std::optional<StorePath> optOutputPath); std::optional<StorePath> optOutputPath,
const ExperimentalFeatureSettings & xpSettings = experimentalFeatureSettings);
void concatLists(Value & v, size_t nrLists, Value * * lists, const PosIdx pos, std::string_view errorCtx); void concatLists(Value & v, size_t nrLists, Value * * lists, const PosIdx pos, std::string_view errorCtx);

View file

@ -83,23 +83,22 @@ StringMap EvalState::realiseContext(const NixStringContext & context)
for (auto & d : drvs) buildReqs.emplace_back(DerivedPath { d }); for (auto & d : drvs) buildReqs.emplace_back(DerivedPath { d });
store->buildPaths(buildReqs); store->buildPaths(buildReqs);
/* Get all the output paths corresponding to the placeholders we had */
for (auto & drv : drvs) { for (auto & drv : drvs) {
auto outputs = resolveDerivedPath(*store, drv); auto outputs = resolveDerivedPath(*store, drv);
for (auto & [outputName, outputPath] : outputs) { for (auto & [outputName, outputPath] : outputs) {
/* Add the output of this derivations to the allowed
paths. */
if (allowedPaths) {
allowPath(outputPath);
}
/* Get all the output paths corresponding to the placeholders we had */
if (experimentalFeatureSettings.isEnabled(Xp::CaDerivations)) {
res.insert_or_assign( res.insert_or_assign(
DownstreamPlaceholder::unknownCaOutput(drv.drvPath, outputName).render(), DownstreamPlaceholder::unknownCaOutput(drv.drvPath, outputName).render(),
store->printStorePath(outputPath) store->printStorePath(outputPath)
); );
} }
} }
/* Add the output of this derivations to the allowed
paths. */
if (allowedPaths) {
for (auto & [_placeholder, outputPath] : res) {
allowPath(store->toRealPath(outputPath));
}
} }
return res; return res;

View file

@ -37,8 +37,15 @@ RC_GTEST_FIXTURE_PROP(
prop_built_path_placeholder_round_trip, prop_built_path_placeholder_round_trip,
(const StorePath & drvPath, const StorePathName & outputName)) (const StorePath & drvPath, const StorePathName & outputName))
{ {
/**
* We set these in tests rather than the regular globals so we don't have
* to worry about race conditions if the tests run concurrently.
*/
ExperimentalFeatureSettings mockXpSettings;
mockXpSettings.set("experimental-features", "ca-derivations");
auto * v = state.allocValue(); auto * v = state.allocValue();
state.mkOutputString(*v, drvPath, outputName.name, std::nullopt); state.mkOutputString(*v, drvPath, outputName.name, std::nullopt, mockXpSettings);
auto [d, _] = state.coerceToDerivedPathUnchecked(noPos, *v, ""); auto [d, _] = state.coerceToDerivedPathUnchecked(noPos, *v, "");
DerivedPath::Built b { DerivedPath::Built b {
.drvPath = drvPath, .drvPath = drvPath,

View file

@ -878,9 +878,11 @@ std::optional<BasicDerivation> Derivation::tryResolve(
for (auto & [inputDrv, inputOutputs] : inputDrvs) { for (auto & [inputDrv, inputOutputs] : inputDrvs) {
for (auto & outputName : inputOutputs) { for (auto & outputName : inputOutputs) {
if (auto actualPath = get(inputDrvOutputs, { inputDrv, outputName })) { if (auto actualPath = get(inputDrvOutputs, { inputDrv, outputName })) {
if (experimentalFeatureSettings.isEnabled(Xp::CaDerivations)) {
inputRewrites.emplace( inputRewrites.emplace(
DownstreamPlaceholder::unknownCaOutput(inputDrv, outputName).render(), DownstreamPlaceholder::unknownCaOutput(inputDrv, outputName).render(),
store.printStorePath(*actualPath)); store.printStorePath(*actualPath));
}
resolved.inputSrcs.insert(*actualPath); resolved.inputSrcs.insert(*actualPath);
} else { } else {
warn("output '%s' of input '%s' missing, aborting the resolving", warn("output '%s' of input '%s' missing, aborting the resolving",

View file

@ -11,8 +11,10 @@ std::string DownstreamPlaceholder::render() const
DownstreamPlaceholder DownstreamPlaceholder::unknownCaOutput( DownstreamPlaceholder DownstreamPlaceholder::unknownCaOutput(
const StorePath & drvPath, const StorePath & drvPath,
std::string_view outputName) std::string_view outputName,
const ExperimentalFeatureSettings & xpSettings)
{ {
xpSettings.require(Xp::CaDerivations);
auto drvNameWithExtension = drvPath.name(); auto drvNameWithExtension = drvPath.name();
auto drvName = drvNameWithExtension.substr(0, drvNameWithExtension.size() - 4); auto drvName = drvNameWithExtension.substr(0, drvNameWithExtension.size() - 4);
auto clearText = "nix-upstream-output:" + std::string { drvPath.hashPart() } + ":" + outputPathName(drvName, outputName); auto clearText = "nix-upstream-output:" + std::string { drvPath.hashPart() } + ":" + outputPathName(drvName, outputName);

View file

@ -52,10 +52,13 @@ public:
* *
* The derivation itself is known (we have a store path for it), but * The derivation itself is known (we have a store path for it), but
* the output doesn't yet have a known store path. * the output doesn't yet have a known store path.
*
* @param xpSettings Stop-gap to avoid globals during unit tests.
*/ */
static DownstreamPlaceholder unknownCaOutput( static DownstreamPlaceholder unknownCaOutput(
const StorePath & drvPath, const StorePath & drvPath,
std::string_view outputName); std::string_view outputName,
const ExperimentalFeatureSettings & xpSettings = experimentalFeatureSettings);
/** /**
* Create a placehold for the output of an unknown derivation. * Create a placehold for the output of an unknown derivation.

View file

@ -5,17 +5,24 @@
namespace nix { namespace nix {
TEST(DownstreamPlaceholder, unknownCaOutput) { TEST(DownstreamPlaceholder, unknownCaOutput) {
/**
* We set these in tests rather than the regular globals so we don't have
* to worry about race conditions if the tests run concurrently.
*/
ExperimentalFeatureSettings mockXpSettings;
mockXpSettings.set("experimental-features", "ca-derivations");
ASSERT_EQ( ASSERT_EQ(
DownstreamPlaceholder::unknownCaOutput( DownstreamPlaceholder::unknownCaOutput(
StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo.drv" }, StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo.drv" },
"out").render(), "out",
mockXpSettings).render(),
"/0c6rn30q4frawknapgwq386zq358m8r6msvywcvc89n6m5p2dgbz"); "/0c6rn30q4frawknapgwq386zq358m8r6msvywcvc89n6m5p2dgbz");
} }
TEST(DownstreamPlaceholder, unknownDerivation) { TEST(DownstreamPlaceholder, unknownDerivation) {
/** /**
* We set these in tests rather than the regular globals so we don't have * Same reason as above
* to worry about race conditions if the tests run concurrently.
*/ */
ExperimentalFeatureSettings mockXpSettings; ExperimentalFeatureSettings mockXpSettings;
mockXpSettings.set("experimental-features", "dynamic-derivations ca-derivations"); mockXpSettings.set("experimental-features", "dynamic-derivations ca-derivations");
@ -24,7 +31,8 @@ TEST(DownstreamPlaceholder, unknownDerivation) {
DownstreamPlaceholder::unknownDerivation( DownstreamPlaceholder::unknownDerivation(
DownstreamPlaceholder::unknownCaOutput( DownstreamPlaceholder::unknownCaOutput(
StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo.drv.drv" }, StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo.drv.drv" },
"out"), "out",
mockXpSettings),
"out", "out",
mockXpSettings).render(), mockXpSettings).render(),
"/0gn6agqxjyyalf0dpihgyf49xq5hqxgw100f0wydnj6yqrhqsb3w"); "/0gn6agqxjyyalf0dpihgyf49xq5hqxgw100f0wydnj6yqrhqsb3w");

View file

@ -22,6 +22,7 @@ StringPairs resolveRewrites(
StringPairs res; StringPairs res;
for (auto & dep : dependencies) for (auto & dep : dependencies)
if (auto drvDep = std::get_if<BuiltPathBuilt>(&dep.path)) if (auto drvDep = std::get_if<BuiltPathBuilt>(&dep.path))
if (experimentalFeatureSettings.isEnabled(Xp::CaDerivations))
for (auto & [ outputName, outputPath ] : drvDep->outputs) for (auto & [ outputName, outputPath ] : drvDep->outputs)
res.emplace( res.emplace(
DownstreamPlaceholder::unknownCaOutput(drvDep->drvPath, outputName).render(), DownstreamPlaceholder::unknownCaOutput(drvDep->drvPath, outputName).render(),