From 13f78c557b402422aefc09cc217b001d83f08baa Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Mon, 4 Mar 2024 07:15:08 +0100 Subject: [PATCH] Merge pull request #9589 from obsidiansystems/floating-content-addressing-derivations-eval-store Fix building CA derivations with and eval store (cherry picked from commit dfc0cee7024a082d90a4f68296f55a82dfd52126) Change-Id: I28feb5a36d4fe75f0ed3e3e2db6eb56b67d0f371 --- src/libstore/build/derivation-goal.cc | 51 +++++++++++++++++++++------ src/libstore/misc.cc | 9 +++-- src/libstore/store-api.hh | 3 +- src/nix-build/nix-build.cc | 6 ++-- tests/functional/ca/eval-store.sh | 10 ++++++ tests/functional/ca/local.mk | 1 + tests/functional/eval-store.sh | 16 +++++++-- 7 files changed, 76 insertions(+), 20 deletions(-) create mode 100644 tests/functional/ca/eval-store.sh diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 01e34448b..e616adb71 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/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(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(drvStore->readDerivation(drvPath)); + break; + } + } + assert(drv); haveDerivation(); } @@ -401,11 +410,15 @@ 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) - inputSrcs.insert(i); + if (worker.evalStore.isValidPath(i)) + inputSrcs.insert(i); copyClosure(worker.evalStore, worker.store, inputSrcs); } @@ -453,7 +466,7 @@ void DerivationGoal::repairClosure() std::map 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); } @@ -1362,7 +1385,10 @@ std::map> 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); } } @@ -1375,7 +1401,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); } } diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index 1035691c7..c8646083b 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -330,8 +330,11 @@ std::map drvOutputReferences( std::map drvOutputReferences( Store & store, const Derivation & drv, - const StorePath & outputPath) + const StorePath & outputPath, + Store * evalStore_) { + auto & evalStore = evalStore_ ? *evalStore_ : store; + std::set inputRealisations; std::function::ChildNode &)> accumRealisations; @@ -339,7 +342,7 @@ std::map drvOutputReferences( accumRealisations = [&](const StorePath & inputDrv, const DerivedPathMap::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) @@ -361,7 +364,7 @@ std::map 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); } } diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 87fb4c04e..d3d7a315e 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -1053,6 +1053,7 @@ const ContentAddress * getDerivationCA(const BasicDerivation & drv); std::map drvOutputReferences( Store & store, const Derivation & drv, - const StorePath & outputPath); + const StorePath & outputPath, + Store * evalStore = nullptr); } diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index 60bc08146..54e8bd8d1 100644 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -458,7 +458,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"; } @@ -511,7 +511,7 @@ static void main_nix_build(int argc, char * * argv) std::function::ChildNode &)> accumInputClosure; accumInputClosure = [&](const StorePath & inputDrv, const DerivedPathMap::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); @@ -649,7 +649,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); diff --git a/tests/functional/ca/eval-store.sh b/tests/functional/ca/eval-store.sh new file mode 100644 index 000000000..9cc499606 --- /dev/null +++ b/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 diff --git a/tests/functional/ca/local.mk b/tests/functional/ca/local.mk index fd87b8d1f..4f86b268f 100644 --- a/tests/functional/ca/local.mk +++ b/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 \ diff --git a/tests/functional/eval-store.sh b/tests/functional/eval-store.sh index a34f3e82f..9937ecbce 100644 --- a/tests/functional/eval-store.sh +++ b/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,7 +35,10 @@ 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 clearStore