From 7616268812cc2fe141132749d6d667b5c4f4d877 Mon Sep 17 00:00:00 2001 From: regnat Date: Fri, 7 May 2021 11:06:17 +0200 Subject: [PATCH 01/13] Always send the realisations as JSON MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Align all the worker protocol with `buildDerivation` which inlines the realisations as one opaque json blob. That way we don’t have to bother changing the remote store protocol when the definition of `Realisation` changes, as long as we keep the json backwards-compatible --- src/libstore/daemon.cc | 25 ++++++++++++++++++------- src/libstore/remote-store.cc | 23 +++++++++++++++++------ src/libstore/worker-protocol.hh | 2 +- 3 files changed, 36 insertions(+), 14 deletions(-) diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 72b3e3e13..e06fb9ce2 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -885,10 +885,15 @@ static void performOp(TunnelLogger * logger, ref store, case wopRegisterDrvOutput: { logger->startWork(); - auto outputId = DrvOutput::parse(readString(from)); - auto outputPath = StorePath(readString(from)); - store->registerDrvOutput(Realisation{ - .id = outputId, .outPath = outputPath}); + if (GET_PROTOCOL_MINOR(clientVersion) < 31) { + auto outputId = DrvOutput::parse(readString(from)); + auto outputPath = StorePath(readString(from)); + store->registerDrvOutput(Realisation{ + .id = outputId, .outPath = outputPath}); + } else { + auto realisation = worker_proto::read(*store, from, Phantom()); + store->registerDrvOutput(realisation); + } logger->stopWork(); break; } @@ -898,9 +903,15 @@ static void performOp(TunnelLogger * logger, ref store, auto outputId = DrvOutput::parse(readString(from)); auto info = store->queryRealisation(outputId); logger->stopWork(); - std::set outPaths; - if (info) outPaths.insert(info->outPath); - worker_proto::write(*store, to, outPaths); + if (GET_PROTOCOL_MINOR(clientVersion) < 31) { + std::set outPaths; + if (info) outPaths.insert(info->outPath); + worker_proto::write(*store, to, outPaths); + } else { + std::set realisations; + if (info) realisations.insert(*info); + worker_proto::write(*store, to, realisations); + } break; } diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index d9b6e9488..aec243637 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -653,8 +653,12 @@ void RemoteStore::registerDrvOutput(const Realisation & info) { auto conn(getConnection()); conn->to << wopRegisterDrvOutput; - conn->to << info.id.to_string(); - conn->to << std::string(info.outPath.to_string()); + if (GET_PROTOCOL_MINOR(conn->daemonVersion) < 31) { + conn->to << info.id.to_string(); + conn->to << std::string(info.outPath.to_string()); + } else { + worker_proto::write(*this, conn->to, info); + } conn.processStderr(); } @@ -664,10 +668,17 @@ std::optional RemoteStore::queryRealisation(const DrvOutput & conn->to << wopQueryRealisation; conn->to << id.to_string(); conn.processStderr(); - auto outPaths = worker_proto::read(*this, conn->from, Phantom>{}); - if (outPaths.empty()) - return std::nullopt; - return {Realisation{.id = id, .outPath = *outPaths.begin()}}; + if (GET_PROTOCOL_MINOR(conn->daemonVersion) < 31) { + auto outPaths = worker_proto::read(*this, conn->from, Phantom>{}); + if (outPaths.empty()) + return std::nullopt; + return {Realisation{.id = id, .outPath = *outPaths.begin()}}; + } else { + auto realisations = worker_proto::read(*this, conn->from, Phantom>{}); + if (realisations.empty()) + return std::nullopt; + return *realisations.begin(); + } } static void writeDerivedPaths(RemoteStore & store, ConnectionHandle & conn, const std::vector & reqs) diff --git a/src/libstore/worker-protocol.hh b/src/libstore/worker-protocol.hh index fdd692cf0..e89183d40 100644 --- a/src/libstore/worker-protocol.hh +++ b/src/libstore/worker-protocol.hh @@ -9,7 +9,7 @@ namespace nix { #define WORKER_MAGIC_1 0x6e697863 #define WORKER_MAGIC_2 0x6478696f -#define PROTOCOL_VERSION (1 << 8 | 30) +#define PROTOCOL_VERSION (1 << 8 | 31) #define GET_PROTOCOL_MAJOR(x) ((x) & 0xff00) #define GET_PROTOCOL_MINOR(x) ((x) & 0x00ff) From 7ce0441d806d12bb073047337545e3901404996d Mon Sep 17 00:00:00 2001 From: regnat Date: Tue, 10 Nov 2020 14:49:25 +0100 Subject: [PATCH 02/13] Add a dependencies field to DrvOutputInfo Currently never used, nor set but will be useful shortly --- src/libstore/realisation.cc | 10 ++++++++++ src/libstore/realisation.hh | 2 ++ 2 files changed, 12 insertions(+) diff --git a/src/libstore/realisation.cc b/src/libstore/realisation.cc index 638065547..27ad6c150 100644 --- a/src/libstore/realisation.cc +++ b/src/libstore/realisation.cc @@ -22,10 +22,14 @@ std::string DrvOutput::to_string() const { } nlohmann::json Realisation::toJSON() const { + nlohmann::json jsonDrvOutputDeps; + for (auto & dep : drvOutputDeps) + jsonDrvOutputDeps.push_back(dep.to_string()); return nlohmann::json{ {"id", id.to_string()}, {"outPath", outPath.to_string()}, {"signatures", signatures}, + {"drvOutputDeps", jsonDrvOutputDeps}, }; } @@ -51,10 +55,16 @@ Realisation Realisation::fromJSON( if (auto signaturesIterator = json.find("signatures"); signaturesIterator != json.end()) signatures.insert(signaturesIterator->begin(), signaturesIterator->end()); + std::set drvOutputDeps; + if (auto jsonDependencies = json.find("drvOutputDeps"); jsonDependencies != json.end()) + for (auto & jsonDep : *jsonDependencies) + drvOutputDeps.insert(DrvOutput::parse(jsonDep.get())); + return Realisation{ .id = DrvOutput::parse(getField("id")), .outPath = StorePath(getField("outPath")), .signatures = signatures, + .drvOutputDeps = drvOutputDeps, }; } diff --git a/src/libstore/realisation.hh b/src/libstore/realisation.hh index f5049c9e9..1e2808ce3 100644 --- a/src/libstore/realisation.hh +++ b/src/libstore/realisation.hh @@ -28,6 +28,8 @@ struct Realisation { StringSet signatures; + std::set drvOutputDeps; + nlohmann::json toJSON() const; static Realisation fromJSON(const nlohmann::json& json, const std::string& whence); From eca6ff06d611ef005e80d419e1b6050393fc056d Mon Sep 17 00:00:00 2001 From: regnat Date: Fri, 7 May 2021 13:57:01 +0200 Subject: [PATCH 03/13] Store the realisation deps on the local store --- src/libstore/ca-specific-schema.sql | 11 +++++- src/libstore/local-store.cc | 61 +++++++++++++++++++++++------ 2 files changed, 59 insertions(+), 13 deletions(-) diff --git a/src/libstore/ca-specific-schema.sql b/src/libstore/ca-specific-schema.sql index 20ee046a1..08af0cc1f 100644 --- a/src/libstore/ca-specific-schema.sql +++ b/src/libstore/ca-specific-schema.sql @@ -3,10 +3,19 @@ -- is enabled create table if not exists Realisations ( + id integer primary key autoincrement not null, drvPath text not null, outputName text not null, -- symbolic output id, usually "out" outputPath integer not null, signatures text, -- space-separated list - primary key (drvPath, outputName), foreign key (outputPath) references ValidPaths(id) on delete cascade ); + +create index if not exists IndexRealisations on Realisations(drvPath, outputName); + +create table if not exists RealisationsRefs ( + referrer integer not null, + realisationReference integer, + foreign key (referrer) references Realisations(id) on delete cascade, + foreign key (realisationReference) references Realisations(id) on delete restrict +); diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 83daa7506..f8d55621d 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -59,6 +59,8 @@ struct LocalStore::State::Stmts { SQLiteStmt QueryAllRealisedOutputs; SQLiteStmt QueryPathFromHashPart; SQLiteStmt QueryValidPaths; + SQLiteStmt QueryRealisationRealisationReferences; + SQLiteStmt AddRealisationRealisationReference; }; int getSchema(Path schemaPath) @@ -316,7 +318,7 @@ LocalStore::LocalStore(const Params & params) )"); state->stmts->QueryRealisedOutput.create(state->db, R"( - select Output.path, Realisations.signatures from Realisations + select Realisations.id, Output.path, Realisations.signatures from Realisations inner join ValidPaths as Output on Output.id = Realisations.outputPath where drvPath = ? and outputName = ? ; @@ -328,6 +330,19 @@ LocalStore::LocalStore(const Params & params) where drvPath = ? ; )"); + state->stmts->QueryRealisationRealisationReferences.create(state->db, + R"( + select drvPath, outputName from Realisations + join RealisationsRefs on realisationReference = Realisations.id + where referrer = ?; + )"); + state->stmts->AddRealisationRealisationReference.create(state->db, + R"( + insert or replace into RealisationsRefs (referrer, realisationReference) + values ( + ?, + (select id from Realisations where drvPath = ? and outputName = ?)); + )"); } } @@ -666,13 +681,17 @@ void LocalStore::registerDrvOutput(const Realisation & info) settings.requireExperimentalFeature("ca-derivations"); auto state(_state.lock()); retrySQLite([&]() { - state->stmts->RegisterRealisedOutput.use() - (info.id.strHash()) - (info.id.outputName) - (printStorePath(info.outPath)) - (concatStringsSep(" ", info.signatures)) + 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.drvOutputDeps) { + state->stmts->AddRealisationRealisationReference + .use()(myId)(outputId.strHash())(outputId.outputName) + .exec(); + } } void LocalStore::cacheDrvOutputMapping(State & state, const uint64_t deriver, const string & outputName, const StorePath & output) @@ -1670,14 +1689,32 @@ std::optional LocalStore::queryRealisation( typedef std::optional Ret; return retrySQLite([&]() -> Ret { auto state(_state.lock()); - auto use(state->stmts->QueryRealisedOutput.use()(id.strHash())( - id.outputName)); - if (!use.next()) + auto useQueryRealisedOutput(state->stmts->QueryRealisedOutput.use()( + id.strHash())(id.outputName)); + if (!useQueryRealisedOutput.next()) return std::nullopt; - auto outputPath = parseStorePath(use.getStr(0)); - auto signatures = tokenizeString(use.getStr(1)); + auto realisationDbId = useQueryRealisedOutput.getInt(0); + auto outputPath = parseStorePath(useQueryRealisedOutput.getStr(1)); + auto signatures = + tokenizeString(useQueryRealisedOutput.getStr(2)); + + std::set drvOutputDeps; + auto useRealisationRefs( + state->stmts->QueryRealisationRealisationReferences.use()( + realisationDbId)); + while (useRealisationRefs.next()) + drvOutputDeps.insert(DrvOutput{ + Hash::parseAnyPrefixed(useRealisationRefs.getStr(0)), + useRealisationRefs.getStr(1), + } + ); + return Ret{Realisation{ - .id = id, .outPath = outputPath, .signatures = signatures}}; + .id = id, + .outPath = outputPath, + .signatures = signatures, + .drvOutputDeps = drvOutputDeps, + }}; }); } } // namespace nix From af3afd25eafb7866406aee04ef049121a3e3ffb0 Mon Sep 17 00:00:00 2001 From: regnat Date: Wed, 19 May 2021 10:26:58 +0200 Subject: [PATCH 04/13] Add a method to compute the closure of a realisation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Only considers the closure in term of `Realisation`, ignores all the opaque inputs. Dunno whether that’s the nicest solution, need to think it through a bit --- src/libstore/realisation.cc | 38 +++++++++++++++++++++++++++++++++++++ src/libstore/realisation.hh | 3 +++ 2 files changed, 41 insertions(+) diff --git a/src/libstore/realisation.cc b/src/libstore/realisation.cc index 27ad6c150..fab10f68c 100644 --- a/src/libstore/realisation.cc +++ b/src/libstore/realisation.cc @@ -1,5 +1,6 @@ #include "realisation.hh" #include "store-api.hh" +#include "closure.hh" #include namespace nix { @@ -21,6 +22,43 @@ std::string DrvOutput::to_string() const { return strHash() + "!" + outputName; } +std::set Realisation::closure(Store & store, std::set startOutputs) +{ + std::set res; + Realisation::closure(store, startOutputs, res); + return res; +} + +void Realisation::closure(Store & store, std::set startOutputs, std::set & res) +{ + auto getDeps = [&](const Realisation& current) -> std::set { + std::set res; + for (auto& currentDep : current.drvOutputDeps) { + if (auto currentRealisation = store.queryRealisation(currentDep)) + res.insert(*currentRealisation); + else + throw Error( + "Unrealised derivation '%s'", currentDep.to_string()); + } + return res; + }; + + computeClosure( + startOutputs, res, + [&](const Realisation& current, + std::function>&)> + processEdges) { + std::promise> promise; + try { + auto res = getDeps(current); + promise.set_value(res); + } catch (...) { + promise.set_exception(std::current_exception()); + } + return processEdges(promise); + }); +} + nlohmann::json Realisation::toJSON() const { nlohmann::json jsonDrvOutputDeps; for (auto & dep : drvOutputDeps) diff --git a/src/libstore/realisation.hh b/src/libstore/realisation.hh index 1e2808ce3..776ab606c 100644 --- a/src/libstore/realisation.hh +++ b/src/libstore/realisation.hh @@ -38,6 +38,9 @@ struct Realisation { bool checkSignature(const PublicKeys & publicKeys, const std::string & sig) const; size_t checkSignatures(const PublicKeys & publicKeys) const; + static std::set closure(Store &, std::set); + static void closure(Store &, std::set, std::set& res); + StorePath getPath() const { return outPath; } GENERATE_CMP(Realisation, me->id, me->outPath); From 8c30acc3e896839283a2c96ec97cc0c811e8ad6a Mon Sep 17 00:00:00 2001 From: regnat Date: Wed, 19 May 2021 10:35:31 +0200 Subject: [PATCH 05/13] Properly track the drvoutput references when building --- src/libstore/build/derivation-goal.cc | 1 + src/libstore/misc.cc | 42 +++++++++++++++++++++++++++ src/libstore/store-api.hh | 5 ++++ 3 files changed, 48 insertions(+) diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 9100d3333..93fc54440 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -927,6 +927,7 @@ void DerivationGoal::resolvedFinished() { auto newRealisation = *realisation; newRealisation.id = DrvOutput{initialOutputs.at(wantedOutput).outputHash, wantedOutput}; newRealisation.signatures.clear(); + newRealisation.drvOutputDeps = drvOutputReferences(worker.store, *drv, realisation->outPath); signRealisation(newRealisation); worker.store.registerDrvOutput(newRealisation); } else { diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index bc5fd968c..8793f9f4a 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -254,5 +254,47 @@ StorePaths Store::topoSortPaths(const StorePathSet & paths) }}); } +std::set drvOutputReferences( + const std::set inputRealisations, + const StorePathSet pathReferences) +{ + std::set res; + std::map> inputsByOutputPath; + for (const auto & input : inputRealisations) + inputsByOutputPath[input.outPath].insert(input.id); + + for (const auto & path : pathReferences) { + auto theseInputs = inputsByOutputPath[path]; + res.insert(theseInputs.begin(), theseInputs.end()); + } + + return res; +} + +std::set drvOutputReferences( + Store & store, + const Derivation & drv, + const StorePath & outputPath) +{ + std::set inputRealisations; + + for (const auto& [inputDrv, outputNames] : drv.inputDrvs) { + auto outputHashes = + staticOutputHashes(store, store.readDerivation(inputDrv)); + for (const auto& outputName : outputNames) { + auto thisRealisation = store.queryRealisation( + DrvOutput{outputHashes.at(outputName), outputName}); + if (!thisRealisation) + throw Error( + "output '%s' of derivation '%s' isn’t built", outputName, + store.printStorePath(inputDrv)); + inputRealisations.insert(*thisRealisation); + } + } + + auto info = store.queryPathInfo(outputPath); + + return drvOutputReferences(Realisation::closure(store, inputRealisations), info->references); +} } diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index f66298991..29af0f495 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -864,4 +864,9 @@ std::pair splitUriAndParams(const std::string & uri) std::optional getDerivationCA(const BasicDerivation & drv); +std::set drvOutputReferences( + Store & store, + const Derivation & drv, + const StorePath & outputPath); + } From 63ebfc73c56978665bf6c58c3e4ad84485355b1a Mon Sep 17 00:00:00 2001 From: regnat Date: Wed, 19 May 2021 10:36:48 +0200 Subject: [PATCH 06/13] Make `copyPaths` copy the whole realisations closure Otherwise registering the realisations on the remote side might fail as it now expects a complete closure --- src/libstore/store-api.cc | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 93fcb068f..08573f709 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -780,20 +780,40 @@ std::map copyPaths(ref srcStore, ref dstStor RepairFlag repair, CheckSigsFlag checkSigs, SubstituteFlag substitute) { StorePathSet storePaths; - std::set realisations; + std::set toplevelRealisations; for (auto & path : paths) { storePaths.insert(path.path()); if (auto realisation = std::get_if(&path.raw)) { settings.requireExperimentalFeature("ca-derivations"); - realisations.insert(*realisation); + toplevelRealisations.insert(*realisation); } } auto pathsMap = copyPaths(srcStore, dstStore, storePaths, repair, checkSigs, substitute); + + ThreadPool pool; + try { - for (auto & realisation : realisations) { - dstStore->registerDrvOutput(realisation, checkSigs); - } - } catch (MissingExperimentalFeature & e) { + // Copy the realisation closure + processGraph( + pool, Realisation::closure(*srcStore, toplevelRealisations), + [&](const Realisation& current) -> std::set { + std::set children; + for (const auto& drvOutput : current.drvOutputDeps) { + auto currentChild = srcStore->queryRealisation(drvOutput); + if (!currentChild) + throw Error( + "Incomplete realisation closure: '%s' is a " + "dependency " + "of '%s' but isn’t registered", + drvOutput.to_string(), current.id.to_string()); + children.insert(*currentChild); + } + return children; + }, + [&](const Realisation& current) -> void { + dstStore->registerDrvOutput(current, checkSigs); + }); + } catch (MissingExperimentalFeature& e) { // Don't fail if the remote doesn't support CA derivations is it might // not be within our control to change that, and we might still want // to at least copy the output paths. From cb46d70794b8677b7927e6ae3d492e6db886c7aa Mon Sep 17 00:00:00 2001 From: regnat Date: Wed, 19 May 2021 12:30:12 +0200 Subject: [PATCH 07/13] Add a db migration script --- src/libstore/local-store.cc | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index f8d55621d..ce0e94865 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -78,7 +78,7 @@ int getSchema(Path schemaPath) void migrateCASchema(SQLite& db, Path schemaPath, AutoCloseFD& lockFd) { - const int nixCASchemaVersion = 1; + const int nixCASchemaVersion = 2; int curCASchema = getSchema(schemaPath); if (curCASchema != nixCASchemaVersion) { if (curCASchema > nixCASchemaVersion) { @@ -96,7 +96,39 @@ void migrateCASchema(SQLite& db, Path schemaPath, AutoCloseFD& lockFd) #include "ca-specific-schema.sql.gen.hh" ; db.exec(schema); + curCASchema = nixCASchemaVersion; } + + if (curCASchema < 2) { + SQLiteTxn txn(db); + // Ugly little sql dance to add a new `id` column and make it the primary key + db.exec(R"( + create table Realisations2 ( + id integer primary key autoincrement not null, + drvPath text not null, + outputName text not null, -- symbolic output id, usually "out" + outputPath integer not null, + signatures text, -- space-separated list + foreign key (outputPath) references ValidPaths(id) on delete cascade + ); + insert into Realisations2 (drvPath, outputName, outputPath, signatures) + select drvPath, outputName, outputPath, signatures from Realisations; + drop table Realisations; + alter table Realisations2 rename to Realisations; + )"); + db.exec(R"( + create index if not exists IndexRealisations on Realisations(drvPath, outputName); + + create table if not exists RealisationsRefs ( + referrer integer not null, + realisationReference integer, + foreign key (referrer) references Realisations(id) on delete cascade, + foreign key (realisationReference) references Realisations(id) on delete restrict + ); + )"); + txn.commit(); + } + writeFile(schemaPath, fmt("%d", nixCASchemaVersion)); lockFile(lockFd.get(), ltRead, true); } From 1f3ff0d193c270f7b97af4aa3e463be01dbe5f2d Mon Sep 17 00:00:00 2001 From: regnat Date: Wed, 26 May 2021 16:09:02 +0200 Subject: [PATCH 08/13] Aso track the output path of the realisation dependencies --- src/libstore/build/derivation-goal.cc | 2 +- src/libstore/local-store.cc | 34 +++++++++++++++------------ src/libstore/misc.cc | 17 ++++++-------- src/libstore/realisation.cc | 20 ++++++++-------- src/libstore/realisation.hh | 8 ++++++- src/libstore/store-api.cc | 2 +- src/libstore/store-api.hh | 2 +- 7 files changed, 46 insertions(+), 39 deletions(-) diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 93fc54440..8c9ef0101 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -927,7 +927,7 @@ void DerivationGoal::resolvedFinished() { auto newRealisation = *realisation; newRealisation.id = DrvOutput{initialOutputs.at(wantedOutput).outputHash, wantedOutput}; newRealisation.signatures.clear(); - newRealisation.drvOutputDeps = drvOutputReferences(worker.store, *drv, realisation->outPath); + newRealisation.dependentRealisations = drvOutputReferences(worker.store, *drv, realisation->outPath); signRealisation(newRealisation); worker.store.registerDrvOutput(newRealisation); } else { diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index ce0e94865..debe70037 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -711,19 +711,19 @@ void LocalStore::registerDrvOutput(const Realisation & info, CheckSigsFlag check void LocalStore::registerDrvOutput(const Realisation & info) { settings.requireExperimentalFeature("ca-derivations"); - auto state(_state.lock()); retrySQLite([&]() { + auto state(_state.lock()); 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) { + state->stmts->AddRealisationRealisationReference + .use()(myId)(outputId.strHash())(outputId.outputName) + .exec(); + } }); - uint64_t myId = state->db.getLastInsertedRowId(); - for (auto& outputId : info.drvOutputDeps) { - state->stmts->AddRealisationRealisationReference - .use()(myId)(outputId.strHash())(outputId.outputName) - .exec(); - } } void LocalStore::cacheDrvOutputMapping(State & state, const uint64_t deriver, const string & outputName, const StorePath & output) @@ -1730,22 +1730,26 @@ std::optional LocalStore::queryRealisation( auto signatures = tokenizeString(useQueryRealisedOutput.getStr(2)); - std::set drvOutputDeps; + std::map dependentRealisations; auto useRealisationRefs( state->stmts->QueryRealisationRealisationReferences.use()( realisationDbId)); - while (useRealisationRefs.next()) - drvOutputDeps.insert(DrvOutput{ - Hash::parseAnyPrefixed(useRealisationRefs.getStr(0)), - useRealisationRefs.getStr(1), - } - ); + 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{ .id = id, .outPath = outputPath, .signatures = signatures, - .drvOutputDeps = drvOutputDeps, + .dependentRealisations = dependentRealisations, }}; }); } diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index 8793f9f4a..80ee15c49 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -254,25 +254,22 @@ StorePaths Store::topoSortPaths(const StorePathSet & paths) }}); } -std::set drvOutputReferences( +std::map drvOutputReferences( const std::set inputRealisations, const StorePathSet pathReferences) { - std::set res; + std::map res; - std::map> inputsByOutputPath; - for (const auto & input : inputRealisations) - inputsByOutputPath[input.outPath].insert(input.id); - - for (const auto & path : pathReferences) { - auto theseInputs = inputsByOutputPath[path]; - res.insert(theseInputs.begin(), theseInputs.end()); + for (const auto & input : inputRealisations) { + if (pathReferences.count(input.outPath)) { + res.insert({input.id, input.outPath}); + } } return res; } -std::set drvOutputReferences( +std::map drvOutputReferences( Store & store, const Derivation & drv, const StorePath & outputPath) diff --git a/src/libstore/realisation.cc b/src/libstore/realisation.cc index fab10f68c..d2d306476 100644 --- a/src/libstore/realisation.cc +++ b/src/libstore/realisation.cc @@ -33,7 +33,7 @@ void Realisation::closure(Store & store, std::set startOutputs, std { auto getDeps = [&](const Realisation& current) -> std::set { std::set res; - for (auto& currentDep : current.drvOutputDeps) { + for (auto& [currentDep, _] : current.dependentRealisations) { if (auto currentRealisation = store.queryRealisation(currentDep)) res.insert(*currentRealisation); else @@ -60,14 +60,14 @@ void Realisation::closure(Store & store, std::set startOutputs, std } nlohmann::json Realisation::toJSON() const { - nlohmann::json jsonDrvOutputDeps; - for (auto & dep : drvOutputDeps) - jsonDrvOutputDeps.push_back(dep.to_string()); + auto jsonDependentRealisations = nlohmann::json::object(); + for (auto & [depId, depOutPath] : dependentRealisations) + jsonDependentRealisations.emplace(depId.to_string(), depOutPath.to_string()); return nlohmann::json{ {"id", id.to_string()}, {"outPath", outPath.to_string()}, {"signatures", signatures}, - {"drvOutputDeps", jsonDrvOutputDeps}, + {"dependentRealisations", jsonDependentRealisations}, }; } @@ -93,16 +93,16 @@ Realisation Realisation::fromJSON( if (auto signaturesIterator = json.find("signatures"); signaturesIterator != json.end()) signatures.insert(signaturesIterator->begin(), signaturesIterator->end()); - std::set drvOutputDeps; - if (auto jsonDependencies = json.find("drvOutputDeps"); jsonDependencies != json.end()) - for (auto & jsonDep : *jsonDependencies) - drvOutputDeps.insert(DrvOutput::parse(jsonDep.get())); + std::map dependentRealisations; + if (auto jsonDependencies = json.find("dependentRealisations"); jsonDependencies != json.end()) + for (auto & [jsonDepId, jsonDepOutPath] : jsonDependencies->get>()) + dependentRealisations.insert({DrvOutput::parse(jsonDepId), StorePath(jsonDepOutPath)}); return Realisation{ .id = DrvOutput::parse(getField("id")), .outPath = StorePath(getField("outPath")), .signatures = signatures, - .drvOutputDeps = drvOutputDeps, + .dependentRealisations = dependentRealisations, }; } diff --git a/src/libstore/realisation.hh b/src/libstore/realisation.hh index 776ab606c..8cda5a752 100644 --- a/src/libstore/realisation.hh +++ b/src/libstore/realisation.hh @@ -28,7 +28,13 @@ struct Realisation { StringSet signatures; - std::set drvOutputDeps; + /** + * The realisations that are required for the current one to be valid. + * + * When importing this realisation, the store will first check that all its + * dependencies exist, and map to the correct output path + */ + std::map dependentRealisations; nlohmann::json toJSON() const; static Realisation fromJSON(const nlohmann::json& json, const std::string& whence); diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 08573f709..6f29c430a 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -798,7 +798,7 @@ std::map copyPaths(ref srcStore, ref dstStor pool, Realisation::closure(*srcStore, toplevelRealisations), [&](const Realisation& current) -> std::set { std::set children; - for (const auto& drvOutput : current.drvOutputDeps) { + for (const auto& [drvOutput, _] : current.dependentRealisations) { auto currentChild = srcStore->queryRealisation(drvOutput); if (!currentChild) throw Error( diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 29af0f495..9a0027641 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -864,7 +864,7 @@ std::pair splitUriAndParams(const std::string & uri) std::optional getDerivationCA(const BasicDerivation & drv); -std::set drvOutputReferences( +std::map drvOutputReferences( Store & store, const Derivation & drv, const StorePath & outputPath); From dcabb461242e5c38e0a2780dc5869b91c13241d6 Mon Sep 17 00:00:00 2001 From: regnat Date: Mon, 21 Jun 2021 16:28:06 +0200 Subject: [PATCH 09/13] Shorten a stupidly long sql query name --- src/libstore/local-store.cc | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index debe70037..45bb5efc8 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -59,8 +59,8 @@ struct LocalStore::State::Stmts { SQLiteStmt QueryAllRealisedOutputs; SQLiteStmt QueryPathFromHashPart; SQLiteStmt QueryValidPaths; - SQLiteStmt QueryRealisationRealisationReferences; - SQLiteStmt AddRealisationRealisationReference; + SQLiteStmt QueryRealisationReferences; + SQLiteStmt AddRealisationReference; }; int getSchema(Path schemaPath) @@ -362,13 +362,13 @@ LocalStore::LocalStore(const Params & params) where drvPath = ? ; )"); - state->stmts->QueryRealisationRealisationReferences.create(state->db, + state->stmts->QueryRealisationReferences.create(state->db, R"( select drvPath, outputName from Realisations join RealisationsRefs on realisationReference = Realisations.id where referrer = ?; )"); - state->stmts->AddRealisationRealisationReference.create(state->db, + state->stmts->AddRealisationReference.create(state->db, R"( insert or replace into RealisationsRefs (referrer, realisationReference) values ( @@ -719,7 +719,7 @@ void LocalStore::registerDrvOutput(const Realisation & info) .exec(); uint64_t myId = state->db.getLastInsertedRowId(); for (auto & [outputId, _] : info.dependentRealisations) { - state->stmts->AddRealisationRealisationReference + state->stmts->AddRealisationReference .use()(myId)(outputId.strHash())(outputId.outputName) .exec(); } @@ -1732,7 +1732,7 @@ std::optional LocalStore::queryRealisation( std::map dependentRealisations; auto useRealisationRefs( - state->stmts->QueryRealisationRealisationReferences.use()( + state->stmts->QueryRealisationReferences.use()( realisationDbId)); while (useRealisationRefs.next()) { auto depHash = useRealisationRefs.getStr(0); From c13d7d0b9770741cc7093dc71ec8fc7978171f18 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Mon, 21 Jun 2021 16:37:45 +0200 Subject: [PATCH 10/13] Pass more values by reference Rather than copying them around everywhere Co-authored-by: Eelco Dolstra --- src/libstore/misc.cc | 4 ++-- src/libstore/realisation.cc | 4 ++-- src/libstore/realisation.hh | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index 80ee15c49..96d73b70e 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -255,8 +255,8 @@ StorePaths Store::topoSortPaths(const StorePathSet & paths) } std::map drvOutputReferences( - const std::set inputRealisations, - const StorePathSet pathReferences) + const std::set & inputRealisations, + const StorePathSet & pathReferences) { std::map res; diff --git a/src/libstore/realisation.cc b/src/libstore/realisation.cc index d2d306476..0d9d4b433 100644 --- a/src/libstore/realisation.cc +++ b/src/libstore/realisation.cc @@ -22,14 +22,14 @@ std::string DrvOutput::to_string() const { return strHash() + "!" + outputName; } -std::set Realisation::closure(Store & store, std::set startOutputs) +std::set Realisation::closure(Store & store, const std::set & startOutputs) { std::set res; Realisation::closure(store, startOutputs, res); return res; } -void Realisation::closure(Store & store, std::set startOutputs, std::set & res) +void Realisation::closure(Store & store, const std::set & startOutputs, std::set & res) { auto getDeps = [&](const Realisation& current) -> std::set { std::set res; diff --git a/src/libstore/realisation.hh b/src/libstore/realisation.hh index 8cda5a752..7fdb65acd 100644 --- a/src/libstore/realisation.hh +++ b/src/libstore/realisation.hh @@ -44,8 +44,8 @@ struct Realisation { bool checkSignature(const PublicKeys & publicKeys, const std::string & sig) const; size_t checkSignatures(const PublicKeys & publicKeys) const; - static std::set closure(Store &, std::set); - static void closure(Store &, std::set, std::set& res); + static std::set closure(Store &, const std::set &); + static void closure(Store &, const std::set &, std::set& res); StorePath getPath() const { return outPath; } From 8d09a4f9a09473e6a32cc118ad10827ec5650700 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Th=C3=A9ophane=20Hufschmitt?= Date: Tue, 22 Jun 2021 09:31:25 +0200 Subject: [PATCH 11/13] Remove a useless string split Co-authored-by: Eelco Dolstra --- src/libstore/store-api.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 6f29c430a..7e057b1ea 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -803,8 +803,7 @@ std::map copyPaths(ref srcStore, ref dstStor if (!currentChild) throw Error( "Incomplete realisation closure: '%s' is a " - "dependency " - "of '%s' but isn’t registered", + "dependency of '%s' but isn’t registered", drvOutput.to_string(), current.id.to_string()); children.insert(*currentChild); } From 7c96a76dd7da69fa527dea7110d1156c1c5fbe9e Mon Sep 17 00:00:00 2001 From: regnat Date: Tue, 22 Jun 2021 09:26:55 +0200 Subject: [PATCH 12/13] Reformat the sql statements --- src/libstore/local-store.cc | 28 +++++++++++++++++----------- 1 file changed, 17 insertions(+), 11 deletions(-) diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 45bb5efc8..b56df735c 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -713,14 +713,18 @@ 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)) + 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) { - state->stmts->AddRealisationReference - .use()(myId)(outputId.strHash())(outputId.outputName) + state->stmts->AddRealisationReference.use() + (myId) + (outputId.strHash()) + (outputId.outputName) .exec(); } }); @@ -1721,8 +1725,9 @@ std::optional LocalStore::queryRealisation( typedef std::optional Ret; return retrySQLite([&]() -> Ret { auto state(_state.lock()); - auto useQueryRealisedOutput(state->stmts->QueryRealisedOutput.use()( - id.strHash())(id.outputName)); + auto useQueryRealisedOutput(state->stmts->QueryRealisedOutput.use() + (id.strHash()) + (id.outputName)); if (!useQueryRealisedOutput.next()) return std::nullopt; auto realisationDbId = useQueryRealisedOutput.getInt(0); @@ -1732,13 +1737,14 @@ std::optional LocalStore::queryRealisation( std::map dependentRealisations; auto useRealisationRefs( - state->stmts->QueryRealisationReferences.use()( - realisationDbId)); + 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)); + auto useQueryRealisedOutput(state->stmts->QueryRealisedOutput.use() + (depHash) + (depOutputName)); assert(useQueryRealisedOutput.next()); auto outputPath = parseStorePath(useQueryRealisedOutput.getStr(1)); auto depId = DrvOutput { Hash::parseAnyPrefixed(depHash), depOutputName }; From ed0e21a88d64d772304d079c39d76feca41c02d3 Mon Sep 17 00:00:00 2001 From: regnat Date: Wed, 23 Jun 2021 07:45:41 +0200 Subject: [PATCH 13/13] Fix indentation --- src/libstore/local-store.cc | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index b56df735c..348d964ca 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -714,10 +714,10 @@ void LocalStore::registerDrvOutput(const Realisation & info) retrySQLite([&]() { auto state(_state.lock()); state->stmts->RegisterRealisedOutput.use() - (info.id.strHash()) - (info.id.outputName) - (printStorePath(info.outPath)) - (concatStringsSep(" ", info.signatures)) + (info.id.strHash()) + (info.id.outputName) + (printStorePath(info.outPath)) + (concatStringsSep(" ", info.signatures)) .exec(); uint64_t myId = state->db.getLastInsertedRowId(); for (auto & [outputId, _] : info.dependentRealisations) {