From b1711071d1a22a1c28448823634b0dc7212e4fe5 Mon Sep 17 00:00:00 2001 From: regnat Date: Thu, 22 Apr 2021 17:54:02 +0200 Subject: [PATCH 1/4] Add a regression test for #4725 --- tests/ca/substitute.sh | 20 +++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/tests/ca/substitute.sh b/tests/ca/substitute.sh index b44fe499a..5d0fc6a87 100644 --- a/tests/ca/substitute.sh +++ b/tests/ca/substitute.sh @@ -8,7 +8,8 @@ sed -i 's/experimental-features .*/& ca-derivations ca-references/' "$NIX_CONF_D rm -rf $TEST_ROOT/binary_cache -export REMOTE_STORE=file://$TEST_ROOT/binary_cache +export REMOTE_STORE_DIR=$TEST_ROOT/binary_cache +export REMOTE_STORE=file://$REMOTE_STORE_DIR buildDrvs () { nix build --file ./content-addressed.nix -L --no-link "$@" @@ -22,3 +23,20 @@ buildDrvs --post-build-hook ../push-to-store.sh clearStore buildDrvs --substitute --substituters $REMOTE_STORE --no-require-sigs -j0 +# Same thing, but +# 1. With non-ca derivations +# 2. Erasing the realisations on the remote store +# +# Even in that case, realising the derivations should still produce the right +# realisations on the local store +# +# Regression test for #4725 +clearStore +nix build --file ../simple.nix -L --no-link --post-build-hook ../push-to-store.sh +clearStore +rm -r "$REMOTE_STORE_DIR/realisations" +nix build --file ../simple.nix -L --no-link --substitute --substituters "$REMOTE_STORE" --no-require-sigs -j0 +if [[ $(sqlite3 "$NIX_STATE_DIR/db/db.sqlite" 'select count(*) from Realisations') -eq 0 ]]; then + echo "Realisations not rebuilt" + exit 1 +fi From 9161e02039e63327830128a3e69034b3cd46c109 Mon Sep 17 00:00:00 2001 From: regnat Date: Thu, 22 Apr 2021 17:55:36 +0200 Subject: [PATCH 2/4] Always register the realisations of input-addressed drvs Fix #4725 --- src/libstore/build/derivation-goal.cc | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 55ced2215..9100d3333 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -1269,12 +1269,23 @@ void DerivationGoal::checkPathValidity() }; } if (settings.isExperimentalFeatureEnabled("ca-derivations")) { - if (auto real = worker.store.queryRealisation( - DrvOutput{initialOutputs.at(i.first).outputHash, i.first})) { + auto drvOutput = DrvOutput{initialOutputs.at(i.first).outputHash, i.first}; + if (auto real = worker.store.queryRealisation(drvOutput)) { info.known = { .path = real->outPath, .status = PathStatus::Valid, }; + } else if (info.known && info.known->status == PathStatus::Valid) { + // We know the output because it' a static output of the + // derivation, and the output path is valid, but we don't have + // its realisation stored (probably because it has been built + // without the `ca-derivations` experimental flag) + worker.store.registerDrvOutput( + Realisation{ + drvOutput, + info.known->path, + } + ); } } } From 6ea9c65aece0b65825c99738340ea4944d083234 Mon Sep 17 00:00:00 2001 From: regnat Date: Fri, 23 Apr 2021 09:34:16 +0200 Subject: [PATCH 3/4] fixup! Add a regression test for #4725 --- tests/ca/substitute.sh | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/ca/substitute.sh b/tests/ca/substitute.sh index 5d0fc6a87..737c851a5 100644 --- a/tests/ca/substitute.sh +++ b/tests/ca/substitute.sh @@ -36,7 +36,12 @@ nix build --file ../simple.nix -L --no-link --post-build-hook ../push-to-store.s clearStore rm -r "$REMOTE_STORE_DIR/realisations" nix build --file ../simple.nix -L --no-link --substitute --substituters "$REMOTE_STORE" --no-require-sigs -j0 -if [[ $(sqlite3 "$NIX_STATE_DIR/db/db.sqlite" 'select count(*) from Realisations') -eq 0 ]]; then +# There's no easy way to check whether a realisation is present on the local +# store − short of manually querying the db, but the build environment doesn't +# have the sqlite binary − so we instead push things again, and check that the +# realisations have correctly been pushed to the remote store +nix copy --to "$REMOTE_STORE" --file ../simple.nix +if [[ -z "$(ls "$REMOTE_STORE_DIR/realisations")" ]]; then echo "Realisations not rebuilt" exit 1 fi From d3df747cb6813fc5f9d0fcc63d6fd16292c92e04 Mon Sep 17 00:00:00 2001 From: regnat Date: Fri, 23 Apr 2021 09:34:43 +0200 Subject: [PATCH 4/4] Revert "Make `nix shell` fallback to static outputs when needed" This reverts commit 8d66f5f1107fe87f70ea24ade045720235cc31fa. This fix isn't needed anymore now that the realisations are always properly registered --- src/libcmd/installables.cc | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index 10855688c..06ef4c669 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -736,18 +736,9 @@ std::set toRealisedPaths( output.first); auto outputId = DrvOutput{outputHashes.at(output.first), output.first}; auto realisation = store->queryRealisation(outputId); - if (!realisation) { - // TODO: remove this check once #4725 is fixed - // as we’ll have the guaranty that if - // `output.second` exists, then the realisation - // will be there too - if (output.second) - res.insert(*output.second); - else - throw Error("cannot operate on an output of unbuilt content-addresed derivation '%s'", outputId.to_string()); - } else { - res.insert(RealisedPath{*realisation}); - } + if (!realisation) + throw Error("cannot operate on an output of unbuilt content-addresed derivation '%s'", outputId.to_string()); + res.insert(RealisedPath{*realisation}); } else { // If ca-derivations isn't enabled, behave as if