From 374fe49ff78c13457c6cfe396f9ed0cb986c903b Mon Sep 17 00:00:00 2001 From: Michael Bishop Date: Sun, 19 Sep 2021 23:07:10 -0300 Subject: set the PER_LINUX32 personality flag, when building for armv6l-linux or armv7l-linux this prevents 32bit builds from detecting a 64bit kernel and picking the wrong target --- src/libstore/build/local-derivation-goal.cc | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'src/libstore/build') diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 518edae9c..21d7d7408 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -1778,6 +1778,10 @@ void LocalDerivationGoal::runChild() if (personality(PER_LINUX32) == -1) throw SysError("cannot set i686-linux personality"); } + if (drv->platform == "armv7l-linux" || drv->platform == "armv6l-linux") { + if (personality(PER_LINUX32) == -1) + throw SysError("cannot set 32bit linux personality"); + } /* Impersonate a Linux 2.6 machine to get some determinism in builds that depend on the kernel version. */ -- cgit v1.2.3 From 621aa65325f11769d33704b00dc15bc7dda49f0f Mon Sep 17 00:00:00 2001 From: Sergei Trofimovich Date: Thu, 30 Sep 2021 22:08:14 +0100 Subject: local-derivation-goal.cc: downgrade "warning: rewriting hashes in..." down to debug Before the changes when building the whole system with `contentAddressedByDefault = true;` we get many noninformative messages: $ nix build -f nixos system --keep-going ... warning: rewriting hashes in '/nix/store/...-clang-11.1.0.drv.chroot/nix/store/...-11.1.0'; cross fingers warning: rewriting hashes in '/nix/store/...-clang-11.1.0.drv.chroot/nix/store/...-11.1.0-dev'; cross fingers warning: rewriting hashes in '/nix/store/...-clang-11.1.0.drv.chroot/nix/store/...-11.1.0-python'; cross fingers error: 2 dependencies of derivation '/nix/store/...-hub-2.14.2.drv' failed to build warning: rewriting hashes in '/nix/store/...-subversion-1.14.1.drv.chroot/nix/store/...-subversion-1.14.1-dev'; cross fingers warning: rewriting hashes in '/nix/store/...-subversion-1.14.1.drv.chroot/nix/store/...-subversion-1.14.1-man'; cross fingers ... Let's downgrade these messages down to debug(). --- src/libstore/build/local-derivation-goal.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/libstore/build') diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index e91e35851..b1852a6bb 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -2208,7 +2208,7 @@ void LocalDerivationGoal::registerOutputs() auto rewriteOutput = [&]() { /* Apply hash rewriting if necessary. */ if (!outputRewrites.empty()) { - warn("rewriting hashes in '%1%'; cross fingers", actualPath); + debug("rewriting hashes in '%1%'; cross fingers", actualPath); /* FIXME: this is in-memory. */ StringSink sink; -- cgit v1.2.3 From ef34fd0656e844ccc54fd105cdda9970d7e944b4 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 4 Oct 2021 13:47:38 +0200 Subject: scanForReferences(): Use a StorePathSet --- src/libstore/build/local-derivation-goal.cc | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) (limited to 'src/libstore/build') diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index b1852a6bb..2ba1b3f59 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -2140,8 +2140,7 @@ void LocalDerivationGoal::registerOutputs() /* Pass blank Sink as we are not ready to hash data at this stage. */ NullSink blank; - auto references = worker.store.parseStorePathSet( - scanForReferences(blank, actualPath, worker.store.printStorePathSet(referenceablePaths))); + auto references = scanForReferences(blank, actualPath, referenceablePaths); outputReferencesIfUnregistered.insert_or_assign( outputName, -- cgit v1.2.3 From 3b7f4c7d9d124283cec7f7d42e0eca3e2de64d5a Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 5 Oct 2021 11:04:46 +0200 Subject: Add FIXME about ptsname --- src/libstore/build/local-derivation-goal.cc | 1 + 1 file changed, 1 insertion(+) (limited to 'src/libstore/build') diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 2ba1b3f59..4cb43bc11 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -711,6 +711,7 @@ void LocalDerivationGoal::startBuilder() if (!builderOut.readSide) throw SysError("opening pseudoterminal master"); + // FIXME: not thread-safe, use ptsname_r std::string slaveName(ptsname(builderOut.readSide.get())); if (buildUser) { -- cgit v1.2.3 From c6718a9d95021476f0cca47b61479e4046090f4e Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 6 Oct 2021 13:54:59 +0200 Subject: Don't reset the logger in a vfork 9c766a40cbbd3a350a9582d0fd8201e3361a63b2 broke logging from the daemon, because commonChildInit is called when starting the build hook in a vfork, so it ends up resetting the parent's logger. So don't vfork. It might be best to get rid of vfork altogether, but that may cause problems, e.g. when we call an external program like git from the evaluator. --- src/libstore/build/local-derivation-goal.cc | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) (limited to 'src/libstore/build') diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 4cb43bc11..3fc156108 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -755,7 +755,6 @@ void LocalDerivationGoal::startBuilder() result.startTime = time(0); /* Fork a child to build the package. */ - ProcessOptions options; #if __linux__ if (useChroot) { @@ -798,8 +797,6 @@ void LocalDerivationGoal::startBuilder() userNamespaceSync.create(); - options.allowVfork = false; - Path maxUserNamespaces = "/proc/sys/user/max_user_namespaces"; static bool userNamespacesEnabled = pathExists(maxUserNamespaces) @@ -857,7 +854,7 @@ void LocalDerivationGoal::startBuilder() writeFull(builderOut.writeSide.get(), fmt("%d %d\n", usingUserNamespace, child)); _exit(0); - }, options); + }); int res = helper.wait(); if (res != 0 && settings.sandboxFallback) { @@ -921,10 +918,9 @@ void LocalDerivationGoal::startBuilder() #endif { fallback: - options.allowVfork = !buildUser && !drv->isBuiltin(); pid = startProcess([&]() { runChild(); - }, options); + }); } /* parent */ -- cgit v1.2.3 From 66c4b20d8bdfb9434329439a7314942b25c7ba50 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 7 Oct 2021 13:34:04 +0200 Subject: Typo --- src/libstore/build/worker.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/libstore/build') diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index a7a6b92a6..55afb5cca 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -239,7 +239,7 @@ void Worker::run(const Goals & _topGoals) } } - /* Call queryMissing() efficiently query substitutes. */ + /* Call queryMissing() to efficiently query substitutes. */ StorePathSet willBuild, willSubstitute, unknown; uint64_t downloadSize, narSize; store.queryMissing(topPaths, willBuild, willSubstitute, unknown, downloadSize, narSize); -- cgit v1.2.3 From 7466048d397ed33d4bc28b6910c834cf1a17b0cb Mon Sep 17 00:00:00 2001 From: regnat Date: Mon, 11 Oct 2021 10:47:02 +0200 Subject: (partially) Revert "Don't copy in rethrow" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts some parts of commit 8430a8f0866e4463a891ccce62779ea9ac0f3b38 which was trying to rethrow some exceptions while we weren’t in the context of a `catch` block, causing some weird “terminate called without an active exception” errors. Fix #5368 --- src/libstore/build/local-derivation-goal.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/libstore/build') diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 3fc156108..8d245f84a 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -948,7 +948,7 @@ void LocalDerivationGoal::startBuilder() FdSource source(builderOut.readSide.get()); auto ex = readError(source); ex.addTrace({}, "while setting up the build environment"); - throw; + throw ex; } debug("sandbox setup: " + msg); msgs.push_back(std::move(msg)); -- cgit v1.2.3 From 8614cf13344eca75074cd4af20fd90238571b0b6 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 16 Aug 2021 20:03:32 +0200 Subject: Non-blocking garbage collector The garbage collector no longer blocks other processes from adding/building store paths or adding GC roots. To prevent the collector from deleting store paths just added by another process, processes need to connect to the garbage collector via a Unix domain socket to register new temporary roots. --- src/libstore/build/local-derivation-goal.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/libstore/build') diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 8d245f84a..bafa83a9d 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -1353,7 +1353,7 @@ void LocalDerivationGoal::startDaemon() AutoCloseFD remote = accept(daemonSocket.get(), (struct sockaddr *) &remoteAddr, &remoteAddrLen); if (!remote) { - if (errno == EINTR) continue; + if (errno == EINTR || errno == EAGAIN) continue; if (errno == EINVAL) break; throw SysError("accepting connection"); } -- cgit v1.2.3 From e989c83b44d7c4d8ffe2e5f4231e4861c5f7732f Mon Sep 17 00:00:00 2001 From: Alexey Novikov Date: Tue, 12 Oct 2021 16:18:44 +0400 Subject: Add error reporting to machine spec paser Currently machine specification (`/etc/nix/machine`) parser fails with a vague exception if the file had incorrect format. This commit adds verbose exceptions and unit-tests for the parser. --- src/libstore/build/entry-points.cc | 1 - 1 file changed, 1 deletion(-) (limited to 'src/libstore/build') diff --git a/src/libstore/build/entry-points.cc b/src/libstore/build/entry-points.cc index 2b77e4354..b1e5d8504 100644 --- a/src/libstore/build/entry-points.cc +++ b/src/libstore/build/entry-points.cc @@ -1,4 +1,3 @@ -#include "machines.hh" #include "worker.hh" #include "substitution-goal.hh" #include "derivation-goal.hh" -- cgit v1.2.3 From b9234142f5e3c69f3372f15ab2e6df19c7bf6b64 Mon Sep 17 00:00:00 2001 From: Alexander Bantyev Date: Sat, 23 Oct 2021 21:30:51 +0300 Subject: addToStore, addToStoreFromDump: add references argument Allow to pass a set of references to be added as info to the added paths. --- src/libstore/build/local-derivation-goal.cc | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) (limited to 'src/libstore/build') diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 8d245f84a..694c408f3 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -1179,7 +1179,8 @@ struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual Lo StorePath addToStore(const string & name, const Path & srcPath, FileIngestionMethod method = FileIngestionMethod::Recursive, HashType hashAlgo = htSHA256, - PathFilter & filter = defaultPathFilter, RepairFlag repair = NoRepair) override + PathFilter & filter = defaultPathFilter, RepairFlag repair = NoRepair, + StorePathSet references = StorePathSet()) override { throw Error("addToStore"); } void addToStore(const ValidPathInfo & info, Source & narSource, @@ -1198,9 +1199,10 @@ struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual Lo } StorePath addToStoreFromDump(Source & dump, const string & name, - FileIngestionMethod method = FileIngestionMethod::Recursive, HashType hashAlgo = htSHA256, RepairFlag repair = NoRepair) override + FileIngestionMethod method = FileIngestionMethod::Recursive, HashType hashAlgo = htSHA256, RepairFlag repair = NoRepair, + StorePathSet references = StorePathSet()) override { - auto path = next->addToStoreFromDump(dump, name, method, hashAlgo, repair); + auto path = next->addToStoreFromDump(dump, name, method, hashAlgo, repair, references); goal.addDependency(path); return path; } -- cgit v1.2.3 From af99941279b80c962ec9cae3e5fa32976a3f5744 Mon Sep 17 00:00:00 2001 From: regnat Date: Mon, 25 Oct 2021 15:53:01 +0200 Subject: Make experimental-features a proper type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rather than having them plain strings scattered through the whole codebase, create an enum containing all the known experimental features. This means that - Nix can now `warn` when an unkwown experimental feature is passed (making it much nicer to spot typos and spot deprecated features) - It’s now easy to remove a feature altogether (once the feature isn’t experimental anymore or is dropped) by just removing the field for the enum and letting the compiler point us to all the now invalid usages of it. --- src/libstore/build/derivation-goal.cc | 6 +++--- src/libstore/build/entry-points.cc | 2 +- src/libstore/build/local-derivation-goal.cc | 6 +++--- 3 files changed, 7 insertions(+), 7 deletions(-) (limited to 'src/libstore/build') diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 0907120db..67cf8b067 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -204,7 +204,7 @@ void DerivationGoal::haveDerivation() trace("have derivation"); if (drv->type() == DerivationType::CAFloating) - settings.requireExperimentalFeature("ca-derivations"); + settings.requireExperimentalFeature(Xp::CaDerivations); retrySubstitution = false; @@ -453,7 +453,7 @@ void DerivationGoal::inputsRealised() if (useDerivation) { auto & fullDrv = *dynamic_cast(drv.get()); - if (settings.isExperimentalFeatureEnabled("ca-derivations") && + if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations) && ((!fullDrv.inputDrvs.empty() && derivationIsCA(fullDrv.type())) || fullDrv.type() == DerivationType::DeferredInputAddressed)) { /* We are be able to resolve this derivation based on the @@ -1273,7 +1273,7 @@ void DerivationGoal::checkPathValidity() : PathStatus::Corrupt, }; } - if (settings.isExperimentalFeatureEnabled("ca-derivations")) { + if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations)) { auto drvOutput = DrvOutput{initialOutputs.at(i.first).outputHash, i.first}; if (auto real = worker.store.queryRealisation(drvOutput)) { info.known = { diff --git a/src/libstore/build/entry-points.cc b/src/libstore/build/entry-points.cc index 2b77e4354..065efc855 100644 --- a/src/libstore/build/entry-points.cc +++ b/src/libstore/build/entry-points.cc @@ -74,7 +74,7 @@ BuildResult Store::buildDerivation(const StorePath & drvPath, const BasicDerivat outputId, Realisation{ outputId, *staticOutput.second} ); - if (settings.isExperimentalFeatureEnabled("ca-derivations") && !derivationHasKnownOutputPaths(drv.type())) { + if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations) && !derivationHasKnownOutputPaths(drv.type())) { auto realisation = this->queryRealisation(outputId); if (realisation) result.builtOutputs.insert_or_assign( diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 8d245f84a..fab6c3a08 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -1259,7 +1259,7 @@ struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual Lo for (auto & [outputName, outputPath] : outputs) if (wantOutput(outputName, bfd.outputs)) { newPaths.insert(outputPath); - if (settings.isExperimentalFeatureEnabled("ca-derivations")) { + if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations)) { auto thisRealisation = next->queryRealisation( DrvOutput{drvHashes.at(outputName), outputName} ); @@ -1320,7 +1320,7 @@ struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual Lo void LocalDerivationGoal::startDaemon() { - settings.requireExperimentalFeature("recursive-nix"); + settings.requireExperimentalFeature(Xp::RecursiveNix); Store::Params params; params["path-info-cache-size"] = "0"; @@ -2561,7 +2561,7 @@ void LocalDerivationGoal::registerOutputs() that for floating CA derivations, which otherwise couldn't be cached, but it's fine to do in all cases. */ - if (settings.isExperimentalFeatureEnabled("ca-derivations")) { + if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations)) { for (auto& [outputName, newInfo] : infos) { auto thisRealisation = Realisation{ .id = DrvOutput{initialOutputs.at(outputName).outputHash, -- cgit v1.2.3 From f2280749b1d58a56f91ad417b487e920ab23b8b6 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 27 Oct 2021 14:21:31 +0200 Subject: If max-jobs == 0, do preferLocalBuild on remote builders --- src/libstore/build/derivation-goal.cc | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'src/libstore/build') diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 67cf8b067..b924d23b2 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -616,7 +616,9 @@ void DerivationGoal::tryToBuild() /* Don't do a remote build if the derivation has the attribute `preferLocalBuild' set. Also, check and repair modes are only supported for local builds. */ - bool buildLocally = buildMode != bmNormal || parsedDrv->willBuildLocally(worker.store); + bool buildLocally = + (buildMode != bmNormal || parsedDrv->willBuildLocally(worker.store)) + && settings.maxBuildJobs.get() != 0; if (!buildLocally) { switch (tryBuildHook()) { -- cgit v1.2.3 From 96670ed2163d3d1a296c9b053833362ec8c06985 Mon Sep 17 00:00:00 2001 From: regnat Date: Wed, 27 Oct 2021 11:36:51 +0200 Subject: Expose an async interface for `queryRealisation` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Doesn’t change much so far because everything is still using it synchronously, but should allow the binary cache to fetch stuff in parallel --- src/libstore/build/drv-output-substitution-goal.hh | 2 +- src/libstore/build/local-derivation-goal.cc | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) (limited to 'src/libstore/build') diff --git a/src/libstore/build/drv-output-substitution-goal.hh b/src/libstore/build/drv-output-substitution-goal.hh index 63ab53d89..1b11a298e 100644 --- a/src/libstore/build/drv-output-substitution-goal.hh +++ b/src/libstore/build/drv-output-substitution-goal.hh @@ -20,7 +20,7 @@ private: // The realisation corresponding to the given output id. // Will be filled once we can get it. - std::optional outputInfo; + std::shared_ptr outputInfo; /* The remaining substituters. */ std::list> subs; diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 2182f0bb4..b076bf998 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -1224,13 +1224,14 @@ struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual Lo // corresponds to an allowed derivation { throw Error("registerDrvOutput"); } - std::optional queryRealisation(const DrvOutput & id) override + void queryRealisationUncached(const DrvOutput & id, + Callback> callback) noexcept override // XXX: This should probably be allowed if the realisation corresponds to // an allowed derivation { if (!goal.isAllowed(id)) - throw InvalidPath("cannot query an unknown output id '%s' in recursive Nix", id.to_string()); - return next->queryRealisation(id); + callback(nullptr); + next->queryRealisation(id, std::move(callback)); } void buildPaths(const std::vector & paths, BuildMode buildMode, std::shared_ptr evalStore) override -- cgit v1.2.3 From f4c869977c391b31eb4f20486f7da03b026e2401 Mon Sep 17 00:00:00 2001 From: regnat Date: Thu, 28 Oct 2021 16:43:54 +0200 Subject: Make the DrvOutputSubstitutionGoal more async --- src/libstore/build/drv-output-substitution-goal.cc | 44 ++++++++++++++++++++-- src/libstore/build/drv-output-substitution-goal.hh | 12 +++++- 2 files changed, 51 insertions(+), 5 deletions(-) (limited to 'src/libstore/build') diff --git a/src/libstore/build/drv-output-substitution-goal.cc b/src/libstore/build/drv-output-substitution-goal.cc index be270d079..b9602e696 100644 --- a/src/libstore/build/drv-output-substitution-goal.cc +++ b/src/libstore/build/drv-output-substitution-goal.cc @@ -1,6 +1,8 @@ #include "drv-output-substitution-goal.hh" +#include "finally.hh" #include "worker.hh" #include "substitution-goal.hh" +#include "callback.hh" namespace nix { @@ -50,14 +52,42 @@ void DrvOutputSubstitutionGoal::tryNext() return; } - auto sub = subs.front(); + sub = subs.front(); subs.pop_front(); // FIXME: Make async - outputInfo = sub->queryRealisation(id); + // outputInfo = sub->queryRealisation(id); + outPipe.create(); + promise = decltype(promise)(); + + sub->queryRealisation( + id, { [&](std::future> res) { + try { + Finally updateStats([this]() { outPipe.writeSide.close(); }); + promise.set_value(res.get()); + } catch (...) { + promise.set_exception(std::current_exception()); + } + } }); + + worker.childStarted(shared_from_this(), {outPipe.readSide.get()}, true, false); + + state = &DrvOutputSubstitutionGoal::realisationFetched; +} + +void DrvOutputSubstitutionGoal::realisationFetched() +{ + worker.childTerminated(this); + + try { + outputInfo = promise.get_future().get(); + } catch (std::exception & e) { + printError(e.what()); + substituterFailed = true; + } + if (!outputInfo) { - tryNext(); - return; + return tryNext(); } for (const auto & [depId, depPath] : outputInfo->dependentRealisations) { @@ -119,4 +149,10 @@ void DrvOutputSubstitutionGoal::work() (this->*state)(); } +void DrvOutputSubstitutionGoal::handleEOF(int fd) +{ + if (fd == outPipe.readSide.get()) worker.wakeUp(shared_from_this()); +} + + } diff --git a/src/libstore/build/drv-output-substitution-goal.hh b/src/libstore/build/drv-output-substitution-goal.hh index 1b11a298e..67ae2624a 100644 --- a/src/libstore/build/drv-output-substitution-goal.hh +++ b/src/libstore/build/drv-output-substitution-goal.hh @@ -3,6 +3,8 @@ #include "store-api.hh" #include "goal.hh" #include "realisation.hh" +#include +#include namespace nix { @@ -25,6 +27,13 @@ private: /* The remaining substituters. */ std::list> subs; + /* The current substituter. */ + std::shared_ptr sub; + + Pipe outPipe; + std::thread thr; + std::promise> promise; + /* Whether a substituter failed. */ bool substituterFailed = false; @@ -36,6 +45,7 @@ public: void init(); void tryNext(); + void realisationFetched(); void outPathValid(); void finished(); @@ -44,7 +54,7 @@ public: string key() override; void work() override; - + void handleEOF(int fd) override; }; } -- cgit v1.2.3 From 0b005bc9d67b3f621bc78e5ecb2cd834172d5563 Mon Sep 17 00:00:00 2001 From: Alexander Bantyev Date: Tue, 9 Nov 2021 12:24:49 +0300 Subject: addToStore, addToStoreFromDump: refactor: pass refs by const reference Co-Authored-By: Eelco Dolstra --- src/libstore/build/local-derivation-goal.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/libstore/build') diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 694c408f3..383c6b990 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -1180,7 +1180,7 @@ struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual Lo StorePath addToStore(const string & name, const Path & srcPath, FileIngestionMethod method = FileIngestionMethod::Recursive, HashType hashAlgo = htSHA256, PathFilter & filter = defaultPathFilter, RepairFlag repair = NoRepair, - StorePathSet references = StorePathSet()) override + const StorePathSet & references = StorePathSet()) override { throw Error("addToStore"); } void addToStore(const ValidPathInfo & info, Source & narSource, @@ -1200,7 +1200,7 @@ struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual Lo StorePath addToStoreFromDump(Source & dump, const string & name, FileIngestionMethod method = FileIngestionMethod::Recursive, HashType hashAlgo = htSHA256, RepairFlag repair = NoRepair, - StorePathSet references = StorePathSet()) override + const StorePathSet & references = StorePathSet()) override { auto path = next->addToStoreFromDump(dump, name, method, hashAlgo, repair, references); goal.addDependency(path); -- cgit v1.2.3 From bceda304982a34c65d5a1dab449cb5bc59f63b83 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 12 Nov 2021 13:41:15 +0100 Subject: Typo --- src/libstore/build/local-derivation-goal.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/libstore/build') diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 2ba79ec46..589b449d0 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -1993,7 +1993,7 @@ void LocalDerivationGoal::runChild() else if (drv->builder == "builtin:unpack-channel") builtinUnpackChannel(drv2); else - throw Error("unsupported builtin function '%1%'", string(drv->builder, 8)); + throw Error("unsupported builtin builder '%1%'", string(drv->builder, 8)); _exit(0); } catch (std::exception & e) { writeFull(STDERR_FILENO, e.what() + std::string("\n")); -- cgit v1.2.3 From a18d9269a516c5f8149b04ccb01034960f4851d8 Mon Sep 17 00:00:00 2001 From: Alex Shabalin Date: Fri, 19 Nov 2021 15:22:31 +0100 Subject: Fix build warnings on MacOS --- src/libstore/build/local-derivation-goal.cc | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'src/libstore/build') diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 3c7bd695e..c9a4a31e7 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -342,7 +342,7 @@ int childEntry(void * arg) return 1; } - +#if __linux__ static void linkOrCopy(const Path & from, const Path & to) { if (link(from.c_str(), to.c_str()) == -1) { @@ -358,6 +358,7 @@ static void linkOrCopy(const Path & from, const Path & to) copyPath(from, to); } } +#endif void LocalDerivationGoal::startBuilder() @@ -917,7 +918,9 @@ void LocalDerivationGoal::startBuilder() } else #endif { +#if __linux__ fallback: +#endif pid = startProcess([&]() { runChild(); }); -- cgit v1.2.3 From 8388d2c7c662e37470240cfde798956fe8e36a6f Mon Sep 17 00:00:00 2001 From: Las Safin Date: Fri, 8 Oct 2021 22:55:08 +0000 Subject: Make recursive-nix work even when not privileged Before this, `setns` would fail when switching to the mount namespace, since we did not have the privileges to do so when not root. Closes #5360 --- src/libstore/build/local-derivation-goal.cc | 11 +++++++++-- src/libstore/build/local-derivation-goal.hh | 3 ++- 2 files changed, 11 insertions(+), 3 deletions(-) (limited to 'src/libstore/build') diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index c9a4a31e7..ebcb561c2 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -260,6 +260,7 @@ void LocalDerivationGoal::cleanupHookFinally() void LocalDerivationGoal::cleanupPreChildKill() { sandboxMountNamespace = -1; + sandboxUserNamespace = -1; } @@ -906,11 +907,14 @@ void LocalDerivationGoal::startBuilder() "nobody:x:65534:65534:Nobody:/:/noshell\n", sandboxUid(), sandboxGid(), settings.sandboxBuildDir)); - /* Save the mount namespace of the child. We have to do this + /* Save the mount- and user namespace of the child. We have to do this *before* the child does a chroot. */ sandboxMountNamespace = open(fmt("/proc/%d/ns/mnt", (pid_t) pid).c_str(), O_RDONLY); if (sandboxMountNamespace.get() == -1) throw SysError("getting sandbox mount namespace"); + sandboxUserNamespace = open(fmt("/proc/%d/ns/user", (pid_t) pid).c_str(), O_RDONLY); + if (sandboxUserNamespace.get() == -1) + throw SysError("getting sandbox user namespace"); /* Signal the builder that we've updated its user namespace. */ writeFull(userNamespaceSync.writeSide.get(), "1"); @@ -1423,7 +1427,7 @@ void LocalDerivationGoal::addDependency(const StorePath & path) Path source = worker.store.Store::toRealPath(path); Path target = chrootRootDir + worker.store.printStorePath(path); - debug("bind-mounting %s -> %s", target, source); + debug("bind-mounting %s -> %s", source, target); if (pathExists(target)) throw Error("store path '%s' already exists in the sandbox", worker.store.printStorePath(path)); @@ -1438,6 +1442,9 @@ void LocalDerivationGoal::addDependency(const StorePath & path) child process.*/ Pid child(startProcess([&]() { + if (usingUserNamespace && (setns(sandboxUserNamespace.get(), 0) == -1)) + throw SysError("entering sandbox user namespace"); + if (setns(sandboxMountNamespace.get(), 0) == -1) throw SysError("entering sandbox mount namespace"); diff --git a/src/libstore/build/local-derivation-goal.hh b/src/libstore/build/local-derivation-goal.hh index 088a57209..bfdf91d89 100644 --- a/src/libstore/build/local-derivation-goal.hh +++ b/src/libstore/build/local-derivation-goal.hh @@ -27,9 +27,10 @@ struct LocalDerivationGoal : public DerivationGoal /* Pipe for synchronising updates to the builder namespaces. */ Pipe userNamespaceSync; - /* The mount namespace of the builder, used to add additional + /* The mount namespace and user namespace of the builder, used to add additional paths to the sandbox as a result of recursive Nix calls. */ AutoCloseFD sandboxMountNamespace; + AutoCloseFD sandboxUserNamespace; /* On Linux, whether we're doing the build in its own user namespace. */ -- cgit v1.2.3 From ae21aab456f4e07fa621d87487c40381e45947dc Mon Sep 17 00:00:00 2001 From: Jan Tojnar Date: Mon, 6 Dec 2021 16:42:57 +0100 Subject: Update manual links Fixes: https://github.com/NixOS/nixos-homepage/issues/762 --- src/libstore/build/derivation-goal.cc | 2 +- src/libstore/build/worker.cc | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'src/libstore/build') diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index b924d23b2..60945403e 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -655,7 +655,7 @@ void DerivationGoal::tryLocalBuild() { throw Error( "unable to build with a primary store that isn't a local store; " "either pass a different '--store' or enable remote builds." - "\nhttps://nixos.org/nix/manual/#chap-distributed-builds"); + "\nhttps://nixos.org/manual/nix/stable/advanced-topics/distributed-builds.html"); } diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index 55afb5cca..f11c5ce68 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -281,11 +281,11 @@ void Worker::run(const Goals & _topGoals) if (getMachines().empty()) throw Error("unable to start any build; either increase '--max-jobs' " "or enable remote builds." - "\nhttps://nixos.org/nix/manual/#chap-distributed-builds"); + "\nhttps://nixos.org/manual/nix/stable/advanced-topics/distributed-builds.html"); else throw Error("unable to start any build; remote machines may not have " "all required system features." - "\nhttps://nixos.org/nix/manual/#chap-distributed-builds"); + "\nhttps://nixos.org/manual/nix/stable/advanced-topics/distributed-builds.html"); } assert(!awake.empty()); -- cgit v1.2.3 From 971382cab0c8ee057706e3dd4a124252d6b3547d Mon Sep 17 00:00:00 2001 From: Gavin Ray Date: Wed, 8 Dec 2021 19:55:34 -0500 Subject: Better diagnostics if no valid signature found I downloaded Nix tonight, and immediately broke it by accidentally removing the default binary caching. After figuring this out, I also failed to fix it properly, due to using the wrong key for Nix's default binary cache If the diagnostic message would have been clearer about what/where a "signature" for a "substituter" is + comes from, it probably would have saved me a few hours. Maybe we can save other noobs the same pain? --- src/libstore/build/substitution-goal.cc | 2 ++ 1 file changed, 2 insertions(+) (limited to 'src/libstore/build') diff --git a/src/libstore/build/substitution-goal.cc b/src/libstore/build/substitution-goal.cc index 29a8cfb87..4c3701b27 100644 --- a/src/libstore/build/substitution-goal.cc +++ b/src/libstore/build/substitution-goal.cc @@ -140,6 +140,8 @@ void PathSubstitutionGoal::tryNext() { warn("substituter '%s' does not have a valid signature for path '%s'", sub->getUri(), worker.store.printStorePath(storePath)); + warn("verify that your nix.conf contains a correct signature in 'trusted-public-keys' for %s", + sub->getUri()); tryNext(); return; } -- cgit v1.2.3 From 3542d4fe16822bcd94433bf9393fbe17a5297948 Mon Sep 17 00:00:00 2001 From: Gavin Ray Date: Fri, 10 Dec 2021 19:02:22 -0500 Subject: Incorporate suggestions from @edolstra --- src/libstore/build/substitution-goal.cc | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'src/libstore/build') diff --git a/src/libstore/build/substitution-goal.cc b/src/libstore/build/substitution-goal.cc index 4c3701b27..5ecf1da7e 100644 --- a/src/libstore/build/substitution-goal.cc +++ b/src/libstore/build/substitution-goal.cc @@ -138,10 +138,8 @@ void PathSubstitutionGoal::tryNext() only after we've downloaded the path. */ if (!sub->isTrusted && worker.store.pathInfoIsUntrusted(*info)) { - warn("substituter '%s' does not have a valid signature for path '%s'", - sub->getUri(), worker.store.printStorePath(storePath)); - warn("verify that your nix.conf contains a correct signature in 'trusted-public-keys' for %s", - sub->getUri()); + warn("the substitute for '%s' from '%s' is not signed by any of the keys in 'trusted-public-keys'", + worker.store.printStorePath(storePath), sub->getUri()); tryNext(); return; } -- cgit v1.2.3 From 55dbb7f1ccc0e991df6b09af0a16ce9246ac3f19 Mon Sep 17 00:00:00 2001 From: regnat Date: Mon, 13 Dec 2021 16:56:44 +0100 Subject: More properly track the status of CA builds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make the build of unresolved derivations return the same status as the resolved one, except in the case of an `AlreadyValid` in which case it will return `ResolvesToAlreadyValid` to mean that the outputs of the unresolved derivation weren’t known, but the resolved one is. --- src/libstore/build/derivation-goal.cc | 26 +++++++++++++++++--------- src/libstore/build/derivation-goal.hh | 4 ++-- 2 files changed, 19 insertions(+), 11 deletions(-) (limited to 'src/libstore/build') diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 60945403e..a713d7222 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -464,7 +464,6 @@ void DerivationGoal::inputsRealised() Derivation drvResolved { *std::move(attempt) }; auto pathResolved = writeDerivation(worker.store, drvResolved); - resolvedDrv = drvResolved; auto msg = fmt("Resolved derivation: '%s' -> '%s'", worker.store.printStorePath(drvPath), @@ -475,9 +474,9 @@ void DerivationGoal::inputsRealised() worker.store.printStorePath(pathResolved), }); - auto resolvedGoal = worker.makeDerivationGoal( + resolvedDrvGoal = worker.makeDerivationGoal( pathResolved, wantedOutputs, buildMode); - addWaitee(resolvedGoal); + addWaitee(resolvedDrvGoal); state = &DerivationGoal::resolvedFinished; return; @@ -938,16 +937,17 @@ void DerivationGoal::buildDone() } void DerivationGoal::resolvedFinished() { - assert(resolvedDrv); + assert(resolvedDrvGoal); + auto resolvedDrv = *resolvedDrvGoal->drv; - auto resolvedHashes = staticOutputHashes(worker.store, *resolvedDrv); + auto resolvedHashes = staticOutputHashes(worker.store, resolvedDrv); StorePathSet outputPaths; // `wantedOutputs` might be empty, which means “all the outputs” auto realWantedOutputs = wantedOutputs; if (realWantedOutputs.empty()) - realWantedOutputs = resolvedDrv->outputNames(); + realWantedOutputs = resolvedDrv.outputNames(); for (auto & wantedOutput : realWantedOutputs) { assert(initialOutputs.count(wantedOutput) != 0); @@ -979,9 +979,17 @@ void DerivationGoal::resolvedFinished() { outputPaths ); - // This is potentially a bit fishy in terms of error reporting. Not sure - // how to do it in a cleaner way - amDone(nrFailed == 0 ? ecSuccess : ecFailed, ex); + auto status = [&]() { + auto resolvedResult = resolvedDrvGoal->getResult(); + switch (resolvedResult.status) { + case BuildResult::AlreadyValid: + return BuildResult::ResolvesToAlreadyValid; + default: + return resolvedResult.status; + } + }(); + + done(status); } HookReply DerivationGoal::tryBuildHook() diff --git a/src/libstore/build/derivation-goal.hh b/src/libstore/build/derivation-goal.hh index 704b77caf..e112542c7 100644 --- a/src/libstore/build/derivation-goal.hh +++ b/src/libstore/build/derivation-goal.hh @@ -50,8 +50,8 @@ struct DerivationGoal : public Goal /* The path of the derivation. */ StorePath drvPath; - /* The path of the corresponding resolved derivation */ - std::optional resolvedDrv; + /* The goal for the corresponding resolved derivation */ + std::shared_ptr resolvedDrvGoal; /* The specific outputs that we need to build. Empty means all of them. */ -- cgit v1.2.3 From 2eec2f765a86b8954f3a74ff148bc70a2d32be27 Mon Sep 17 00:00:00 2001 From: regnat Date: Mon, 13 Dec 2021 16:58:43 +0100 Subject: Add a crude tracing mechansim for the build results MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a `_NIX_TRACE_BUILT_OUTPUTS` environment variable that can be set to a filename in which the result of each build will be logged. This is intentionally crude and undocumented as it’s only meant to be a temporary thing to assess the usefulness of CA derivations. Any other use would need a cleaner re-implementation first. --- src/libstore/build/derivation-goal.cc | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'src/libstore/build') diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index a713d7222..c6ab5c3d1 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -17,6 +17,7 @@ #include #include +#include #include #include #include @@ -1337,6 +1338,13 @@ void DerivationGoal::done(BuildResult::Status status, std::optional ex) } worker.updateProgress(); + + auto traceBuiltOutputsFile = getEnv("_NIX_TRACE_BUILT_OUTPUTS").value_or(""); + if (traceBuiltOutputsFile != "") { + std::fstream fs; + fs.open(traceBuiltOutputsFile, std::fstream::out); + fs << worker.store.printStorePath(drvPath) << "\t" << result.toString() << std::endl; + } } -- cgit v1.2.3 From 46d86e06ba54dc708fa8fd7d0109845fa2ac402e Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 13 Dec 2021 20:28:53 +0100 Subject: Simplify --- src/libstore/build/local-derivation-goal.cc | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) (limited to 'src/libstore/build') diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index edfa46052..66d950b42 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -1779,15 +1779,14 @@ void LocalDerivationGoal::runChild() i686-linux build on an x86_64-linux machine. */ struct utsname utsbuf; uname(&utsbuf); - if (drv->platform == "i686-linux" && - (settings.thisSystem == "x86_64-linux" || - (!strcmp(utsbuf.sysname, "Linux") && !strcmp(utsbuf.machine, "x86_64")))) { + if ((drv->platform == "i686-linux" + && (settings.thisSystem == "x86_64-linux" + || (!strcmp(utsbuf.sysname, "Linux") && !strcmp(utsbuf.machine, "x86_64")))) + || drv->platform == "armv7l-linux" + || drv->platform == "armv6l-linux") + { if (personality(PER_LINUX32) == -1) - throw SysError("cannot set i686-linux personality"); - } - if (drv->platform == "armv7l-linux" || drv->platform == "armv6l-linux") { - if (personality(PER_LINUX32) == -1) - throw SysError("cannot set 32bit linux personality"); + throw SysError("cannot set 32-bit personality"); } /* Impersonate a Linux 2.6 machine to get some determinism in -- cgit v1.2.3 From be64fb9b5160f50d5b127598c84ffb77b980799b Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 8 Dec 2021 14:03:44 +0100 Subject: DerivationGoal::loadDerivation(): Don't use derivationFromPath() This causes a recursive call to ensurePath(), which is not a good idea. --- src/libstore/build/derivation-goal.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/libstore/build') diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index c6ab5c3d1..d34e53fe8 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -194,7 +194,7 @@ void DerivationGoal::loadDerivation() assert(worker.evalStore.isValidPath(drvPath)); /* Get the derivation. */ - drv = std::make_unique(worker.evalStore.derivationFromPath(drvPath)); + drv = std::make_unique(worker.evalStore.readDerivation(drvPath)); haveDerivation(); } -- cgit v1.2.3 From 19fd6e585d8f4cad3e358361b6f983b0d7d3b8a9 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 10 Jan 2022 16:52:25 +0100 Subject: 'target' points to 'source' --- src/libstore/build/local-derivation-goal.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/libstore/build') diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index d3434d63e..8dd1965e1 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -1427,7 +1427,7 @@ void LocalDerivationGoal::addDependency(const StorePath & path) Path source = worker.store.Store::toRealPath(path); Path target = chrootRootDir + worker.store.printStorePath(path); - debug("bind-mounting %s -> %s", source, target); + debug("bind-mounting %s -> %s", target, source); if (pathExists(target)) throw Error("store path '%s' already exists in the sandbox", worker.store.printStorePath(path)); -- cgit v1.2.3 From e9a4abdb5d6fe8e128372a77d879b0187b1bacfe Mon Sep 17 00:00:00 2001 From: Guillaume Maudoux Date: Tue, 11 Jan 2022 11:57:45 +0100 Subject: Make --repair-path also repair corrupt optimised links There already existed a smoke test for the link content length, but it appears that there exists some corruptions pernicious enough to replace the file content with zeros, and keeping the same length. --repair-path now goes as far as checking the content of the link, making it true to its name and actually repairing the path for such coruption cases. --- src/libstore/build/local-derivation-goal.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/libstore/build') diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 66d950b42..9ad28c6a9 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -2460,7 +2460,7 @@ void LocalDerivationGoal::registerOutputs() } if (curRound == nrRounds) { - localStore.optimisePath(actualPath); // FIXME: combine with scanForReferences() + localStore.optimisePath(actualPath, false); // FIXME: combine with scanForReferences() worker.markContentsGood(newInfo.path); } -- cgit v1.2.3 From 9f9f39a24b515d5d3ddd2bbd4cdadb0b1924bc3b Mon Sep 17 00:00:00 2001 From: Guillaume Maudoux Date: Tue, 11 Jan 2022 13:34:57 +0100 Subject: Prefer RepairFlag over bool when applicable --- src/libstore/build/local-derivation-goal.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/libstore/build') diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 9ad28c6a9..61991a7e5 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -2460,7 +2460,7 @@ void LocalDerivationGoal::registerOutputs() } if (curRound == nrRounds) { - localStore.optimisePath(actualPath, false); // FIXME: combine with scanForReferences() + localStore.optimisePath(actualPath, NoRepair); // FIXME: combine with scanForReferences() worker.markContentsGood(newInfo.path); } -- cgit v1.2.3 From 776eb97a4328ccfacbd4e795b4659b7367838db0 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 17 Jan 2022 19:28:42 +0100 Subject: serialise.hh: Use std::string_view --- src/libstore/build/derivation-goal.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/libstore/build') diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index d34e53fe8..151217b8b 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -278,7 +278,7 @@ void DerivationGoal::outputsSubstitutionTried() if (nrFailed > 0 && nrFailed > nrNoSubstituters + nrIncompleteClosure && !settings.tryFallback) { done(BuildResult::TransientFailure, - fmt("some substitutes for the outputs of derivation '%s' failed (usually happens due to networking issues); try '--fallback' to build derivation from source ", + Error("some substitutes for the outputs of derivation '%s' failed (usually happens due to networking issues); try '--fallback' to build derivation from source ", worker.store.printStorePath(drvPath))); return; } -- cgit v1.2.3 From d62a9390fcdc0f9e4971e5fab2f667567237b252 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 17 Jan 2022 22:20:05 +0100 Subject: Get rid of std::shared_ptr and ref These were needed back in the pre-C++11 era because we didn't have move semantics. But now we do. --- src/libstore/build/local-derivation-goal.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'src/libstore/build') diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index e4d2add73..0d0afea2d 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -2226,8 +2226,8 @@ void LocalDerivationGoal::registerOutputs() StringSink sink; dumpPath(actualPath, sink); deletePath(actualPath); - sink.s = make_ref(rewriteStrings(*sink.s, outputRewrites)); - StringSource source(*sink.s); + sink.s = rewriteStrings(sink.s, outputRewrites); + StringSource source(sink.s); restorePath(actualPath, source); } }; @@ -2295,7 +2295,7 @@ void LocalDerivationGoal::registerOutputs() StringSink sink; dumpPath(actualPath, sink); RewritingSink rsink2(oldHashPart, std::string(finalPath.hashPart()), nextSink); - rsink2(*sink.s); + rsink2(sink.s); rsink2.flush(); }); Path tmpPath = actualPath + ".tmp"; -- cgit v1.2.3 From c437e1326d900e2563c5859489f3be2cb557a3d3 Mon Sep 17 00:00:00 2001 From: Sebastian Ullrich Date: Sat, 12 Feb 2022 16:28:36 +0100 Subject: Fix using sandbox without user namespaces --- src/libstore/build/local-derivation-goal.cc | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) (limited to 'src/libstore/build') diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 0d0afea2d..b76ad702b 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -912,9 +912,12 @@ void LocalDerivationGoal::startBuilder() sandboxMountNamespace = open(fmt("/proc/%d/ns/mnt", (pid_t) pid).c_str(), O_RDONLY); if (sandboxMountNamespace.get() == -1) throw SysError("getting sandbox mount namespace"); - sandboxUserNamespace = open(fmt("/proc/%d/ns/user", (pid_t) pid).c_str(), O_RDONLY); - if (sandboxUserNamespace.get() == -1) - throw SysError("getting sandbox user namespace"); + + if (usingUserNamespace) { + sandboxUserNamespace = open(fmt("/proc/%d/ns/user", (pid_t) pid).c_str(), O_RDONLY); + if (sandboxUserNamespace.get() == -1) + throw SysError("getting sandbox user namespace"); + } /* Signal the builder that we've updated its user namespace. */ writeFull(userNamespaceSync.writeSide.get(), "1"); -- cgit v1.2.3 From 2d6d9a28ebb17b1ba1fe0dc4d56b6aa311f94d39 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 15 Feb 2022 15:08:06 +0100 Subject: addToStoreFromDump(): Take std::string_view --- src/libstore/build/local-derivation-goal.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'src/libstore/build') diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index b76ad702b..8861d2c7b 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -1208,7 +1208,7 @@ struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual Lo return path; } - StorePath addToStoreFromDump(Source & dump, const string & name, + StorePath addToStoreFromDump(Source & dump, std::string_view name, FileIngestionMethod method = FileIngestionMethod::Recursive, HashType hashAlgo = htSHA256, RepairFlag repair = NoRepair, const StorePathSet & references = StorePathSet()) override { -- cgit v1.2.3 From fe9afb65bb35737a144acd612170b2e284298a2f Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 21 Feb 2022 16:28:23 +0100 Subject: Remove std::set alias --- src/libstore/build/derivation-goal.cc | 2 +- src/libstore/build/goal.hh | 4 ++-- src/libstore/build/worker.cc | 4 ++-- src/libstore/build/worker.hh | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) (limited to 'src/libstore/build') diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 151217b8b..27f8c6257 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -1091,7 +1091,7 @@ HookReply DerivationGoal::tryBuildHook() /* Create the log file and pipe. */ Path logFile = openLogFile(); - set fds; + std::set fds; fds.insert(hook->fromHook.readSide.get()); fds.insert(hook->builderOut.readSide.get()); worker.childStarted(shared_from_this(), fds, false, false); diff --git a/src/libstore/build/goal.hh b/src/libstore/build/goal.hh index 192e416d2..0db0c1938 100644 --- a/src/libstore/build/goal.hh +++ b/src/libstore/build/goal.hh @@ -18,8 +18,8 @@ struct CompareGoalPtrs { }; /* Set of goals. */ -typedef set Goals; -typedef set> WeakGoals; +typedef std::set Goals; +typedef std::set> WeakGoals; /* A map of paths to goals (and the other way around). */ typedef std::map WeakGoalMap; diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index f11c5ce68..0a6108233 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -161,7 +161,7 @@ unsigned Worker::getNrLocalBuilds() } -void Worker::childStarted(GoalPtr goal, const set & fds, +void Worker::childStarted(GoalPtr goal, const std::set & fds, bool inBuildSlot, bool respectTimeouts) { Child child; @@ -377,7 +377,7 @@ void Worker::waitForInput() GoalPtr goal = j->goal.lock(); assert(goal); - set fds2(j->fds); + std::set fds2(j->fds); std::vector buffer(4096); for (auto & k : fds2) { if (pollStatus.at(fdToPollStatus.at(k)).revents) { diff --git a/src/libstore/build/worker.hh b/src/libstore/build/worker.hh index 6a3b99c02..a1e036a96 100644 --- a/src/libstore/build/worker.hh +++ b/src/libstore/build/worker.hh @@ -38,7 +38,7 @@ struct Child { WeakGoalPtr goal; Goal * goal2; // ugly hackery - set fds; + std::set fds; bool respectTimeouts; bool inBuildSlot; steady_time_point lastOutput; /* time we last got output on stdout/stderr */ @@ -167,7 +167,7 @@ public: /* Registers a running child process. `inBuildSlot' means that the process counts towards the jobs limit. */ - void childStarted(GoalPtr goal, const set & fds, + void childStarted(GoalPtr goal, const std::set & fds, bool inBuildSlot, bool respectTimeouts); /* Unregisters a running child process. `wakeSleepers' should be -- cgit v1.2.3 From df552ff53e68dff8ca360adbdbea214ece1d08ee Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 25 Feb 2022 16:00:00 +0100 Subject: Remove std::string alias (for real this time) Also use std::string_view in a few more places. --- src/libstore/build/derivation-goal.cc | 14 ++--- src/libstore/build/derivation-goal.hh | 4 +- src/libstore/build/drv-output-substitution-goal.cc | 2 +- src/libstore/build/drv-output-substitution-goal.hh | 2 +- src/libstore/build/goal.cc | 4 +- src/libstore/build/goal.hh | 8 +-- src/libstore/build/local-derivation-goal.cc | 64 +++++++++++++--------- src/libstore/build/local-derivation-goal.hh | 2 +- src/libstore/build/substitution-goal.cc | 2 +- src/libstore/build/substitution-goal.hh | 4 +- src/libstore/build/worker.cc | 2 +- 11 files changed, 60 insertions(+), 48 deletions(-) (limited to 'src/libstore/build') diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 27f8c6257..95da1841d 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -116,7 +116,7 @@ DerivationGoal::~DerivationGoal() } -string DerivationGoal::key() +std::string DerivationGoal::key() { /* Ensure that derivations get built in order of their name, i.e. a derivation named "aardvark" always comes before @@ -1013,7 +1013,7 @@ HookReply DerivationGoal::tryBuildHook() /* Read the first line of input, which should be a word indicating whether the hook wishes to perform the build. */ - string reply; + std::string reply; while (true) { auto s = [&]() { try { @@ -1025,8 +1025,8 @@ HookReply DerivationGoal::tryBuildHook() }(); if (handleJSONLogMessage(s, worker.act, worker.hook->activities, true)) ; - else if (string(s, 0, 2) == "# ") { - reply = string(s, 2); + else if (s.substr(0, 2) == "# ") { + reply = s.substr(2); break; } else { @@ -1140,10 +1140,10 @@ Path DerivationGoal::openLogFile() logDir = localStore->logDir; else logDir = settings.nixLogDir; - Path dir = fmt("%s/%s/%s/", logDir, LocalFSStore::drvsLogDir, string(baseName, 0, 2)); + Path dir = fmt("%s/%s/%s/", logDir, LocalFSStore::drvsLogDir, baseName.substr(0, 2)); createDirs(dir); - Path logFileName = fmt("%s/%s%s", dir, string(baseName, 2), + Path logFileName = fmt("%s/%s%s", dir, baseName.substr(2), settings.compressLog ? ".bz2" : ""); fdLogFile = open(logFileName.c_str(), O_CREAT | O_WRONLY | O_TRUNC | O_CLOEXEC, 0666); @@ -1176,7 +1176,7 @@ bool DerivationGoal::isReadDesc(int fd) } -void DerivationGoal::handleChildOutput(int fd, const string & data) +void DerivationGoal::handleChildOutput(int fd, std::string_view data) { if (isReadDesc(fd)) { diff --git a/src/libstore/build/derivation-goal.hh b/src/libstore/build/derivation-goal.hh index e112542c7..d947482ef 100644 --- a/src/libstore/build/derivation-goal.hh +++ b/src/libstore/build/derivation-goal.hh @@ -145,7 +145,7 @@ struct DerivationGoal : public Goal void timedOut(Error && ex) override; - string key() override; + std::string key() override; void work() override; @@ -200,7 +200,7 @@ struct DerivationGoal : public Goal virtual bool isReadDesc(int fd); /* Callback used by the worker to write to the log. */ - void handleChildOutput(int fd, const string & data) override; + void handleChildOutput(int fd, std::string_view data) override; void handleEOF(int fd) override; void flushLine(); diff --git a/src/libstore/build/drv-output-substitution-goal.cc b/src/libstore/build/drv-output-substitution-goal.cc index b9602e696..e6e810cdd 100644 --- a/src/libstore/build/drv-output-substitution-goal.cc +++ b/src/libstore/build/drv-output-substitution-goal.cc @@ -137,7 +137,7 @@ void DrvOutputSubstitutionGoal::finished() amDone(ecSuccess); } -string DrvOutputSubstitutionGoal::key() +std::string DrvOutputSubstitutionGoal::key() { /* "a$" ensures substitution goals happen before derivation goals. */ diff --git a/src/libstore/build/drv-output-substitution-goal.hh b/src/libstore/build/drv-output-substitution-goal.hh index 67ae2624a..948dbda8f 100644 --- a/src/libstore/build/drv-output-substitution-goal.hh +++ b/src/libstore/build/drv-output-substitution-goal.hh @@ -51,7 +51,7 @@ public: void timedOut(Error && ex) override { abort(); }; - string key() override; + std::string key() override; void work() override; void handleEOF(int fd) override; diff --git a/src/libstore/build/goal.cc b/src/libstore/build/goal.cc index 7c985128b..d2420b107 100644 --- a/src/libstore/build/goal.cc +++ b/src/libstore/build/goal.cc @@ -5,8 +5,8 @@ namespace nix { bool CompareGoalPtrs::operator() (const GoalPtr & a, const GoalPtr & b) const { - string s1 = a->key(); - string s2 = b->key(); + std::string s1 = a->key(); + std::string s2 = b->key(); return s1 < s2; } diff --git a/src/libstore/build/goal.hh b/src/libstore/build/goal.hh index 0db0c1938..68e08bdf9 100644 --- a/src/libstore/build/goal.hh +++ b/src/libstore/build/goal.hh @@ -50,7 +50,7 @@ struct Goal : public std::enable_shared_from_this unsigned int nrIncompleteClosure; /* Name of this goal for debugging purposes. */ - string name; + std::string name; /* Whether the goal is finished. */ ExitCode exitCode; @@ -75,7 +75,7 @@ struct Goal : public std::enable_shared_from_this virtual void waiteeDone(GoalPtr waitee, ExitCode result); - virtual void handleChildOutput(int fd, const string & data) + virtual void handleChildOutput(int fd, std::string_view data) { abort(); } @@ -87,7 +87,7 @@ struct Goal : public std::enable_shared_from_this void trace(const FormatOrString & fs); - string getName() + std::string getName() { return name; } @@ -97,7 +97,7 @@ struct Goal : public std::enable_shared_from_this by the worker (important!), etc. */ virtual void timedOut(Error && ex) = 0; - virtual string key() = 0; + virtual std::string key() = 0; void amDone(ExitCode result, std::optional ex = {}); diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 8861d2c7b..220f80602 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -481,12 +481,12 @@ void LocalDerivationGoal::startBuilder() temporary build directory. The text files have the format used by `nix-store --register-validity'. However, the deriver fields are left empty. */ - string s = get(drv->env, "exportReferencesGraph").value_or(""); + auto s = get(drv->env, "exportReferencesGraph").value_or(""); Strings ss = tokenizeString(s); if (ss.size() % 2 != 0) throw BuildError("odd number of tokens in 'exportReferencesGraph': '%1%'", s); for (Strings::iterator i = ss.begin(); i != ss.end(); ) { - string fileName = *i++; + auto fileName = *i++; static std::regex regex("[A-Za-z_][A-Za-z0-9_.-]*"); if (!std::regex_match(fileName, regex)) throw Error("invalid file name '%s' in 'exportReferencesGraph'", fileName); @@ -517,10 +517,10 @@ void LocalDerivationGoal::startBuilder() i.pop_back(); } size_t p = i.find('='); - if (p == string::npos) + if (p == std::string::npos) dirsInChroot[i] = {i, optional}; else - dirsInChroot[string(i, 0, p)] = {string(i, p + 1), optional}; + dirsInChroot[i.substr(0, p)] = {i.substr(p + 1), optional}; } dirsInChroot[tmpDirInSandbox] = tmpDir; @@ -671,9 +671,10 @@ void LocalDerivationGoal::startBuilder() auto state = stBegin; auto lines = runProgram(settings.preBuildHook, false, args); auto lastPos = std::string::size_type{0}; - for (auto nlPos = lines.find('\n'); nlPos != string::npos; - nlPos = lines.find('\n', lastPos)) { - auto line = std::string{lines, lastPos, nlPos - lastPos}; + for (auto nlPos = lines.find('\n'); nlPos != std::string::npos; + nlPos = lines.find('\n', lastPos)) + { + auto line = lines.substr(lastPos, nlPos - lastPos); lastPos = nlPos + 1; if (state == stBegin) { if (line == "extra-sandbox-paths" || line == "extra-chroot-dirs") { @@ -686,10 +687,10 @@ void LocalDerivationGoal::startBuilder() state = stBegin; } else { auto p = line.find('='); - if (p == string::npos) + if (p == std::string::npos) dirsInChroot[line] = line; else - dirsInChroot[string(line, 0, p)] = string(line, p + 1); + dirsInChroot[line.substr(0, p)] = line.substr(p + 1); } } } @@ -941,7 +942,7 @@ void LocalDerivationGoal::startBuilder() /* Check if setting up the build environment failed. */ std::vector msgs; while (true) { - string msg = [&]() { + std::string msg = [&]() { try { return readLine(builderOut.readSide.get()); } catch (Error & e) { @@ -953,8 +954,8 @@ void LocalDerivationGoal::startBuilder() throw; } }(); - if (string(msg, 0, 1) == "\2") break; - if (string(msg, 0, 1) == "\1") { + if (msg.substr(0, 1) == "\2") break; + if (msg.substr(0, 1) == "\1") { FdSource source(builderOut.readSide.get()); auto ex = readError(source); ex.addTrace({}, "while setting up the build environment"); @@ -990,7 +991,7 @@ void LocalDerivationGoal::initTmpDir() { env[i.first] = i.second; } else { auto hash = hashString(htSHA256, i.first); - string fn = ".attr-" + hash.to_string(Base32, false); + std::string fn = ".attr-" + hash.to_string(Base32, false); Path p = tmpDir + "/" + fn; writeFile(p, rewriteStrings(i.second, inputRewrites)); chownToBuilder(p); @@ -1081,7 +1082,7 @@ void LocalDerivationGoal::writeStructuredAttrs() for (auto & [i, v] : json["outputs"].get()) { /* The placeholder must have a rewrite, so we use it to cover both the cases where we know or don't know the output path ahead of time. */ - rewritten[i] = rewriteStrings(v, inputRewrites); + rewritten[i] = rewriteStrings((std::string) v, inputRewrites); } json["outputs"] = rewritten; @@ -1187,10 +1188,14 @@ struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual Lo std::optional queryPathFromHashPart(const std::string & hashPart) override { throw Error("queryPathFromHashPart"); } - StorePath addToStore(const string & name, const Path & srcPath, - FileIngestionMethod method = FileIngestionMethod::Recursive, HashType hashAlgo = htSHA256, - PathFilter & filter = defaultPathFilter, RepairFlag repair = NoRepair, - const StorePathSet & references = StorePathSet()) override + StorePath addToStore( + std::string_view name, + const Path & srcPath, + FileIngestionMethod method, + HashType hashAlgo, + PathFilter & filter, + RepairFlag repair, + const StorePathSet & references) override { throw Error("addToStore"); } void addToStore(const ValidPathInfo & info, Source & narSource, @@ -1200,17 +1205,24 @@ struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual Lo goal.addDependency(info.path); } - StorePath addTextToStore(const string & name, const string & s, - const StorePathSet & references, RepairFlag repair = NoRepair) override + StorePath addTextToStore( + std::string_view name, + std::string_view s, + const StorePathSet & references, + RepairFlag repair = NoRepair) override { auto path = next->addTextToStore(name, s, references, repair); goal.addDependency(path); return path; } - StorePath addToStoreFromDump(Source & dump, std::string_view name, - FileIngestionMethod method = FileIngestionMethod::Recursive, HashType hashAlgo = htSHA256, RepairFlag repair = NoRepair, - const StorePathSet & references = StorePathSet()) override + StorePath addToStoreFromDump( + Source & dump, + std::string_view name, + FileIngestionMethod method, + HashType hashAlgo, + RepairFlag repair, + const StorePathSet & references) override { auto path = next->addToStoreFromDump(dump, name, method, hashAlgo, repair, references); goal.addDependency(path); @@ -1992,7 +2004,7 @@ void LocalDerivationGoal::runChild() args.push_back(rewriteStrings(i, inputRewrites)); /* Indicate that we managed to set up the build environment. */ - writeFull(STDERR_FILENO, string("\2\n")); + writeFull(STDERR_FILENO, std::string("\2\n")); /* Execute the program. This should not return. */ if (drv->isBuiltin()) { @@ -2010,7 +2022,7 @@ void LocalDerivationGoal::runChild() else if (drv->builder == "builtin:unpack-channel") builtinUnpackChannel(drv2); else - throw Error("unsupported builtin builder '%1%'", string(drv->builder, 8)); + throw Error("unsupported builtin builder '%1%'", drv->builder.substr(8)); _exit(0); } catch (std::exception & e) { writeFull(STDERR_FILENO, e.what() + std::string("\n")); @@ -2694,7 +2706,7 @@ void LocalDerivationGoal::checkOutputs(const std::map & out } if (!badPaths.empty()) { - string badPathsStr; + std::string badPathsStr; for (auto & i : badPaths) { badPathsStr += "\n "; badPathsStr += worker.store.printStorePath(i); diff --git a/src/libstore/build/local-derivation-goal.hh b/src/libstore/build/local-derivation-goal.hh index bfdf91d89..2d1222d2f 100644 --- a/src/libstore/build/local-derivation-goal.hh +++ b/src/libstore/build/local-derivation-goal.hh @@ -58,7 +58,7 @@ struct LocalDerivationGoal : public DerivationGoal typedef map DirsInChroot; // maps target path to source path DirsInChroot dirsInChroot; - typedef map Environment; + typedef map Environment; Environment env; #if __APPLE__ diff --git a/src/libstore/build/substitution-goal.cc b/src/libstore/build/substitution-goal.cc index 5ecf1da7e..c1bb1941d 100644 --- a/src/libstore/build/substitution-goal.cc +++ b/src/libstore/build/substitution-goal.cc @@ -272,7 +272,7 @@ void PathSubstitutionGoal::finished() } -void PathSubstitutionGoal::handleChildOutput(int fd, const string & data) +void PathSubstitutionGoal::handleChildOutput(int fd, std::string_view data) { } diff --git a/src/libstore/build/substitution-goal.hh b/src/libstore/build/substitution-goal.hh index 70c806d23..d8399d2a8 100644 --- a/src/libstore/build/substitution-goal.hh +++ b/src/libstore/build/substitution-goal.hh @@ -59,7 +59,7 @@ public: void timedOut(Error && ex) override { abort(); }; - string key() override + std::string key() override { /* "a$" ensures substitution goals happen before derivation goals. */ @@ -77,7 +77,7 @@ public: void finished(); /* Callback used by the worker to write to the log. */ - void handleChildOutput(int fd, const string & data) override; + void handleChildOutput(int fd, std::string_view data) override; void handleEOF(int fd) override; void cleanup() override; diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index 0a6108233..f72c1cc9c 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -394,7 +394,7 @@ void Worker::waitForInput() } else { printMsg(lvlVomit, "%1%: read %2% bytes", goal->getName(), rd); - string data((char *) buffer.data(), rd); + std::string data((char *) buffer.data(), rd); j->lastOutput = after; goal->handleChildOutput(k, data); } -- cgit v1.2.3 From a949673a5b4a08429e9866bc8558e961bb8fe130 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 28 Feb 2022 15:21:03 +0100 Subject: Fix Darwin build Fixes #6169 --- src/libstore/build/local-derivation-goal.cc | 4 ++-- src/libstore/build/local-derivation-goal.hh | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) (limited to 'src/libstore/build') diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 220f80602..a2a52e8f9 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -1933,7 +1933,7 @@ void LocalDerivationGoal::runChild() "can't map '%1%' to '%2%': mismatched impure paths not supported on Darwin", i.first, i.second.source); - string path = i.first; + std::string path = i.first; struct stat st; if (lstat(path.c_str(), &st)) { if (i.second.optional && errno == ENOENT) @@ -1985,7 +1985,7 @@ void LocalDerivationGoal::runChild() args.push_back("IMPORT_DIR=" + settings.nixDataDir + "/nix/sandbox/"); if (allowLocalNetworking) { args.push_back("-D"); - args.push_back(string("_ALLOW_LOCAL_NETWORKING=1")); + args.push_back(std::string("_ALLOW_LOCAL_NETWORKING=1")); } args.push_back(drv->builder); } else { diff --git a/src/libstore/build/local-derivation-goal.hh b/src/libstore/build/local-derivation-goal.hh index 2d1222d2f..95692c60d 100644 --- a/src/libstore/build/local-derivation-goal.hh +++ b/src/libstore/build/local-derivation-goal.hh @@ -62,7 +62,7 @@ struct LocalDerivationGoal : public DerivationGoal Environment env; #if __APPLE__ - typedef string SandboxProfile; + typedef std::string SandboxProfile; SandboxProfile additionalSandboxProfile; #endif -- cgit v1.2.3 From b91500a14e5f1bab33cd9dccbe9a85ad2832062a Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 28 Feb 2022 15:24:24 +0100 Subject: Fix clang warning --- src/libstore/build/local-derivation-goal.cc | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) (limited to 'src/libstore/build') diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index a2a52e8f9..7e69d4145 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -2383,14 +2383,10 @@ void LocalDerivationGoal::registerOutputs() [&](DerivationOutputCAFloating dof) { return newInfoFromCA(dof); }, - [&](DerivationOutputDeferred) { + [&](DerivationOutputDeferred) -> ValidPathInfo { // No derivation should reach that point without having been // rewritten first assert(false); - // Ugly, but the compiler insists on having this return a value - // of type `ValidPathInfo` despite the `assert(false)`, so - // let's provide it - return *(ValidPathInfo*)0; }, }, output.output); -- cgit v1.2.3 From 7a04839ea53986a0175bf085e7d061ab8a32d59b Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Mon, 31 Jan 2022 22:05:28 +0100 Subject: ssh-ng: also store build logs to make them accessible by `nix log` Right now when building a derivation remotely via $ nix build -j0 -f . hello -L --builders 'ssh://builder' it's possible later to read through the entire build-log by running `nix log -f . hello`. This isn't possible however when using `ssh-ng` rather than `ssh`. The reason for that is that there are two different ways to transfer logs in Nix through e.g. an SSH tunnel (that are used by `ssh`/`ssh-ng` respectively): * `ssh://` receives its logs from the fd pointing to `builderOut`. This is directly passed to the "log-sink" (and to the logger on each `\n`), hence `nix log` works here. * `ssh-ng://` however expects JSON-like messages (i.e. `@nix {log data in here}`) and passes it directly to the logger without doing anything with the `logSink`. However it's certainly possible to extract log-lines from this format as these have their own message-type in the JSON payload (i.e. `resBuildLogLine`). This is basically what I changed in this patch: if the code-path for `builderOut` is not reached and a `logSink` is initialized, the message was successfully processed by the JSON logger (i.e. it's in the expected format) and the line is of the expected type (i.e. `resBuildLogLine`), the line will be written to the log-sink as well. Closes #5079 --- src/libstore/build/derivation-goal.cc | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) (limited to 'src/libstore/build') diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 95da1841d..ef1135601 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -1175,10 +1175,10 @@ bool DerivationGoal::isReadDesc(int fd) return fd == hook->builderOut.readSide.get(); } - void DerivationGoal::handleChildOutput(int fd, std::string_view data) { - if (isReadDesc(fd)) + auto isWrittenToLog = isReadDesc(fd); + if (isWrittenToLog) { logSize += data.size(); if (settings.maxLogSize && logSize > settings.maxLogSize) { @@ -1207,7 +1207,14 @@ void DerivationGoal::handleChildOutput(int fd, std::string_view data) if (hook && fd == hook->fromHook.readSide.get()) { for (auto c : data) if (c == '\n') { - handleJSONLogMessage(currentHookLine, worker.act, hook->activities, true); + auto s = handleJSONLogMessage(currentHookLine, worker.act, hook->activities, true); + if (s && !isWrittenToLog && logSink) { + auto json = nlohmann::json::parse(std::string(currentHookLine, 5)); + if (json["type"] == resBuildLogLine) { + auto f = json["fields"]; + (*logSink)((f.size() > 0 ? f.at(0).get() : "") + "\n"); + } + } currentHookLine.clear(); } else currentHookLine += c; -- cgit v1.2.3 From cd92ea588504db697103d3ff56166fe61e6da131 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Sat, 19 Feb 2022 22:34:50 +0100 Subject: libstore/derivation-goal: avoid double-parsing of JSON messages To avoid that JSON messages are parsed twice in case of remote builds with `ssh-ng://`, I split up the original `handleJSONLogMessage` into three parts: * `parseJSONMessage(const std::string&)` checks if it's a message in the form of `@nix {...}` and tries to parse it (and prints an error if the parsing fails). * `handleJSONLogMessage(nlohmann::json&, ...)` reads the fields from the message and passes them to the logger. * `handleJSONLogMessage(const std::string&, ...)` behaves as before, but uses the two functions mentioned above as implementation. In case of `ssh-ng://`-logs the first two methods are invoked manually. --- src/libstore/build/derivation-goal.cc | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) (limited to 'src/libstore/build') diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index ef1135601..c49be5e79 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -1207,12 +1207,14 @@ void DerivationGoal::handleChildOutput(int fd, std::string_view data) if (hook && fd == hook->fromHook.readSide.get()) { for (auto c : data) if (c == '\n') { - auto s = handleJSONLogMessage(currentHookLine, worker.act, hook->activities, true); - if (s && !isWrittenToLog && logSink) { - auto json = nlohmann::json::parse(std::string(currentHookLine, 5)); - if (json["type"] == resBuildLogLine) { - auto f = json["fields"]; - (*logSink)((f.size() > 0 ? f.at(0).get() : "") + "\n"); + auto json = parseJSONMessage(currentHookLine); + if (json) { + auto s = handleJSONLogMessage(*json, worker.act, hook->activities, true); + if (s && !isWrittenToLog && logSink) { + if ((*json)["type"] == resBuildLogLine) { + auto f = (*json)["fields"]; + (*logSink)((f.size() > 0 ? f.at(0).get() : "") + "\n"); + } } } currentHookLine.clear(); -- cgit v1.2.3 From 102cb390864f8a142ad6730d3456784e2778a493 Mon Sep 17 00:00:00 2001 From: Maximilian Bosch Date: Sun, 20 Feb 2022 16:02:44 +0100 Subject: libstore/build: add a few explanatory comments; simplify --- src/libstore/build/derivation-goal.cc | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) (limited to 'src/libstore/build') diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index c49be5e79..ae250ffaf 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -1177,6 +1177,7 @@ bool DerivationGoal::isReadDesc(int fd) void DerivationGoal::handleChildOutput(int fd, std::string_view data) { + // local & `ssh://`-builds are dealt with here. auto isWrittenToLog = isReadDesc(fd); if (isWrittenToLog) { @@ -1210,11 +1211,11 @@ void DerivationGoal::handleChildOutput(int fd, std::string_view data) auto json = parseJSONMessage(currentHookLine); if (json) { auto s = handleJSONLogMessage(*json, worker.act, hook->activities, true); - if (s && !isWrittenToLog && logSink) { - if ((*json)["type"] == resBuildLogLine) { - auto f = (*json)["fields"]; - (*logSink)((f.size() > 0 ? f.at(0).get() : "") + "\n"); - } + // ensure that logs from a builder using `ssh-ng://` as protocol + // are also available to `nix log`. + if (s && !isWrittenToLog && logSink && (*json)["type"] == resBuildLogLine) { + auto f = (*json)["fields"]; + (*logSink)((f.size() > 0 ? f.at(0).get() : "") + "\n"); } } currentHookLine.clear(); -- cgit v1.2.3 From e862833ec662c1bffbe31b9a229147de391e801a Mon Sep 17 00:00:00 2001 From: John Ericson Date: Tue, 1 Mar 2022 19:43:07 +0000 Subject: Move `BuildResult` defintion to its own header Just like we did for `ValidPathInfo` in d92d4f85a5c8a2a2385c084500a8b6bd54b54e6c. --- src/libstore/build/derivation-goal.hh | 1 + 1 file changed, 1 insertion(+) (limited to 'src/libstore/build') diff --git a/src/libstore/build/derivation-goal.hh b/src/libstore/build/derivation-goal.hh index d947482ef..6f158bdbf 100644 --- a/src/libstore/build/derivation-goal.hh +++ b/src/libstore/build/derivation-goal.hh @@ -2,6 +2,7 @@ #include "parsed-derivations.hh" #include "lock.hh" +#include "build-result.hh" #include "store-api.hh" #include "pathlocks.hh" #include "goal.hh" -- cgit v1.2.3 From 6636202356b94ca4128462493770e7fedf997b0e Mon Sep 17 00:00:00 2001 From: John Ericson Date: Tue, 1 Mar 2022 18:31:36 +0000 Subject: Factor out a `GcStore` interface Starts progress on #5729. The idea is that we should not have these default methods throwing "unimplemented". This is a small step in that direction. I kept `addTempRoot` because it is a no-op, rather than failure. Also, as a practical matter, it is called all over the place, while doing other tasks, so the downcasting would be annoying. Maybe in the future I could move the "real" `addTempRoot` to `GcStore`, and the existing usecases use a `tryAddTempRoot` wrapper to downcast or do nothing, but I wasn't sure whether that was a good idea so with a bias to less churn I didn't do it yet. --- src/libstore/build/local-derivation-goal.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'src/libstore/build') diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 7e69d4145..581d63d0e 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -1,4 +1,5 @@ #include "local-derivation-goal.hh" +#include "gc-store.hh" #include "hook-instance.hh" #include "worker.hh" #include "builtins.hh" @@ -1127,7 +1128,7 @@ struct RestrictedStoreConfig : virtual LocalFSStoreConfig /* A wrapper around LocalStore that only allows building/querying of paths that are in the input closures of the build or were added via recursive Nix calls. */ -struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual LocalFSStore +struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual LocalFSStore, public virtual GcStore { ref next; -- cgit v1.2.3 From a4604f19284254ac98f19a13ff7c2216de7fe176 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 8 Mar 2022 19:50:46 +0100 Subject: Add Store::buildPathsWithResults() This function is like buildPaths(), except that it returns a vector of BuildResults containing the exact statuses and output paths of each derivation / substitution. This is convenient for functions like Installable::build(), because they then don't need to do another series of calls to get the outputs of CA derivations. It's also a precondition to impure derivations, where we *can't* query the output of those derivations since they're not stored in the Nix database. Note that PathSubstitutionGoal can now also return a BuildStatus. --- src/libstore/build/derivation-goal.cc | 177 +++++++++++---------- src/libstore/build/derivation-goal.hh | 31 ++-- src/libstore/build/drv-output-substitution-goal.cc | 4 +- src/libstore/build/entry-points.cc | 51 +++--- src/libstore/build/goal.hh | 4 + src/libstore/build/local-derivation-goal.cc | 96 ++++++----- src/libstore/build/local-derivation-goal.hh | 2 +- src/libstore/build/substitution-goal.cc | 18 ++- src/libstore/build/substitution-goal.hh | 2 + 9 files changed, 209 insertions(+), 176 deletions(-) (limited to 'src/libstore/build') diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index ae250ffaf..cd582bb01 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -135,7 +135,7 @@ void DerivationGoal::killChild() void DerivationGoal::timedOut(Error && ex) { killChild(); - done(BuildResult::TimedOut, ex); + done(BuildResult::TimedOut, {}, ex); } @@ -182,7 +182,7 @@ void DerivationGoal::loadDerivation() trace("loading derivation"); if (nrFailed != 0) { - done(BuildResult::MiscFailure, Error("cannot build missing derivation '%s'", worker.store.printStorePath(drvPath))); + done(BuildResult::MiscFailure, {}, Error("cannot build missing derivation '%s'", worker.store.printStorePath(drvPath))); return; } @@ -215,28 +215,20 @@ void DerivationGoal::haveDerivation() auto outputHashes = staticOutputHashes(worker.evalStore, *drv); for (auto & [outputName, outputHash] : outputHashes) - initialOutputs.insert({ + initialOutputs.insert({ outputName, - InitialOutput{ + InitialOutput { .wanted = true, // Will be refined later .outputHash = outputHash } - }); + }); /* Check what outputs paths are not already valid. */ - checkPathValidity(); - bool allValid = true; - for (auto & [_, status] : initialOutputs) { - if (!status.wanted) continue; - if (!status.known || !status.known->isValid()) { - allValid = false; - break; - } - } + auto [allValid, validOutputs] = checkPathValidity(); /* If they are all valid, then we're done. */ if (allValid && buildMode == bmNormal) { - done(BuildResult::AlreadyValid); + done(BuildResult::AlreadyValid, std::move(validOutputs)); return; } @@ -277,7 +269,7 @@ void DerivationGoal::outputsSubstitutionTried() trace("all outputs substituted (maybe)"); if (nrFailed > 0 && nrFailed > nrNoSubstituters + nrIncompleteClosure && !settings.tryFallback) { - done(BuildResult::TransientFailure, + done(BuildResult::TransientFailure, {}, Error("some substitutes for the outputs of derivation '%s' failed (usually happens due to networking issues); try '--fallback' to build derivation from source ", worker.store.printStorePath(drvPath))); return; @@ -301,23 +293,17 @@ void DerivationGoal::outputsSubstitutionTried() return; } - checkPathValidity(); - size_t nrInvalid = 0; - for (auto & [_, status] : initialOutputs) { - if (!status.wanted) continue; - if (!status.known || !status.known->isValid()) - nrInvalid++; - } + auto [allValid, validOutputs] = checkPathValidity(); - if (buildMode == bmNormal && nrInvalid == 0) { - done(BuildResult::Substituted); + if (buildMode == bmNormal && allValid) { + done(BuildResult::Substituted, std::move(validOutputs)); return; } - if (buildMode == bmRepair && nrInvalid == 0) { + if (buildMode == bmRepair && allValid) { repairClosure(); return; } - if (buildMode == bmCheck && nrInvalid > 0) + if (buildMode == bmCheck && !allValid) throw Error("some outputs of '%s' are not valid, so checking is not possible", worker.store.printStorePath(drvPath)); @@ -409,7 +395,7 @@ void DerivationGoal::repairClosure() } if (waitees.empty()) { - done(BuildResult::AlreadyValid); + done(BuildResult::AlreadyValid, assertPathValidity()); return; } @@ -423,7 +409,7 @@ void DerivationGoal::closureRepaired() if (nrFailed > 0) throw Error("some paths in the output closure of derivation '%s' could not be repaired", worker.store.printStorePath(drvPath)); - done(BuildResult::AlreadyValid); + done(BuildResult::AlreadyValid, assertPathValidity()); } @@ -434,7 +420,7 @@ void DerivationGoal::inputsRealised() if (nrFailed != 0) { if (!useDerivation) throw Error("some dependencies of '%s' are missing", worker.store.printStorePath(drvPath)); - done(BuildResult::DependencyFailed, Error( + done(BuildResult::DependencyFailed, {}, Error( "%s dependencies of derivation '%s' failed to build", nrFailed, worker.store.printStorePath(drvPath))); return; @@ -523,10 +509,11 @@ void DerivationGoal::inputsRealised() state = &DerivationGoal::tryToBuild; worker.wakeUp(shared_from_this()); - result = BuildResult(); + buildResult = BuildResult(); } -void DerivationGoal::started() { +void DerivationGoal::started() +{ auto msg = fmt( buildMode == bmRepair ? "repairing outputs of '%s'" : buildMode == bmCheck ? "checking outputs of '%s'" : @@ -588,19 +575,12 @@ void DerivationGoal::tryToBuild() omitted, but that would be less efficient.) Note that since we now hold the locks on the output paths, no other process can build this derivation, so no further checks are necessary. */ - checkPathValidity(); - bool allValid = true; - for (auto & [_, status] : initialOutputs) { - if (!status.wanted) continue; - if (!status.known || !status.known->isValid()) { - allValid = false; - break; - } - } + auto [allValid, validOutputs] = checkPathValidity(); + if (buildMode != bmCheck && allValid) { debug("skipping build of derivation '%s', someone beat us to it", worker.store.printStorePath(drvPath)); outputLocks.setDeletion(true); - done(BuildResult::AlreadyValid); + done(BuildResult::AlreadyValid, std::move(validOutputs)); return; } @@ -626,7 +606,7 @@ void DerivationGoal::tryToBuild() /* Yes, it has started doing so. Wait until we get EOF from the hook. */ actLock.reset(); - result.startTime = time(0); // inexact + buildResult.startTime = time(0); // inexact state = &DerivationGoal::buildDone; started(); return; @@ -830,8 +810,8 @@ void DerivationGoal::buildDone() debug("builder process for '%s' finished", worker.store.printStorePath(drvPath)); - result.timesBuilt++; - result.stopTime = time(0); + buildResult.timesBuilt++; + buildResult.stopTime = time(0); /* So the child is gone now. */ worker.childTerminated(this); @@ -876,11 +856,11 @@ void DerivationGoal::buildDone() /* Compute the FS closure of the outputs and register them as being valid. */ - registerOutputs(); + auto builtOutputs = registerOutputs(); StorePathSet outputPaths; - for (auto & [_, path] : finalOutputs) - outputPaths.insert(path); + for (auto & [_, output] : buildResult.builtOutputs) + outputPaths.insert(output.outPath); runPostBuildHook( worker.store, *logger, @@ -890,7 +870,7 @@ void DerivationGoal::buildDone() if (buildMode == bmCheck) { cleanupPostOutputsRegisteredModeCheck(); - done(BuildResult::Built); + done(BuildResult::Built, std::move(builtOutputs)); return; } @@ -911,6 +891,8 @@ void DerivationGoal::buildDone() outputLocks.setDeletion(true); outputLocks.unlock(); + done(BuildResult::Built, std::move(builtOutputs)); + } catch (BuildError & e) { outputLocks.unlock(); @@ -930,14 +912,13 @@ void DerivationGoal::buildDone() BuildResult::PermanentFailure; } - done(st, e); + done(st, {}, e); return; } - - done(BuildResult::Built); } -void DerivationGoal::resolvedFinished() { +void DerivationGoal::resolvedFinished() +{ assert(resolvedDrvGoal); auto resolvedDrv = *resolvedDrvGoal->drv; @@ -950,11 +931,13 @@ void DerivationGoal::resolvedFinished() { if (realWantedOutputs.empty()) realWantedOutputs = resolvedDrv.outputNames(); + DrvOutputs builtOutputs; + for (auto & wantedOutput : realWantedOutputs) { assert(initialOutputs.count(wantedOutput) != 0); assert(resolvedHashes.count(wantedOutput) != 0); auto realisation = worker.store.queryRealisation( - DrvOutput{resolvedHashes.at(wantedOutput), wantedOutput} + DrvOutput{resolvedHashes.at(wantedOutput), wantedOutput} ); // We've just built it, but maybe the build failed, in which case the // realisation won't be there @@ -966,10 +949,11 @@ void DerivationGoal::resolvedFinished() { signRealisation(newRealisation); worker.store.registerDrvOutput(newRealisation); outputPaths.insert(realisation->outPath); + builtOutputs.emplace(realisation->id, *realisation); } else { // If we don't have a realisation, then it must mean that something // failed when building the resolved drv - assert(!result.success()); + assert(!buildResult.success()); } } @@ -981,7 +965,7 @@ void DerivationGoal::resolvedFinished() { ); auto status = [&]() { - auto resolvedResult = resolvedDrvGoal->getResult(); + auto & resolvedResult = resolvedDrvGoal->buildResult; switch (resolvedResult.status) { case BuildResult::AlreadyValid: return BuildResult::ResolvesToAlreadyValid; @@ -990,7 +974,7 @@ void DerivationGoal::resolvedFinished() { } }(); - done(status); + done(status, std::move(builtOutputs)); } HookReply DerivationGoal::tryBuildHook() @@ -1100,7 +1084,7 @@ HookReply DerivationGoal::tryBuildHook() } -void DerivationGoal::registerOutputs() +DrvOutputs DerivationGoal::registerOutputs() { /* When using a build hook, the build hook can register the output as valid (by doing `nix-store --import'). If so we don't have @@ -1109,21 +1093,7 @@ void DerivationGoal::registerOutputs() We can only early return when the outputs are known a priori. For floating content-addressed derivations this isn't the case. */ - for (auto & [outputName, optOutputPath] : worker.store.queryPartialDerivationOutputMap(drvPath)) { - if (!wantOutput(outputName, wantedOutputs)) - continue; - if (!optOutputPath) - throw BuildError( - "output '%s' from derivation '%s' does not have a known output path", - outputName, worker.store.printStorePath(drvPath)); - auto & outputPath = *optOutputPath; - if (!worker.store.isValidPath(outputPath)) - throw BuildError( - "output '%s' from derivation '%s' is supposed to be at '%s' but that path is not valid", - outputName, worker.store.printStorePath(drvPath), worker.store.printStorePath(outputPath)); - - finalOutputs.insert_or_assign(outputName, outputPath); - } + return assertPathValidity(); } Path DerivationGoal::openLogFile() @@ -1185,7 +1155,7 @@ void DerivationGoal::handleChildOutput(int fd, std::string_view data) if (settings.maxLogSize && logSize > settings.maxLogSize) { killChild(); done( - BuildResult::LogLimitExceeded, + BuildResult::LogLimitExceeded, {}, Error("%s killed after writing more than %d bytes of log output", getName(), settings.maxLogSize)); return; @@ -1274,10 +1244,12 @@ OutputPathMap DerivationGoal::queryDerivationOutputMap() } -void DerivationGoal::checkPathValidity() +std::pair DerivationGoal::checkPathValidity() { bool checkHash = buildMode == bmRepair; auto wantedOutputsLeft = wantedOutputs; + DrvOutputs validOutputs; + for (auto & i : queryPartialDerivationOutputMap()) { InitialOutput & info = initialOutputs.at(i.first); info.wanted = wantOutput(i.first, wantedOutputs); @@ -1294,26 +1266,28 @@ void DerivationGoal::checkPathValidity() : PathStatus::Corrupt, }; } + auto drvOutput = DrvOutput{initialOutputs.at(i.first).outputHash, i.first}; if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations)) { - auto drvOutput = DrvOutput{initialOutputs.at(i.first).outputHash, i.first}; if (auto real = worker.store.queryRealisation(drvOutput)) { info.known = { .path = real->outPath, .status = PathStatus::Valid, }; - } else if (info.known && info.known->status == PathStatus::Valid) { - // We know the output because it' a static output of the + } else if (info.known && info.known->isValid()) { + // We know the output because it's a static output of the // derivation, and the output path is valid, but we don't have // its realisation stored (probably because it has been built - // without the `ca-derivations` experimental flag) + // without the `ca-derivations` experimental flag). worker.store.registerDrvOutput( - Realisation{ + Realisation { drvOutput, info.known->path, } ); } } + if (info.wanted && info.known && info.known->isValid()) + validOutputs.emplace(drvOutput, Realisation { drvOutput, info.known->path }); } // If we requested all the outputs via the empty set, we are always fine. // If we requested specific elements, the loop above removes all the valid @@ -1322,24 +1296,51 @@ void DerivationGoal::checkPathValidity() throw Error("derivation '%s' does not have wanted outputs %s", worker.store.printStorePath(drvPath), concatStringsSep(", ", quoteStrings(wantedOutputsLeft))); + + bool allValid = true; + for (auto & [_, status] : initialOutputs) { + if (!status.wanted) continue; + if (!status.known || !status.known->isValid()) { + allValid = false; + break; + } + } + + return { allValid, validOutputs }; +} + + +DrvOutputs DerivationGoal::assertPathValidity() +{ + auto [allValid, validOutputs] = checkPathValidity(); + if (!allValid) + throw Error("some outputs are unexpectedly invalid"); + return validOutputs; } -void DerivationGoal::done(BuildResult::Status status, std::optional ex) +void DerivationGoal::done( + BuildResult::Status status, + DrvOutputs builtOutputs, + std::optional ex) { - result.status = status; + buildResult.drvPath = drvPath; + buildResult.status = status; if (ex) - result.errorMsg = ex->what(); - amDone(result.success() ? ecSuccess : ecFailed, ex); - if (result.status == BuildResult::TimedOut) + // FIXME: strip: "error: " + buildResult.errorMsg = ex->what(); + amDone(buildResult.success() ? ecSuccess : ecFailed, ex); + if (buildResult.status == BuildResult::TimedOut) worker.timedOut = true; - if (result.status == BuildResult::PermanentFailure) + if (buildResult.status == BuildResult::PermanentFailure) worker.permanentFailure = true; mcExpectedBuilds.reset(); mcRunningBuilds.reset(); - if (result.success()) { + if (buildResult.success()) { + assert(!builtOutputs.empty()); + buildResult.builtOutputs = std::move(builtOutputs); if (status == BuildResult::Built) worker.doneBuilds++; } else { @@ -1353,7 +1354,7 @@ void DerivationGoal::done(BuildResult::Status status, std::optional ex) if (traceBuiltOutputsFile != "") { std::fstream fs; fs.open(traceBuiltOutputsFile, std::fstream::out); - fs << worker.store.printStorePath(drvPath) << "\t" << result.toString() << std::endl; + fs << worker.store.printStorePath(drvPath) << "\t" << buildResult.toString() << std::endl; } } diff --git a/src/libstore/build/derivation-goal.hh b/src/libstore/build/derivation-goal.hh index 6f158bdbf..ea2db89b2 100644 --- a/src/libstore/build/derivation-goal.hh +++ b/src/libstore/build/derivation-goal.hh @@ -2,7 +2,6 @@ #include "parsed-derivations.hh" #include "lock.hh" -#include "build-result.hh" #include "store-api.hh" #include "pathlocks.hh" #include "goal.hh" @@ -105,20 +104,8 @@ struct DerivationGoal : public Goal typedef void (DerivationGoal::*GoalState)(); GoalState state; - /* The final output paths of the build. - - - For input-addressed derivations, always the precomputed paths - - - For content-addressed derivations, calcuated from whatever the hash - ends up being. (Note that fixed outputs derivations that produce the - "wrong" output still install that data under its true content-address.) - */ - OutputPathMap finalOutputs; - BuildMode buildMode; - BuildResult result; - /* The current round, if we're building multiple times. */ size_t curRound = 1; @@ -153,8 +140,6 @@ struct DerivationGoal : public Goal /* Add wanted outputs to an already existing derivation goal. */ void addWantedOutputs(const StringSet & outputs); - BuildResult getResult() { return result; } - /* The states. */ void getDerivation(); void loadDerivation(); @@ -176,7 +161,7 @@ struct DerivationGoal : public Goal /* Check that the derivation outputs all exist and register them as valid. */ - virtual void registerOutputs(); + virtual DrvOutputs registerOutputs(); /* Open a log file and a pipe to it. */ Path openLogFile(); @@ -211,8 +196,17 @@ struct DerivationGoal : public Goal std::map> queryPartialDerivationOutputMap(); OutputPathMap queryDerivationOutputMap(); - /* Return the set of (in)valid paths. */ - void checkPathValidity(); + /* Update 'initialOutputs' to determine the current status of the + outputs of the derivation. Also returns a Boolean denoting + whether all outputs are valid and non-corrupt, and a + 'DrvOutputs' structure containing the valid and wanted + outputs. */ + std::pair checkPathValidity(); + + /* Aborts if any output is not valid or corrupt, and otherwise + returns a 'DrvOutputs' structure containing the wanted + outputs. */ + DrvOutputs assertPathValidity(); /* Forcibly kill the child process, if any. */ virtual void killChild(); @@ -223,6 +217,7 @@ struct DerivationGoal : public Goal void done( BuildResult::Status status, + DrvOutputs builtOutputs = {}, std::optional ex = {}); StorePathSet exportReferences(const StorePathSet & storePaths); diff --git a/src/libstore/build/drv-output-substitution-goal.cc b/src/libstore/build/drv-output-substitution-goal.cc index e6e810cdd..946ec1aff 100644 --- a/src/libstore/build/drv-output-substitution-goal.cc +++ b/src/libstore/build/drv-output-substitution-goal.cc @@ -32,7 +32,7 @@ void DrvOutputSubstitutionGoal::init() void DrvOutputSubstitutionGoal::tryNext() { - trace("Trying next substituter"); + trace("trying next substituter"); if (subs.size() == 0) { /* None left. Terminate this goal and let someone else deal @@ -119,7 +119,7 @@ void DrvOutputSubstitutionGoal::realisationFetched() void DrvOutputSubstitutionGoal::outPathValid() { assert(outputInfo); - trace("Output path substituted"); + trace("output path substituted"); if (nrFailed > 0) { debug("The output path of the derivation output '%s' could not be substituted", id.to_string()); diff --git a/src/libstore/build/entry-points.cc b/src/libstore/build/entry-points.cc index 9b4cfd835..b2f87aa82 100644 --- a/src/libstore/build/entry-points.cc +++ b/src/libstore/build/entry-points.cc @@ -47,6 +47,35 @@ void Store::buildPaths(const std::vector & reqs, BuildMode buildMod } } +std::vector Store::buildPathsWithResults( + const std::vector & reqs, + BuildMode buildMode, + std::shared_ptr evalStore) +{ + Worker worker(*this, evalStore ? *evalStore : *this); + + Goals goals; + for (const auto & br : reqs) { + std::visit(overloaded { + [&](const DerivedPath::Built & bfd) { + goals.insert(worker.makeDerivationGoal(bfd.drvPath, bfd.outputs, buildMode)); + }, + [&](const DerivedPath::Opaque & bo) { + goals.insert(worker.makePathSubstitutionGoal(bo.path, buildMode == bmRepair ? Repair : NoRepair)); + }, + }, br.raw()); + } + + worker.run(goals); + + std::vector results; + + for (auto & i : goals) + results.push_back(i->buildResult); + + return results; +} + BuildResult Store::buildDerivation(const StorePath & drvPath, const BasicDerivation & drv, BuildMode buildMode) { @@ -57,31 +86,11 @@ BuildResult Store::buildDerivation(const StorePath & drvPath, const BasicDerivat try { worker.run(Goals{goal}); - result = goal->getResult(); + result = goal->buildResult; } catch (Error & e) { result.status = BuildResult::MiscFailure; result.errorMsg = e.msg(); } - // XXX: Should use `goal->queryPartialDerivationOutputMap()` once it's - // extended to return the full realisation for each output - auto staticDrvOutputs = drv.outputsAndOptPaths(*this); - auto outputHashes = staticOutputHashes(*this, drv); - for (auto & [outputName, staticOutput] : staticDrvOutputs) { - auto outputId = DrvOutput{outputHashes.at(outputName), outputName}; - if (staticOutput.second) - result.builtOutputs.insert_or_assign( - outputId, - Realisation{ outputId, *staticOutput.second} - ); - if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations) && !derivationHasKnownOutputPaths(drv.type())) { - auto realisation = this->queryRealisation(outputId); - if (realisation) - result.builtOutputs.insert_or_assign( - outputId, - *realisation - ); - } - } return result; } diff --git a/src/libstore/build/goal.hh b/src/libstore/build/goal.hh index 68e08bdf9..fcf3d0084 100644 --- a/src/libstore/build/goal.hh +++ b/src/libstore/build/goal.hh @@ -2,6 +2,7 @@ #include "types.hh" #include "store-api.hh" +#include "build-result.hh" namespace nix { @@ -55,6 +56,9 @@ struct Goal : public std::enable_shared_from_this /* Whether the goal is finished. */ ExitCode exitCode; + /* Build result. */ + BuildResult buildResult; + /* Exception containing an error message, if any. */ std::optional ex; diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 581d63d0e..a372728f5 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -194,7 +194,7 @@ void LocalDerivationGoal::tryLocalBuild() { outputLocks.unlock(); buildUser.reset(); worker.permanentFailure = true; - done(BuildResult::InputRejected, e); + done(BuildResult::InputRejected, {}, e); return; } @@ -756,7 +756,7 @@ void LocalDerivationGoal::startBuilder() if (tcsetattr(builderOut.writeSide.get(), TCSANOW, &term)) throw SysError("putting pseudoterminal into raw mode"); - result.startTime = time(0); + buildResult.startTime = time(0); /* Fork a child to build the package. */ @@ -1260,6 +1260,16 @@ struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual Lo } void buildPaths(const std::vector & paths, BuildMode buildMode, std::shared_ptr evalStore) override + { + for (auto & result : buildPathsWithResults(paths, buildMode, evalStore)) + if (!result.success()) + result.rethrow(); + } + + std::vector buildPathsWithResults( + const std::vector & paths, + BuildMode buildMode = bmNormal, + std::shared_ptr evalStore = nullptr) override { assert(!evalStore); @@ -1273,26 +1283,13 @@ struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual Lo throw InvalidPath("cannot build '%s' in recursive Nix because path is unknown", req.to_string(*next)); } - next->buildPaths(paths, buildMode); - - for (auto & path : paths) { - auto p = std::get_if(&path); - if (!p) continue; - auto & bfd = *p; - auto drv = readDerivation(bfd.drvPath); - auto drvHashes = staticOutputHashes(*this, drv); - auto outputs = next->queryDerivationOutputMap(bfd.drvPath); - for (auto & [outputName, outputPath] : outputs) - if (wantOutput(outputName, bfd.outputs)) { - newPaths.insert(outputPath); - if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations)) { - auto thisRealisation = next->queryRealisation( - DrvOutput{drvHashes.at(outputName), outputName} - ); - assert(thisRealisation); - newRealisations.insert(*thisRealisation); - } - } + auto results = next->buildPathsWithResults(paths, buildMode); + + for (auto & result : results) { + for (auto & [outputName, output] : result.builtOutputs) { + newPaths.insert(output.outPath); + newRealisations.insert(output); + } } StorePathSet closure; @@ -1301,6 +1298,8 @@ struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual Lo goal.addDependency(path); for (auto & real : Realisation::closure(*next, newRealisations)) goal.addedDrvOutputs.insert(real.id); + + return results; } BuildResult buildDerivation(const StorePath & drvPath, const BasicDerivation & drv, @@ -2069,7 +2068,7 @@ void LocalDerivationGoal::runChild() } -void LocalDerivationGoal::registerOutputs() +DrvOutputs LocalDerivationGoal::registerOutputs() { /* When using a build hook, the build hook can register the output as valid (by doing `nix-store --import'). If so we don't have @@ -2078,10 +2077,8 @@ void LocalDerivationGoal::registerOutputs() We can only early return when the outputs are known a priori. For floating content-addressed derivations this isn't the case. */ - if (hook) { - DerivationGoal::registerOutputs(); - return; - } + if (hook) + return DerivationGoal::registerOutputs(); std::map infos; @@ -2204,6 +2201,8 @@ void LocalDerivationGoal::registerOutputs() std::reverse(sortedOutputNames.begin(), sortedOutputNames.end()); + OutputPathMap finalOutputs; + for (auto & outputName : sortedOutputNames) { auto output = drv->outputs.at(outputName); auto & scratchPath = scratchOutputs.at(outputName); @@ -2340,6 +2339,7 @@ void LocalDerivationGoal::registerOutputs() }; ValidPathInfo newInfo = std::visit(overloaded { + [&](const DerivationOutputInputAddressed & output) { /* input-addressed case */ auto requiredFinalPath = output.path; @@ -2359,6 +2359,7 @@ void LocalDerivationGoal::registerOutputs() newInfo0.references.insert(newInfo0.path); return newInfo0; }, + [&](const DerivationOutputCAFixed & dof) { auto newInfo0 = newInfoFromCA(DerivationOutputCAFloating { .method = dof.hash.method, @@ -2381,14 +2382,17 @@ void LocalDerivationGoal::registerOutputs() } return newInfo0; }, - [&](DerivationOutputCAFloating dof) { + + [&](DerivationOutputCAFloating & dof) { return newInfoFromCA(dof); }, + [&](DerivationOutputDeferred) -> ValidPathInfo { // No derivation should reach that point without having been // rewritten first assert(false); }, + }, output.output); /* FIXME: set proper permissions in restorePath() so @@ -2499,11 +2503,12 @@ void LocalDerivationGoal::registerOutputs() } if (buildMode == bmCheck) { - // In case of FOD mismatches on `--check` an error must be thrown as this is also - // a source for non-determinism. + /* In case of fixed-output derivations, if there are + mismatches on `--check` an error must be thrown as this is + also a source for non-determinism. */ if (delayedException) std::rethrow_exception(delayedException); - return; + return assertPathValidity(); } /* Apply output checks. */ @@ -2515,7 +2520,7 @@ void LocalDerivationGoal::registerOutputs() assert(prevInfos.size() == infos.size()); for (auto i = prevInfos.begin(), j = infos.begin(); i != prevInfos.end(); ++i, ++j) if (!(*i == *j)) { - result.isNonDeterministic = true; + buildResult.isNonDeterministic = true; Path prev = worker.store.printStorePath(i->second.path) + checkSuffix; bool prevExists = keepPreviousRound && pathExists(prev); hintformat hint = prevExists @@ -2553,7 +2558,7 @@ void LocalDerivationGoal::registerOutputs() if (curRound < nrRounds) { prevInfos = std::move(infos); - return; + return {}; } /* Remove the .check directories if we're done. FIXME: keep them @@ -2588,17 +2593,24 @@ void LocalDerivationGoal::registerOutputs() means it's safe to link the derivation to the output hash. We must do that for floating CA derivations, which otherwise couldn't be cached, but it's fine to do in all cases. */ + DrvOutputs builtOutputs; - if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations)) { - for (auto& [outputName, newInfo] : infos) { - auto thisRealisation = Realisation{ - .id = DrvOutput{initialOutputs.at(outputName).outputHash, - outputName}, - .outPath = newInfo.path}; + for (auto & [outputName, newInfo] : infos) { + auto thisRealisation = Realisation { + .id = DrvOutput { + initialOutputs.at(outputName).outputHash, + outputName + }, + .outPath = newInfo.path + }; + if (settings.isExperimentalFeatureEnabled(Xp::CaDerivations)) { signRealisation(thisRealisation); worker.store.registerDrvOutput(thisRealisation); } + builtOutputs.emplace(thisRealisation.id, thisRealisation); } + + return builtOutputs; } void LocalDerivationGoal::signRealisation(Realisation & realisation) @@ -2607,7 +2619,7 @@ void LocalDerivationGoal::signRealisation(Realisation & realisation) } -void LocalDerivationGoal::checkOutputs(const std::map & outputs) +void LocalDerivationGoal::checkOutputs(const std::map & outputs) { std::map outputsByPath; for (auto & output : outputs) @@ -2679,8 +2691,8 @@ void LocalDerivationGoal::checkOutputs(const std::map & out for (auto & i : *value) { if (worker.store.isStorePath(i)) spec.insert(worker.store.parseStorePath(i)); - else if (finalOutputs.count(i)) - spec.insert(finalOutputs.at(i)); + else if (outputs.count(i)) + spec.insert(outputs.at(i).path); else throw BuildError("derivation contains an illegal reference specifier '%s'", i); } diff --git a/src/libstore/build/local-derivation-goal.hh b/src/libstore/build/local-derivation-goal.hh index 95692c60d..d456e9cae 100644 --- a/src/libstore/build/local-derivation-goal.hh +++ b/src/libstore/build/local-derivation-goal.hh @@ -169,7 +169,7 @@ struct LocalDerivationGoal : public DerivationGoal /* Check that the derivation outputs all exist and register them as valid. */ - void registerOutputs() override; + DrvOutputs registerOutputs() override; void signRealisation(Realisation &) override; diff --git a/src/libstore/build/substitution-goal.cc b/src/libstore/build/substitution-goal.cc index c1bb1941d..ec500baf8 100644 --- a/src/libstore/build/substitution-goal.cc +++ b/src/libstore/build/substitution-goal.cc @@ -24,6 +24,14 @@ PathSubstitutionGoal::~PathSubstitutionGoal() } +void PathSubstitutionGoal::done(ExitCode result, BuildResult::Status status) +{ + buildResult.outPath = storePath; + buildResult.status = status; + amDone(result); +} + + void PathSubstitutionGoal::work() { (this->*state)(); @@ -38,7 +46,7 @@ void PathSubstitutionGoal::init() /* If the path already exists we're done. */ if (!repair && worker.store.isValidPath(storePath)) { - amDone(ecSuccess); + done(ecSuccess, BuildResult::AlreadyValid); return; } @@ -65,7 +73,7 @@ void PathSubstitutionGoal::tryNext() /* Hack: don't indicate failure if there were no substituters. In that case the calling derivation should just do a build. */ - amDone(substituterFailed ? ecFailed : ecNoSubstituters); + done(substituterFailed ? ecFailed : ecNoSubstituters, BuildResult::NoSubstituters); if (substituterFailed) { worker.failedSubstitutions++; @@ -163,7 +171,9 @@ void PathSubstitutionGoal::referencesValid() if (nrFailed > 0) { debug("some references of path '%s' could not be realised", worker.store.printStorePath(storePath)); - amDone(nrNoSubstituters > 0 || nrIncompleteClosure > 0 ? ecIncompleteClosure : ecFailed); + done( + nrNoSubstituters > 0 || nrIncompleteClosure > 0 ? ecIncompleteClosure : ecFailed, + BuildResult::DependencyFailed); return; } @@ -268,7 +278,7 @@ void PathSubstitutionGoal::finished() worker.updateProgress(); - amDone(ecSuccess); + done(ecSuccess, BuildResult::Substituted); } diff --git a/src/libstore/build/substitution-goal.hh b/src/libstore/build/substitution-goal.hh index d8399d2a8..946f13841 100644 --- a/src/libstore/build/substitution-goal.hh +++ b/src/libstore/build/substitution-goal.hh @@ -53,6 +53,8 @@ struct PathSubstitutionGoal : public Goal /* Content address for recomputing store path */ std::optional ca; + void done(ExitCode result, BuildResult::Status status); + public: PathSubstitutionGoal(const StorePath & storePath, Worker & worker, RepairFlag repair = NoRepair, std::optional ca = std::nullopt); ~PathSubstitutionGoal(); -- cgit v1.2.3 From 761242afa08d5c9280ba6bd63a310b4334b83bb2 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 9 Mar 2022 12:25:35 +0100 Subject: BuildResult: Use DerivedPath --- src/libstore/build/derivation-goal.cc | 6 +++--- src/libstore/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 +-- 5 files changed, 20 insertions(+), 16 deletions(-) (limited to 'src/libstore/build') 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); } -- cgit v1.2.3 From 4d98143914120d0163f5c50f30ce8a5289433f8f Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 9 Mar 2022 20:31:50 +0100 Subject: BuildResult: Remove unused drvPath field --- src/libstore/build/derivation-goal.cc | 1 - 1 file changed, 1 deletion(-) (limited to 'src/libstore/build') diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 325635e2e..afed9bf16 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -1324,7 +1324,6 @@ void DerivationGoal::done( DrvOutputs builtOutputs, std::optional ex) { - buildResult.drvPath = drvPath; buildResult.status = status; if (ex) // FIXME: strip: "error: " -- cgit v1.2.3