Merge pull request #8692 from obsidiansystems/add-another-xp-check

Feature gate `DownstreamPlaceholder::unknownCaOutput`

src/libexpr/eval.cc

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

src/libexpr/eval.hh

@@ -689,12 +689,15 @@ public:
      * 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 used (see `DownstreamPlaceholder`).
+     *
+     * @param xpSettings Stop-gap to avoid globals during unit tests.
      */
     void mkOutputString(
         Value & value,
         const StorePath & drvPath,
         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);
 

src/libexpr/primops.cc

@@ -84,23 +84,22 @@ StringMap EvalState::realiseContext(const NixStringContext & context)
     for (auto & d : drvs) buildReqs.emplace_back(DerivedPath { d });
     store->buildPaths(buildReqs);
 
-    /* Get all the output paths corresponding to the placeholders we had */
     for (auto & drv : drvs) {
         auto outputs = resolveDerivedPath(*store, drv);
         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(
                     DownstreamPlaceholder::unknownCaOutput(drv.drvPath, outputName).render(),
                     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;

src/libexpr/tests/derived-path.cc

@@ -37,8 +37,15 @@ RC_GTEST_FIXTURE_PROP(
     prop_built_path_placeholder_round_trip,
     (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();
-    state.mkOutputString(*v, drvPath, outputName.name, std::nullopt);
+    state.mkOutputString(*v, drvPath, outputName.name, std::nullopt, mockXpSettings);
     auto [d, _] = state.coerceToDerivedPathUnchecked(noPos, *v, "");
     DerivedPath::Built b {
         .drvPath = drvPath,

src/libstore/derivations.cc

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

src/libstore/downstream-placeholder.cc

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

src/libstore/downstream-placeholder.hh

@@ -52,10 +52,13 @@ public:
      *
      * The derivation itself is known (we have a store path for it), but
      * the output doesn't yet have a known store path.
+     *
+     * @param xpSettings Stop-gap to avoid globals during unit tests.
      */
     static DownstreamPlaceholder unknownCaOutput(
         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.

src/libstore/tests/downstream-placeholder.cc

@@ -5,17 +5,24 @@
 namespace nix {
 
 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(
         DownstreamPlaceholder::unknownCaOutput(
             StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo.drv" },
-            "out").render(),
+            "out",
+            mockXpSettings).render(),
         "/0c6rn30q4frawknapgwq386zq358m8r6msvywcvc89n6m5p2dgbz");
 }
 
 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;
     mockXpSettings.set("experimental-features", "dynamic-derivations ca-derivations");
@@ -24,7 +31,8 @@ TEST(DownstreamPlaceholder, unknownDerivation) {
         DownstreamPlaceholder::unknownDerivation(
             DownstreamPlaceholder::unknownCaOutput(
                 StorePath { "g1w7hy3qg1w7hy3qg1w7hy3qg1w7hy3q-foo.drv.drv" },
-                "out"),
+                "out",
+                mockXpSettings),
             "out",
             mockXpSettings).render(),
         "/0gn6agqxjyyalf0dpihgyf49xq5hqxgw100f0wydnj6yqrhqsb3w");

src/nix/app.cc

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