From 255d145ba7ac907d1cba8d088da556b591627756 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Tue, 2 Mar 2021 03:50:41 +0000 Subject: [PATCH] Use `BuildableReq` for `buildPaths` and `ensurePath` This avoids an ambiguity where the `StorePathWithOutputs { drvPath, {} }` could mean "build `brvPath`" or "substitute `drvPath`" depending on context. It also brings the internals closer in line to the new CLI, by generalizing the `Buildable` type is used there and makes that distinction already. In doing so, relegate `StorePathWithOutputs` to being a type just for backwards compatibility (CLI and RPC). --- src/libcmd/installables.cc | 7 +-- src/libexpr/get-drvs.cc | 1 + src/libexpr/primops.cc | 12 +++-- src/libmain/shared.cc | 2 +- src/libmain/shared.hh | 3 +- src/libstore/build/derivation-goal.cc | 4 +- src/libstore/build/entry-points.cc | 16 +++--- src/libstore/build/local-derivation-goal.cc | 52 +++++++++++++------ src/libstore/build/local-derivation-goal.hh | 1 + src/libstore/build/worker.cc | 6 +-- src/libstore/buildable.cc | 47 +++++++++++++++++ src/libstore/buildable.hh | 29 +++++++++-- src/libstore/daemon.cc | 21 +++++--- src/libstore/legacy-ssh-store.cc | 16 ++++-- src/libstore/misc.cc | 49 +++++++++--------- src/libstore/path-with-outputs.cc | 35 +++++++++++++ src/libstore/path-with-outputs.hh | 9 ++++ src/libstore/remote-store.cc | 57 +++++++++++++++++---- src/libstore/remote-store.hh | 4 +- src/libstore/store-api.cc | 8 +-- src/libstore/store-api.hh | 6 +-- src/libstore/worker-protocol.hh | 22 ++++++++ src/nix-build/nix-build.cc | 4 +- src/nix-env/nix-env.cc | 30 ++++++----- src/nix-env/user-env.cc | 9 +++- src/nix-store/nix-store.cc | 11 ++-- src/nix/bundle.cc | 4 +- src/nix/develop.cc | 3 +- src/nix/flake.cc | 5 +- src/nix/profile.cc | 15 +++--- src/nix/run.cc | 2 +- 31 files changed, 364 insertions(+), 126 deletions(-) diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index ca416b9ee..b68c5f6a7 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -679,19 +679,20 @@ Buildables build(ref store, Realise mode, Buildables buildables; - std::vector pathsToBuild; + std::vector pathsToBuild; for (auto & i : installables) { for (auto & b : i->toBuildables()) { std::visit(overloaded { [&](BuildableOpaque bo) { - pathsToBuild.push_back({bo.path}); + pathsToBuild.push_back(bo); }, [&](BuildableFromDrv bfd) { StringSet outputNames; for (auto & output : bfd.outputs) outputNames.insert(output.first); - pathsToBuild.push_back({bfd.drvPath, outputNames}); + pathsToBuild.push_back( + BuildableReqFromDrv{bfd.drvPath, outputNames}); }, }, b); buildables.push_back(std::move(b)); diff --git a/src/libexpr/get-drvs.cc b/src/libexpr/get-drvs.cc index 7793f26ff..f774e6493 100644 --- a/src/libexpr/get-drvs.cc +++ b/src/libexpr/get-drvs.cc @@ -2,6 +2,7 @@ #include "util.hh" #include "eval-inline.hh" #include "store-api.hh" +#include "path-with-outputs.hh" #include #include diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 1d1afa768..24bc34b74 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -35,7 +35,7 @@ InvalidPathError::InvalidPathError(const Path & path) : void EvalState::realiseContext(const PathSet & context) { - std::vector drvs; + std::vector drvs; for (auto & i : context) { auto [ctxS, outputName] = decodeContext(i); @@ -43,7 +43,7 @@ void EvalState::realiseContext(const PathSet & context) if (!store->isValidPath(ctx)) throw InvalidPathError(store->printStorePath(ctx)); if (!outputName.empty() && ctx.isDerivation()) { - drvs.push_back(StorePathWithOutputs{ctx, {outputName}}); + drvs.push_back({ctx, {outputName}}); } } @@ -51,14 +51,16 @@ void EvalState::realiseContext(const PathSet & context) if (!evalSettings.enableImportFromDerivation) throw EvalError("attempted to realize '%1%' during evaluation but 'allow-import-from-derivation' is false", - store->printStorePath(drvs.begin()->path)); + store->printStorePath(drvs.begin()->drvPath)); /* For performance, prefetch all substitute info. */ StorePathSet willBuild, willSubstitute, unknown; uint64_t downloadSize, narSize; - store->queryMissing(drvs, willBuild, willSubstitute, unknown, downloadSize, narSize); + std::vector buildReqs; + for (auto & d : drvs) buildReqs.emplace_back(BuildableReq { d }); + store->queryMissing(buildReqs, willBuild, willSubstitute, unknown, downloadSize, narSize); - store->buildPaths(drvs); + store->buildPaths(buildReqs); /* Add the output of this derivations to the allowed paths. */ diff --git a/src/libmain/shared.cc b/src/libmain/shared.cc index 5baaff3e9..20027e099 100644 --- a/src/libmain/shared.cc +++ b/src/libmain/shared.cc @@ -36,7 +36,7 @@ void printGCWarning() } -void printMissing(ref store, const std::vector & paths, Verbosity lvl) +void printMissing(ref store, const std::vector & paths, Verbosity lvl) { uint64_t downloadSize, narSize; StorePathSet willBuild, willSubstitute, unknown; diff --git a/src/libmain/shared.hh b/src/libmain/shared.hh index edc7b5efa..18e0fb57d 100644 --- a/src/libmain/shared.hh +++ b/src/libmain/shared.hh @@ -4,6 +4,7 @@ #include "args.hh" #include "common-args.hh" #include "path.hh" +#include "buildable.hh" #include @@ -42,7 +43,7 @@ struct StorePathWithOutputs; void printMissing( ref store, - const std::vector & paths, + const std::vector & paths, Verbosity lvl = lvlInfo); void printMissing(ref store, const StorePathSet & willBuild, diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 2e7be517e..8680d0bce 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -73,7 +73,7 @@ DerivationGoal::DerivationGoal(const StorePath & drvPath, state = &DerivationGoal::getDerivation; name = fmt( "building of '%s' from .drv file", - StorePathWithOutputs { drvPath, wantedOutputs }.to_string(worker.store)); + to_string(worker.store, BuildableReqFromDrv { drvPath, wantedOutputs })); trace("created"); mcExpectedBuilds = std::make_unique>(worker.expectedBuilds); @@ -94,7 +94,7 @@ DerivationGoal::DerivationGoal(const StorePath & drvPath, const BasicDerivation state = &DerivationGoal::haveDerivation; name = fmt( "building of '%s' from in-memory derivation", - StorePathWithOutputs { drvPath, drv.outputNames() }.to_string(worker.store)); + to_string(worker.store, BuildableReqFromDrv { drvPath, drv.outputNames() })); trace("created"); mcExpectedBuilds = std::make_unique>(worker.expectedBuilds); diff --git a/src/libstore/build/entry-points.cc b/src/libstore/build/entry-points.cc index 686364440..d1973d78b 100644 --- a/src/libstore/build/entry-points.cc +++ b/src/libstore/build/entry-points.cc @@ -6,16 +6,20 @@ namespace nix { -void Store::buildPaths(const std::vector & drvPaths, BuildMode buildMode) +void Store::buildPaths(const std::vector & reqs, BuildMode buildMode) { Worker worker(*this); Goals goals; - for (auto & path : drvPaths) { - if (path.path.isDerivation()) - goals.insert(worker.makeDerivationGoal(path.path, path.outputs, buildMode)); - else - goals.insert(worker.makePathSubstitutionGoal(path.path, buildMode == bmRepair ? Repair : NoRepair)); + for (auto & br : reqs) { + std::visit(overloaded { + [&](BuildableReqFromDrv bfd) { + goals.insert(worker.makeDerivationGoal(bfd.drvPath, bfd.outputs, buildMode)); + }, + [&](BuildableOpaque bo) { + goals.insert(worker.makePathSubstitutionGoal(bo.path, buildMode == bmRepair ? Repair : NoRepair)); + }, + }, br); } worker.run(goals); diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 8ef43c225..c245527c9 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -1190,6 +1190,26 @@ void LocalDerivationGoal::writeStructuredAttrs() chownToBuilder(tmpDir + "/.attrs.sh"); } + +static StorePath pathPartOfReq(const BuildableReq & req) +{ + return std::visit(overloaded { + [&](BuildableOpaque bo) { + return bo.path; + }, + [&](BuildableReqFromDrv bfd) { + return bfd.drvPath; + }, + }, req); +} + + +bool LocalDerivationGoal::isAllowed(const BuildableReq & req) +{ + return this->isAllowed(pathPartOfReq(req)); +} + + struct RestrictedStoreConfig : virtual LocalFSStoreConfig { using LocalFSStoreConfig::LocalFSStoreConfig; @@ -1312,25 +1332,27 @@ struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual Lo // an allowed derivation { throw Error("queryRealisation"); } - void buildPaths(const std::vector & paths, BuildMode buildMode) override + void buildPaths(const std::vector & paths, BuildMode buildMode) override { if (buildMode != bmNormal) throw Error("unsupported build mode"); StorePathSet newPaths; - for (auto & path : paths) { - if (!goal.isAllowed(path.path)) - throw InvalidPath("cannot build unknown path '%s' in recursive Nix", printStorePath(path.path)); + for (auto & req : paths) { + if (!goal.isAllowed(req)) + throw InvalidPath("cannot build '%s' in recursive Nix because path is unknown", to_string(*next, req)); } next->buildPaths(paths, buildMode); for (auto & path : paths) { - if (!path.path.isDerivation()) continue; - auto outputs = next->queryDerivationOutputMap(path.path); - for (auto & output : outputs) - if (wantOutput(output.first, path.outputs)) - newPaths.insert(output.second); + auto p = std::get_if(&path); + if (!p) continue; + auto & bfd = *p; + auto outputs = next->queryDerivationOutputMap(bfd.drvPath); + for (auto & [outputName, outputPath] : outputs) + if (wantOutput(outputName, bfd.outputs)) + newPaths.insert(outputPath); } StorePathSet closure; @@ -1358,7 +1380,7 @@ struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual Lo void addSignatures(const StorePath & storePath, const StringSet & sigs) override { unsupported("addSignatures"); } - void queryMissing(const std::vector & targets, + void queryMissing(const std::vector & targets, StorePathSet & willBuild, StorePathSet & willSubstitute, StorePathSet & unknown, uint64_t & downloadSize, uint64_t & narSize) override { @@ -1366,12 +1388,12 @@ struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual Lo client about what paths will be built/substituted or are already present. Probably not a big deal. */ - std::vector allowed; - for (auto & path : targets) { - if (goal.isAllowed(path.path)) - allowed.emplace_back(path); + std::vector allowed; + for (auto & req : targets) { + if (goal.isAllowed(req)) + allowed.emplace_back(req); else - unknown.insert(path.path); + unknown.insert(pathPartOfReq(req)); } next->queryMissing(allowed, willBuild, willSubstitute, diff --git a/src/libstore/build/local-derivation-goal.hh b/src/libstore/build/local-derivation-goal.hh index 47b818a8b..edb93f84e 100644 --- a/src/libstore/build/local-derivation-goal.hh +++ b/src/libstore/build/local-derivation-goal.hh @@ -116,6 +116,7 @@ struct LocalDerivationGoal : public DerivationGoal { return inputPaths.count(path) || addedPaths.count(path); } + bool isAllowed(const BuildableReq & req); friend struct RestrictedStore; diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index 616b17e61..fef4cb0cb 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -226,14 +226,14 @@ void Worker::waitForAWhile(GoalPtr goal) void Worker::run(const Goals & _topGoals) { - std::vector topPaths; + std::vector topPaths; for (auto & i : _topGoals) { topGoals.insert(i); if (auto goal = dynamic_cast(i.get())) { - topPaths.push_back({goal->drvPath, goal->wantedOutputs}); + topPaths.push_back(BuildableReqFromDrv{goal->drvPath, goal->wantedOutputs}); } else if (auto goal = dynamic_cast(i.get())) { - topPaths.push_back({goal->storePath}); + topPaths.push_back(BuildableOpaque{goal->storePath}); } } diff --git a/src/libstore/buildable.cc b/src/libstore/buildable.cc index 5cba45b1d..63ca1779e 100644 --- a/src/libstore/buildable.cc +++ b/src/libstore/buildable.cc @@ -11,6 +11,7 @@ nlohmann::json BuildableOpaque::toJSON(ref store) const { return res; } +template<> nlohmann::json BuildableFromDrv::toJSON(ref store) const { nlohmann::json res; res["drvPath"] = store->printStorePath(drvPath); @@ -30,4 +31,50 @@ nlohmann::json buildablesToJSON(const Buildables & buildables, ref store) return res; } + +std::string BuildableOpaque::to_string(const Store & store) const { + return store.printStorePath(path); +} + +template<> +std::string BuildableReqFromDrv::to_string(const Store & store) const { + return store.printStorePath(drvPath) + + "!" + + (outputs.empty() ? std::string { "*" } : concatStringsSep(",", outputs)); +} + +std::string to_string(const Store & store, const BuildableReq & req) +{ + return std::visit( + [&](const auto & req) { return req.to_string(store); }, + req); +} + + +BuildableOpaque BuildableOpaque::parse(const Store & store, std::string_view s) +{ + return {store.parseStorePath(s)}; +} + +template<> +BuildableReqFromDrv BuildableReqFromDrv::parse(const Store & store, std::string_view s) +{ + size_t n = s.find("!"); + assert(n != s.npos); + auto drvPath = store.parseStorePath(s.substr(0, n)); + auto outputsS = s.substr(n + 1); + std::set outputs; + if (outputsS != "*") + outputs = tokenizeString>(outputsS); + return {drvPath, outputs}; +} + +BuildableReq parseBuildableReq(const Store & store, std::string_view s) +{ + size_t n = s.find("!"); + return n == s.npos + ? (BuildableReq) BuildableOpaque::parse(store, s) + : (BuildableReq) BuildableReqFromDrv::parse(store, s); +} + } diff --git a/src/libstore/buildable.hh b/src/libstore/buildable.hh index 6177237be..db78316bd 100644 --- a/src/libstore/buildable.hh +++ b/src/libstore/buildable.hh @@ -2,6 +2,7 @@ #include "util.hh" #include "path.hh" +#include "path.hh" #include @@ -13,19 +14,37 @@ class Store; struct BuildableOpaque { StorePath path; + nlohmann::json toJSON(ref store) const; + std::string to_string(const Store & store) const; + static BuildableOpaque parse(const Store & store, std::string_view); }; -struct BuildableFromDrv { +template +struct BuildableForFromDrv { StorePath drvPath; - std::map> outputs; + Outputs outputs; + nlohmann::json toJSON(ref store) const; + std::string to_string(const Store & store) const; + static BuildableForFromDrv parse(const Store & store, std::string_view); }; -typedef std::variant< +template +using BuildableFor = std::variant< BuildableOpaque, - BuildableFromDrv -> Buildable; + BuildableForFromDrv +>; + +typedef BuildableForFromDrv> BuildableReqFromDrv; +typedef BuildableFor> BuildableReq; + +std::string to_string(const Store & store, const BuildableReq &); + +BuildableReq parseBuildableReq(const Store & store, std::string_view); + +typedef BuildableForFromDrv>> BuildableFromDrv; +typedef BuildableFor>> Buildable; typedef std::vector Buildables; diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 48706bff8..6b527dcb2 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -2,6 +2,7 @@ #include "monitor-fd.hh" #include "worker-protocol.hh" #include "store-api.hh" +#include "path-with-outputs.hh" #include "finally.hh" #include "affinity.hh" #include "archive.hh" @@ -259,6 +260,18 @@ static void writeValidPathInfo( } } +static std::vector readBuildableReqs(Store & store, unsigned int clientVersion, Source & from) +{ + std::vector reqs; + if (GET_PROTOCOL_MINOR(clientVersion) >= 29) { + reqs = worker_proto::read(store, from, Phantom> {}); + } else { + for (auto & s : readStrings(from)) + reqs.push_back(parsePathWithOutputs(store, s).toBuildableReq()); + } + return reqs; +} + static void performOp(TunnelLogger * logger, ref store, TrustedFlag trusted, RecursiveFlag recursive, unsigned int clientVersion, Source & from, BufferedSink & to, unsigned int op) @@ -493,9 +506,7 @@ static void performOp(TunnelLogger * logger, ref store, } case wopBuildPaths: { - std::vector drvs; - for (auto & s : readStrings(from)) - drvs.push_back(parsePathWithOutputs(*store, s)); + auto drvs = readBuildableReqs(*store, clientVersion, from); BuildMode mode = bmNormal; if (GET_PROTOCOL_MINOR(clientVersion) >= 15) { mode = (BuildMode) readInt(from); @@ -859,9 +870,7 @@ static void performOp(TunnelLogger * logger, ref store, } case wopQueryMissing: { - std::vector targets; - for (auto & s : readStrings(from)) - targets.push_back(parsePathWithOutputs(*store, s)); + auto targets = readBuildableReqs(*store, clientVersion, from); logger->startWork(); StorePathSet willBuild, willSubstitute, unknown; uint64_t downloadSize, narSize; diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index a9f53bad9..1cb977be6 100644 --- a/src/libstore/legacy-ssh-store.cc +++ b/src/libstore/legacy-ssh-store.cc @@ -3,6 +3,7 @@ #include "remote-store.hh" #include "serve-protocol.hh" #include "store-api.hh" +#include "path-with-outputs.hh" #include "worker-protocol.hh" #include "ssh.hh" #include "derivations.hh" @@ -266,14 +267,23 @@ public: return status; } - void buildPaths(const std::vector & drvPaths, BuildMode buildMode) override + void buildPaths(const std::vector & drvPaths, BuildMode buildMode) override { auto conn(connections->get()); conn->to << cmdBuildPaths; Strings ss; - for (auto & p : drvPaths) - ss.push_back(p.to_string(*this)); + for (auto & p : drvPaths) { + auto sOrDrvPath = StorePathWithOutputs::tryFromBuildableReq(p); + std::visit(overloaded { + [&](StorePathWithOutputs s) { + ss.push_back(s.to_string(*this)); + }, + [&](StorePath drvPath) { + throw Error("wanted to fetch '%s' but the legacy ssh protocol doesn't support merely substituting drv files via the build paths command. It would build them instead. Try using ssh-ng://", printStorePath(drvPath)); + }, + }, sOrDrvPath); + } conn->to << ss; putBuildSettings(*conn); diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index f58816ad8..e702a4f9e 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -117,7 +117,7 @@ std::optional getDerivationCA(const BasicDerivation & drv) return std::nullopt; } -void Store::queryMissing(const std::vector & targets, +void Store::queryMissing(const std::vector & targets, StorePathSet & willBuild_, StorePathSet & willSubstitute_, StorePathSet & unknown_, uint64_t & downloadSize_, uint64_t & narSize_) { @@ -145,7 +145,7 @@ void Store::queryMissing(const std::vector & targets, Sync state_(State{{}, unknown_, willSubstitute_, willBuild_, downloadSize_, narSize_}); - std::function doPath; + std::function doPath; auto mustBuildDrv = [&](const StorePath & drvPath, const Derivation & drv) { { @@ -154,7 +154,7 @@ void Store::queryMissing(const std::vector & targets, } for (auto & i : drv.inputDrvs) - pool.enqueue(std::bind(doPath, StorePathWithOutputs { i.first, i.second })); + pool.enqueue(std::bind(doPath, BuildableReqFromDrv { i.first, i.second })); }; auto checkOutput = [&]( @@ -177,24 +177,25 @@ void Store::queryMissing(const std::vector & targets, drvState->outPaths.insert(outPath); if (!drvState->left) { for (auto & path : drvState->outPaths) - pool.enqueue(std::bind(doPath, StorePathWithOutputs { path } )); + pool.enqueue(std::bind(doPath, BuildableOpaque { path } )); } } } }; - doPath = [&](const StorePathWithOutputs & path) { + doPath = [&](const BuildableReq & req) { { auto state(state_.lock()); - if (!state->done.insert(path.to_string(*this)).second) return; + if (!state->done.insert(to_string(*this, req)).second) return; } - if (path.path.isDerivation()) { - if (!isValidPath(path.path)) { + std::visit(overloaded { + [&](BuildableReqFromDrv bfd) { + if (!isValidPath(bfd.drvPath)) { // FIXME: we could try to substitute the derivation. auto state(state_.lock()); - state->unknown.insert(path.path); + state->unknown.insert(bfd.drvPath); return; } @@ -202,52 +203,54 @@ void Store::queryMissing(const std::vector & targets, /* true for regular derivations, and CA derivations for which we have a trust mapping for all wanted outputs. */ auto knownOutputPaths = true; - for (auto & [outputName, pathOpt] : queryPartialDerivationOutputMap(path.path)) { + for (auto & [outputName, pathOpt] : queryPartialDerivationOutputMap(bfd.drvPath)) { if (!pathOpt) { knownOutputPaths = false; break; } - if (wantOutput(outputName, path.outputs) && !isValidPath(*pathOpt)) + if (wantOutput(outputName, bfd.outputs) && !isValidPath(*pathOpt)) invalid.insert(*pathOpt); } if (knownOutputPaths && invalid.empty()) return; - auto drv = make_ref(derivationFromPath(path.path)); - ParsedDerivation parsedDrv(StorePath(path.path), *drv); + auto drv = make_ref(derivationFromPath(bfd.drvPath)); + ParsedDerivation parsedDrv(StorePath(bfd.drvPath), *drv); if (knownOutputPaths && settings.useSubstitutes && parsedDrv.substitutesAllowed()) { auto drvState = make_ref>(DrvState(invalid.size())); for (auto & output : invalid) - pool.enqueue(std::bind(checkOutput, path.path, drv, output, drvState)); + pool.enqueue(std::bind(checkOutput, bfd.drvPath, drv, output, drvState)); } else - mustBuildDrv(path.path, *drv); + mustBuildDrv(bfd.drvPath, *drv); - } else { + }, + [&](BuildableOpaque bo) { - if (isValidPath(path.path)) return; + if (isValidPath(bo.path)) return; SubstitutablePathInfos infos; - querySubstitutablePathInfos({{path.path, std::nullopt}}, infos); + querySubstitutablePathInfos({{bo.path, std::nullopt}}, infos); if (infos.empty()) { auto state(state_.lock()); - state->unknown.insert(path.path); + state->unknown.insert(bo.path); return; } - auto info = infos.find(path.path); + auto info = infos.find(bo.path); assert(info != infos.end()); { auto state(state_.lock()); - state->willSubstitute.insert(path.path); + state->willSubstitute.insert(bo.path); state->downloadSize += info->second.downloadSize; state->narSize += info->second.narSize; } for (auto & ref : info->second.references) - pool.enqueue(std::bind(doPath, StorePathWithOutputs { ref })); - } + pool.enqueue(std::bind(doPath, BuildableOpaque { ref })); + }, + }, req); }; for (auto & path : targets) diff --git a/src/libstore/path-with-outputs.cc b/src/libstore/path-with-outputs.cc index a898ad09c..353286ac6 100644 --- a/src/libstore/path-with-outputs.cc +++ b/src/libstore/path-with-outputs.cc @@ -1,3 +1,4 @@ +#include "path-with-outputs.hh" #include "store-api.hh" namespace nix { @@ -10,6 +11,40 @@ std::string StorePathWithOutputs::to_string(const Store & store) const } +BuildableReq StorePathWithOutputs::toBuildableReq() const +{ + if (!outputs.empty() || path.isDerivation()) + return BuildableReqFromDrv { path, outputs }; + else + return BuildableOpaque { path }; +} + + +std::vector toBuildableReqs(const std::vector ss) +{ + std::vector reqs; + for (auto & s : ss) reqs.push_back(s.toBuildableReq()); + return reqs; +} + + +std::variant StorePathWithOutputs::tryFromBuildableReq(const BuildableReq & p) +{ + return std::visit(overloaded { + [&](BuildableOpaque bo) -> std::variant { + if (bo.path.isDerivation()) { + // drv path gets interpreted as "build", not "get drv file itself" + return bo.path; + } + return StorePathWithOutputs { bo.path }; + }, + [&](BuildableReqFromDrv bfd) -> std::variant { + return StorePathWithOutputs { bfd.drvPath, bfd.outputs }; + }, + }, p); +} + + std::pair parsePathWithOutputs(std::string_view s) { size_t n = s.find("!"); diff --git a/src/libstore/path-with-outputs.hh b/src/libstore/path-with-outputs.hh index 0e34b5aa1..870cac08e 100644 --- a/src/libstore/path-with-outputs.hh +++ b/src/libstore/path-with-outputs.hh @@ -1,6 +1,9 @@ #pragma once +#include + #include "path.hh" +#include "buildable.hh" namespace nix { @@ -10,8 +13,14 @@ struct StorePathWithOutputs std::set outputs; std::string to_string(const Store & store) const; + + BuildableReq toBuildableReq() const; + + static std::variant tryFromBuildableReq(const BuildableReq &); }; +std::vector toBuildableReqs(const std::vector); + std::pair parsePathWithOutputs(std::string_view s); class Store; diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index ccf095dc2..de1c95ed6 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -1,5 +1,6 @@ #include "serialise.hh" #include "util.hh" +#include "path-with-outputs.hh" #include "remote-fs-accessor.hh" #include "remote-store.hh" #include "worker-protocol.hh" @@ -50,6 +51,19 @@ void write(const Store & store, Sink & out, const ContentAddress & ca) out << renderContentAddress(ca); } + +BuildableReq read(const Store & store, Source & from, Phantom _) +{ + auto s = readString(from); + return parseBuildableReq(store, s); +} + +void write(const Store & store, Sink & out, const BuildableReq & req) +{ + out << to_string(store, req); +} + + Realisation read(const Store & store, Source & from, Phantom _) { std::string rawInput = readString(from); @@ -58,8 +72,12 @@ Realisation read(const Store & store, Source & from, Phantom _) "remote-protocol" ); } + void write(const Store & store, Sink & out, const Realisation & realisation) -{ out << realisation.toJSON().dump(); } +{ + out << realisation.toJSON().dump(); +} + DrvOutput read(const Store & store, Source & from, Phantom _) { @@ -652,16 +670,36 @@ std::optional RemoteStore::queryRealisation(const DrvOutput & return {Realisation{.id = id, .outPath = *outPaths.begin()}}; } +static void writeBuildableReqs(RemoteStore & store, ConnectionHandle & conn, const std::vector & reqs) +{ + if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 29) { + worker_proto::write(store, conn->to, reqs); + } else { + Strings ss; + for (auto & p : reqs) { + auto sOrDrvPath = StorePathWithOutputs::tryFromBuildableReq(p); + std::visit(overloaded { + [&](StorePathWithOutputs s) { + ss.push_back(s.to_string(store)); + }, + [&](StorePath drvPath) { + throw Error("trying to request '%s', but daemon protocol %d.%d is too old (< 1.29) to request a derivation file", + store.printStorePath(drvPath), + GET_PROTOCOL_MAJOR(conn->daemonVersion), + GET_PROTOCOL_MINOR(conn->daemonVersion)); + }, + }, sOrDrvPath); + } + conn->to << ss; + } +} -void RemoteStore::buildPaths(const std::vector & drvPaths, BuildMode buildMode) +void RemoteStore::buildPaths(const std::vector & drvPaths, BuildMode buildMode) { auto conn(getConnection()); conn->to << wopBuildPaths; assert(GET_PROTOCOL_MINOR(conn->daemonVersion) >= 13); - Strings ss; - for (auto & p : drvPaths) - ss.push_back(p.to_string(*this)); - conn->to << ss; + writeBuildableReqs(*this, conn, drvPaths); if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 15) conn->to << buildMode; else @@ -800,7 +838,7 @@ void RemoteStore::addSignatures(const StorePath & storePath, const StringSet & s } -void RemoteStore::queryMissing(const std::vector & targets, +void RemoteStore::queryMissing(const std::vector & targets, StorePathSet & willBuild, StorePathSet & willSubstitute, StorePathSet & unknown, uint64_t & downloadSize, uint64_t & narSize) { @@ -811,10 +849,7 @@ void RemoteStore::queryMissing(const std::vector & targets // to prevent a deadlock. goto fallback; conn->to << wopQueryMissing; - Strings ss; - for (auto & p : targets) - ss.push_back(p.to_string(*this)); - conn->to << ss; + writeBuildableReqs(*this, conn, targets); conn.processStderr(); willBuild = worker_proto::read(*this, conn->from, Phantom {}); willSubstitute = worker_proto::read(*this, conn->from, Phantom {}); diff --git a/src/libstore/remote-store.hh b/src/libstore/remote-store.hh index b3a9910a3..20d366038 100644 --- a/src/libstore/remote-store.hh +++ b/src/libstore/remote-store.hh @@ -85,7 +85,7 @@ public: std::optional queryRealisation(const DrvOutput &) override; - void buildPaths(const std::vector & paths, BuildMode buildMode) override; + void buildPaths(const std::vector & paths, BuildMode buildMode) override; BuildResult buildDerivation(const StorePath & drvPath, const BasicDerivation & drv, BuildMode buildMode) override; @@ -108,7 +108,7 @@ public: void addSignatures(const StorePath & storePath, const StringSet & sigs) override; - void queryMissing(const std::vector & targets, + void queryMissing(const std::vector & targets, StorePathSet & willBuild, StorePathSet & willSubstitute, StorePathSet & unknown, uint64_t & downloadSize, uint64_t & narSize) override; diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index e3500872c..8b60bdc62 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -529,10 +529,10 @@ void Store::queryPathInfo(const StorePath & storePath, void Store::substitutePaths(const StorePathSet & paths) { - std::vector paths2; + std::vector paths2; for (auto & path : paths) if (!path.isDerivation()) - paths2.push_back({path}); + paths2.push_back(BuildableOpaque{path}); uint64_t downloadSize, narSize; StorePathSet willBuild, willSubstitute, unknown; queryMissing(paths2, @@ -540,8 +540,8 @@ void Store::substitutePaths(const StorePathSet & paths) if (!willSubstitute.empty()) try { - std::vector subs; - for (auto & p : willSubstitute) subs.push_back({p}); + std::vector subs; + for (auto & p : willSubstitute) subs.push_back(BuildableOpaque{p}); buildPaths(subs); } catch (Error & e) { logWarning(e.info()); diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index da7ac4460..59d0983df 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -2,7 +2,7 @@ #include "realisation.hh" #include "path.hh" -#include "path-with-outputs.hh" +#include "buildable.hh" #include "hash.hh" #include "content-address.hh" #include "serialise.hh" @@ -494,7 +494,7 @@ public: recursively building any sub-derivations. For inputs that are not derivations, substitute them. */ virtual void buildPaths( - const std::vector & paths, + const std::vector & paths, BuildMode buildMode = bmNormal); /* Build a single non-materialized derivation (i.e. not from an @@ -656,7 +656,7 @@ public: /* Given a set of paths that are to be built, return the set of derivations that will be built, and the set of output paths that will be substituted. */ - virtual void queryMissing(const std::vector & targets, + virtual void queryMissing(const std::vector & targets, StorePathSet & willBuild, StorePathSet & willSubstitute, StorePathSet & unknown, uint64_t & downloadSize, uint64_t & narSize); diff --git a/src/libstore/worker-protocol.hh b/src/libstore/worker-protocol.hh index be071dd78..0255726ac 100644 --- a/src/libstore/worker-protocol.hh +++ b/src/libstore/worker-protocol.hh @@ -86,9 +86,11 @@ namespace worker_proto { MAKE_WORKER_PROTO(, std::string); MAKE_WORKER_PROTO(, StorePath); MAKE_WORKER_PROTO(, ContentAddress); +MAKE_WORKER_PROTO(, BuildableReq); MAKE_WORKER_PROTO(, Realisation); MAKE_WORKER_PROTO(, DrvOutput); +MAKE_WORKER_PROTO(template, std::vector); MAKE_WORKER_PROTO(template, std::set); #define X_ template @@ -113,6 +115,26 @@ MAKE_WORKER_PROTO(X_, Y_); MAKE_WORKER_PROTO(, std::optional); MAKE_WORKER_PROTO(, std::optional); +template +std::vector read(const Store & store, Source & from, Phantom> _) +{ + std::vector resSet; + auto size = readNum(from); + while (size--) { + resSet.push_back(read(store, from, Phantom {})); + } + return resSet; +} + +template +void write(const Store & store, Sink & out, const std::vector & resSet) +{ + out << resSet.size(); + for (auto & key : resSet) { + write(store, out, key); + } +} + template std::set read(const Store & store, Source & from, Phantom> _) { diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index 65b85b304..6f8a61261 100755 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -12,6 +12,7 @@ #include "affinity.hh" #include "util.hh" #include "shared.hh" +#include "path-with-outputs.hh" #include "eval.hh" #include "eval-inline.hh" #include "get-drvs.hh" @@ -321,7 +322,8 @@ static void main_nix_build(int argc, char * * argv) state->printStats(); - auto buildPaths = [&](const std::vector & paths) { + auto buildPaths = [&](const std::vector & paths0) { + auto paths = toBuildableReqs(paths0); /* Note: we do this even when !printMissing to efficiently fetch binary cache data. */ uint64_t downloadSize, narSize; diff --git a/src/nix-env/nix-env.cc b/src/nix-env/nix-env.cc index 0f10a4cbb..af1c69b87 100644 --- a/src/nix-env/nix-env.cc +++ b/src/nix-env/nix-env.cc @@ -6,6 +6,7 @@ #include "globals.hh" #include "names.hh" #include "profiles.hh" +#include "path-with-outputs.hh" #include "shared.hh" #include "store-api.hh" #include "local-fs-store.hh" @@ -418,13 +419,13 @@ static void queryInstSources(EvalState & state, static void printMissing(EvalState & state, DrvInfos & elems) { - std::vector targets; + std::vector targets; for (auto & i : elems) { Path drvPath = i.queryDrvPath(); if (drvPath != "") - targets.push_back({state.store->parseStorePath(drvPath)}); + targets.push_back(BuildableReqFromDrv{state.store->parseStorePath(drvPath)}); else - targets.push_back({state.store->parseStorePath(i.queryOutPath())}); + targets.push_back(BuildableOpaque{state.store->parseStorePath(i.queryOutPath())}); } printMissing(state.store, targets); @@ -693,17 +694,18 @@ static void opSet(Globals & globals, Strings opFlags, Strings opArgs) if (globals.forceName != "") drv.setName(globals.forceName); - if (drv.queryDrvPath() != "") { - std::vector paths{{globals.state->store->parseStorePath(drv.queryDrvPath())}}; - printMissing(globals.state->store, paths); - if (globals.dryRun) return; - globals.state->store->buildPaths(paths, globals.state->repair ? bmRepair : bmNormal); - } else { - printMissing(globals.state->store, - {{globals.state->store->parseStorePath(drv.queryOutPath())}}); - if (globals.dryRun) return; - globals.state->store->ensurePath(globals.state->store->parseStorePath(drv.queryOutPath())); - } + std::vector paths { + (drv.queryDrvPath() != "") + ? (BuildableReq) (BuildableReqFromDrv { + globals.state->store->parseStorePath(drv.queryDrvPath()) + }) + : (BuildableReq) (BuildableOpaque { + globals.state->store->parseStorePath(drv.queryOutPath()) + }), + }; + printMissing(globals.state->store, paths); + if (globals.dryRun) return; + globals.state->store->buildPaths(paths, globals.state->repair ? bmRepair : bmNormal); debug(format("switching to new user environment")); Path generation = createGeneration( diff --git a/src/nix-env/user-env.cc b/src/nix-env/user-env.cc index 168ac492b..0ccf960fb 100644 --- a/src/nix-env/user-env.cc +++ b/src/nix-env/user-env.cc @@ -2,6 +2,7 @@ #include "util.hh" #include "derivations.hh" #include "store-api.hh" +#include "path-with-outputs.hh" #include "local-fs-store.hh" #include "globals.hh" #include "shared.hh" @@ -41,7 +42,9 @@ bool createUserEnv(EvalState & state, DrvInfos & elems, drvsToBuild.push_back({state.store->parseStorePath(i.queryDrvPath())}); debug(format("building user environment dependencies")); - state.store->buildPaths(drvsToBuild, state.repair ? bmRepair : bmNormal); + state.store->buildPaths( + toBuildableReqs(drvsToBuild), + state.repair ? bmRepair : bmNormal); /* Construct the whole top level derivation. */ StorePathSet references; @@ -136,7 +139,9 @@ bool createUserEnv(EvalState & state, DrvInfos & elems, debug("building user environment"); std::vector topLevelDrvs; topLevelDrvs.push_back({topLevelDrv}); - state.store->buildPaths(topLevelDrvs, state.repair ? bmRepair : bmNormal); + state.store->buildPaths( + toBuildableReqs(topLevelDrvs), + state.repair ? bmRepair : bmNormal); /* Switch the current user environment to the output path. */ auto store2 = state.store.dynamic_pointer_cast(); diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index bfd1299fc..21c1e547b 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -10,6 +10,7 @@ #include "worker-protocol.hh" #include "graphml.hh" #include "legacy.hh" +#include "path-with-outputs.hh" #include #include @@ -62,7 +63,7 @@ static PathSet realisePath(StorePathWithOutputs path, bool build = true) auto store2 = std::dynamic_pointer_cast(store); if (path.path.isDerivation()) { - if (build) store->buildPaths({path}); + if (build) store->buildPaths({path.toBuildableReq()}); auto outputPaths = store->queryDerivationOutputMap(path.path); Derivation drv = store->derivationFromPath(path.path); rootNr++; @@ -132,7 +133,9 @@ static void opRealise(Strings opFlags, Strings opArgs) uint64_t downloadSize, narSize; StorePathSet willBuild, willSubstitute, unknown; - store->queryMissing(paths, willBuild, willSubstitute, unknown, downloadSize, narSize); + store->queryMissing( + toBuildableReqs(paths), + willBuild, willSubstitute, unknown, downloadSize, narSize); if (ignoreUnknown) { std::vector paths2; @@ -148,7 +151,7 @@ static void opRealise(Strings opFlags, Strings opArgs) if (dryRun) return; /* Build all paths at the same time to exploit parallelism. */ - store->buildPaths(paths, buildMode); + store->buildPaths(toBuildableReqs(paths), buildMode); if (!ignoreUnknown) for (auto & i : paths) { @@ -879,7 +882,7 @@ static void opServe(Strings opFlags, Strings opArgs) try { MonitorFdHup monitor(in.fd); - store->buildPaths(paths); + store->buildPaths(toBuildableReqs(paths)); out << 0; } catch (Error & e) { assert(e.status); diff --git a/src/nix/bundle.cc b/src/nix/bundle.cc index 48f4eb6e3..e86fbb3f7 100644 --- a/src/nix/bundle.cc +++ b/src/nix/bundle.cc @@ -70,7 +70,7 @@ struct CmdBundle : InstallableCommand auto evalState = getEvalState(); auto app = installable->toApp(*evalState); - store->buildPaths(app.context); + store->buildPaths(toBuildableReqs(app.context)); auto [bundlerFlakeRef, bundlerName] = parseFlakeRefWithFragment(bundler, absPath(".")); const flake::LockFlags lockFlags{ .writeLockFile = false }; @@ -110,7 +110,7 @@ struct CmdBundle : InstallableCommand StorePath outPath = store->parseStorePath(evalState->coerceToPath(*attr2->pos, *attr2->value, context2)); - store->buildPaths({{drvPath}}); + store->buildPaths({ BuildableReqFromDrv { drvPath } }); auto outPathS = store->printStorePath(outPath); diff --git a/src/nix/develop.cc b/src/nix/develop.cc index d0b140570..616e2073e 100644 --- a/src/nix/develop.cc +++ b/src/nix/develop.cc @@ -3,6 +3,7 @@ #include "common-args.hh" #include "shared.hh" #include "store-api.hh" +#include "path-with-outputs.hh" #include "derivations.hh" #include "affinity.hh" #include "progress-bar.hh" @@ -159,7 +160,7 @@ StorePath getDerivationEnvironment(ref store, const StorePath & drvPath) auto shellDrvPath = writeDerivation(*store, drv); /* Build the derivation. */ - store->buildPaths({{shellDrvPath}}); + store->buildPaths({BuildableReqFromDrv{shellDrvPath}}); for (auto & [_0, outputAndOptPath] : drv.outputsAndOptPaths(*store)) { auto & [_1, optPath] = outputAndOptPath; diff --git a/src/nix/flake.cc b/src/nix/flake.cc index a2b6c0303..9d6d22a43 100644 --- a/src/nix/flake.cc +++ b/src/nix/flake.cc @@ -7,6 +7,7 @@ #include "get-drvs.hh" #include "store-api.hh" #include "derivations.hh" +#include "path-with-outputs.hh" #include "attr-path.hh" #include "fetchers.hh" #include "registry.hh" @@ -292,7 +293,7 @@ struct CmdFlakeCheck : FlakeCommand } }; - std::vector drvPaths; + std::vector drvPaths; auto checkApp = [&](const std::string & attrPath, Value & v, const Pos & pos) { try { @@ -461,7 +462,7 @@ struct CmdFlakeCheck : FlakeCommand fmt("%s.%s.%s", name, attr.name, attr2.name), *attr2.value, *attr2.pos); if ((std::string) attr.name == settings.thisSystem.get()) - drvPaths.push_back({drvPath}); + drvPaths.push_back(BuildableReqFromDrv{drvPath}); } } } diff --git a/src/nix/profile.cc b/src/nix/profile.cc index 4d275f577..b96e71844 100644 --- a/src/nix/profile.cc +++ b/src/nix/profile.cc @@ -233,7 +233,7 @@ struct CmdProfileInstall : InstallablesCommand, MixDefaultProfile { ProfileManifest manifest(*getEvalState(), *profile); - std::vector pathsToBuild; + std::vector pathsToBuild; for (auto & installable : installables) { if (auto installable2 = std::dynamic_pointer_cast(installable)) { @@ -249,7 +249,7 @@ struct CmdProfileInstall : InstallablesCommand, MixDefaultProfile attrPath, }; - pathsToBuild.push_back({drv.drvPath, StringSet{drv.outputName}}); + pathsToBuild.push_back(BuildableReqFromDrv{drv.drvPath, StringSet{drv.outputName}}); manifest.elements.emplace_back(std::move(element)); } else { @@ -260,12 +260,15 @@ struct CmdProfileInstall : InstallablesCommand, MixDefaultProfile std::visit(overloaded { [&](BuildableOpaque bo) { - pathsToBuild.push_back({bo.path, {}}); + pathsToBuild.push_back(bo); element.storePaths.insert(bo.path); }, [&](BuildableFromDrv bfd) { + // TODO: Why are we querying if we know the output + // names already? Is it just to figure out what the + // default one is? for (auto & output : store->queryDerivationOutputMap(bfd.drvPath)) { - pathsToBuild.push_back({bfd.drvPath, {output.first}}); + pathsToBuild.push_back(BuildableReqFromDrv{bfd.drvPath, {output.first}}); element.storePaths.insert(output.second); } }, @@ -388,7 +391,7 @@ struct CmdProfileUpgrade : virtual SourceExprCommand, MixDefaultProfile, MixProf auto matchers = getMatchers(store); // FIXME: code duplication - std::vector pathsToBuild; + std::vector pathsToBuild; for (size_t i = 0; i < manifest.elements.size(); ++i) { auto & element(manifest.elements[i]); @@ -423,7 +426,7 @@ struct CmdProfileUpgrade : virtual SourceExprCommand, MixDefaultProfile, MixProf attrPath, }; - pathsToBuild.push_back({drv.drvPath, StringSet{"out"}}); // FIXME + pathsToBuild.push_back(BuildableReqFromDrv{drv.drvPath, {"out"}}); // FIXME } } diff --git a/src/nix/run.cc b/src/nix/run.cc index ec9388234..2e9bb41cc 100644 --- a/src/nix/run.cc +++ b/src/nix/run.cc @@ -182,7 +182,7 @@ struct CmdRun : InstallableCommand, RunCommon auto app = installable->toApp(*state); - state->store->buildPaths(app.context); + state->store->buildPaths(toBuildableReqs(app.context)); Strings allArgs{app.program}; for (auto & i : args) allArgs.push_back(i);