diff --git a/src/libstore/build/drv-output-substitution-goal.cc b/src/libstore/build/drv-output-substitution-goal.cc index 1703e845d..be270d079 100644 --- a/src/libstore/build/drv-output-substitution-goal.cc +++ b/src/libstore/build/drv-output-substitution-goal.cc @@ -17,6 +17,13 @@ DrvOutputSubstitutionGoal::DrvOutputSubstitutionGoal(const DrvOutput& id, Worker void DrvOutputSubstitutionGoal::init() { trace("init"); + + /* If the derivation already exists, we’re done */ + if (worker.store.queryRealisation(id)) { + amDone(ecSuccess); + return; + } + subs = settings.useSubstitutes ? getDefaultSubstituters() : std::list>(); tryNext(); } @@ -53,9 +60,23 @@ void DrvOutputSubstitutionGoal::tryNext() return; } - for (const auto & [drvOutputDep, _] : outputInfo->dependentRealisations) { - if (drvOutputDep != id) { - addWaitee(worker.makeDrvOutputSubstitutionGoal(drvOutputDep)); + for (const auto & [depId, depPath] : outputInfo->dependentRealisations) { + if (depId != id) { + if (auto localOutputInfo = worker.store.queryRealisation(depId); + localOutputInfo && localOutputInfo->outPath != depPath) { + warn( + "substituter '%s' has an incompatible realisation for '%s', ignoring.\n" + "Local: %s\n" + "Remote: %s", + sub->getUri(), + depId.to_string(), + worker.store.printStorePath(localOutputInfo->outPath), + worker.store.printStorePath(depPath) + ); + tryNext(); + return; + } + addWaitee(worker.makeDrvOutputSubstitutionGoal(depId)); } } diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index c2256635a..d7c7f8e1d 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -53,6 +53,7 @@ struct LocalStore::State::Stmts { SQLiteStmt InvalidatePath; SQLiteStmt AddDerivationOutput; SQLiteStmt RegisterRealisedOutput; + SQLiteStmt UpdateRealisedOutput; SQLiteStmt QueryValidDerivers; SQLiteStmt QueryDerivationOutputs; SQLiteStmt QueryRealisedOutput; @@ -345,6 +346,15 @@ LocalStore::LocalStore(const Params & params) values (?, ?, (select id from ValidPaths where path = ?), ?) ; )"); + state->stmts->UpdateRealisedOutput.create(state->db, + R"( + update Realisations + set signatures = ? + where + drvPath = ? and + outputName = ? + ; + )"); state->stmts->QueryRealisedOutput.create(state->db, R"( select Realisations.id, Output.path, Realisations.signatures from Realisations @@ -710,14 +720,46 @@ void LocalStore::registerDrvOutput(const Realisation & info) settings.requireExperimentalFeature("ca-derivations"); retrySQLite([&]() { auto state(_state.lock()); - state->stmts->RegisterRealisedOutput.use() - (info.id.strHash()) - (info.id.outputName) - (printStorePath(info.outPath)) - (concatStringsSep(" ", info.signatures)) - .exec(); + if (auto oldR = queryRealisation_(*state, info.id)) { + if (info.isCompatibleWith(*oldR)) { + auto combinedSignatures = oldR->signatures; + combinedSignatures.insert(info.signatures.begin(), + info.signatures.end()); + state->stmts->UpdateRealisedOutput.use() + (concatStringsSep(" ", combinedSignatures)) + (info.id.strHash()) + (info.id.outputName) + .exec(); + } else { + throw Error("Trying to register a realisation of '%s', but we already " + "have another one locally.\n" + "Local: %s\n" + "Remote: %s", + info.id.to_string(), + printStorePath(oldR->outPath), + printStorePath(info.outPath) + ); + } + } else { + state->stmts->RegisterRealisedOutput.use() + (info.id.strHash()) + (info.id.outputName) + (printStorePath(info.outPath)) + (concatStringsSep(" ", info.signatures)) + .exec(); + } uint64_t myId = state->db.getLastInsertedRowId(); - for (auto & [outputId, _] : info.dependentRealisations) { + for (auto & [outputId, depPath] : info.dependentRealisations) { + auto localRealisation = queryRealisationCore_(*state, outputId); + if (!localRealisation) + throw Error("unable to register the derivation '%s' as it " + "depends on the non existent '%s'", + info.id.to_string(), outputId.to_string()); + if (localRealisation->second.outPath != depPath) + throw Error("unable to register the derivation '%s' as it " + "depends on a realisation of '%s' that doesn’t" + "match what we have locally", + info.id.to_string(), outputId.to_string()); state->stmts->AddRealisationReference.use() (myId) (outputId.strHash()) @@ -1734,46 +1776,68 @@ void LocalStore::createUser(const std::string & userName, uid_t userId) } } -std::optional LocalStore::queryRealisation( - const DrvOutput& id) { - typedef std::optional Ret; - return retrySQLite([&]() -> Ret { - auto state(_state.lock()); - auto useQueryRealisedOutput(state->stmts->QueryRealisedOutput.use() +std::optional> LocalStore::queryRealisationCore_( + LocalStore::State & state, + const DrvOutput & id) +{ + auto useQueryRealisedOutput( + state.stmts->QueryRealisedOutput.use() (id.strHash()) (id.outputName)); - if (!useQueryRealisedOutput.next()) - return std::nullopt; - auto realisationDbId = useQueryRealisedOutput.getInt(0); - auto outputPath = parseStorePath(useQueryRealisedOutput.getStr(1)); - auto signatures = - tokenizeString(useQueryRealisedOutput.getStr(2)); + if (!useQueryRealisedOutput.next()) + return std::nullopt; + auto realisationDbId = useQueryRealisedOutput.getInt(0); + auto outputPath = parseStorePath(useQueryRealisedOutput.getStr(1)); + auto signatures = + tokenizeString(useQueryRealisedOutput.getStr(2)); - std::map dependentRealisations; - auto useRealisationRefs( - state->stmts->QueryRealisationReferences.use() - (realisationDbId)); - while (useRealisationRefs.next()) { - auto depHash = useRealisationRefs.getStr(0); - auto depOutputName = useRealisationRefs.getStr(1); - auto useQueryRealisedOutput(state->stmts->QueryRealisedOutput.use() - (depHash) - (depOutputName)); - assert(useQueryRealisedOutput.next()); - auto outputPath = parseStorePath(useQueryRealisedOutput.getStr(1)); - auto depId = DrvOutput { Hash::parseAnyPrefixed(depHash), depOutputName }; - dependentRealisations.insert({depId, outputPath}); - } - - return Ret{Realisation{ + return {{ + realisationDbId, + Realisation{ .id = id, .outPath = outputPath, .signatures = signatures, - .dependentRealisations = dependentRealisations, - }}; - }); + } + }}; } +std::optional LocalStore::queryRealisation_( + LocalStore::State & state, + const DrvOutput & id) +{ + auto maybeCore = queryRealisationCore_(state, id); + if (!maybeCore) + return std::nullopt; + auto [realisationDbId, res] = *maybeCore; + + std::map dependentRealisations; + auto useRealisationRefs( + state.stmts->QueryRealisationReferences.use() + (realisationDbId)); + while (useRealisationRefs.next()) { + auto depId = DrvOutput { + Hash::parseAnyPrefixed(useRealisationRefs.getStr(0)), + useRealisationRefs.getStr(1), + }; + auto dependentRealisation = queryRealisationCore_(state, depId); + assert(dependentRealisation); // Enforced by the db schema + auto outputPath = dependentRealisation->second.outPath; + dependentRealisations.insert({depId, outputPath}); + } + + res.dependentRealisations = dependentRealisations; + + return { res }; +} + +std::optional +LocalStore::queryRealisation(const DrvOutput & id) +{ + return retrySQLite>([&]() { + auto state(_state.lock()); + return queryRealisation_(*state, id); + }); +} FixedOutputHash LocalStore::hashCAPath( const FileIngestionMethod & method, const HashType & hashType, diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index 15c7fc306..a01d48c4b 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -203,6 +203,8 @@ public: void registerDrvOutput(const Realisation & info, CheckSigsFlag checkSigs) override; void cacheDrvOutputMapping(State & state, const uint64_t deriver, const string & outputName, const StorePath & output); + std::optional queryRealisation_(State & state, const DrvOutput & id); + std::optional> queryRealisationCore_(State & state, const DrvOutput & id); std::optional queryRealisation(const DrvOutput&) override; private: diff --git a/src/libstore/realisation.cc b/src/libstore/realisation.cc index 0d9d4b433..eadec594c 100644 --- a/src/libstore/realisation.cc +++ b/src/libstore/realisation.cc @@ -140,6 +140,16 @@ StorePath RealisedPath::path() const { return std::visit([](auto && arg) { return arg.getPath(); }, raw); } +bool Realisation::isCompatibleWith(const Realisation & other) const +{ + assert (id == other.id); + if (outPath == other.outPath) { + assert(dependentRealisations == other.dependentRealisations); + return true; + } + return false; +} + void RealisedPath::closure( Store& store, const RealisedPath::Set& startPaths, diff --git a/src/libstore/realisation.hh b/src/libstore/realisation.hh index 7fdb65acd..05d2bc44f 100644 --- a/src/libstore/realisation.hh +++ b/src/libstore/realisation.hh @@ -47,6 +47,8 @@ struct Realisation { static std::set closure(Store &, const std::set &); static void closure(Store &, const std::set &, std::set& res); + bool isCompatibleWith(const Realisation & other) const; + StorePath getPath() const { return outPath; } GENERATE_CMP(Realisation, me->id, me->outPath); diff --git a/tests/ca/duplicate-realisation-in-closure.sh b/tests/ca/duplicate-realisation-in-closure.sh new file mode 100644 index 000000000..bfe2a4e08 --- /dev/null +++ b/tests/ca/duplicate-realisation-in-closure.sh @@ -0,0 +1,26 @@ +source ./common.sh + +sed -i 's/experimental-features .*/& ca-derivations ca-references/' "$NIX_CONF_DIR"/nix.conf + +export REMOTE_STORE_DIR="$TEST_ROOT/remote_store" +export REMOTE_STORE="file://$REMOTE_STORE_DIR" + +rm -rf $REMOTE_STORE_DIR +clearStore + +# Build dep1 and push that to the binary cache. +# This entails building (and pushing) current-time. +nix copy --to "$REMOTE_STORE" -f nondeterministic.nix dep1 +clearStore +sleep 2 # To make sure that `$(date)` will be different +# Build dep2. +# As we’ve cleared the cache, we’ll have to rebuild current-time. And because +# the current time isn’t the same as before, this will yield a new (different) +# realisation +nix build -f nondeterministic.nix dep2 + +# Build something that depends both on dep1 and dep2. +# If everything goes right, we should rebuild dep2 rather than fetch it from +# the cache (because that would mean duplicating `current-time` in the closure), +# and have `dep1 == dep2`. +nix build --substituters "$REMOTE_STORE" -f nondeterministic.nix toplevel --no-require-sigs diff --git a/tests/ca/nondeterministic.nix b/tests/ca/nondeterministic.nix new file mode 100644 index 000000000..d6d099a3e --- /dev/null +++ b/tests/ca/nondeterministic.nix @@ -0,0 +1,35 @@ +with import ./config.nix; + +let mkCADerivation = args: mkDerivation ({ + __contentAddressed = true; + outputHashMode = "recursive"; + outputHashAlgo = "sha256"; +} // args); +in + +rec { + currentTime = mkCADerivation { + name = "current-time"; + buildCommand = '' + mkdir $out + echo $(date) > $out/current-time + ''; + }; + dep = seed: mkCADerivation { + name = "dep"; + inherit seed; + buildCommand = '' + echo ${currentTime} > $out + ''; + }; + dep1 = dep 1; + dep2 = dep 2; + toplevel = mkCADerivation { + name = "toplevel"; + buildCommand = '' + test ${dep1} == ${dep2} + touch $out + ''; + }; +} + diff --git a/tests/local.mk b/tests/local.mk index 3fa609788..82cec1df3 100644 --- a/tests/local.mk +++ b/tests/local.mk @@ -47,6 +47,7 @@ nix_tests = \ compute-levels.sh \ ca/build.sh \ ca/build-with-garbage-path.sh \ + ca/duplicate-realisation-in-closure.sh \ ca/substitute.sh \ ca/signatures.sh \ ca/nix-shell.sh \