From 761242afa08d5c9280ba6bd63a310b4334b83bb2 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 9 Mar 2022 12:25:35 +0100 Subject: [PATCH] BuildResult: Use DerivedPath --- src/libcmd/installables.cc | 20 ++++++++------- src/libstore/build-result.hh | 6 ++--- src/libstore/build/derivation-goal.cc | 6 ++--- .../build/drv-output-substitution-goal.cc | 8 ++++-- src/libstore/build/entry-points.cc | 15 ++++++----- src/libstore/build/goal.hh | 4 ++- src/libstore/build/substitution-goal.cc | 3 +-- src/libstore/legacy-ssh-store.cc | 4 +-- src/libstore/remote-store.cc | 25 +++++++++---------- 9 files changed, 48 insertions(+), 43 deletions(-) diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index d1ed19acf..b7623d4ba 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -826,15 +826,17 @@ BuiltPaths Installable::build( for (auto & buildResult : store->buildPathsWithResults(pathsToBuild, bMode, evalStore)) { if (!buildResult.success()) buildResult.rethrow(); - if (buildResult.drvPath) { - std::map outputs; - for (auto & path : buildResult.builtOutputs) - outputs.emplace(path.first.outputName, path.second.outPath); - res.push_back(BuiltPath::Built{*buildResult.drvPath, outputs}); - } else if (buildResult.outPath) { - res.push_back(BuiltPath::Opaque{*buildResult.outPath}); - } else - abort(); + std::visit(overloaded { + [&](const DerivedPath::Built & bfd) { + std::map outputs; + for (auto & path : buildResult.builtOutputs) + outputs.emplace(path.first.outputName, path.second.outPath); + res.push_back(BuiltPath::Built { bfd.drvPath, outputs }); + }, + [&](const DerivedPath::Opaque & bo) { + res.push_back(BuiltPath::Opaque { bo.path }); + }, + }, buildResult.path.raw()); } return res; } diff --git a/src/libstore/build-result.hh b/src/libstore/build-result.hh index c16027808..d39d23a76 100644 --- a/src/libstore/build-result.hh +++ b/src/libstore/build-result.hh @@ -64,13 +64,13 @@ struct BuildResult non-determinism.) */ bool isNonDeterministic = false; + /* The derivation we built or the store path we substituted. */ + DerivedPath path; + /* For derivations, the derivation path and the wanted outputs. */ std::optional drvPath; DrvOutputs builtOutputs; - /* For substitutions, the substituted path. */ - std::optional outPath; - /* The start/stop times of the build (or one of the rounds, if it was repeated). */ time_t startTime = 0, stopTime = 0; diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index cd582bb01..325635e2e 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -66,7 +66,7 @@ namespace nix { DerivationGoal::DerivationGoal(const StorePath & drvPath, const StringSet & wantedOutputs, Worker & worker, BuildMode buildMode) - : Goal(worker) + : Goal(worker, DerivedPath::Built { .drvPath = drvPath, .outputs = wantedOutputs }) , useDerivation(true) , drvPath(drvPath) , wantedOutputs(wantedOutputs) @@ -85,7 +85,7 @@ DerivationGoal::DerivationGoal(const StorePath & drvPath, DerivationGoal::DerivationGoal(const StorePath & drvPath, const BasicDerivation & drv, const StringSet & wantedOutputs, Worker & worker, BuildMode buildMode) - : Goal(worker) + : Goal(worker, DerivedPath::Built { .drvPath = drvPath, .outputs = wantedOutputs }) , useDerivation(false) , drvPath(drvPath) , wantedOutputs(wantedOutputs) @@ -509,7 +509,7 @@ void DerivationGoal::inputsRealised() state = &DerivationGoal::tryToBuild; worker.wakeUp(shared_from_this()); - buildResult = BuildResult(); + buildResult = BuildResult { .path = buildResult.path }; } void DerivationGoal::started() diff --git a/src/libstore/build/drv-output-substitution-goal.cc b/src/libstore/build/drv-output-substitution-goal.cc index 946ec1aff..e50292c1e 100644 --- a/src/libstore/build/drv-output-substitution-goal.cc +++ b/src/libstore/build/drv-output-substitution-goal.cc @@ -6,8 +6,12 @@ namespace nix { -DrvOutputSubstitutionGoal::DrvOutputSubstitutionGoal(const DrvOutput& id, Worker & worker, RepairFlag repair, std::optional ca) - : Goal(worker) +DrvOutputSubstitutionGoal::DrvOutputSubstitutionGoal( + const DrvOutput & id, + Worker & worker, + RepairFlag repair, + std::optional ca) + : Goal(worker, DerivedPath::Opaque { StorePath::dummy }) , id(id) { state = &DrvOutputSubstitutionGoal::init; diff --git a/src/libstore/build/entry-points.cc b/src/libstore/build/entry-points.cc index b2f87aa82..bea7363db 100644 --- a/src/libstore/build/entry-points.cc +++ b/src/libstore/build/entry-points.cc @@ -82,17 +82,16 @@ BuildResult Store::buildDerivation(const StorePath & drvPath, const BasicDerivat Worker worker(*this, *this); auto goal = worker.makeBasicDerivationGoal(drvPath, drv, {}, buildMode); - BuildResult result; - try { worker.run(Goals{goal}); - result = goal->buildResult; + return goal->buildResult; } catch (Error & e) { - result.status = BuildResult::MiscFailure; - result.errorMsg = e.msg(); - } - - return result; + return BuildResult { + .status = BuildResult::MiscFailure, + .errorMsg = e.msg(), + .path = DerivedPath::Built { .drvPath = drvPath }, + }; + }; } diff --git a/src/libstore/build/goal.hh b/src/libstore/build/goal.hh index fcf3d0084..07c752bb9 100644 --- a/src/libstore/build/goal.hh +++ b/src/libstore/build/goal.hh @@ -62,7 +62,9 @@ struct Goal : public std::enable_shared_from_this /* Exception containing an error message, if any. */ std::optional ex; - Goal(Worker & worker) : worker(worker) + Goal(Worker & worker, DerivedPath path) + : worker(worker) + , buildResult { .path = std::move(path) } { nrFailed = nrNoSubstituters = nrIncompleteClosure = 0; exitCode = ecBusy; diff --git a/src/libstore/build/substitution-goal.cc b/src/libstore/build/substitution-goal.cc index ec500baf8..31e6dbc9f 100644 --- a/src/libstore/build/substitution-goal.cc +++ b/src/libstore/build/substitution-goal.cc @@ -6,7 +6,7 @@ namespace nix { PathSubstitutionGoal::PathSubstitutionGoal(const StorePath & storePath, Worker & worker, RepairFlag repair, std::optional ca) - : Goal(worker) + : Goal(worker, DerivedPath::Opaque { storePath }) , storePath(storePath) , repair(repair) , ca(ca) @@ -26,7 +26,6 @@ PathSubstitutionGoal::~PathSubstitutionGoal() void PathSubstitutionGoal::done(ExitCode result, BuildResult::Status status) { - buildResult.outPath = storePath; buildResult.status = status; amDone(result); } diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index 16c2b90c6..dd34b19c6 100644 --- a/src/libstore/legacy-ssh-store.cc +++ b/src/libstore/legacy-ssh-store.cc @@ -279,7 +279,7 @@ public: conn->to.flush(); - BuildResult status; + BuildResult status { .path = DerivedPath::Built { .drvPath = drvPath } }; status.status = (BuildResult::Status) readInt(conn->from); conn->from >> status.errorMsg; @@ -317,7 +317,7 @@ public: conn->to.flush(); - BuildResult result; + BuildResult result { .path = DerivedPath::Opaque { StorePath::dummy } }; result.status = (BuildResult::Status) readInt(conn->from); if (!result.success()) { diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index a0883db11..347e32094 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -93,7 +93,8 @@ void write(const Store & store, Sink & out, const DrvOutput & drvOutput) BuildResult read(const Store & store, Source & from, Phantom _) { - BuildResult res; + auto path = worker_proto::read(store, from, Phantom {}); + BuildResult res { .path = path }; res.status = (BuildResult::Status) readInt(from); from >> res.errorMsg @@ -101,14 +102,13 @@ BuildResult read(const Store & store, Source & from, Phantom _) >> res.isNonDeterministic >> res.startTime >> res.stopTime; - res.drvPath = worker_proto::read(store, from, Phantom> {}); res.builtOutputs = worker_proto::read(store, from, Phantom {}); - res.outPath = worker_proto::read(store, from, Phantom> {}); return res; } void write(const Store & store, Sink & to, const BuildResult & res) { + worker_proto::write(store, to, res.path); to << res.status << res.errorMsg @@ -116,9 +116,7 @@ void write(const Store & store, Sink & to, const BuildResult & res) << res.isNonDeterministic << res.startTime << res.stopTime; - worker_proto::write(store, to, res.drvPath); worker_proto::write(store, to, res.builtOutputs); - worker_proto::write(store, to, res.outPath); } @@ -842,15 +840,16 @@ std::vector RemoteStore::buildPathsWithResults( std::visit( overloaded { [&](const DerivedPath::Opaque & bo) { - BuildResult res; - res.status = BuildResult::Substituted; - res.outPath = bo.path; - results.push_back(res); + results.push_back(BuildResult { + .status = BuildResult::Substituted, + .path = bo, + }); }, [&](const DerivedPath::Built & bfd) { - BuildResult res; - res.status = BuildResult::Built; - res.drvPath = bfd.drvPath; + BuildResult res { + .status = BuildResult::Built, + .path = bfd, + }; OutputPathMap outputs; auto drv = evalStore->readDerivation(bfd.drvPath); @@ -905,7 +904,7 @@ BuildResult RemoteStore::buildDerivation(const StorePath & drvPath, const BasicD writeDerivation(conn->to, *this, drv); conn->to << buildMode; conn.processStderr(); - BuildResult res; + BuildResult res { .path = DerivedPath::Built { .drvPath = drvPath } }; res.status = (BuildResult::Status) readInt(conn->from); conn->from >> res.errorMsg; if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 29) {