Get rid of impureOutputHash

I do not believe there is any problem with computing `hashDerivationModulo` the normal way with impure derivations.
NixOS · Mar 31, 2022 · 27b5d2a · 27b5d2a
1 parent c9a29d0
commit 27b5d2a
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 38 deletions.
diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc
@@ -209,29 +209,6 @@ void DerivationGoal::haveDerivation()
     if (!drv->type().hasKnownOutputPaths())
         settings.requireExperimentalFeature(Xp::CaDerivations);
 
-    if (!drv->type().isPure()) {
-        settings.requireExperimentalFeature(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
-                    }
-                }
-            });
-        }
-
-        gaveUpOnSubstitution();
-        return;
-    }
-
     for (auto & i : drv->outputsAndOptPaths(worker.store))
         if (i.second.second)
             worker.store.addTempRoot(*i.second.second);
@@ -246,6 +223,15 @@ void DerivationGoal::haveDerivation()
             }
         });
 
+    if (!drv->type().isPure()) {
+        settings.requireExperimentalFeature(Xp::ImpureDerivations);
+
+        /* We don't yet have any safe way to cache and impure derivation at
+           this step. */
+        gaveUpOnSubstitution();
+        return;
+    }
+
     /* Check what outputs paths are not already valid. */
     auto [allValid, validOutputs] = checkPathValidity();
 

diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc
@@ -627,16 +627,6 @@ DrvHash hashDerivationModulo(Store & store, const Derivation & drv, bool maskOut
         };
     }
 
-    if (!type.isPure()) {
-        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
@@ -649,7 +639,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());
 
@@ -889,6 +879,4 @@ std::optional<BasicDerivation> Derivation::tryResolve(
     return resolved;
 }
 
-const Hash impureOutputHash = hashString(htSHA256, "impure");
-
 }
diff --git a/src/libstore/derivations.hh b/src/libstore/derivations.hh
@@ -319,6 +319,4 @@ std::string hashPlaceholder(const std::string_view outputName);
    dependency which is a CA derivation. */
 std::string downstreamPlaceholder(const Store & store, const StorePath & drvPath, std::string_view outputName);
 
-extern const Hash impureOutputHash;
-
 }