Merge pull request #9589 from obsidiansystems/floating-content-addressing-derivations-eval-store

Fix building CA derivations with and eval store

src/libstore/build/derivation-goal.cc

@@ -196,10 +196,19 @@ void DerivationGoal::loadDerivation()
        things being garbage collected while we're busy. */
     worker.evalStore.addTempRoot(drvPath);
 
-    assert(worker.evalStore.isValidPath(drvPath));
+    /* Get the derivation. It is probably in the eval store, but it might be inthe main store:
 
-    /* Get the derivation. */
-    drv = std::make_unique<Derivation>(worker.evalStore.readDerivation(drvPath));
+         - Resolved derivation are resolved against main store realisations, and so must be stored there.
+
+         - Dynamic derivations are built, and so are found in the main store.
+     */
+    for (auto * drvStore : { &worker.evalStore, &worker.store }) {
+        if (drvStore->isValidPath(drvPath)) {
+            drv = std::make_unique<Derivation>(drvStore->readDerivation(drvPath));
+            break;
+        }
+    }
+    assert(drv);
 
     haveDerivation();
 }
@@ -401,10 +410,14 @@ void DerivationGoal::gaveUpOnSubstitution()
     }
 
     /* Copy the input sources from the eval store to the build
-       store. */
+       store.
+
+       Note that some inputs might not be in the eval store because they
+       are (resolved) derivation outputs in a resolved derivation. */
     if (&worker.evalStore != &worker.store) {
         RealisedPath::Set inputSrcs;
         for (auto & i : drv->inputSrcs)
+            if (worker.evalStore.isValidPath(i))
                 inputSrcs.insert(i);
         copyClosure(worker.evalStore, worker.store, inputSrcs);
     }
@@ -453,7 +466,7 @@ void DerivationGoal::repairClosure()
     std::map<StorePath, StorePath> outputsToDrv;
     for (auto & i : inputClosure)
         if (i.isDerivation()) {
-            auto depOutputs = worker.store.queryPartialDerivationOutputMap(i);
+            auto depOutputs = worker.store.queryPartialDerivationOutputMap(i, &worker.evalStore);
             for (auto & j : depOutputs)
                 if (j.second)
                     outputsToDrv.insert_or_assign(*j.second, i);
@@ -604,7 +617,13 @@ void DerivationGoal::inputsRealised()
                     return *outPath;
                 }
                 else {
-                    auto outMap = worker.evalStore.queryDerivationOutputMap(depDrvPath);
+                    auto outMap = [&]{
+                        for (auto * drvStore : { &worker.evalStore, &worker.store })
+                            if (drvStore->isValidPath(depDrvPath))
+                                return worker.store.queryDerivationOutputMap(depDrvPath, drvStore);
+                        assert(false);
+                    }();
+
                     auto outMapPath = outMap.find(outputName);
                     if (outMapPath == outMap.end()) {
                         throw Error(
@@ -1085,8 +1104,12 @@ void DerivationGoal::resolvedFinished()
                 auto newRealisation = realisation;
                 newRealisation.id = DrvOutput { initialOutput->outputHash, outputName };
                 newRealisation.signatures.clear();
-                if (!drv->type().isFixed())
-                    newRealisation.dependentRealisations = drvOutputReferences(worker.store, *drv, realisation.outPath);
+                if (!drv->type().isFixed()) {
+                    auto & drvStore = worker.evalStore.isValidPath(drvPath)
+                        ? worker.evalStore
+                        : worker.store;
+                    newRealisation.dependentRealisations = drvOutputReferences(worker.store, *drv, realisation.outPath, &drvStore);
+                }
                 signRealisation(newRealisation);
                 worker.store.registerDrvOutput(newRealisation);
             }
@@ -1379,7 +1402,10 @@ std::map<std::string, std::optional<StorePath>> DerivationGoal::queryPartialDeri
             res.insert_or_assign(name, output.path(worker.store, drv->name, name));
         return res;
     } else {
-        return worker.store.queryPartialDerivationOutputMap(drvPath);
+        for (auto * drvStore : { &worker.evalStore, &worker.store })
+            if (drvStore->isValidPath(drvPath))
+                return worker.store.queryPartialDerivationOutputMap(drvPath, drvStore);
+        assert(false);
     }
 }
 
@@ -1392,7 +1418,10 @@ OutputPathMap DerivationGoal::queryDerivationOutputMap()
             res.insert_or_assign(name, *output.second);
         return res;
     } else {
-        return worker.store.queryDerivationOutputMap(drvPath);
+        for (auto * drvStore : { &worker.evalStore, &worker.store })
+            if (drvStore->isValidPath(drvPath))
+                return worker.store.queryDerivationOutputMap(drvPath, drvStore);
+        assert(false);
     }
 }
 

src/libstore/misc.cc

@@ -331,8 +331,11 @@ std::map<DrvOutput, StorePath> drvOutputReferences(
 std::map<DrvOutput, StorePath> drvOutputReferences(
     Store & store,
     const Derivation & drv,
-    const StorePath & outputPath)
+    const StorePath & outputPath,
+    Store * evalStore_)
 {
+    auto & evalStore = evalStore_ ? *evalStore_ : store;
+
     std::set<Realisation> inputRealisations;
 
     std::function<void(const StorePath &, const DerivedPathMap<StringSet>::ChildNode &)> accumRealisations;
@@ -340,7 +343,7 @@ std::map<DrvOutput, StorePath> drvOutputReferences(
     accumRealisations = [&](const StorePath & inputDrv, const DerivedPathMap<StringSet>::ChildNode & inputNode) {
         if (!inputNode.value.empty()) {
             auto outputHashes =
-                staticOutputHashes(store, store.readDerivation(inputDrv));
+                staticOutputHashes(evalStore, evalStore.readDerivation(inputDrv));
             for (const auto & outputName : inputNode.value) {
                 auto outputHash = get(outputHashes, outputName);
                 if (!outputHash)
@@ -362,7 +365,7 @@ std::map<DrvOutput, StorePath> drvOutputReferences(
                 SingleDerivedPath next = SingleDerivedPath::Built { d, outputName };
                 accumRealisations(
                     // TODO deep resolutions for dynamic derivations, issue #8947, would go here.
-                    resolveDerivedPath(store, next),
+                    resolveDerivedPath(store, next, evalStore_),
                     childNode);
             }
         }

src/libstore/store-api.hh

@@ -943,6 +943,7 @@ const ContentAddress * getDerivationCA(const BasicDerivation & drv);
 std::map<DrvOutput, StorePath> drvOutputReferences(
     Store & store,
     const Derivation & drv,
-    const StorePath & outputPath);
+    const StorePath & outputPath,
+    Store * evalStore = nullptr);
 
 }

src/nix-build/nix-build.cc

@@ -462,7 +462,7 @@ static void main_nix_build(int argc, char * * argv)
         if (dryRun) return;
 
         if (shellDrv) {
-            auto shellDrvOutputs = store->queryPartialDerivationOutputMap(shellDrv.value());
+            auto shellDrvOutputs = store->queryPartialDerivationOutputMap(shellDrv.value(), &*evalStore);
             shell = store->printStorePath(shellDrvOutputs.at("out").value()) + "/bin/bash";
         }
 
@@ -515,7 +515,7 @@ static void main_nix_build(int argc, char * * argv)
             std::function<void(const StorePath &, const DerivedPathMap<StringSet>::ChildNode &)> accumInputClosure;
 
             accumInputClosure = [&](const StorePath & inputDrv, const DerivedPathMap<StringSet>::ChildNode & inputNode) {
-                auto outputs = evalStore->queryPartialDerivationOutputMap(inputDrv);
+                auto outputs = store->queryPartialDerivationOutputMap(inputDrv, &*evalStore);
                 for (auto & i : inputNode.value) {
                     auto o = outputs.at(i);
                     store->computeFSClosure(*o, inputs);
@@ -653,7 +653,7 @@ static void main_nix_build(int argc, char * * argv)
             if (counter)
                 drvPrefix += fmt("-%d", counter + 1);
 
-            auto builtOutputs = evalStore->queryPartialDerivationOutputMap(drvPath);
+            auto builtOutputs = store->queryPartialDerivationOutputMap(drvPath, &*evalStore);
 
             auto maybeOutputPath = builtOutputs.at(outputName);
             assert(maybeOutputPath);

tests/functional/ca/eval-store.sh

@@ -0,0 +1,10 @@
+#!/usr/bin/env bash
+
+# Ensure that garbage collection works properly with ca derivations
+
+source common.sh
+
+export NIX_TESTS_CA_BY_DEFAULT=1
+
+cd ..
+source eval-store.sh

tests/functional/ca/local.mk

@@ -5,6 +5,7 @@ ca-tests := \
   $(d)/concurrent-builds.sh \
   $(d)/derivation-json.sh \
   $(d)/duplicate-realisation-in-closure.sh \
+  $(d)/eval-store.sh \
   $(d)/gc.sh \
   $(d)/import-derivation.sh \
   $(d)/new-build-cmd.sh \

tests/functional/eval-store.sh

@@ -11,7 +11,16 @@ rm -rf "$eval_store"
 
 nix build -f dependencies.nix --eval-store "$eval_store" -o "$TEST_ROOT/result"
 [[ -e $TEST_ROOT/result/foobar ]]
-(! ls $NIX_STORE_DIR/*.drv)
+if [[ ! -n "${NIX_TESTS_CA_BY_DEFAULT:-}" ]]; then
+    # Resolved CA derivations are written to store for building
+    #
+    # TODO when we something more systematic
+    # (https://github.com/NixOS/nix/issues/5025) that distinguishes
+    # between scratch storage for building and the final destination
+    # store, we'll be able to make this unconditional again -- resolved
+    # derivations should only appear in the scratch store.
+    (! ls $NIX_STORE_DIR/*.drv)
+fi
 ls $eval_store/nix/store/*.drv
 
 clearStore
@@ -26,5 +35,8 @@ rm -rf "$eval_store"
 
 nix-build dependencies.nix --eval-store "$eval_store" -o "$TEST_ROOT/result"
 [[ -e $TEST_ROOT/result/foobar ]]
-(! ls $NIX_STORE_DIR/*.drv)
+if [[ ! -n "${NIX_TESTS_CA_BY_DEFAULT:-}" ]]; then
+    # See above
+    (! ls $NIX_STORE_DIR/*.drv)
+fi
 ls $eval_store/nix/store/*.drv