Fix building CA derivations with and eval store

I don't love the way this code looks. There are two larger problems:

- eval, build/scratch, destination stores (#5025) should have different
  types to reflect the fact that they are used for different purposes
  and those purposes correspond to different operations. It should be
  impossible to "use the wrong store" in my cases.

- Since drvs can end up in both the eval and build/scratch store, we
  should have some sort of union/layered store (not on the file sytem
  level, just conceptual level) that allows accessing both. This would
  get rid of the ugly "check both" boilerplate in this PR.

Still, it might be better to land this now / soon after minimal cleanup,
so we have a concrete idea of what problem better abstractions are
supposed to solve.
This commit is contained in:
John Ericson 2023-12-10 21:21:21 -05:00
parent 8cddda4f89
commit 9f39dda66c
7 changed files with 76 additions and 20 deletions

View file

@ -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,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<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);
}
}

View file

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

View file

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

View file

@ -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);

View file

@ -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

View file

@ -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 \

View file

@ -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