Merge pull request #6346 from Ericson2314/impure-derivations-ng

Get rid of `impureOutputHash`; fix possible bug

Author: Ericson2314

src/libstore/build/derivation-goal.cc

@@ -185,41 +185,44 @@ Goal::Co DerivationGoal::haveDerivation()
     if (!drv->type().hasKnownOutputPaths())
         experimentalFeatureSettings.require(Xp::CaDerivations);
 
-    if (drv->type().isImpure()) {
-        experimentalFeatureSettings.require(Xp::ImpureDerivations);
-
-        for (auto & [outputName, output] : drv->outputs) {
-            auto randomPath = StorePath::random(outputPathName(drv->name, outputName));
-            assert(!worker.store.isValidPath(randomPath));
-            initialOutputs.insert({
-                outputName,
-                InitialOutput {
-                    .wanted = true,
-                    .outputHash = impureOutputHash,
-                    .known = InitialOutputStatus {
-                        .path = randomPath,
-                        .status = PathStatus::Absent
-                    }
-                }
-            });
-        }
-
-        co_return gaveUpOnSubstitution();
-    }
-
     for (auto & i : drv->outputsAndOptPaths(worker.store))
         if (i.second.second)
             worker.store.addTempRoot(*i.second.second);
 
-    auto outputHashes = staticOutputHashes(worker.evalStore, *drv);
-    for (auto & [outputName, outputHash] : outputHashes)
-        initialOutputs.insert({
-            outputName,
-            InitialOutput {
+    {
+        bool impure = drv->type().isImpure();
+
+        if (impure) experimentalFeatureSettings.require(Xp::ImpureDerivations);
+
+        auto outputHashes = staticOutputHashes(worker.evalStore, *drv);
+        for (auto & [outputName, outputHash] : outputHashes) {
+            InitialOutput v{
                 .wanted = true, // Will be refined later
                 .outputHash = outputHash
+            };
+
+            /* TODO we might want to also allow randomizing the paths
+               for regular CA derivations, e.g. for sake of checking
+               determinism. */
+            if (impure) {
+                v.known = InitialOutputStatus {
+                    .path = StorePath::random(outputPathName(drv->name, outputName)),
+                    .status = PathStatus::Absent,
+                };
             }
-        });
+
+            initialOutputs.insert({
+                outputName,
+                std::move(v),
+            });
+        }
+
+        if (impure) {
+            /* We don't yet have any safe way to cache an impure derivation at
+               this step. */
+            co_return gaveUpOnSubstitution();
+        }
+    }
 
     {
         /* Check what outputs paths are not already valid. */

src/libstore/derivations.cc

@@ -843,16 +843,6 @@ DrvHash hashDerivationModulo(Store & store, const Derivation & drv, bool maskOut
         };
     }
 
-    if (type.isImpure()) {
-        std::map<std::string, Hash> outputHashes;
-        for (const auto & [outputName, _] : drv.outputs)
-            outputHashes.insert_or_assign(outputName, impureOutputHash);
-        return DrvHash {
-            .hashes = outputHashes,
-            .kind = DrvHash::Kind::Deferred,
-        };
-    }
-
     auto kind = std::visit(overloaded {
         [](const DerivationType::InputAddressed & ia) {
             /* This might be a "pesimistically" deferred output, so we don't
@@ -865,7 +855,7 @@ DrvHash hashDerivationModulo(Store & store, const Derivation & drv, bool maskOut
                 : DrvHash::Kind::Deferred;
         },
         [](const DerivationType::Impure &) -> DrvHash::Kind {
-            assert(false);
+            return DrvHash::Kind::Deferred;
         }
     }, drv.type().raw);
 

src/libstore/derivations.hh

@@ -526,6 +526,4 @@ void writeDerivation(Sink & out, const StoreDirConfig & store, const BasicDeriva
  */
 std::string hashPlaceholder(const OutputNameView outputName);
 
-extern const Hash impureOutputHash;
-
 }