Get rid of impureOutputHash

I do not believe there is any problem with computing
`hashDerivationModulo` the normal way with impure derivations.

Conversely, the way this used to work is very suspicious because two
almost-equal derivations that only differ in depending on different
impure derivations could have the same drv hash modulo. That is very
suspicious because there is no reason to think those two different
impure derivations will end up producing the same content-addressed
data!

Co-authored-by: Alain Zscheile <[email protected]>

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;
-
 }