Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix building CA derivations with and eval store #9589

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 40 additions & 11 deletions src/libstore/build/derivation-goal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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 }) {
tomberek marked this conversation as resolved.
Show resolved Hide resolved
if (drvStore->isValidPath(drvPath)) {
drv = std::make_unique<Derivation>(drvStore->readDerivation(drvPath));
break;
}
}
assert(drv);

haveDerivation();
}
Expand Down Expand Up @@ -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))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have a temporary root for it in the evalStore?

Copy link
Member Author

@Ericson2314 Ericson2314 Dec 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is rooted by the derivation in evalStore (if that is rooted), if the derivation is in store then this whole thing is a no-op

inputSrcs.insert(i);
copyClosure(worker.evalStore, worker.store, inputSrcs);
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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 })
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thought: {,} is a hidden overlay store

if (drvStore->isValidPath(depDrvPath))
return worker.store.queryDerivationOutputMap(depDrvPath, drvStore);
assert(false);
}();

auto outMapPath = outMap.find(outputName);
if (outMapPath == outMap.end()) {
throw Error(
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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);
}
}

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

Expand Down
9 changes: 6 additions & 3 deletions src/libstore/misc.cc
Original file line number Diff line number Diff line change
Expand Up @@ -331,16 +331,19 @@ 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;

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)
Expand All @@ -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);
}
}
Expand Down
3 changes: 2 additions & 1 deletion src/libstore/store-api.hh
Original file line number Diff line number Diff line change
Expand Up @@ -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);

}
6 changes: 3 additions & 3 deletions src/nix-build/nix-build.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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";
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
10 changes: 10 additions & 0 deletions tests/functional/ca/eval-store.sh
Original file line number Diff line number Diff line change
@@ -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
1 change: 1 addition & 0 deletions tests/functional/ca/local.mk
Original file line number Diff line number Diff line change
Expand Up @@ -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 \
Expand Down
16 changes: 14 additions & 2 deletions tests/functional/eval-store.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Loading