6 files changed, 157 insertions, 84 deletions

diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc
index 3d1c4fbc1..53f212c1d 100644
--- a/src/libstore/build/derivation-goal.cc
+++ b/src/libstore/build/derivation-goal.cc
@@ -204,9 +204,34 @@ void DerivationGoal::haveDerivation()
 {
     trace("have derivation");
 
+    parsedDrv = std::make_unique<ParsedDerivation>(drvPath, *drv);
+
     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);
@@ -230,9 +255,6 @@ void DerivationGoal::haveDerivation()
         return;
     }
 
-    parsedDrv = std::make_unique<ParsedDerivation>(drvPath, *drv);
-
-
     /* We are first going to try to create the invalid output paths
        through substitutes.  If that doesn't work, we'll build
        them. */
@@ -266,6 +288,8 @@ void DerivationGoal::outputsSubstitutionTried()
 {
     trace("all outputs substituted (maybe)");
 
+    assert(drv->type().isPure());
+
     if (nrFailed > 0 && nrFailed > nrNoSubstituters + nrIncompleteClosure && !settings.tryFallback) {
         done(BuildResult::TransientFailure, {},
             Error("some substitutes for the outputs of derivation '%s' failed (usually happens due to networking issues); try '--fallback' to build derivation from source ",
@@ -315,9 +339,21 @@ void DerivationGoal::outputsSubstitutionTried()
 void DerivationGoal::gaveUpOnSubstitution()
 {
     /* The inputs must be built before we can build this goal. */
+    inputDrvOutputs.clear();
     if (useDerivation)
-        for (auto & i : dynamic_cast<Derivation *>(drv.get())->inputDrvs)
+        for (auto & i : dynamic_cast<Derivation *>(drv.get())->inputDrvs) {
+            /* Ensure that pure, non-fixed-output derivations don't
+               depend on impure derivations. */
+            if (drv->type().isPure() && !drv->type().isFixed()) {
+                auto inputDrv = worker.evalStore.readDerivation(i.first);
+                if (!inputDrv.type().isPure())
+                    throw Error("pure derivation '%s' depends on impure derivation '%s'",
+                        worker.store.printStorePath(drvPath),
+                        worker.store.printStorePath(i.first));
+            }
+
             addWaitee(worker.makeDerivationGoal(i.first, i.second, buildMode == bmRepair ? bmRepair : bmNormal));
+        }
 
     /* Copy the input sources from the eval store to the build
        store. */
@@ -345,6 +381,8 @@ void DerivationGoal::gaveUpOnSubstitution()
 
 void DerivationGoal::repairClosure()
 {
+    assert(drv->type().isPure());
+
     /* If we're repairing, we now know that our own outputs are valid.
        Now check whether the other paths in the outputs closure are
        good.  If not, then start derivation goals for the derivations
@@ -452,22 +490,24 @@ void DerivationGoal::inputsRealised()
                        drvs. */
                     : true);
             },
+            [&](const DerivationType::Impure &) {
+                return true;
+            }
         }, drvType.raw());
 
-        if (resolveDrv)
-        {
+        if (resolveDrv && !fullDrv.inputDrvs.empty()) {
             settings.requireExperimentalFeature(Xp::CaDerivations);
 
             /* We are be able to resolve this derivation based on the
-               now-known results of dependencies. If so, we become a stub goal
-               aliasing that resolved derivation goal */
-            std::optional attempt = fullDrv.tryResolve(worker.store);
+               now-known results of dependencies. If so, we become a
+               stub goal aliasing that resolved derivation goal. */
+            std::optional attempt = fullDrv.tryResolve(worker.store, inputDrvOutputs);
             assert(attempt);
             Derivation drvResolved { *std::move(attempt) };
 
             auto pathResolved = writeDerivation(worker.store, drvResolved);
 
-            auto msg = fmt("Resolved derivation: '%s' -> '%s'",
+            auto msg = fmt("resolved derivation: '%s' -> '%s'",
                 worker.store.printStorePath(drvPath),
                 worker.store.printStorePath(pathResolved));
             act = std::make_unique<Activity>(*logger, lvlInfo, actBuildWaiting, msg,
@@ -488,21 +528,13 @@ void DerivationGoal::inputsRealised()
             /* Add the relevant output closures of the input derivation
                `i' as input paths.  Only add the closures of output paths
                that are specified as inputs. */
-            assert(worker.evalStore.isValidPath(drvPath));
-            auto outputs = worker.evalStore.queryPartialDerivationOutputMap(depDrvPath);
-            for (auto & j : wantedDepOutputs) {
-                if (outputs.count(j) > 0) {
-                    auto optRealizedInput = outputs.at(j);
-                    if (!optRealizedInput)
-                        throw Error(
-                            "derivation '%s' requires output '%s' from input derivation '%s', which is supposedly realized already, yet we still don't know what path corresponds to that output",
-                            worker.store.printStorePath(drvPath), j, worker.store.printStorePath(depDrvPath));
-                    worker.store.computeFSClosure(*optRealizedInput, inputPaths);
-                } else
+            for (auto & j : wantedDepOutputs)
+                if (auto outPath = get(inputDrvOutputs, { depDrvPath, j }))
+                    worker.store.computeFSClosure(*outPath, inputPaths);
+                else
                     throw Error(
                         "derivation '%s' requires non-existent output '%s' from input derivation '%s'",
                         worker.store.printStorePath(drvPath), j, worker.store.printStorePath(depDrvPath));
-            }
         }
     }
 
@@ -923,7 +955,7 @@ void DerivationGoal::buildDone()
             st =
                 dynamic_cast<NotDeterministic*>(&e) ? BuildResult::NotDeterministic :
                 statusOk(status) ? BuildResult::OutputRejected :
-                derivationType.isImpure() || diskFull ? BuildResult::TransientFailure :
+                !derivationType.isSandboxed() || diskFull ? BuildResult::TransientFailure :
                 BuildResult::PermanentFailure;
         }
 
@@ -934,60 +966,53 @@ void DerivationGoal::buildDone()
 
 void DerivationGoal::resolvedFinished()
 {
+    trace("resolved derivation finished");
+
     assert(resolvedDrvGoal);
     auto resolvedDrv = *resolvedDrvGoal->drv;
+    auto & resolvedResult = resolvedDrvGoal->buildResult;
 
-    auto resolvedHashes = staticOutputHashes(worker.store, resolvedDrv);
+    DrvOutputs builtOutputs;
 
-    StorePathSet outputPaths;
+    if (resolvedResult.success()) {
+        auto resolvedHashes = staticOutputHashes(worker.store, resolvedDrv);
 
-    // `wantedOutputs` might be empty, which means “all the outputs”
-    auto realWantedOutputs = wantedOutputs;
-    if (realWantedOutputs.empty())
-        realWantedOutputs = resolvedDrv.outputNames();
+        StorePathSet outputPaths;
 
-    DrvOutputs builtOutputs;
+        // `wantedOutputs` might be empty, which means “all the outputs”
+        auto realWantedOutputs = wantedOutputs;
+        if (realWantedOutputs.empty())
+            realWantedOutputs = resolvedDrv.outputNames();
+
+        for (auto & wantedOutput : realWantedOutputs) {
+            assert(initialOutputs.count(wantedOutput) != 0);
+            assert(resolvedHashes.count(wantedOutput) != 0);
+            auto realisation = resolvedResult.builtOutputs.at(
+                DrvOutput { resolvedHashes.at(wantedOutput), wantedOutput });
+            if (drv->type().isPure()) {
+                auto newRealisation = realisation;
+                newRealisation.id = DrvOutput { initialOutputs.at(wantedOutput).outputHash, wantedOutput };
+                newRealisation.signatures.clear();
+                if (!drv->type().isFixed())
+                    newRealisation.dependentRealisations = drvOutputReferences(worker.store, *drv, realisation.outPath);
+                signRealisation(newRealisation);
+                worker.store.registerDrvOutput(newRealisation);
+            }
+            outputPaths.insert(realisation.outPath);
+            builtOutputs.emplace(realisation.id, realisation);
+        }
 
-    for (auto & wantedOutput : realWantedOutputs) {
-        assert(initialOutputs.count(wantedOutput) != 0);
-        assert(resolvedHashes.count(wantedOutput) != 0);
-        auto realisation = worker.store.queryRealisation(
-            DrvOutput{resolvedHashes.at(wantedOutput), wantedOutput}
+        runPostBuildHook(
+            worker.store,
+            *logger,
+            drvPath,
+            outputPaths
         );
-        // We've just built it, but maybe the build failed, in which case the
-        // realisation won't be there
-        if (realisation) {
-            auto newRealisation = *realisation;
-            newRealisation.id = DrvOutput{initialOutputs.at(wantedOutput).outputHash, wantedOutput};
-            newRealisation.signatures.clear();
-            newRealisation.dependentRealisations = drvOutputReferences(worker.store, *drv, realisation->outPath);
-            signRealisation(newRealisation);
-            worker.store.registerDrvOutput(newRealisation);
-            outputPaths.insert(realisation->outPath);
-            builtOutputs.emplace(realisation->id, *realisation);
-        } else {
-            // If we don't have a realisation, then it must mean that something
-            // failed when building the resolved drv
-            assert(!buildResult.success());
-        }
     }
 
-    runPostBuildHook(
-        worker.store,
-        *logger,
-        drvPath,
-        outputPaths
-    );
-
-    auto status = [&]() {
-        auto & resolvedResult = resolvedDrvGoal->buildResult;
-        switch (resolvedResult.status) {
-            case BuildResult::AlreadyValid:
-                return BuildResult::ResolvesToAlreadyValid;
-            default:
-                return resolvedResult.status;
-        }
-    }();
+    auto status = resolvedResult.status;
+    if (status == BuildResult::AlreadyValid)
+        status = BuildResult::ResolvesToAlreadyValid;
 
     done(status, std::move(builtOutputs));
 }
@@ -1236,6 +1261,7 @@ void DerivationGoal::flushLine()
 
 std::map<std::string, std::optional<StorePath>> DerivationGoal::queryPartialDerivationOutputMap()
 {
+    assert(drv->type().isPure());
     if (!useDerivation || drv->type().hasKnownOutputPaths()) {
         std::map<std::string, std::optional<StorePath>> res;
         for (auto & [name, output] : drv->outputs)
@@ -1248,6 +1274,7 @@ std::map<std::string, std::optional<StorePath>> DerivationGoal::queryPartialDeri
 
 OutputPathMap DerivationGoal::queryDerivationOutputMap()
 {
+    assert(drv->type().isPure());
     if (!useDerivation || drv->type().hasKnownOutputPaths()) {
         OutputPathMap res;
         for (auto & [name, output] : drv->outputsAndOptPaths(worker.store))
@@ -1261,6 +1288,8 @@ OutputPathMap DerivationGoal::queryDerivationOutputMap()
 
 std::pair<bool, DrvOutputs> DerivationGoal::checkPathValidity()
 {
+    if (!drv->type().isPure()) return { false, {} };
+
     bool checkHash = buildMode == bmRepair;
     auto wantedOutputsLeft = wantedOutputs;
     DrvOutputs validOutputs;
@@ -1304,6 +1333,7 @@ std::pair<bool, DrvOutputs> DerivationGoal::checkPathValidity()
         if (info.wanted && info.known && info.known->isValid())
             validOutputs.emplace(drvOutput, Realisation { drvOutput, info.known->path });
     }
+
     // If we requested all the outputs via the empty set, we are always fine.
     // If we requested specific elements, the loop above removes all the valid
     // ones, so any that are left must be invalid.
@@ -1341,9 +1371,7 @@ void DerivationGoal::done(
 {
     buildResult.status = status;
     if (ex)
-        // FIXME: strip: "error: "
-        buildResult.errorMsg = ex->what();
-    amDone(buildResult.success() ? ecSuccess : ecFailed, ex);
+        buildResult.errorMsg = fmt("%s", normaltxt(ex->info().msg));
     if (buildResult.status == BuildResult::TimedOut)
         worker.timedOut = true;
     if (buildResult.status == BuildResult::PermanentFailure)
@@ -1370,7 +1398,21 @@ void DerivationGoal::done(
         fs.open(traceBuiltOutputsFile, std::fstream::out);
         fs << worker.store.printStorePath(drvPath) << "\t" << buildResult.toString() << std::endl;
     }
+
+    amDone(buildResult.success() ? ecSuccess : ecFailed, ex);
 }
 
 
+void DerivationGoal::waiteeDone(GoalPtr waitee, ExitCode result)
+{
+    Goal::waiteeDone(waitee, result);
+
+    if (waitee->buildResult.success())
+        if (auto bfd = std::get_if<DerivedPath::Built>(&waitee->buildResult.path))
+            for (auto & [output, realisation] : waitee->buildResult.builtOutputs)
+                inputDrvOutputs.insert_or_assign(
+                    { bfd->drvPath, output.outputName },
+                    realisation.outPath);
+}
+
 }
diff --git a/src/libstore/build/derivation-goal.hh b/src/libstore/build/derivation-goal.hh
index f556b6f25..2d8bfd592 100644
--- a/src/libstore/build/derivation-goal.hh
+++ b/src/libstore/build/derivation-goal.hh
@@ -57,6 +57,11 @@ struct DerivationGoal : public Goal
        them. */
     StringSet wantedOutputs;
 
+    /* Mapping from input derivations + output names to actual store
+       paths. This is filled in by waiteeDone() as each dependency
+       finishes, before inputsRealised() is reached, */
+    std::map<std::pair<StorePath, std::string>, StorePath> inputDrvOutputs;
+
     /* Whether additional wanted outputs have been added. */
     bool needRestart = false;
 
@@ -224,6 +229,8 @@ struct DerivationGoal : public Goal
         DrvOutputs builtOutputs = {},
         std::optional<Error> ex = {});
 
+    void waiteeDone(GoalPtr waitee, ExitCode result) override;
+
     StorePathSet exportReferences(const StorePathSet & storePaths);
 };
 
diff --git a/src/libstore/build/drv-output-substitution-goal.cc b/src/libstore/build/drv-output-substitution-goal.cc
index e50292c1e..b7f7b5ab1 100644
--- a/src/libstore/build/drv-output-substitution-goal.cc
+++ b/src/libstore/build/drv-output-substitution-goal.cc
@@ -41,7 +41,7 @@ void DrvOutputSubstitutionGoal::tryNext()
     if (subs.size() == 0) {
         /* None left.  Terminate this goal and let someone else deal
            with it. */
-        debug("drv output '%s' is required, but there is no substituter that can provide it", id.to_string());
+        debug("derivation output '%s' is required, but there is no substituter that can provide it", id.to_string());
 
         /* Hack: don't indicate failure if there were no substituters.
            In that case the calling derivation should just do a
diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc
index 008ce0050..1467c18af 100644
--- a/src/libstore/build/local-derivation-goal.cc
+++ b/src/libstore/build/local-derivation-goal.cc
@@ -395,7 +395,7 @@ void LocalDerivationGoal::startBuilder()
         else if (settings.sandboxMode == smDisabled)
             useChroot = false;
         else if (settings.sandboxMode == smRelaxed)
-            useChroot = !(derivationType.isImpure()) && !noChroot;
+            useChroot = derivationType.isSandboxed() && !noChroot;
     }
 
     auto & localStore = getLocalStore();
@@ -608,7 +608,7 @@ void LocalDerivationGoal::startBuilder()
                 "nogroup:x:65534:\n", sandboxGid()));
 
         /* Create /etc/hosts with localhost entry. */
-        if (!(derivationType.isImpure()))
+        if (derivationType.isSandboxed())
             writeFile(chrootRootDir + "/etc/hosts", "127.0.0.1 localhost\n::1 localhost\n");
 
         /* Make the closure of the inputs available in the chroot,
@@ -704,6 +704,9 @@ void LocalDerivationGoal::startBuilder()
 
     /* Run the builder. */
     printMsg(lvlChatty, "executing builder '%1%'", drv->builder);
+    printMsg(lvlChatty, "using builder args '%1%'", concatStringsSep(" ", drv->args));
+    for (auto & i : drv->env)
+        printMsg(lvlVomit, "setting builder env variable '%1%'='%2%'", i.first, i.second);
 
     /* Create the log file. */
     Path logFile = openLogFile();
@@ -796,7 +799,7 @@ void LocalDerivationGoal::startBuilder()
            us.
         */
 
-        if (!(derivationType.isImpure()))
+        if (derivationType.isSandboxed())
             privateNetwork = true;
 
         userNamespaceSync.create();
@@ -1060,7 +1063,7 @@ void LocalDerivationGoal::initEnv()
        to the builder is generally impure, but the output of
        fixed-output derivations is by definition pure (since we
        already know the cryptographic hash of the output). */
-    if (derivationType.isImpure()) {
+    if (!derivationType.isSandboxed()) {
         for (auto & i : parsedDrv->getStringsAttr("impureEnvVars").value_or(Strings()))
             env[i] = getEnv(i).value_or("");
     }
@@ -1674,7 +1677,7 @@ void LocalDerivationGoal::runChild()
             /* Fixed-output derivations typically need to access the
                network, so give them access to /etc/resolv.conf and so
                on. */
-            if (derivationType.isImpure()) {
+            if (!derivationType.isSandboxed()) {
                 // Only use nss functions to resolve hosts and
                 // services. Don’t use it for anything else that may
                 // be configured for this system. This limits the
@@ -1918,7 +1921,7 @@ void LocalDerivationGoal::runChild()
 
                 sandboxProfile += "(import \"sandbox-defaults.sb\")\n";
 
-                if (derivationType.isImpure())
+                if (!derivationType.isSandboxed())
                     sandboxProfile += "(import \"sandbox-network.sb\")\n";
 
                 /* Add the output paths we'll use at build-time to the chroot */
@@ -2390,6 +2393,13 @@ DrvOutputs LocalDerivationGoal::registerOutputs()
                 assert(false);
             },
 
+            [&](const DerivationOutput::Impure & doi) {
+                return newInfoFromCA(DerivationOutput::CAFloating {
+                    .method = doi.method,
+                    .hashType = doi.hashType,
+                });
+            },
+
         }, output.raw());
 
         /* FIXME: set proper permissions in restorePath() so
@@ -2600,7 +2610,9 @@ DrvOutputs LocalDerivationGoal::registerOutputs()
             },
             .outPath = newInfo.path
         };
-        if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations)) {
+        if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations)
+            && drv->type().isPure())
+        {
             signRealisation(thisRealisation);
             worker.store.registerDrvOutput(thisRealisation);
         }
diff --git a/src/libstore/build/substitution-goal.cc b/src/libstore/build/substitution-goal.cc
index 2aaa89a57..2aee09f21 100644
--- a/src/libstore/build/substitution-goal.cc
+++ b/src/libstore/build/substitution-goal.cc
@@ -24,9 +24,16 @@ PathSubstitutionGoal::~PathSubstitutionGoal()
 }
 
 
-void PathSubstitutionGoal::done(ExitCode result, BuildResult::Status status)
+void PathSubstitutionGoal::done(
+    ExitCode result,
+    BuildResult::Status status,
+    std::optional<std::string> errorMsg)
 {
     buildResult.status = status;
+    if (errorMsg) {
+        debug(*errorMsg);
+        buildResult.errorMsg = *errorMsg;
+    }
     amDone(result);
 }
 
@@ -67,12 +74,14 @@ void PathSubstitutionGoal::tryNext()
     if (subs.size() == 0) {
         /* None left.  Terminate this goal and let someone else deal
            with it. */
-        debug("path '%s' is required, but there is no substituter that can build it", worker.store.printStorePath(storePath));
 
         /* Hack: don't indicate failure if there were no substituters.
            In that case the calling derivation should just do a
            build. */
-        done(substituterFailed ? ecFailed : ecNoSubstituters, BuildResult::NoSubstituters);
+        done(
+            substituterFailed ? ecFailed : ecNoSubstituters,
+            BuildResult::NoSubstituters,
+            fmt("path '%s' is required, but there is no substituter that can build it", worker.store.printStorePath(storePath)));
 
         if (substituterFailed) {
             worker.failedSubstitutions++;
@@ -171,10 +180,10 @@ void PathSubstitutionGoal::referencesValid()
     trace("all references realised");
 
     if (nrFailed > 0) {
-        debug("some references of path '%s' could not be realised", worker.store.printStorePath(storePath));
         done(
             nrNoSubstituters > 0 || nrIncompleteClosure > 0 ? ecIncompleteClosure : ecFailed,
-            BuildResult::DependencyFailed);
+            BuildResult::DependencyFailed,
+            fmt("some references of path '%s' could not be realised", worker.store.printStorePath(storePath)));
         return;
     }
 
diff --git a/src/libstore/build/substitution-goal.hh b/src/libstore/build/substitution-goal.hh
index 946f13841..a73f8e666 100644
--- a/src/libstore/build/substitution-goal.hh
+++ b/src/libstore/build/substitution-goal.hh
@@ -53,7 +53,10 @@ struct PathSubstitutionGoal : public Goal
     /* Content address for recomputing store path */
     std::optional<ContentAddress> ca;
 
-    void done(ExitCode result, BuildResult::Status status);
+    void done(
+        ExitCode result,
+        BuildResult::Status status,
+        std::optional<std::string> errorMsg = {});
 
 public:
     PathSubstitutionGoal(const StorePath & storePath, Worker & worker, RepairFlag repair = NoRepair, std::optional<ContentAddress> ca = std::nullopt);