From b8f7177a7b2e884cbfb8bbe3c1ee8d586159fbb3 Mon Sep 17 00:00:00 2001 From: regnat Date: Wed, 19 May 2021 16:19:46 +0200 Subject: [PATCH] Properly fail when trying to register an incoherent realisation --- src/libstore/local-store.cc | 136 +++++++++++++++++++++++++----------- src/libstore/local-store.hh | 2 + src/libstore/realisation.cc | 6 ++ src/libstore/realisation.hh | 2 + 4 files changed, 107 insertions(+), 39 deletions(-) diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index c2256635a..8df1d55b9 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,41 @@ 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", + info.id.to_string()); + } + } 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 +1771,67 @@ 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..76aec74ce 100644 --- a/src/libstore/realisation.cc +++ b/src/libstore/realisation.cc @@ -140,6 +140,12 @@ StorePath RealisedPath::path() const { return std::visit([](auto && arg) { return arg.getPath(); }, raw); } +bool Realisation::isCompatibleWith(const Realisation & other) const +{ + assert (id == other.id); + return outPath == other.outPath; +} + 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);