From 0f6cb33763e1114e738d7ae28baa35ed261bdfb0 Mon Sep 17 00:00:00 2001 From: Samuli Thomasson Date: Thu, 12 Jun 2025 21:27:30 +0200 Subject: [PATCH 01/54] fix throwing output paths out of sandbox paths It seems obvious that erasing any output paths from pathsInChroot needs to happen after getPathsInSandbox(), not before. Signed-off-by: Samuli Thomasson --- .../unix/build/linux-derivation-builder.cc | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/libstore/unix/build/linux-derivation-builder.cc b/src/libstore/unix/build/linux-derivation-builder.cc index c27b87163..b23c8003f 100644 --- a/src/libstore/unix/build/linux-derivation-builder.cc +++ b/src/libstore/unix/build/linux-derivation-builder.cc @@ -368,6 +368,13 @@ struct ChrootLinuxDerivationBuilder : LinuxDerivationBuilder if (buildUser && chown(chrootStoreDir.c_str(), 0, buildUser->getGID()) == -1) throw SysError("cannot change ownership of '%1%'", chrootStoreDir); + pathsInChroot = getPathsInSandbox(); + + for (auto & i : inputPaths) { + auto p = store.printStorePath(i); + pathsInChroot.insert_or_assign(p, store.toRealPath(p)); + } + /* If we're repairing, checking or rebuilding part of a multiple-outputs derivation, it's possible that we're rebuilding a path that is in settings.sandbox-paths @@ -391,13 +398,6 @@ struct ChrootLinuxDerivationBuilder : LinuxDerivationBuilder chownToBuilder(*cgroup + "/cgroup.threads"); // chownToBuilder(*cgroup + "/cgroup.subtree_control"); } - - pathsInChroot = getPathsInSandbox(); - - for (auto & i : inputPaths) { - auto p = store.printStorePath(i); - pathsInChroot.insert_or_assign(p, store.toRealPath(p)); - } } Strings getPreBuildHookArgs() override From 47b7d910e96583f93d76fa0e436644466280fe48 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Fri, 20 Jun 2025 18:42:23 -0400 Subject: [PATCH 02/54] Properly prefix libfetchers header This is a minor oversight from 012453d1e60. --- src/libfetchers/include/nix/fetchers/input-cache.hh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libfetchers/include/nix/fetchers/input-cache.hh b/src/libfetchers/include/nix/fetchers/input-cache.hh index 9b1c5a310..f9278053a 100644 --- a/src/libfetchers/include/nix/fetchers/input-cache.hh +++ b/src/libfetchers/include/nix/fetchers/input-cache.hh @@ -1,4 +1,4 @@ -#include "fetchers.hh" +#include "nix/fetchers/fetchers.hh" namespace nix::fetchers { From a55806a0ddb864ba86ffc5b32b693ef35a8b4676 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 3 Mar 2025 10:33:54 -0500 Subject: [PATCH 03/54] Get rid of `addWantedOutputs` This is just a code cleanup; it should not be behavior change. `addWantedOutputs` is removed by introducing `DerivationTrampolineGoal`. `DerivationGoal` now only tracks a single output, and is back to tracking a plain store path `drvPath`, not a deriving path one. Its `addWantedOutputs` method is gone. These changes will allow subsequent PRs to simplify it greatly. Because the purpose of each goal is back to being immutable, we can also once again make `Goal::buildResult` a public field, and get rid of the `getBuildResult` method. This simplifies things also. `DerivationTrampolineGoal` is, as the nane is supposed to indicate, a cheap "trampoline" goal. It takes immutable sets of wanted outputs, and just kicks of `DerivationGoal`s for them. Since now "actual work" is done in these goals, it is not wasteful to have separate ones for separate sets of outputs, even if those outputs (and the derivations they are from) overlap. This design is described in more detail in the doc comments on the goal types, which I've now greatly expanded. --- This separation of concerns will make it possible for future work on issues like #11928, and to continue the path of having more goal types, but each goal type does fewer things (issue #12628). --- This commit in some sense reverts f4f28cdd0e01a9bfb0901e8a53ead719265fa9b8, but that one kept around `addWantedOutputs`. I am quite sure it was having two layers of goals with `addWantedOutputs` that caused the issues --- restarting logic like `addWantedOutputs` has is very tempermental! In this version of the change, we have *zero* layers of `addWantedOutputs` --- no goal type needs it, or otherwise has a mutable objective --- and so I think this change is safe. Co-authored-by: Sergei Zimmerman <145775305+xokdvium@users.noreply.github.com> --- .../build/derivation-building-goal.cc | 26 +-- src/libstore/build/derivation-goal.cc | 205 ++++-------------- .../build/derivation-trampoline-goal.cc | 175 +++++++++++++++ src/libstore/build/entry-points.cc | 26 +-- src/libstore/build/goal.cc | 24 -- src/libstore/build/worker.cc | 122 +++++------ src/libstore/derived-path-map.cc | 4 +- .../store/build/derivation-building-goal.hh | 4 +- .../nix/store/build/derivation-goal.hh | 74 +++---- .../store/build/derivation-trampoline-goal.hh | 134 ++++++++++++ src/libstore/include/nix/store/build/goal.hh | 14 -- .../include/nix/store/build/worker.hh | 27 ++- .../include/nix/store/derived-path-map.hh | 2 +- src/libstore/include/nix/store/local-store.hh | 1 - src/libstore/include/nix/store/meson.build | 1 + src/libstore/meson.build | 1 + 16 files changed, 480 insertions(+), 360 deletions(-) create mode 100644 src/libstore/build/derivation-trampoline-goal.cc create mode 100644 src/libstore/include/nix/store/build/derivation-trampoline-goal.hh diff --git a/src/libstore/build/derivation-building-goal.cc b/src/libstore/build/derivation-building-goal.cc index 9a91b3592..3019d9d72 100644 --- a/src/libstore/build/derivation-building-goal.cc +++ b/src/libstore/build/derivation-building-goal.cc @@ -1,5 +1,5 @@ #include "nix/store/build/derivation-building-goal.hh" -#include "nix/store/build/derivation-goal.hh" +#include "nix/store/build/derivation-trampoline-goal.hh" #ifndef _WIN32 // TODO enable build hook on Windows # include "nix/store/build/hook-instance.hh" # include "nix/store/build/derivation-builder.hh" @@ -72,7 +72,7 @@ std::string DerivationBuildingGoal::key() /* Ensure that derivations get built in order of their name, i.e. a derivation named "aardvark" always comes before "baboon". And substitution goals always happen before - derivation goals (due to "b$"). */ + derivation goals (due to "bd$"). */ return "bd$" + std::string(drvPath.name()) + "$" + worker.store.printStorePath(drvPath); } @@ -266,7 +266,7 @@ Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution() auto mEntry = get(inputGoals, drvPath); if (!mEntry) return std::nullopt; - auto buildResult = (*mEntry)->getBuildResult(DerivedPath::Built{drvPath, OutputsSpec::Names{outputName}}); + auto & buildResult = (*mEntry)->buildResult; if (!buildResult.success()) return std::nullopt; auto i = get(buildResult.builtOutputs, outputName); @@ -296,9 +296,11 @@ Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution() worker.store.printStorePath(pathResolved), }); - // FIXME wanted outputs - auto resolvedDrvGoal = worker.makeDerivationGoal( - makeConstantStorePathRef(pathResolved), OutputsSpec::All{}, buildMode); + /* TODO https://github.com/NixOS/nix/issues/13247 we should + let the calling goal do this, so it has a change to pass + just the output(s) it cares about. */ + auto resolvedDrvGoal = worker.makeDerivationTrampolineGoal( + pathResolved, OutputsSpec::All{}, drvResolved, buildMode); { Goals waitees{resolvedDrvGoal}; co_await await(std::move(waitees)); @@ -306,20 +308,16 @@ Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution() trace("resolved derivation finished"); - auto resolvedDrv = *resolvedDrvGoal->drv; - auto resolvedResult = resolvedDrvGoal->getBuildResult(DerivedPath::Built{ - .drvPath = makeConstantStorePathRef(pathResolved), - .outputs = OutputsSpec::All{}, - }); + auto resolvedResult = resolvedDrvGoal->buildResult; SingleDrvOutputs builtOutputs; if (resolvedResult.success()) { - auto resolvedHashes = staticOutputHashes(worker.store, resolvedDrv); + auto resolvedHashes = staticOutputHashes(worker.store, drvResolved); StorePathSet outputPaths; - for (auto & outputName : resolvedDrv.outputNames()) { + for (auto & outputName : drvResolved.outputNames()) { auto initialOutput = get(initialOutputs, outputName); auto resolvedHash = get(resolvedHashes, outputName); if ((!initialOutput) || (!resolvedHash)) @@ -340,7 +338,7 @@ Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution() throw Error( "derivation '%s' doesn't have expected output '%s' (derivation-goal.cc/realisation)", - resolvedDrvGoal->drvReq->to_string(worker.store), outputName); + worker.store.printStorePath(pathResolved), outputName); }(); if (!drv->type().isImpure()) { diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index 3fcc376ed..128d360c9 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -24,35 +24,18 @@ namespace nix { -DerivationGoal::DerivationGoal(ref drvReq, - const OutputsSpec & wantedOutputs, Worker & worker, BuildMode buildMode) - : Goal(worker, loadDerivation()) - , drvReq(drvReq) - , wantedOutputs(wantedOutputs) - , buildMode(buildMode) -{ - name = fmt( - "building of '%s' from .drv file", - DerivedPath::Built { drvReq, wantedOutputs }.to_string(worker.store)); - trace("created"); - - mcExpectedBuilds = std::make_unique>(worker.expectedBuilds); - worker.updateProgress(); -} - - -DerivationGoal::DerivationGoal(const StorePath & drvPath, const BasicDerivation & drv, - const OutputsSpec & wantedOutputs, Worker & worker, BuildMode buildMode) - : Goal(worker, haveDerivation(drvPath)) - , drvReq(makeConstantStorePathRef(drvPath)) - , wantedOutputs(wantedOutputs) +DerivationGoal::DerivationGoal(const StorePath & drvPath, const Derivation & drv, + const OutputName & wantedOutput, Worker & worker, BuildMode buildMode) + : Goal(worker, haveDerivation()) + , drvPath(drvPath) + , wantedOutput(wantedOutput) , buildMode(buildMode) { this->drv = std::make_unique(drv); name = fmt( "building of '%s' from in-memory derivation", - DerivedPath::Built { drvReq, drv.outputNames() }.to_string(worker.store)); + DerivedPath::Built { makeConstantStorePathRef(drvPath), drv.outputNames() }.to_string(worker.store)); trace("created"); mcExpectedBuilds = std::make_unique>(worker.expectedBuilds); @@ -61,114 +44,20 @@ DerivationGoal::DerivationGoal(const StorePath & drvPath, const BasicDerivation } -static StorePath pathPartOfReq(const SingleDerivedPath & req) -{ - return std::visit( - overloaded{ - [&](const SingleDerivedPath::Opaque & bo) { return bo.path; }, - [&](const SingleDerivedPath::Built & bfd) { return pathPartOfReq(*bfd.drvPath); }, - }, - req.raw()); -} - - std::string DerivationGoal::key() { /* Ensure that derivations get built in order of their name, i.e. a derivation named "aardvark" always comes before "baboon". And substitution goals always happen before derivation goals (due to "b$"). */ - return "b$" + std::string(pathPartOfReq(*drvReq).name()) + "$" + drvReq->to_string(worker.store); + return "b$" + std::string(drvPath.name()) + "$" + SingleDerivedPath::Built{ + .drvPath = makeConstantStorePathRef(drvPath), + .output = wantedOutput, + }.to_string(worker.store); } -void DerivationGoal::addWantedOutputs(const OutputsSpec & outputs) -{ - auto newWanted = wantedOutputs.union_(outputs); - switch (needRestart) { - case NeedRestartForMoreOutputs::OutputsUnmodifiedDontNeed: - if (!newWanted.isSubsetOf(wantedOutputs)) - needRestart = NeedRestartForMoreOutputs::OutputsAddedDoNeed; - break; - case NeedRestartForMoreOutputs::OutputsAddedDoNeed: - /* No need to check whether we added more outputs, because a - restart is already queued up. */ - break; - case NeedRestartForMoreOutputs::BuildInProgressWillNotNeed: - /* We are already building all outputs, so it doesn't matter if - we now want more. */ - break; - }; - wantedOutputs = newWanted; -} - - -Goal::Co DerivationGoal::loadDerivation() { - trace("need to load derivation from file"); - - { - /* The first thing to do is to make sure that the derivation - exists. If it doesn't, it may be built from another - derivation, or merely substituted. We can make goal to get it - and not worry about which method it takes to get the - derivation. */ - - if (auto optDrvPath = [this]() -> std::optional { - if (buildMode != bmNormal) - return std::nullopt; - - auto drvPath = StorePath::dummy; - try { - drvPath = resolveDerivedPath(worker.store, *drvReq); - } catch (MissingRealisation &) { - return std::nullopt; - } - auto cond = worker.evalStore.isValidPath(drvPath) || worker.store.isValidPath(drvPath); - return cond ? std::optional{drvPath} : std::nullopt; - }()) { - trace( - fmt("already have drv '%s' for '%s', can go straight to building", - worker.store.printStorePath(*optDrvPath), - drvReq->to_string(worker.store))); - } else { - trace("need to obtain drv we want to build"); - Goals waitees{worker.makeGoal(DerivedPath::fromSingle(*drvReq))}; - co_await await(std::move(waitees)); - } - - trace("loading derivation"); - - if (nrFailed != 0) { - co_return amDone(ecFailed, Error("cannot build missing derivation '%s'", drvReq->to_string(worker.store))); - } - - StorePath drvPath = resolveDerivedPath(worker.store, *drvReq); - - /* `drvPath' should already be a root, but let's be on the safe - side: if the user forgot to make it a root, we wouldn't want - things being garbage collected while we're busy. */ - worker.evalStore.addTempRoot(drvPath); - - /* Get the derivation. It is probably in the eval store, but it might be inthe main store: - - - Resolved derivation are resolved against main store realisations, and so must be stored there. - - - Dynamic derivations are built, and so are found in the main store. - */ - for (auto * drvStore : { &worker.evalStore, &worker.store }) { - if (drvStore->isValidPath(drvPath)) { - drv = std::make_unique(drvStore->readDerivation(drvPath)); - break; - } - } - assert(drv); - - co_return haveDerivation(drvPath); - } -} - - -Goal::Co DerivationGoal::haveDerivation(StorePath drvPath) +Goal::Co DerivationGoal::haveDerivation() { trace("have derivation"); @@ -205,18 +94,26 @@ Goal::Co DerivationGoal::haveDerivation(StorePath drvPath) trace("outer build done"); - buildResult = g->getBuildResult(DerivedPath::Built{ - .drvPath = makeConstantStorePathRef(drvPath), - .outputs = wantedOutputs, - }); + buildResult = g->buildResult; if (buildMode == bmCheck) { /* In checking mode, the builder will not register any outputs. So we want to make sure the ones that we wanted to check are properly there. */ - buildResult.builtOutputs = assertPathValidity(drvPath); + buildResult.builtOutputs = assertPathValidity(); } + for (auto it = buildResult.builtOutputs.begin(); it != buildResult.builtOutputs.end(); ) { + if (it->first != wantedOutput) { + it = buildResult.builtOutputs.erase(it); + } else { + ++it; + } + } + + if (buildResult.success()) + assert(buildResult.builtOutputs.count(wantedOutput) > 0); + co_return amDone(g->exitCode, g->ex); }; @@ -261,11 +158,11 @@ Goal::Co DerivationGoal::haveDerivation(StorePath drvPath) { /* Check what outputs paths are not already valid. */ - auto [allValid, validOutputs] = checkPathValidity(drvPath); + auto [allValid, validOutputs] = checkPathValidity(); /* If they are all valid, then we're done. */ if (allValid && buildMode == bmNormal) { - co_return done(drvPath, BuildResult::AlreadyValid, std::move(validOutputs)); + co_return done(BuildResult::AlreadyValid, std::move(validOutputs)); } } @@ -302,25 +199,20 @@ Goal::Co DerivationGoal::haveDerivation(StorePath drvPath) assert(!drv->type().isImpure()); if (nrFailed > 0 && nrFailed > nrNoSubstituters && !settings.tryFallback) { - co_return done(drvPath, BuildResult::TransientFailure, {}, + co_return 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))); } nrFailed = nrNoSubstituters = 0; - if (needRestart == NeedRestartForMoreOutputs::OutputsAddedDoNeed) { - needRestart = NeedRestartForMoreOutputs::OutputsUnmodifiedDontNeed; - co_return haveDerivation(std::move(drvPath)); - } - - auto [allValid, validOutputs] = checkPathValidity(drvPath); + auto [allValid, validOutputs] = checkPathValidity(); if (buildMode == bmNormal && allValid) { - co_return done(drvPath, BuildResult::Substituted, std::move(validOutputs)); + co_return done(BuildResult::Substituted, std::move(validOutputs)); } if (buildMode == bmRepair && allValid) { - co_return repairClosure(std::move(drvPath)); + co_return repairClosure(); } if (buildMode == bmCheck && !allValid) throw Error("some outputs of '%s' are not valid, so checking is not possible", @@ -343,7 +235,7 @@ struct value_comparison }; -Goal::Co DerivationGoal::repairClosure(StorePath drvPath) +Goal::Co DerivationGoal::repairClosure() { assert(!drv->type().isImpure()); @@ -353,11 +245,10 @@ Goal::Co DerivationGoal::repairClosure(StorePath drvPath) that produced those outputs. */ /* Get the output closure. */ - auto outputs = queryDerivationOutputMap(drvPath); + auto outputs = queryDerivationOutputMap(); StorePathSet outputClosure; - for (auto & i : outputs) { - if (!wantedOutputs.contains(i.first)) continue; - worker.store.computeFSClosure(i.second, outputClosure); + if (auto mPath = get(outputs, wantedOutput)) { + worker.store.computeFSClosure(*mPath, outputClosure); } /* Filter out our own outputs (which we have already checked). */ @@ -411,11 +302,11 @@ Goal::Co DerivationGoal::repairClosure(StorePath drvPath) throw Error("some paths in the output closure of derivation '%s' could not be repaired", worker.store.printStorePath(drvPath)); } - co_return done(drvPath, BuildResult::AlreadyValid, assertPathValidity(drvPath)); + co_return done(BuildResult::AlreadyValid, assertPathValidity()); } -std::map> DerivationGoal::queryPartialDerivationOutputMap(const StorePath & drvPath) +std::map> DerivationGoal::queryPartialDerivationOutputMap() { assert(!drv->type().isImpure()); @@ -431,7 +322,7 @@ std::map> DerivationGoal::queryPartialDeri return res; } -OutputPathMap DerivationGoal::queryDerivationOutputMap(const StorePath & drvPath) +OutputPathMap DerivationGoal::queryDerivationOutputMap() { assert(!drv->type().isImpure()); @@ -447,28 +338,21 @@ OutputPathMap DerivationGoal::queryDerivationOutputMap(const StorePath & drvPath } -std::pair DerivationGoal::checkPathValidity(const StorePath & drvPath) +std::pair DerivationGoal::checkPathValidity() { if (drv->type().isImpure()) return { false, {} }; bool checkHash = buildMode == bmRepair; - auto wantedOutputsLeft = std::visit(overloaded { - [&](const OutputsSpec::All &) { - return StringSet {}; - }, - [&](const OutputsSpec::Names & names) { - return static_cast(names); - }, - }, wantedOutputs.raw); + StringSet wantedOutputsLeft{wantedOutput}; SingleDrvOutputs validOutputs; - for (auto & i : queryPartialDerivationOutputMap(drvPath)) { + for (auto & i : queryPartialDerivationOutputMap()) { auto initialOutput = get(initialOutputs, i.first); if (!initialOutput) // this is an invalid output, gets caught with (!wantedOutputsLeft.empty()) continue; auto & info = *initialOutput; - info.wanted = wantedOutputs.contains(i.first); + info.wanted = wantedOutput == i.first; if (info.wanted) wantedOutputsLeft.erase(i.first); if (i.second) { @@ -527,9 +411,9 @@ std::pair DerivationGoal::checkPathValidity(const StoreP } -SingleDrvOutputs DerivationGoal::assertPathValidity(const StorePath & drvPath) +SingleDrvOutputs DerivationGoal::assertPathValidity() { - auto [allValid, validOutputs] = checkPathValidity(drvPath); + auto [allValid, validOutputs] = checkPathValidity(); if (!allValid) throw Error("some outputs are unexpectedly invalid"); return validOutputs; @@ -537,7 +421,6 @@ SingleDrvOutputs DerivationGoal::assertPathValidity(const StorePath & drvPath) Goal::Done DerivationGoal::done( - const StorePath & drvPath, BuildResult::Status status, SingleDrvOutputs builtOutputs, std::optional ex) @@ -553,7 +436,7 @@ Goal::Done DerivationGoal::done( mcExpectedBuilds.reset(); if (buildResult.success()) { - auto wantedBuiltOutputs = filterDrvOutputs(wantedOutputs, std::move(builtOutputs)); + auto wantedBuiltOutputs = filterDrvOutputs(OutputsSpec::Names{wantedOutput}, std::move(builtOutputs)); assert(!wantedBuiltOutputs.empty()); buildResult.builtOutputs = std::move(wantedBuiltOutputs); if (status == BuildResult::Built) diff --git a/src/libstore/build/derivation-trampoline-goal.cc b/src/libstore/build/derivation-trampoline-goal.cc new file mode 100644 index 000000000..e8ca47dfe --- /dev/null +++ b/src/libstore/build/derivation-trampoline-goal.cc @@ -0,0 +1,175 @@ +#include "nix/store/build/derivation-trampoline-goal.hh" +#include "nix/store/build/worker.hh" +#include "nix/store/derivations.hh" + +namespace nix { + +DerivationTrampolineGoal::DerivationTrampolineGoal( + ref drvReq, const OutputsSpec & wantedOutputs, Worker & worker, BuildMode buildMode) + : Goal(worker, init()) + , drvReq(drvReq) + , wantedOutputs(wantedOutputs) + , buildMode(buildMode) +{ + commonInit(); +} + +DerivationTrampolineGoal::DerivationTrampolineGoal( + const StorePath & drvPath, + const OutputsSpec & wantedOutputs, + const Derivation & drv, + Worker & worker, + BuildMode buildMode) + : Goal(worker, haveDerivation(drvPath, drv)) + , drvReq(makeConstantStorePathRef(drvPath)) + , wantedOutputs(wantedOutputs) + , buildMode(buildMode) +{ + commonInit(); +} + +void DerivationTrampolineGoal::commonInit() +{ + name = + fmt("outer obtaining drv from '%s' and then building outputs %s", + drvReq->to_string(worker.store), + std::visit( + overloaded{ + [&](const OutputsSpec::All) -> std::string { return "* (all of them)"; }, + [&](const OutputsSpec::Names os) { return concatStringsSep(", ", quoteStrings(os)); }, + }, + wantedOutputs.raw)); + trace("created outer"); + + worker.updateProgress(); +} + +DerivationTrampolineGoal::~DerivationTrampolineGoal() {} + +static StorePath pathPartOfReq(const SingleDerivedPath & req) +{ + return std::visit( + overloaded{ + [&](const SingleDerivedPath::Opaque & bo) { return bo.path; }, + [&](const SingleDerivedPath::Built & bfd) { return pathPartOfReq(*bfd.drvPath); }, + }, + req.raw()); +} + +std::string DerivationTrampolineGoal::key() +{ + /* Ensure that derivations get built in order of their name, + i.e. a derivation named "aardvark" always comes before "baboon". And + substitution goals, derivation goals, and derivation building goals always happen before + derivation goals (due to "bt$"). */ + return "bt$" + std::string(pathPartOfReq(*drvReq).name()) + "$" + DerivedPath::Built{ + .drvPath = drvReq, + .outputs = wantedOutputs, + }.to_string(worker.store); +} + +void DerivationTrampolineGoal::timedOut(Error && ex) {} + +Goal::Co DerivationTrampolineGoal::init() +{ + trace("need to load derivation from file"); + + /* The first thing to do is to make sure that the derivation + exists. If it doesn't, it may be built from another derivation, + or merely substituted. We can make goal to get it and not worry + about which method it takes to get the derivation. */ + if (auto optDrvPath = [this]() -> std::optional { + if (buildMode != bmNormal) + return std::nullopt; + + auto drvPath = StorePath::dummy; + try { + drvPath = resolveDerivedPath(worker.store, *drvReq); + } catch (MissingRealisation &) { + return std::nullopt; + } + auto cond = worker.evalStore.isValidPath(drvPath) || worker.store.isValidPath(drvPath); + return cond ? std::optional{drvPath} : std::nullopt; + }()) { + trace( + fmt("already have drv '%s' for '%s', can go straight to building", + worker.store.printStorePath(*optDrvPath), + drvReq->to_string(worker.store))); + } else { + trace("need to obtain drv we want to build"); + Goals waitees{worker.makeGoal(DerivedPath::fromSingle(*drvReq))}; + co_await await(std::move(waitees)); + } + + trace("outer load and build derivation"); + + if (nrFailed != 0) { + co_return amDone(ecFailed, Error("cannot build missing derivation '%s'", drvReq->to_string(worker.store))); + } + + StorePath drvPath = resolveDerivedPath(worker.store, *drvReq); + + /* `drvPath' should already be a root, but let's be on the safe + side: if the user forgot to make it a root, we wouldn't want + things being garbage collected while we're busy. */ + worker.evalStore.addTempRoot(drvPath); + + /* Get the derivation. It is probably in the eval store, but it might be in the main store: + + - Resolved derivation are resolved against main store realisations, and so must be stored there. + + - Dynamic derivations are built, and so are found in the main store. + */ + auto drv = [&] { + for (auto * drvStore : {&worker.evalStore, &worker.store}) + if (drvStore->isValidPath(drvPath)) + return drvStore->readDerivation(drvPath); + assert(false); + }(); + + co_return haveDerivation(std::move(drvPath), std::move(drv)); +} + +Goal::Co DerivationTrampolineGoal::haveDerivation(StorePath drvPath, Derivation drv) +{ + trace("have derivation, will kick off derivations goals per wanted output"); + + auto resolvedWantedOutputs = std::visit( + overloaded{ + [&](const OutputsSpec::Names & names) -> OutputsSpec::Names { return names; }, + [&](const OutputsSpec::All &) -> OutputsSpec::Names { + StringSet outputs; + for (auto & [outputName, _] : drv.outputs) + outputs.insert(outputName); + return outputs; + }, + }, + wantedOutputs.raw); + + Goals concreteDrvGoals; + + /* Build this step! */ + + for (auto & output : resolvedWantedOutputs) { + auto g = upcast_goal(worker.makeDerivationGoal(drvPath, drv, output, buildMode)); + g->preserveException = true; + /* We will finish with it ourselves, as if we were the derivational goal. */ + concreteDrvGoals.insert(std::move(g)); + } + + // Copy on purpose + co_await await(Goals(concreteDrvGoals)); + + trace("outer build done"); + + auto & g = *concreteDrvGoals.begin(); + buildResult = g->buildResult; + for (auto & g2 : concreteDrvGoals) { + for (auto && [x, y] : g2->buildResult.builtOutputs) + buildResult.builtOutputs.insert_or_assign(x, y); + } + + co_return amDone(g->exitCode, g->ex); +} + +} diff --git a/src/libstore/build/entry-points.cc b/src/libstore/build/entry-points.cc index 39fd471c4..6c842554c 100644 --- a/src/libstore/build/entry-points.cc +++ b/src/libstore/build/entry-points.cc @@ -1,8 +1,7 @@ +#include "nix/store/derivations.hh" #include "nix/store/build/worker.hh" #include "nix/store/build/substitution-goal.hh" -#ifndef _WIN32 // TODO Enable building on Windows -# include "nix/store/build/derivation-goal.hh" -#endif +#include "nix/store/build/derivation-trampoline-goal.hh" #include "nix/store/local-store.hh" #include "nix/util/strings.hh" @@ -28,12 +27,9 @@ void Store::buildPaths(const std::vector & reqs, BuildMode buildMod ex = std::move(i->ex); } if (i->exitCode != Goal::ecSuccess) { -#ifndef _WIN32 // TODO Enable building on Windows - if (auto i2 = dynamic_cast(i.get())) + if (auto i2 = dynamic_cast(i.get())) failed.insert(i2->drvReq->to_string(*this)); - else -#endif - if (auto i2 = dynamic_cast(i.get())) + else if (auto i2 = dynamic_cast(i.get())) failed.insert(printStorePath(i2->storePath)); } } @@ -70,7 +66,7 @@ std::vector Store::buildPathsWithResults( for (auto & [req, goalPtr] : state) results.emplace_back(KeyedBuildResult { - goalPtr->getBuildResult(req), + goalPtr->buildResult, /* .path = */ req, }); @@ -81,19 +77,11 @@ BuildResult Store::buildDerivation(const StorePath & drvPath, const BasicDerivat BuildMode buildMode) { Worker worker(*this, *this); -#ifndef _WIN32 // TODO Enable building on Windows - auto goal = worker.makeBasicDerivationGoal(drvPath, drv, OutputsSpec::All {}, buildMode); -#else - std::shared_ptr goal; - throw UnimplementedError("Building derivations not yet implemented on windows."); -#endif + auto goal = worker.makeDerivationTrampolineGoal(drvPath, OutputsSpec::All {}, drv, buildMode); try { worker.run(Goals{goal}); - return goal->getBuildResult(DerivedPath::Built { - .drvPath = makeConstantStorePathRef(drvPath), - .outputs = OutputsSpec::All {}, - }); + return goal->buildResult; } catch (Error & e) { return BuildResult { .status = BuildResult::MiscFailure, diff --git a/src/libstore/build/goal.cc b/src/libstore/build/goal.cc index 8a8d79283..88b0c28c0 100644 --- a/src/libstore/build/goal.cc +++ b/src/libstore/build/goal.cc @@ -101,30 +101,6 @@ bool CompareGoalPtrs::operator() (const GoalPtr & a, const GoalPtr & b) const { return s1 < s2; } - -BuildResult Goal::getBuildResult(const DerivedPath & req) const { - BuildResult res { buildResult }; - - if (auto pbp = std::get_if(&req)) { - auto & bp = *pbp; - - /* Because goals are in general shared between derived paths - that share the same derivation, we need to filter their - results to get back just the results we care about. - */ - - for (auto it = res.builtOutputs.begin(); it != res.builtOutputs.end();) { - if (bp.outputs.contains(it->first)) - ++it; - else - it = res.builtOutputs.erase(it); - } - } - - return res; -} - - void addToWeakGoals(WeakGoals & goals, GoalPtr p) { if (goals.find(p) != goals.end()) diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index 12d4aaa2d..38ac73768 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -5,6 +5,7 @@ #include "nix/store/build/drv-output-substitution-goal.hh" #include "nix/store/build/derivation-goal.hh" #include "nix/store/build/derivation-building-goal.hh" +#include "nix/store/build/derivation-trampoline-goal.hh" #ifndef _WIN32 // TODO Enable building on Windows # include "nix/store/build/hook-instance.hh" #endif @@ -53,52 +54,40 @@ std::shared_ptr Worker::initGoalIfNeeded(std::weak_ptr & goal_weak, Args & return goal; } -std::shared_ptr Worker::makeDerivationGoalCommon( +std::shared_ptr Worker::makeDerivationTrampolineGoal( ref drvReq, const OutputsSpec & wantedOutputs, - std::function()> mkDrvGoal) + BuildMode buildMode) { - std::weak_ptr & goal_weak = derivationGoals.ensureSlot(*drvReq).value; - std::shared_ptr goal = goal_weak.lock(); - if (!goal) { - goal = mkDrvGoal(); - goal_weak = goal; - wakeUp(goal); - } else { - goal->addWantedOutputs(wantedOutputs); - } - return goal; + return initGoalIfNeeded( + derivationTrampolineGoals.ensureSlot(*drvReq).value[wantedOutputs], + drvReq, wantedOutputs, *this, buildMode); } -std::shared_ptr Worker::makeDerivationGoal(ref drvReq, - const OutputsSpec & wantedOutputs, BuildMode buildMode) +std::shared_ptr Worker::makeDerivationTrampolineGoal( + const StorePath & drvPath, + const OutputsSpec & wantedOutputs, + const Derivation & drv, + BuildMode buildMode) { - return makeDerivationGoalCommon(drvReq, wantedOutputs, [&]() -> std::shared_ptr { - return std::make_shared(drvReq, wantedOutputs, *this, buildMode); - }); + return initGoalIfNeeded( + derivationTrampolineGoals.ensureSlot(DerivedPath::Opaque{drvPath}).value[wantedOutputs], + drvPath, wantedOutputs, drv, *this, buildMode); } -std::shared_ptr Worker::makeBasicDerivationGoal(const StorePath & drvPath, - const BasicDerivation & drv, const OutputsSpec & wantedOutputs, BuildMode buildMode) + +std::shared_ptr Worker::makeDerivationGoal(const StorePath & drvPath, + const Derivation & drv, const OutputName & wantedOutput, BuildMode buildMode) { - return makeDerivationGoalCommon(makeConstantStorePathRef(drvPath), wantedOutputs, [&]() -> std::shared_ptr { - return std::make_shared(drvPath, drv, wantedOutputs, *this, buildMode); - }); + return initGoalIfNeeded(derivationGoals[drvPath][wantedOutput], drvPath, drv, wantedOutput, *this, buildMode); } std::shared_ptr Worker::makeDerivationBuildingGoal(const StorePath & drvPath, const Derivation & drv, BuildMode buildMode) { - std::weak_ptr & goal_weak = derivationBuildingGoals[drvPath]; - auto goal = goal_weak.lock(); // FIXME - if (!goal) { - goal = std::make_shared(drvPath, drv, *this, buildMode); - goal_weak = goal; - wakeUp(goal); - } - return goal; + return initGoalIfNeeded(derivationBuildingGoals[drvPath], drvPath, drv, *this, buildMode); } @@ -118,7 +107,7 @@ GoalPtr Worker::makeGoal(const DerivedPath & req, BuildMode buildMode) { return std::visit(overloaded { [&](const DerivedPath::Built & bfd) -> GoalPtr { - return makeDerivationGoal(bfd.drvPath, bfd.outputs, buildMode); + return makeDerivationTrampolineGoal(bfd.drvPath, bfd.outputs, buildMode); }, [&](const DerivedPath::Opaque & bo) -> GoalPtr { return makePathSubstitutionGoal(bo.path, buildMode == bmRepair ? Repair : NoRepair); @@ -126,46 +115,52 @@ GoalPtr Worker::makeGoal(const DerivedPath & req, BuildMode buildMode) }, req.raw()); } - -template -static void cullMap(std::map & goalMap, F f) +/** + * This function is polymorphic (both via type parameters and + * overloading) and recursive in order to work on a various types of + * trees + * + * @return Whether the tree node we are processing is not empty / should + * be kept alive. In the case of this overloading the node in question + * is the leaf, the weak reference itself. If the weak reference points + * to the goal we are looking for, our caller can delete it. In the + * inductive case where the node is an interior node, we'll likewise + * return whether the interior node is non-empty. If it is empty + * (because we just deleted its last child), then our caller can + * likewise delete it. + */ +template +static bool removeGoal(std::shared_ptr goal, std::weak_ptr & gp) { - for (auto i = goalMap.begin(); i != goalMap.end();) - if (!f(i->second)) + return gp.lock() != goal; +} + +template +static bool removeGoal(std::shared_ptr goal, std::map & goalMap) +{ + /* !!! inefficient */ + for (auto i = goalMap.begin(); i != goalMap.end();) { + if (!removeGoal(goal, i->second)) i = goalMap.erase(i); - else ++i; + else + ++i; + } + return !goalMap.empty(); } - -template -static void removeGoal(std::shared_ptr goal, std::map> & goalMap) +template +static bool removeGoal(std::shared_ptr goal, typename DerivedPathMap>>::ChildNode & node) { - /* !!! inefficient */ - cullMap(goalMap, [&](const std::weak_ptr & gp) -> bool { - return gp.lock() != goal; - }); -} - -template -static void removeGoal(std::shared_ptr goal, std::map>::ChildNode> & goalMap); - -template -static void removeGoal(std::shared_ptr goal, std::map>::ChildNode> & goalMap) -{ - /* !!! inefficient */ - cullMap(goalMap, [&](DerivedPathMap>::ChildNode & node) -> bool { - if (node.value.lock() == goal) - node.value.reset(); - removeGoal(goal, node.childMap); - return !node.value.expired() || !node.childMap.empty(); - }); + return removeGoal(goal, node.value) || removeGoal(goal, node.childMap); } void Worker::removeGoal(GoalPtr goal) { - if (auto drvGoal = std::dynamic_pointer_cast(goal)) - nix::removeGoal(drvGoal, derivationGoals.map); + if (auto drvGoal = std::dynamic_pointer_cast(goal)) + nix::removeGoal(drvGoal, derivationTrampolineGoals.map); + else if (auto drvGoal = std::dynamic_pointer_cast(goal)) + nix::removeGoal(drvGoal, derivationGoals); else if (auto drvBuildingGoal = std::dynamic_pointer_cast(goal)) nix::removeGoal(drvBuildingGoal, derivationBuildingGoals); else if (auto subGoal = std::dynamic_pointer_cast(goal)) @@ -312,13 +307,12 @@ void Worker::run(const Goals & _topGoals) for (auto & i : _topGoals) { topGoals.insert(i); - if (auto goal = dynamic_cast(i.get())) { + if (auto goal = dynamic_cast(i.get())) { topPaths.push_back(DerivedPath::Built { .drvPath = goal->drvReq, .outputs = goal->wantedOutputs, }); - } else - if (auto goal = dynamic_cast(i.get())) { + } else if (auto goal = dynamic_cast(i.get())) { topPaths.push_back(DerivedPath::Opaque{goal->storePath}); } } diff --git a/src/libstore/derived-path-map.cc b/src/libstore/derived-path-map.cc index 408d1a6b9..e34deb744 100644 --- a/src/libstore/derived-path-map.cc +++ b/src/libstore/derived-path-map.cc @@ -52,7 +52,7 @@ typename DerivedPathMap::ChildNode * DerivedPathMap::findSlot(const Single // instantiations -#include "nix/store/build/derivation-goal.hh" +#include "nix/store/build/derivation-trampoline-goal.hh" namespace nix { template<> @@ -69,7 +69,7 @@ std::strong_ordering DerivedPathMap::ChildNode::operator <=> ( template struct DerivedPathMap::ChildNode; template struct DerivedPathMap; -template struct DerivedPathMap>; +template struct DerivedPathMap>>; }; diff --git a/src/libstore/include/nix/store/build/derivation-building-goal.hh b/src/libstore/include/nix/store/build/derivation-building-goal.hh index bff2e7a89..569d1ddbb 100644 --- a/src/libstore/include/nix/store/build/derivation-building-goal.hh +++ b/src/libstore/include/nix/store/build/derivation-building-goal.hh @@ -29,7 +29,9 @@ void runPostBuildHook( const StorePathSet & outputPaths); /** - * A goal for building some or all of the outputs of a derivation. + * A goal for building a derivation. Substitution, (or any other method of + * obtaining the outputs) will not be attempted, so it is the calling goal's + * responsibility to try to substitute first. */ struct DerivationBuildingGoal : public Goal { diff --git a/src/libstore/include/nix/store/build/derivation-goal.hh b/src/libstore/include/nix/store/build/derivation-goal.hh index 9d4257cb3..ac9ec5346 100644 --- a/src/libstore/include/nix/store/build/derivation-goal.hh +++ b/src/libstore/include/nix/store/build/derivation-goal.hh @@ -22,47 +22,33 @@ void runPostBuildHook( const StorePathSet & outputPaths); /** - * A goal for building some or all of the outputs of a derivation. + * A goal for realising a single output of a derivation. Various sorts of + * fetching (which will be done by other goal types) is tried, and if none of + * those succeed, the derivation is attempted to be built. + * + * This is a purely "administrative" goal type, which doesn't do any + * "real work" of substituting (that would be `PathSubstitutionGoal` or + * `DrvOutputSubstitutionGoal`) or building (that would be a + * `DerivationBuildingGoal`). This goal type creates those types of + * goals to attempt each way of realisation a derivation; they are tried + * sequentially in order of preference. + * + * The derivation must already be gotten (in memory, in C++, parsed) and passed + * to the caller. If the derivation itself needs to be gotten first, a + * `DerivationTrampolineGoal` goal must be used instead. */ struct DerivationGoal : public Goal { /** The path of the derivation. */ - ref drvReq; + StorePath drvPath; /** * The specific outputs that we need to build. */ - OutputsSpec wantedOutputs; + OutputName wantedOutput; /** - * See `needRestart`; just for that field. - */ - enum struct NeedRestartForMoreOutputs { - /** - * The goal state machine is progressing based on the current value of - * `wantedOutputs. No actions are needed. - */ - OutputsUnmodifiedDontNeed, - /** - * `wantedOutputs` has been extended, but the state machine is - * proceeding according to its old value, so we need to restart. - */ - OutputsAddedDoNeed, - /** - * The goal state machine has progressed to the point of doing a build, - * in which case all outputs will be produced, so extensions to - * `wantedOutputs` no longer require a restart. - */ - BuildInProgressWillNotNeed, - }; - - /** - * Whether additional wanted outputs have been added. - */ - NeedRestartForMoreOutputs needRestart = NeedRestartForMoreOutputs::OutputsUnmodifiedDontNeed; - - /** - * The derivation stored at `drvReq`. + * The derivation stored at drvPath. */ std::unique_ptr drv; @@ -76,11 +62,8 @@ struct DerivationGoal : public Goal std::unique_ptr> mcExpectedBuilds; - DerivationGoal(ref drvReq, - const OutputsSpec & wantedOutputs, Worker & worker, - BuildMode buildMode = bmNormal); - DerivationGoal(const StorePath & drvPath, const BasicDerivation & drv, - const OutputsSpec & wantedOutputs, Worker & worker, + DerivationGoal(const StorePath & drvPath, const Derivation & drv, + const OutputName & wantedOutput, Worker & worker, BuildMode buildMode = bmNormal); ~DerivationGoal() = default; @@ -88,24 +71,18 @@ struct DerivationGoal : public Goal std::string key() override; - /** - * Add wanted outputs to an already existing derivation goal. - */ - void addWantedOutputs(const OutputsSpec & outputs); - /** * The states. */ - Co loadDerivation(); - Co haveDerivation(StorePath drvPath); + Co haveDerivation(); /** * Wrappers around the corresponding Store methods that first consult the * derivation. This is currently needed because when there is no drv file * there also is no DB entry. */ - std::map> queryPartialDerivationOutputMap(const StorePath & drvPath); - OutputPathMap queryDerivationOutputMap(const StorePath & drvPath); + std::map> queryPartialDerivationOutputMap(); + OutputPathMap queryDerivationOutputMap(); /** * Update 'initialOutputs' to determine the current status of the @@ -113,18 +90,17 @@ struct DerivationGoal : public Goal * whether all outputs are valid and non-corrupt, and a * 'SingleDrvOutputs' structure containing the valid outputs. */ - std::pair checkPathValidity(const StorePath & drvPath); + std::pair checkPathValidity(); /** * Aborts if any output is not valid or corrupt, and otherwise * returns a 'SingleDrvOutputs' structure containing all outputs. */ - SingleDrvOutputs assertPathValidity(const StorePath & drvPath); + SingleDrvOutputs assertPathValidity(); - Co repairClosure(StorePath drvPath); + Co repairClosure(); Done done( - const StorePath & drvPath, BuildResult::Status status, SingleDrvOutputs builtOutputs = {}, std::optional ex = {}); diff --git a/src/libstore/include/nix/store/build/derivation-trampoline-goal.hh b/src/libstore/include/nix/store/build/derivation-trampoline-goal.hh new file mode 100644 index 000000000..648337695 --- /dev/null +++ b/src/libstore/include/nix/store/build/derivation-trampoline-goal.hh @@ -0,0 +1,134 @@ +#pragma once +///@file + +#include "nix/store/parsed-derivations.hh" +#include "nix/store/store-api.hh" +#include "nix/store/pathlocks.hh" +#include "nix/store/build/goal.hh" + +namespace nix { + +/** + * This is the "outermost" goal type relating to derivations --- by that + * we mean that this one calls all the others for a given derivation. + * + * This is a purely "administrative" goal type, which doesn't do any "real + * work". See `DerivationGoal` for what we mean by such an administrative goal. + * + * # Rationale + * + * It exists to solve two problems: + * + * 1. We want to build a derivation we don't yet have. + * + * Traditionally, that simply means we try to substitute the missing + * derivation; simple enough. However, with (currently experimental) + * dynamic derivations, derivations themselves can be the outputs of + * other derivations. That means the general case is that a + * `DerivationTrampolineGoal` needs to create *another* + * `DerivationTrampolineGoal` goal to realize the derivation it needs. + * That goal in turn might need to create a third + * `DerivationTrampolineGoal`, the induction down to a statically known + * derivation as the base case is arbitrary deep. + * + * 2. Only a subset of outputs is needed, but such subsets are discovered + * dynamically. + * + * Consider derivations: + * + * - A has outputs x, y, and z + * + * - B needs A^x,y + * + * - C needs A^y,z and B's single output + * + * With the current `Worker` architecture, we're first discover + * needing `A^y,z` and then discover needing `A^x,y`. Of course, we + * don't want to download `A^y` twice, either. + * + * The way we handle sharing work for `A^y` is to have + * `DerivationGoal` just handle a single output, and do slightly more + * work (though it is just an "administrative" goal too), and + * `DerivationTrampolineGoal` handle sets of goals, but have it (once the + * derivation itself has been gotten) *just* create + * `DerivationGoal`s. + * + * That means it is fine to create man `DerivationTrampolineGoal` with + * overlapping sets of outputs, because all the "real work" will be + * coordinated via `DerivationGoal`s, and sharing will be discovered. + * + * Both these problems *can* be solved by having just a more powerful + * `DerivationGoal`, but that makes `DerivationGoal` more complex. + * However the more complex `DerivationGoal` has these downsides: + * + * 1. It needs to cope with only sometimes knowing a `StorePath drvPath` + * (as opposed to a more general `SingleDerivedPath drvPath` with will + * be only resolved to a `StorePath` part way through the control flow). + * + * 2. It needs complicated "restarting logic" to cope with the set of + * "wanted outputs" growing over time. + * + * (1) is not so bad, but (2) is quite scary, and has been a source of + * bugs in the past. By splitting out `DerivationTrampolineGoal`, we + * crucially avoid a need for (2), letting goal sharing rather than + * ad-hoc retry mechanisms accomplish the deduplication we need. Solving + * (1) is just a by-product and extra bonus of creating + * `DerivationTrampolineGoal`. + * + * # Misc Notes + * + * If we already have the derivation (e.g. if the evaluator has created + * the derivation locally and then instructed the store to build it), we + * can skip the derivation-getting goal entirely as a small + * optimization. + */ +struct DerivationTrampolineGoal : public Goal +{ + /** + * How to obtain a store path of the derivation to build. + */ + ref drvReq; + + /** + * The specific outputs that we need to build. + */ + OutputsSpec wantedOutputs; + + DerivationTrampolineGoal( + ref drvReq, + const OutputsSpec & wantedOutputs, + Worker & worker, + BuildMode buildMode = bmNormal); + + DerivationTrampolineGoal( + const StorePath & drvPath, + const OutputsSpec & wantedOutputs, + const Derivation & drv, + Worker & worker, + BuildMode buildMode = bmNormal); + + virtual ~DerivationTrampolineGoal(); + + void timedOut(Error && ex) override; + + std::string key() override; + + JobCategory jobCategory() const override + { + return JobCategory::Administration; + }; + +private: + + BuildMode buildMode; + + Co init(); + Co haveDerivation(StorePath drvPath, Derivation drv); + + /** + * Shared between both constructors + */ + void commonInit(); +}; + +} diff --git a/src/libstore/include/nix/store/build/goal.hh b/src/libstore/include/nix/store/build/goal.hh index 577ce1e84..ee69c9cc7 100644 --- a/src/libstore/include/nix/store/build/goal.hh +++ b/src/libstore/include/nix/store/build/goal.hh @@ -105,13 +105,11 @@ public: */ ExitCode exitCode = ecBusy; -protected: /** * Build result. */ BuildResult buildResult; -public: /** * Suspend our goal and wait until we get `work`-ed again. * `co_await`-able by @ref Co. @@ -358,18 +356,6 @@ protected: public: virtual void cleanup() { } - /** - * Project a `BuildResult` with just the information that pertains - * to the given request. - * - * In general, goals may be aliased between multiple requests, and - * the stored `BuildResult` has information for the union of all - * requests. We don't want to leak what the other request are for - * sake of both privacy and determinism, and this "safe accessor" - * ensures we don't. - */ - BuildResult getBuildResult(const DerivedPath &) const; - /** * Hack to say that this goal should not log `ex`, but instead keep * it around. Set by a waitee which sees itself as the designated diff --git a/src/libstore/include/nix/store/build/worker.hh b/src/libstore/include/nix/store/build/worker.hh index c70c72377..491b8f494 100644 --- a/src/libstore/include/nix/store/build/worker.hh +++ b/src/libstore/include/nix/store/build/worker.hh @@ -14,6 +14,7 @@ namespace nix { /* Forward definition. */ +struct DerivationTrampolineGoal; struct DerivationGoal; struct DerivationBuildingGoal; struct PathSubstitutionGoal; @@ -33,6 +34,7 @@ class DrvOutputSubstitutionGoal; */ GoalPtr upcast_goal(std::shared_ptr subGoal); GoalPtr upcast_goal(std::shared_ptr subGoal); +GoalPtr upcast_goal(std::shared_ptr subGoal); typedef std::chrono::time_point steady_time_point; @@ -106,8 +108,9 @@ private: * same derivation / path. */ - DerivedPathMap> derivationGoals; + DerivedPathMap>> derivationTrampolineGoals; + std::map>> derivationGoals; std::map> derivationBuildingGoals; std::map> substitutionGoals; std::map> drvOutputSubstitutionGoals; @@ -204,16 +207,20 @@ private: template std::shared_ptr initGoalIfNeeded(std::weak_ptr & goal_weak, Args && ...args); - std::shared_ptr makeDerivationGoalCommon( - ref drvReq, const OutputsSpec & wantedOutputs, - std::function()> mkDrvGoal); -public: - std::shared_ptr makeDerivationGoal( + std::shared_ptr makeDerivationTrampolineGoal( ref drvReq, const OutputsSpec & wantedOutputs, BuildMode buildMode = bmNormal); - std::shared_ptr makeBasicDerivationGoal( - const StorePath & drvPath, const BasicDerivation & drv, - const OutputsSpec & wantedOutputs, BuildMode buildMode = bmNormal); + +public: + std::shared_ptr makeDerivationTrampolineGoal( + const StorePath & drvPath, + const OutputsSpec & wantedOutputs, + const Derivation & drv, + BuildMode buildMode = bmNormal); + + std::shared_ptr makeDerivationGoal( + const StorePath & drvPath, const Derivation & drv, + const OutputName & wantedOutput, BuildMode buildMode = bmNormal); /** * @ref DerivationBuildingGoal "derivation goal" @@ -232,7 +239,7 @@ public: * Make a goal corresponding to the `DerivedPath`. * * It will be a `DerivationGoal` for a `DerivedPath::Built` or - * a `SubstitutionGoal` for a `DerivedPath::Opaque`. + * a `PathSubstitutionGoal` for a `DerivedPath::Opaque`. */ GoalPtr makeGoal(const DerivedPath & req, BuildMode buildMode = bmNormal); diff --git a/src/libstore/include/nix/store/derived-path-map.hh b/src/libstore/include/nix/store/derived-path-map.hh index 16ffeb05e..6dae73fab 100644 --- a/src/libstore/include/nix/store/derived-path-map.hh +++ b/src/libstore/include/nix/store/derived-path-map.hh @@ -22,7 +22,7 @@ namespace nix { * @param V A type to instantiate for each output. It should probably * should be an "optional" type so not every interior node has to have a * value. For example, the scheduler uses - * `DerivedPathMap>` to + * `DerivedPathMap>` to * remember which goals correspond to which outputs. `* const Something` * or `std::optional` would also be good choices for * "optional" types. diff --git a/src/libstore/include/nix/store/local-store.hh b/src/libstore/include/nix/store/local-store.hh index 9a118fcc5..1aa22e311 100644 --- a/src/libstore/include/nix/store/local-store.hh +++ b/src/libstore/include/nix/store/local-store.hh @@ -403,7 +403,6 @@ private: void addBuildLog(const StorePath & drvPath, std::string_view log) override; friend struct PathSubstitutionGoal; - friend struct SubstitutionGoal; friend struct DerivationGoal; }; diff --git a/src/libstore/include/nix/store/meson.build b/src/libstore/include/nix/store/meson.build index a18430417..44d9815de 100644 --- a/src/libstore/include/nix/store/meson.build +++ b/src/libstore/include/nix/store/meson.build @@ -15,6 +15,7 @@ headers = [config_pub_h] + files( 'build/derivation-goal.hh', 'build/derivation-building-goal.hh', 'build/derivation-building-misc.hh', + 'build/derivation-trampoline-goal.hh', 'build/drv-output-substitution-goal.hh', 'build/goal.hh', 'build/substitution-goal.hh', diff --git a/src/libstore/meson.build b/src/libstore/meson.build index 94b8951fd..d82bcddc1 100644 --- a/src/libstore/meson.build +++ b/src/libstore/meson.build @@ -255,6 +255,7 @@ sources = files( 'build-result.cc', 'build/derivation-goal.cc', 'build/derivation-building-goal.cc', + 'build/derivation-trampoline-goal.cc', 'build/drv-output-substitution-goal.cc', 'build/entry-points.cc', 'build/goal.cc', From af05ce0f6d6543c48d1f182023888083359467bf Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 4 Jul 2025 15:39:47 +0200 Subject: [PATCH 04/54] queryMissing(): Return a struct ...instead of having a bunch of pass-by-reference arguments. --- src/libmain/shared.cc | 6 ++--- src/libstore/build/worker.cc | 4 +-- src/libstore/daemon.cc | 12 ++++----- .../include/nix/store/remote-store.hh | 4 +-- src/libstore/include/nix/store/store-api.hh | 16 +++++++++--- src/libstore/misc.cc | 26 ++++++++----------- src/libstore/remote-store.cc | 18 ++++++------- src/libstore/restricted-store.cc | 24 +++++++---------- src/libstore/store-api.cc | 9 +++---- src/nix-build/nix-build.cc | 7 ++--- src/nix-store/nix-store.cc | 12 +++------ 11 files changed, 59 insertions(+), 79 deletions(-) diff --git a/src/libmain/shared.cc b/src/libmain/shared.cc index 3b431f021..75c9ebadd 100644 --- a/src/libmain/shared.cc +++ b/src/libmain/shared.cc @@ -46,10 +46,8 @@ void printGCWarning() void printMissing(ref store, const std::vector & paths, Verbosity lvl) { - uint64_t downloadSize, narSize; - StorePathSet willBuild, willSubstitute, unknown; - store->queryMissing(paths, willBuild, willSubstitute, unknown, downloadSize, narSize); - printMissing(store, willBuild, willSubstitute, unknown, downloadSize, narSize, lvl); + auto missing = store->queryMissing(paths); + printMissing(store, missing.willBuild, missing.willSubstitute, missing.unknown, missing.downloadSize, missing.narSize, lvl); } diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index 12d4aaa2d..8e10e948a 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -324,9 +324,7 @@ void Worker::run(const Goals & _topGoals) } /* Call queryMissing() to efficiently query substitutes. */ - StorePathSet willBuild, willSubstitute, unknown; - uint64_t downloadSize, narSize; - store.queryMissing(topPaths, willBuild, willSubstitute, unknown, downloadSize, narSize); + store.queryMissing(topPaths); debug("entered goal loop"); diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index dfc068bc7..bf4a9d959 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -948,14 +948,12 @@ static void performOp(TunnelLogger * logger, ref store, case WorkerProto::Op::QueryMissing: { auto targets = WorkerProto::Serialise::read(*store, rconn); logger->startWork(); - StorePathSet willBuild, willSubstitute, unknown; - uint64_t downloadSize, narSize; - store->queryMissing(targets, willBuild, willSubstitute, unknown, downloadSize, narSize); + auto missing = store->queryMissing(targets); logger->stopWork(); - WorkerProto::write(*store, wconn, willBuild); - WorkerProto::write(*store, wconn, willSubstitute); - WorkerProto::write(*store, wconn, unknown); - conn.to << downloadSize << narSize; + WorkerProto::write(*store, wconn, missing.willBuild); + WorkerProto::write(*store, wconn, missing.willSubstitute); + WorkerProto::write(*store, wconn, missing.unknown); + conn.to << missing.downloadSize << missing.narSize; break; } diff --git a/src/libstore/include/nix/store/remote-store.hh b/src/libstore/include/nix/store/remote-store.hh index dd2396fe3..18c02456f 100644 --- a/src/libstore/include/nix/store/remote-store.hh +++ b/src/libstore/include/nix/store/remote-store.hh @@ -149,9 +149,7 @@ struct RemoteStore : void addSignatures(const StorePath & storePath, const StringSet & sigs) override; - void queryMissing(const std::vector & targets, - StorePathSet & willBuild, StorePathSet & willSubstitute, StorePathSet & unknown, - uint64_t & downloadSize, uint64_t & narSize) override; + MissingPaths queryMissing(const std::vector & targets) override; void addBuildLog(const StorePath & drvPath, std::string_view log) override; diff --git a/src/libstore/include/nix/store/store-api.hh b/src/libstore/include/nix/store/store-api.hh index 72ca1f437..e0a3e67d1 100644 --- a/src/libstore/include/nix/store/store-api.hh +++ b/src/libstore/include/nix/store/store-api.hh @@ -71,6 +71,18 @@ struct KeyedBuildResult; typedef std::map> StorePathCAMap; +/** + * Information about what paths will be built or substituted, returned + * by Store::queryMissing(). + */ +struct MissingPaths +{ + StorePathSet willBuild; + StorePathSet willSubstitute; + StorePathSet unknown; + uint64_t downloadSize{0}; + uint64_t narSize{0}; +}; /** * About the class hierarchy of the store types: @@ -694,9 +706,7 @@ public: * derivations that will be built, and the set of output paths that * will be substituted. */ - virtual void queryMissing(const std::vector & targets, - StorePathSet & willBuild, StorePathSet & willSubstitute, StorePathSet & unknown, - uint64_t & downloadSize, uint64_t & narSize); + virtual MissingPaths queryMissing(const std::vector & targets); /** * Sort a set of paths topologically under the references diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index dabae647f..7c97dbc57 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -98,23 +98,17 @@ const ContentAddress * getDerivationCA(const BasicDerivation & drv) return nullptr; } -void Store::queryMissing(const std::vector & targets, - StorePathSet & willBuild_, StorePathSet & willSubstitute_, StorePathSet & unknown_, - uint64_t & downloadSize_, uint64_t & narSize_) +MissingPaths Store::queryMissing(const std::vector & targets) { Activity act(*logger, lvlDebug, actUnknown, "querying info about missing paths"); - downloadSize_ = narSize_ = 0; - // FIXME: make async. ThreadPool pool(fileTransferSettings.httpConnections); struct State { std::unordered_set done; - StorePathSet & unknown, & willSubstitute, & willBuild; - uint64_t & downloadSize; - uint64_t & narSize; + MissingPaths res; }; struct DrvState @@ -125,7 +119,7 @@ void Store::queryMissing(const std::vector & targets, DrvState(size_t left) : left(left) { } }; - Sync state_(State{{}, unknown_, willSubstitute_, willBuild_, downloadSize_, narSize_}); + Sync state_; std::function doPath; @@ -143,7 +137,7 @@ void Store::queryMissing(const std::vector & targets, auto mustBuildDrv = [&](const StorePath & drvPath, const Derivation & drv) { { auto state(state_.lock()); - state->willBuild.insert(drvPath); + state->res.willBuild.insert(drvPath); } for (const auto & [inputDrv, inputNode] : drv.inputDrvs.map) { @@ -203,7 +197,7 @@ void Store::queryMissing(const std::vector & targets, if (!isValidPath(drvPath)) { // FIXME: we could try to substitute the derivation. auto state(state_.lock()); - state->unknown.insert(drvPath); + state->res.unknown.insert(drvPath); return; } @@ -282,7 +276,7 @@ void Store::queryMissing(const std::vector & targets, if (infos.empty()) { auto state(state_.lock()); - state->unknown.insert(bo.path); + state->res.unknown.insert(bo.path); return; } @@ -291,9 +285,9 @@ void Store::queryMissing(const std::vector & targets, { auto state(state_.lock()); - state->willSubstitute.insert(bo.path); - state->downloadSize += info->second.downloadSize; - state->narSize += info->second.narSize; + state->res.willSubstitute.insert(bo.path); + state->res.downloadSize += info->second.downloadSize; + state->res.narSize += info->second.narSize; } for (auto & ref : info->second.references) @@ -306,6 +300,8 @@ void Store::queryMissing(const std::vector & targets, pool.enqueue(std::bind(doPath, path)); pool.process(); + + return std::move(state_.lock()->res); } diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 3151f319c..1b8bad048 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -855,9 +855,7 @@ void RemoteStore::addSignatures(const StorePath & storePath, const StringSet & s } -void RemoteStore::queryMissing(const std::vector & targets, - StorePathSet & willBuild, StorePathSet & willSubstitute, StorePathSet & unknown, - uint64_t & downloadSize, uint64_t & narSize) +MissingPaths RemoteStore::queryMissing(const std::vector & targets) { { auto conn(getConnection()); @@ -868,16 +866,16 @@ void RemoteStore::queryMissing(const std::vector & targets, conn->to << WorkerProto::Op::QueryMissing; WorkerProto::write(*this, *conn, targets); conn.processStderr(); - willBuild = WorkerProto::Serialise::read(*this, *conn); - willSubstitute = WorkerProto::Serialise::read(*this, *conn); - unknown = WorkerProto::Serialise::read(*this, *conn); - conn->from >> downloadSize >> narSize; - return; + MissingPaths res; + res.willBuild = WorkerProto::Serialise::read(*this, *conn); + res.willSubstitute = WorkerProto::Serialise::read(*this, *conn); + res.unknown = WorkerProto::Serialise::read(*this, *conn); + conn->from >> res.downloadSize >> res.narSize; + return res; } fallback: - return Store::queryMissing(targets, willBuild, willSubstitute, - unknown, downloadSize, narSize); + return Store::queryMissing(targets); } diff --git a/src/libstore/restricted-store.cc b/src/libstore/restricted-store.cc index 0485f5584..69435122a 100644 --- a/src/libstore/restricted-store.cc +++ b/src/libstore/restricted-store.cc @@ -143,13 +143,7 @@ struct RestrictedStore : public virtual IndirectRootStore, public virtual GcStor unsupported("addSignatures"); } - void queryMissing( - const std::vector & targets, - StorePathSet & willBuild, - StorePathSet & willSubstitute, - StorePathSet & unknown, - uint64_t & downloadSize, - uint64_t & narSize) override; + MissingPaths queryMissing(const std::vector & targets) override; virtual std::optional getBuildLogExact(const StorePath & path) override { @@ -306,19 +300,14 @@ std::vector RestrictedStore::buildPathsWithResults( return results; } -void RestrictedStore::queryMissing( - const std::vector & targets, - StorePathSet & willBuild, - StorePathSet & willSubstitute, - StorePathSet & unknown, - uint64_t & downloadSize, - uint64_t & narSize) +MissingPaths RestrictedStore::queryMissing(const std::vector & targets) { /* This is slightly impure since it leaks information to the client about what paths will be built/substituted or are already present. Probably not a big deal. */ std::vector allowed; + StorePathSet unknown; for (auto & req : targets) { if (goal.isAllowed(req)) allowed.emplace_back(req); @@ -326,7 +315,12 @@ void RestrictedStore::queryMissing( unknown.insert(pathPartOfReq(req)); } - next->queryMissing(allowed, willBuild, willSubstitute, unknown, downloadSize, narSize); + auto res = next->queryMissing(allowed); + + for (auto & p : unknown) + res.unknown.insert(p); + + return res; } } diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 296f2251a..9aeab1d1f 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -790,15 +790,12 @@ void Store::substitutePaths(const StorePathSet & paths) for (auto & path : paths) if (!path.isDerivation()) paths2.emplace_back(DerivedPath::Opaque{path}); - uint64_t downloadSize, narSize; - StorePathSet willBuild, willSubstitute, unknown; - queryMissing(paths2, - willBuild, willSubstitute, unknown, downloadSize, narSize); + auto missing = queryMissing(paths2); - if (!willSubstitute.empty()) + if (!missing.willSubstitute.empty()) try { std::vector subs; - for (auto & p : willSubstitute) subs.emplace_back(DerivedPath::Opaque{p}); + for (auto & p : missing.willSubstitute) subs.emplace_back(DerivedPath::Opaque{p}); buildPaths(subs); } catch (Error & e) { logWarning(e.info()); diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index cde9d6742..36e6f3eab 100644 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -422,13 +422,10 @@ static void main_nix_build(int argc, char * * argv) auto buildPaths = [&](const std::vector & paths) { /* Note: we do this even when !printMissing to efficiently fetch binary cache data. */ - uint64_t downloadSize, narSize; - StorePathSet willBuild, willSubstitute, unknown; - store->queryMissing(paths, - willBuild, willSubstitute, unknown, downloadSize, narSize); + auto missing = store->queryMissing(paths); if (settings.printMissing) - printMissing(ref(store), willBuild, willSubstitute, unknown, downloadSize, narSize); + printMissing(ref(store), missing.willBuild, missing.willSubstitute, missing.unknown, missing.downloadSize, missing.narSize); if (!dryRun) store->buildPaths(paths, buildMode, evalStore); diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index 0d758f44b..c508029b5 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -146,23 +146,19 @@ static void opRealise(Strings opFlags, Strings opArgs) for (auto & i : opArgs) paths.push_back(followLinksToStorePathWithOutputs(*store, i)); - uint64_t downloadSize, narSize; - StorePathSet willBuild, willSubstitute, unknown; - store->queryMissing( - toDerivedPaths(paths), - willBuild, willSubstitute, unknown, downloadSize, narSize); + auto missing = store->queryMissing(toDerivedPaths(paths)); /* Filter out unknown paths from `paths`. */ if (ignoreUnknown) { std::vector paths2; for (auto & i : paths) - if (!unknown.count(i.path)) paths2.push_back(i); + if (!missing.unknown.count(i.path)) paths2.push_back(i); paths = std::move(paths2); - unknown = StorePathSet(); + missing.unknown = StorePathSet(); } if (settings.printMissing) - printMissing(ref(store), willBuild, willSubstitute, unknown, downloadSize, narSize); + printMissing(ref(store), missing.willBuild, missing.willSubstitute, missing.unknown, missing.downloadSize, missing.narSize); if (dryRun) return; From 5d308ccca558230d374ea0579c80cf68d950feda Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 4 Jul 2025 16:27:08 +0200 Subject: [PATCH 05/54] printMissing(): Take a MissingPaths argument --- src/libmain/include/nix/main/shared.hh | 8 +++--- src/libmain/shared.cc | 34 +++++++++++++------------- src/nix-build/nix-build.cc | 2 +- src/nix-store/nix-store.cc | 2 +- 4 files changed, 24 insertions(+), 22 deletions(-) diff --git a/src/libmain/include/nix/main/shared.hh b/src/libmain/include/nix/main/shared.hh index 2ff57135b..4d4b816e7 100644 --- a/src/libmain/include/nix/main/shared.hh +++ b/src/libmain/include/nix/main/shared.hh @@ -35,15 +35,17 @@ void printVersion(const std::string & programName); void printGCWarning(); class Store; +struct MissingPaths; void printMissing( ref store, const std::vector & paths, Verbosity lvl = lvlInfo); -void printMissing(ref store, const StorePathSet & willBuild, - const StorePathSet & willSubstitute, const StorePathSet & unknown, - uint64_t downloadSize, uint64_t narSize, Verbosity lvl = lvlInfo); +void printMissing( + ref store, + const MissingPaths & missing, + Verbosity lvl = lvlInfo); std::string getArg(const std::string & opt, Strings::iterator & i, const Strings::iterator & end); diff --git a/src/libmain/shared.cc b/src/libmain/shared.cc index 75c9ebadd..0982810d1 100644 --- a/src/libmain/shared.cc +++ b/src/libmain/shared.cc @@ -46,41 +46,41 @@ void printGCWarning() void printMissing(ref store, const std::vector & paths, Verbosity lvl) { - auto missing = store->queryMissing(paths); - printMissing(store, missing.willBuild, missing.willSubstitute, missing.unknown, missing.downloadSize, missing.narSize, lvl); + printMissing(store, store->queryMissing(paths), lvl); } -void printMissing(ref store, const StorePathSet & willBuild, - const StorePathSet & willSubstitute, const StorePathSet & unknown, - uint64_t downloadSize, uint64_t narSize, Verbosity lvl) +void printMissing( + ref store, + const MissingPaths & missing, + Verbosity lvl) { - if (!willBuild.empty()) { - if (willBuild.size() == 1) + if (!missing.willBuild.empty()) { + if (missing.willBuild.size() == 1) printMsg(lvl, "this derivation will be built:"); else - printMsg(lvl, "these %d derivations will be built:", willBuild.size()); - auto sorted = store->topoSortPaths(willBuild); + printMsg(lvl, "these %d derivations will be built:", missing.willBuild.size()); + auto sorted = store->topoSortPaths(missing.willBuild); reverse(sorted.begin(), sorted.end()); for (auto & i : sorted) printMsg(lvl, " %s", store->printStorePath(i)); } - if (!willSubstitute.empty()) { - const float downloadSizeMiB = downloadSize / (1024.f * 1024.f); - const float narSizeMiB = narSize / (1024.f * 1024.f); - if (willSubstitute.size() == 1) { + if (!missing.willSubstitute.empty()) { + const float downloadSizeMiB = missing.downloadSize / (1024.f * 1024.f); + const float narSizeMiB = missing.narSize / (1024.f * 1024.f); + if (missing.willSubstitute.size() == 1) { printMsg(lvl, "this path will be fetched (%.2f MiB download, %.2f MiB unpacked):", downloadSizeMiB, narSizeMiB); } else { printMsg(lvl, "these %d paths will be fetched (%.2f MiB download, %.2f MiB unpacked):", - willSubstitute.size(), + missing.willSubstitute.size(), downloadSizeMiB, narSizeMiB); } std::vector willSubstituteSorted = {}; - std::for_each(willSubstitute.begin(), willSubstitute.end(), + std::for_each(missing.willSubstitute.begin(), missing.willSubstitute.end(), [&](const StorePath &p) { willSubstituteSorted.push_back(&p); }); std::sort(willSubstituteSorted.begin(), willSubstituteSorted.end(), [](const StorePath *lhs, const StorePath *rhs) { @@ -93,10 +93,10 @@ void printMissing(ref store, const StorePathSet & willBuild, printMsg(lvl, " %s", store->printStorePath(*p)); } - if (!unknown.empty()) { + if (!missing.unknown.empty()) { printMsg(lvl, "don't know how to build these paths%s:", (settings.readOnlyMode ? " (may be caused by read-only store access)" : "")); - for (auto & i : unknown) + for (auto & i : missing.unknown) printMsg(lvl, " %s", store->printStorePath(i)); } } diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index 36e6f3eab..e2f4c0330 100644 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -425,7 +425,7 @@ static void main_nix_build(int argc, char * * argv) auto missing = store->queryMissing(paths); if (settings.printMissing) - printMissing(ref(store), missing.willBuild, missing.willSubstitute, missing.unknown, missing.downloadSize, missing.narSize); + printMissing(ref(store), missing); if (!dryRun) store->buildPaths(paths, buildMode, evalStore); diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index c508029b5..3da7a8ac1 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -158,7 +158,7 @@ static void opRealise(Strings opFlags, Strings opArgs) } if (settings.printMissing) - printMissing(ref(store), missing.willBuild, missing.willSubstitute, missing.unknown, missing.downloadSize, missing.narSize); + printMissing(ref(store), missing); if (dryRun) return; From f039f6886a9880b006f78df2ab273a0d66f20db9 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 4 Jul 2025 16:32:37 +0200 Subject: [PATCH 06/54] nix-build: Drop unnecessary call to queryMissing() This is already done by Worker::run(). --- src/nix-build/nix-build.cc | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index e2f4c0330..7e0b40252 100644 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -420,12 +420,8 @@ static void main_nix_build(int argc, char * * argv) state->maybePrintStats(); auto buildPaths = [&](const std::vector & paths) { - /* Note: we do this even when !printMissing to efficiently - fetch binary cache data. */ - auto missing = store->queryMissing(paths); - if (settings.printMissing) - printMissing(ref(store), missing); + printMissing(ref(store), paths); if (!dryRun) store->buildPaths(paths, buildMode, evalStore); From fb5e22e3184ccd18c038eecf67e9fd350a8d9b4f Mon Sep 17 00:00:00 2001 From: DavHau Date: Fri, 27 Jun 2025 15:35:46 +0700 Subject: [PATCH 07/54] build-cores: detect cores automatically if set to 0 This changes makes nix detect a machines available cores automatically whenever build-cores is set to 0. So far, nix simply passed NIX_BUILD_CORES=0 whenever build-cores is set to 0. (only when build-cores is unset it was detecting cores automatically) The behavior of passing NIX_BUILD_CORES=0 leads to a performance penalty when sourcing nixpkgs' generic builder's `setup.sh`, as setup.sh has to execute `nproc`. This significantly slows down sourcing of setup.sh --- doc/manual/rl-next/build-cores-auto-detect.md | 6 ++++ src/libstore/globals.cc | 2 +- src/libstore/include/nix/store/globals.hh | 12 +++---- src/libstore/unix/build/derivation-builder.cc | 2 +- src/nix-build/nix-build.cc | 2 +- tests/functional/build-cores.nix | 11 +++++++ tests/functional/build-cores.sh | 32 +++++++++++++++++++ tests/functional/meson.build | 1 + 8 files changed, 58 insertions(+), 10 deletions(-) create mode 100644 doc/manual/rl-next/build-cores-auto-detect.md create mode 100644 tests/functional/build-cores.nix create mode 100755 tests/functional/build-cores.sh diff --git a/doc/manual/rl-next/build-cores-auto-detect.md b/doc/manual/rl-next/build-cores-auto-detect.md new file mode 100644 index 000000000..67ab6995b --- /dev/null +++ b/doc/manual/rl-next/build-cores-auto-detect.md @@ -0,0 +1,6 @@ +--- +synopsis: "`build-cores = 0` now auto-detects CPU cores" +prs: [13402] +--- + +When `build-cores` is set to `0`, nix now automatically detects the number of available CPU cores and passes this value via `NIX_BUILD_CORES`, instead of passing `0` directly. This matches the behavior when `build-cores` is unset. This prevents the builder from having to detect the number of cores. diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc index de5128347..1f80cb379 100644 --- a/src/libstore/globals.cc +++ b/src/libstore/globals.cc @@ -140,7 +140,7 @@ std::vector getUserConfigFiles() return files; } -unsigned int Settings::getDefaultCores() +unsigned int Settings::getDefaultCores() const { const unsigned int concurrency = std::max(1U, std::thread::hardware_concurrency()); const unsigned int maxCPU = getMaxCPU(); diff --git a/src/libstore/include/nix/store/globals.hh b/src/libstore/include/nix/store/globals.hh index 0ac689b55..8dfdf11cb 100644 --- a/src/libstore/include/nix/store/globals.hh +++ b/src/libstore/include/nix/store/globals.hh @@ -43,8 +43,6 @@ const uint32_t maxIdsPerBuild = class Settings : public Config { - unsigned int getDefaultCores(); - StringSet getDefaultSystemFeatures(); StringSet getDefaultExtraPlatforms(); @@ -57,6 +55,8 @@ public: Settings(); + unsigned int getDefaultCores() const; + Path nixPrefix; /** @@ -153,7 +153,7 @@ public: Setting buildCores{ this, - getDefaultCores(), + 0, "cores", R"( Sets the value of the `NIX_BUILD_CORES` environment variable in the [invocation of the `builder` executable](@docroot@/language/derivations.md#builder-execution) of a derivation. @@ -166,15 +166,13 @@ public: --> For instance, in Nixpkgs, if the attribute `enableParallelBuilding` for the `mkDerivation` build helper is set to `true`, it passes the `-j${NIX_BUILD_CORES}` flag to GNU Make. - The value `0` means that the `builder` should use all available CPU cores in the system. + If set to `0`, nix will detect the number of CPU cores and pass this number via NIX_BUILD_CORES. > **Note** > > The number of parallel local Nix build jobs is independently controlled with the [`max-jobs`](#conf-max-jobs) setting. )", - {"build-cores"}, - // Don't document the machine-specific default value - false}; + {"build-cores"}}; /** * Read-only mode. Don't copy stuff to the store, don't change diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index fd62aa664..cf6c0a5b1 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -1083,7 +1083,7 @@ void DerivationBuilderImpl::initEnv() env["NIX_STORE"] = store.storeDir; /* The maximum number of cores to utilize for parallel building. */ - env["NIX_BUILD_CORES"] = fmt("%d", settings.buildCores); + env["NIX_BUILD_CORES"] = fmt("%d", settings.buildCores ? settings.buildCores : settings.getDefaultCores()); /* In non-structured mode, set all bindings either directory in the environment or via a file, as specified by diff --git a/src/nix-build/nix-build.cc b/src/nix-build/nix-build.cc index cde9d6742..04ea44c3d 100644 --- a/src/nix-build/nix-build.cc +++ b/src/nix-build/nix-build.cc @@ -542,7 +542,7 @@ static void main_nix_build(int argc, char * * argv) env["NIX_BUILD_TOP"] = env["TMPDIR"] = env["TEMPDIR"] = env["TMP"] = env["TEMP"] = tmpDir.path().string(); env["NIX_STORE"] = store->storeDir; - env["NIX_BUILD_CORES"] = std::to_string(settings.buildCores); + env["NIX_BUILD_CORES"] = fmt("%d", settings.buildCores ? settings.buildCores : settings.getDefaultCores()); auto parsedDrv = StructuredAttrs::tryParse(drv.env); DerivationOptions drvOptions; diff --git a/tests/functional/build-cores.nix b/tests/functional/build-cores.nix new file mode 100644 index 000000000..9b763f7d8 --- /dev/null +++ b/tests/functional/build-cores.nix @@ -0,0 +1,11 @@ +with import ./config.nix; + +{ + # Test derivation that checks the NIX_BUILD_CORES environment variable + testCores = mkDerivation { + name = "test-build-cores"; + buildCommand = '' + echo "$NIX_BUILD_CORES" > $out + ''; + }; +} diff --git a/tests/functional/build-cores.sh b/tests/functional/build-cores.sh new file mode 100755 index 000000000..a226774c6 --- /dev/null +++ b/tests/functional/build-cores.sh @@ -0,0 +1,32 @@ +#!/usr/bin/env bash + +source common.sh + +clearStoreIfPossible + +echo "Testing build-cores configuration behavior..." + +# Test 1: When build-cores is set to a non-zero value, NIX_BUILD_CORES should have that value +echo "Testing build-cores=4..." +rm -f "$TEST_ROOT"/build-cores-output +nix-build --cores 4 build-cores.nix -A testCores -o "$TEST_ROOT"/build-cores-output +result=$(cat "$(readlink "$TEST_ROOT"/build-cores-output)") +if [[ "$result" != "4" ]]; then + echo "FAIL: Expected NIX_BUILD_CORES=4, got $result" + exit 1 +fi +echo "PASS: build-cores=4 correctly sets NIX_BUILD_CORES=4" +rm -f "$TEST_ROOT"/build-cores-output + +# Test 2: When build-cores is set to 0, NIX_BUILD_CORES should be resolved to getDefaultCores() +echo "Testing build-cores=0..." +nix-build --cores 0 build-cores.nix -A testCores -o "$TEST_ROOT"/build-cores-output +result=$(cat "$(readlink "$TEST_ROOT"/build-cores-output)") +if [[ "$result" == "0" ]]; then + echo "FAIL: NIX_BUILD_CORES should not be 0 when build-cores=0" + exit 1 +fi +echo "PASS: build-cores=0 resolves to NIX_BUILD_CORES=$result (should be > 0)" +rm -f "$TEST_ROOT"/build-cores-output + +echo "All build-cores tests passed!" diff --git a/tests/functional/meson.build b/tests/functional/meson.build index cd1bc6319..501ed45c7 100644 --- a/tests/functional/meson.build +++ b/tests/functional/meson.build @@ -145,6 +145,7 @@ suites = [ 'placeholders.sh', 'ssh-relay.sh', 'build.sh', + 'build-cores.sh', 'build-delete.sh', 'output-normalization.sh', 'selfref-gc.sh', From 01388b3e78582553cc30ee772db5b5aba8d89edd Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 16 Jun 2025 12:09:43 +0200 Subject: [PATCH 08/54] Give unit tests access to a $HOME directory Also, don't try to access cache.nixos.org in the libstore unit tests. --- src/libflake-tests/meson.build | 1 + src/libflake-tests/package.nix | 20 ++++++++------------ src/libstore-tests/meson.build | 1 + src/libstore-tests/nix_api_store.cc | 16 +--------------- src/libstore-tests/package.nix | 18 +++++++----------- 5 files changed, 18 insertions(+), 38 deletions(-) diff --git a/src/libflake-tests/meson.build b/src/libflake-tests/meson.build index 593b0e18d..8c082c7e0 100644 --- a/src/libflake-tests/meson.build +++ b/src/libflake-tests/meson.build @@ -60,6 +60,7 @@ test( env : { '_NIX_TEST_UNIT_DATA': meson.current_source_dir() / 'data', 'NIX_CONFIG': 'extra-experimental-features = flakes', + 'HOME': meson.current_build_dir() / 'test-home', }, protocol : 'gtest', ) diff --git a/src/libflake-tests/package.nix b/src/libflake-tests/package.nix index 714f3791a..1a8afd6ea 100644 --- a/src/libflake-tests/package.nix +++ b/src/libflake-tests/package.nix @@ -56,18 +56,14 @@ mkMesonExecutable (finalAttrs: { { meta.broken = !stdenv.hostPlatform.emulatorAvailable buildPackages; } - ( - lib.optionalString stdenv.hostPlatform.isWindows '' - export HOME="$PWD/home-dir" - mkdir -p "$HOME" - '' - + '' - export _NIX_TEST_UNIT_DATA=${resolvePath ./data} - export NIX_CONFIG="extra-experimental-features = flakes" - ${stdenv.hostPlatform.emulator buildPackages} ${lib.getExe finalAttrs.finalPackage} - touch $out - '' - ); + ('' + export _NIX_TEST_UNIT_DATA=${resolvePath ./data} + export NIX_CONFIG="extra-experimental-features = flakes" + export HOME="$TMPDIR/home" + mkdir -p "$HOME" + ${stdenv.hostPlatform.emulator buildPackages} ${lib.getExe finalAttrs.finalPackage} + touch $out + ''); }; }; diff --git a/src/libstore-tests/meson.build b/src/libstore-tests/meson.build index 8a1ff40f0..8b9893b23 100644 --- a/src/libstore-tests/meson.build +++ b/src/libstore-tests/meson.build @@ -100,6 +100,7 @@ test( this_exe, env : { '_NIX_TEST_UNIT_DATA': meson.current_source_dir() / 'data', + 'HOME': meson.current_build_dir() / 'test-home', }, protocol : 'gtest', ) diff --git a/src/libstore-tests/nix_api_store.cc b/src/libstore-tests/nix_api_store.cc index 3d9f7908b..05373cb88 100644 --- a/src/libstore-tests/nix_api_store.cc +++ b/src/libstore-tests/nix_api_store.cc @@ -28,10 +28,6 @@ TEST_F(nix_api_store_test, nix_store_get_uri) TEST_F(nix_api_util_context, nix_store_get_storedir_default) { - if (nix::getEnv("HOME").value_or("") == "/homeless-shelter") { - // skipping test in sandbox because nix_store_open tries to create /nix/var/nix/profiles - GTEST_SKIP(); - } nix_libstore_init(ctx); Store * store = nix_store_open(ctx, nullptr, nullptr); assert_ctx_ok(); @@ -141,10 +137,6 @@ TEST_F(nix_api_store_test, nix_store_real_path) TEST_F(nix_api_util_context, nix_store_real_path_relocated) { - if (nix::getEnv("HOME").value_or("") == "/homeless-shelter") { - // Can't open default store from within sandbox - GTEST_SKIP(); - } auto tmp = nix::createTempDir(); std::string storeRoot = tmp + "/store"; std::string stateDir = tmp + "/state"; @@ -184,13 +176,7 @@ TEST_F(nix_api_util_context, nix_store_real_path_relocated) TEST_F(nix_api_util_context, nix_store_real_path_binary_cache) { - if (nix::getEnv("HOME").value_or("") == "/homeless-shelter") { - // TODO: override NIX_CACHE_HOME? - // skipping test in sandbox because narinfo cache can't be written - GTEST_SKIP(); - } - - Store * store = nix_store_open(ctx, "https://cache.nixos.org", nullptr); + Store * store = nix_store_open(ctx, nix::fmt("file://%s/binary-cache", nix::createTempDir()).c_str(), nullptr); assert_ctx_ok(); ASSERT_NE(store, nullptr); diff --git a/src/libstore-tests/package.nix b/src/libstore-tests/package.nix index b39ee7fa7..1f3701c7f 100644 --- a/src/libstore-tests/package.nix +++ b/src/libstore-tests/package.nix @@ -73,17 +73,13 @@ mkMesonExecutable (finalAttrs: { { meta.broken = !stdenv.hostPlatform.emulatorAvailable buildPackages; } - ( - lib.optionalString stdenv.hostPlatform.isWindows '' - export HOME="$PWD/home-dir" - mkdir -p "$HOME" - '' - + '' - export _NIX_TEST_UNIT_DATA=${data + "/src/libstore-tests/data"} - ${stdenv.hostPlatform.emulator buildPackages} ${lib.getExe finalAttrs.finalPackage} - touch $out - '' - ); + ('' + export _NIX_TEST_UNIT_DATA=${data + "/src/libstore-tests/data"} + export HOME="$TMPDIR/home" + mkdir -p "$HOME" + ${stdenv.hostPlatform.emulator buildPackages} ${lib.getExe finalAttrs.finalPackage} + touch $out + ''); }; }; From f29acd5bbc0505ea6a78a51e4828a22630dd0ff1 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 7 Jul 2025 11:12:55 +0200 Subject: [PATCH 09/54] Use writableTmpDirAsHomeHook --- src/libflake-tests/package.nix | 4 ++-- src/libstore-tests/package.nix | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libflake-tests/package.nix b/src/libflake-tests/package.nix index 1a8afd6ea..397ef4192 100644 --- a/src/libflake-tests/package.nix +++ b/src/libflake-tests/package.nix @@ -3,6 +3,7 @@ buildPackages, stdenv, mkMesonExecutable, + writableTmpDirAsHomeHook, nix-flake, nix-flake-c, @@ -55,12 +56,11 @@ mkMesonExecutable (finalAttrs: { runCommand "${finalAttrs.pname}-run" { meta.broken = !stdenv.hostPlatform.emulatorAvailable buildPackages; + buildInputs = [ writableTmpDirAsHomeHook ]; } ('' export _NIX_TEST_UNIT_DATA=${resolvePath ./data} export NIX_CONFIG="extra-experimental-features = flakes" - export HOME="$TMPDIR/home" - mkdir -p "$HOME" ${stdenv.hostPlatform.emulator buildPackages} ${lib.getExe finalAttrs.finalPackage} touch $out ''); diff --git a/src/libstore-tests/package.nix b/src/libstore-tests/package.nix index 1f3701c7f..62c7e136b 100644 --- a/src/libstore-tests/package.nix +++ b/src/libstore-tests/package.nix @@ -3,6 +3,7 @@ buildPackages, stdenv, mkMesonExecutable, + writableTmpDirAsHomeHook, nix-store, nix-store-c, @@ -72,11 +73,10 @@ mkMesonExecutable (finalAttrs: { runCommand "${finalAttrs.pname}-run" { meta.broken = !stdenv.hostPlatform.emulatorAvailable buildPackages; + buildInputs = [ writableTmpDirAsHomeHook ]; } ('' export _NIX_TEST_UNIT_DATA=${data + "/src/libstore-tests/data"} - export HOME="$TMPDIR/home" - mkdir -p "$HOME" ${stdenv.hostPlatform.emulator buildPackages} ${lib.getExe finalAttrs.finalPackage} touch $out ''); From 778156072444fdd06812b89e86169bf45f2425df Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 7 Jul 2025 11:26:06 +0200 Subject: [PATCH 10/54] libstore-tests: Fix impurity trying to access the Nix daemon This failed on macOS: nix-store-tests-run> C++ exception with description "../nix_api_store.cc:33: nix_err_code(ctx) != NIX_OK, message: error: getting status of '/nix/var/nix/daemon-socket/socket': Operation not permitted" thrown in the test body. --- src/libstore-tests/meson.build | 1 + src/libstore-tests/package.nix | 1 + 2 files changed, 2 insertions(+) diff --git a/src/libstore-tests/meson.build b/src/libstore-tests/meson.build index 8b9893b23..79f21620e 100644 --- a/src/libstore-tests/meson.build +++ b/src/libstore-tests/meson.build @@ -101,6 +101,7 @@ test( env : { '_NIX_TEST_UNIT_DATA': meson.current_source_dir() / 'data', 'HOME': meson.current_build_dir() / 'test-home', + 'NIX_REMOTE': meson.current_build_dir() / 'test-home' / 'store', }, protocol : 'gtest', ) diff --git a/src/libstore-tests/package.nix b/src/libstore-tests/package.nix index 62c7e136b..f606604ba 100644 --- a/src/libstore-tests/package.nix +++ b/src/libstore-tests/package.nix @@ -77,6 +77,7 @@ mkMesonExecutable (finalAttrs: { } ('' export _NIX_TEST_UNIT_DATA=${data + "/src/libstore-tests/data"} + export NIX_REMOTE=$HOME/store ${stdenv.hostPlatform.emulator buildPackages} ${lib.getExe finalAttrs.finalPackage} touch $out ''); From 8a9e625ba5483f08aaef7c4b7074042bcc7f0f57 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 7 Jul 2025 14:06:24 +0200 Subject: [PATCH 11/54] release notes: 2.30.0 --- doc/manual/rl-next/build-dir-mandatory.md | 9 -- doc/manual/rl-next/deprecate__json.md | 11 -- doc/manual/rl-next/eval-profiler.md | 13 --- doc/manual/rl-next/json-logger.md | 6 - doc/manual/rl-next/nix-profile-add.md | 6 - .../rl-next/outpath-and-sourceinfo-fixes.md | 17 --- doc/manual/rl-next/revert-77.md | 17 --- doc/manual/source/SUMMARY.md.in | 1 + doc/manual/source/release-notes/rl-2.30.md | 106 ++++++++++++++++++ .../data/release-credits-email-to-handle.json | 21 +++- .../data/release-credits-handle-to-name.json | 18 ++- 11 files changed, 144 insertions(+), 81 deletions(-) delete mode 100644 doc/manual/rl-next/build-dir-mandatory.md delete mode 100644 doc/manual/rl-next/deprecate__json.md delete mode 100644 doc/manual/rl-next/eval-profiler.md delete mode 100644 doc/manual/rl-next/json-logger.md delete mode 100644 doc/manual/rl-next/nix-profile-add.md delete mode 100644 doc/manual/rl-next/outpath-and-sourceinfo-fixes.md delete mode 100644 doc/manual/rl-next/revert-77.md create mode 100644 doc/manual/source/release-notes/rl-2.30.md diff --git a/doc/manual/rl-next/build-dir-mandatory.md b/doc/manual/rl-next/build-dir-mandatory.md deleted file mode 100644 index cb45a4315..000000000 --- a/doc/manual/rl-next/build-dir-mandatory.md +++ /dev/null @@ -1,9 +0,0 @@ ---- -synopsis: "`build-dir` no longer defaults to `$TMPDIR`" ---- - -The directory in which temporary build directories are created no longer defaults -to `TMPDIR` or `/tmp`, to avoid builders making their directories -world-accessible. This behavior allowed escaping the build sandbox and can -cause build impurities even when not used maliciously. We now default to `builds` -in `NIX_STATE_DIR` (which is `/nix/var/nix/builds` in the default configuration). diff --git a/doc/manual/rl-next/deprecate__json.md b/doc/manual/rl-next/deprecate__json.md deleted file mode 100644 index 7fc05832f..000000000 --- a/doc/manual/rl-next/deprecate__json.md +++ /dev/null @@ -1,11 +0,0 @@ ---- -synopsis: Deprecate manually making structured attrs with `__json = ...;` -prs: [13220] ---- - -The proper way to create a derivation using [structured attrs] in the Nix language is by using `__structuredAttrs = true` with [`builtins.derivation`]. -However, by exploiting how structured attrs are implementated, it has also been possible to create them by setting the `__json` environment variable to a serialized JSON string. -This sneaky alternative method is now deprecated, and may be disallowed in future versions of Nix. - -[structured attrs]: @docroot@/language/advanced-attributes.md#adv-attr-structuredAttrs -[`builtins.derivation`]: @docroot@/language/builtins.html#builtins-derivation diff --git a/doc/manual/rl-next/eval-profiler.md b/doc/manual/rl-next/eval-profiler.md deleted file mode 100644 index 6b2bf2ce7..000000000 --- a/doc/manual/rl-next/eval-profiler.md +++ /dev/null @@ -1,13 +0,0 @@ ---- -synopsis: Add stack sampling evaluation profiler -prs: [13220] ---- - -Nix evaluator now supports stack sampling evaluation profiling via `--eval-profiler flamegraph` setting. -It collects collapsed call stack information to output file specified by -`--eval-profile-file` (`nix.profile` by default) in a format directly consumable -by `flamegraph.pl` and compatible tools like [speedscope](https://speedscope.app/). -Sampling frequency can be configured via `--eval-profiler-frequency` (99 Hz by default). - -Unlike existing `--trace-function-calls` this profiler includes the name of the function -being called when it's available. diff --git a/doc/manual/rl-next/json-logger.md b/doc/manual/rl-next/json-logger.md deleted file mode 100644 index 867b5d8b3..000000000 --- a/doc/manual/rl-next/json-logger.md +++ /dev/null @@ -1,6 +0,0 @@ ---- -synopsis: "`json-log-path` setting" -prs: [13003] ---- - -New setting `json-log-path` that sends a copy of all Nix log messages (in JSON format) to a file or Unix domain socket. diff --git a/doc/manual/rl-next/nix-profile-add.md b/doc/manual/rl-next/nix-profile-add.md deleted file mode 100644 index a2fbcaa8d..000000000 --- a/doc/manual/rl-next/nix-profile-add.md +++ /dev/null @@ -1,6 +0,0 @@ ---- -synopsis: "Rename `nix profile install` to `nix profile add`" -prs: [13224] ---- - -The command `nix profile install` has been renamed to `nix profile add` (though the former is still available as an alias). This is because the verb "add" is a better antonym for the verb "remove" (i.e. `nix profile remove`). Nix also does not have install hooks or general behavior often associated with "installing". diff --git a/doc/manual/rl-next/outpath-and-sourceinfo-fixes.md b/doc/manual/rl-next/outpath-and-sourceinfo-fixes.md deleted file mode 100644 index 479a2f5cb..000000000 --- a/doc/manual/rl-next/outpath-and-sourceinfo-fixes.md +++ /dev/null @@ -1,17 +0,0 @@ ---- -synopsis: Non-flake inputs now contain a `sourceInfo` attribute -issues: 13164 -prs: 13170 ---- - -Flakes have always a `sourceInfo` attribute which describes the source of the flake. -The `sourceInfo.outPath` is often identical to the flake's `outPath`, however it can differ when the flake is located in a subdirectory of its source. - -Non-flake inputs (i.e. inputs with `flake = false`) can also be located at some path _within_ a wider source. -This usually happens when defining a relative path input within the same source as the parent flake, e.g. `inputs.foo.url = ./some-file.nix`. -Such relative inputs will now inherit their parent's `sourceInfo`. - -This also means it is now possible to use `?dir=subdir` on non-flake inputs. - -This iterates on the work done in 2.26 to improve relative path support ([#10089](https://github.com/NixOS/nix/pull/10089)), -and resolves a regression introduced in 2.28 relating to nested relative path inputs ([#13164](https://github.com/NixOS/nix/issues/13164)). diff --git a/doc/manual/rl-next/revert-77.md b/doc/manual/rl-next/revert-77.md deleted file mode 100644 index c2cef74d3..000000000 --- a/doc/manual/rl-next/revert-77.md +++ /dev/null @@ -1,17 +0,0 @@ ---- -synopsis: Revert incomplete closure mixed download and build feature -issues: [77, 12628] -prs: [13176] ---- - -Since Nix 1.3 (299141ecbd08bae17013226dbeae71e842b4fdd7 in 2013) Nix has attempted to mix together upstream fresh builds and downstream substitutions when remote substuters contain an "incomplete closure" (have some store objects, but not the store objects they reference). -This feature is now removed. - -Worst case, removing this feature could cause more building downstream, but it should not cause outright failures, since this is not happening for opaque store objects that we don't know how to build if we decide not to substitute. -In practice, however, we doubt even the more building is very likely to happen. -Remote stores that are missing dependencies in arbitrary ways (e.g. corruption) don't seem to be very common. - -On the contrary, when remote stores fail to implement the [closure property](@docroot@/store/store-object.md#closure-property), it is usually an *intentional* choice on the part of the remote store, because it wishes to serve as an "overlay" store over another store, such as `https://cache.nixos.org`. -If an "incomplete closure" is encountered in that situation, the right fix is not to do some sort of "franken-building" as this feature implemented, but instead to make sure both substituters are enabled in the settings. - -(In the future, we should make it easier for remote stores to indicate this to clients, to catch settings that won't work in general before a missing dependency is actually encountered.) diff --git a/doc/manual/source/SUMMARY.md.in b/doc/manual/source/SUMMARY.md.in index 8326a96e3..bfb921567 100644 --- a/doc/manual/source/SUMMARY.md.in +++ b/doc/manual/source/SUMMARY.md.in @@ -137,6 +137,7 @@ - [Contributing](development/contributing.md) - [Releases](release-notes/index.md) {{#include ./SUMMARY-rl-next.md}} + - [Release 2.30 (2025-07-07)](release-notes/rl-2.30.md) - [Release 2.29 (2025-05-14)](release-notes/rl-2.29.md) - [Release 2.28 (2025-04-02)](release-notes/rl-2.28.md) - [Release 2.27 (2025-03-03)](release-notes/rl-2.27.md) diff --git a/doc/manual/source/release-notes/rl-2.30.md b/doc/manual/source/release-notes/rl-2.30.md new file mode 100644 index 000000000..b205bad51 --- /dev/null +++ b/doc/manual/source/release-notes/rl-2.30.md @@ -0,0 +1,106 @@ +# Release 2.30.0 (2025-07-07) + +- `build-dir` no longer defaults to `$TMPDIR` + + The directory in which temporary build directories are created no longer defaults + to `TMPDIR` or `/tmp`, to avoid builders making their directories + world-accessible. This behavior allowed escaping the build sandbox and can + cause build impurities even when not used maliciously. We now default to `builds` + in `NIX_STATE_DIR` (which is `/nix/var/nix/builds` in the default configuration). + +- Deprecate manually making structured attrs with `__json = ...;` [#13220](https://github.com/NixOS/nix/pull/13220) + + The proper way to create a derivation using [structured attrs] in the Nix language is by using `__structuredAttrs = true` with [`builtins.derivation`]. + However, by exploiting how structured attrs are implementated, it has also been possible to create them by setting the `__json` environment variable to a serialized JSON string. + This sneaky alternative method is now deprecated, and may be disallowed in future versions of Nix. + + [structured attrs]: @docroot@/language/advanced-attributes.md#adv-attr-structuredAttrs + [`builtins.derivation`]: @docroot@/language/builtins.html#builtins-derivation + +- Add stack sampling evaluation profiler [#13220](https://github.com/NixOS/nix/pull/13220) + + Nix evaluator now supports stack sampling evaluation profiling via `--eval-profiler flamegraph` setting. + It collects collapsed call stack information to output file specified by + `--eval-profile-file` (`nix.profile` by default) in a format directly consumable + by `flamegraph.pl` and compatible tools like [speedscope](https://speedscope.app/). + Sampling frequency can be configured via `--eval-profiler-frequency` (99 Hz by default). + + Unlike existing `--trace-function-calls` this profiler includes the name of the function + being called when it's available. + +- `json-log-path` setting [#13003](https://github.com/NixOS/nix/pull/13003) + + New setting `json-log-path` that sends a copy of all Nix log messages (in JSON format) to a file or Unix domain socket. + +- Rename `nix profile install` to `nix profile add` [#13224](https://github.com/NixOS/nix/pull/13224) + + The command `nix profile install` has been renamed to `nix profile add` (though the former is still available as an alias). This is because the verb "add" is a better antonym for the verb "remove" (i.e. `nix profile remove`). Nix also does not have install hooks or general behavior often associated with "installing". + +- Non-flake inputs now contain a `sourceInfo` attribute [#13164](https://github.com/NixOS/nix/issues/13164) [#13170](https://github.com/NixOS/nix/pull/13170) + + Flakes have always a `sourceInfo` attribute which describes the source of the flake. + The `sourceInfo.outPath` is often identical to the flake's `outPath`, however it can differ when the flake is located in a subdirectory of its source. + + Non-flake inputs (i.e. inputs with `flake = false`) can also be located at some path _within_ a wider source. + This usually happens when defining a relative path input within the same source as the parent flake, e.g. `inputs.foo.url = ./some-file.nix`. + Such relative inputs will now inherit their parent's `sourceInfo`. + + This also means it is now possible to use `?dir=subdir` on non-flake inputs. + + This iterates on the work done in 2.26 to improve relative path support ([#10089](https://github.com/NixOS/nix/pull/10089)), + and resolves a regression introduced in 2.28 relating to nested relative path inputs ([#13164](https://github.com/NixOS/nix/issues/13164)). + +- Revert incomplete closure mixed download and build feature [#77](https://github.com/NixOS/nix/issues/77) [#12628](https://github.com/NixOS/nix/issues/12628) [#13176](https://github.com/NixOS/nix/pull/13176) + + Since Nix 1.3 (299141ecbd08bae17013226dbeae71e842b4fdd7 in 2013) Nix has attempted to mix together upstream fresh builds and downstream substitutions when remote substuters contain an "incomplete closure" (have some store objects, but not the store objects they reference). + This feature is now removed. + + Worst case, removing this feature could cause more building downstream, but it should not cause outright failures, since this is not happening for opaque store objects that we don't know how to build if we decide not to substitute. + In practice, however, we doubt even the more building is very likely to happen. + Remote stores that are missing dependencies in arbitrary ways (e.g. corruption) don't seem to be very common. + + On the contrary, when remote stores fail to implement the [closure property](@docroot@/store/store-object.md#closure-property), it is usually an *intentional* choice on the part of the remote store, because it wishes to serve as an "overlay" store over another store, such as `https://cache.nixos.org`. + If an "incomplete closure" is encountered in that situation, the right fix is not to do some sort of "franken-building" as this feature implemented, but instead to make sure both substituters are enabled in the settings. + + (In the future, we should make it easier for remote stores to indicate this to clients, to catch settings that won't work in general before a missing dependency is actually encountered.) + + +# Contributors + + +This release was made possible by the following 32 contributors: + +- Robert Hensing [**(@roberth)**](https://github.com/roberth) +- Jörg Thalheim [**(@Mic92)**](https://github.com/Mic92) +- Egor Konovalov [**(@egorkonovalov)**](https://github.com/egorkonovalov) +- PopeRigby [**(@poperigby)**](https://github.com/poperigby) +- Peder Bergebakken Sundt [**(@pbsds)**](https://github.com/pbsds) +- Farid Zakaria [**(@fzakaria)**](https://github.com/fzakaria) +- Gwenn Le Bihan [**(@gwennlbh)**](https://github.com/gwennlbh) +- Jade Masker [**(@donottellmetonottellyou)**](https://github.com/donottellmetonottellyou) +- Nikita Krasnov [**(@synalice)**](https://github.com/synalice) +- tomberek [**(@tomberek)**](https://github.com/tomberek) +- Wolfgang Walther [**(@wolfgangwalther)**](https://github.com/wolfgangwalther) +- Samuli Thomasson [**(@SimSaladin)**](https://github.com/SimSaladin) +- h0nIg [**(@h0nIg)**](https://github.com/h0nIg) +- Valentin Gagarin [**(@fricklerhandwerk)**](https://github.com/fricklerhandwerk) +- Vladimír Čunát [**(@vcunat)**](https://github.com/vcunat) +- Graham Christensen [**(@grahamc)**](https://github.com/grahamc) +- kstrafe [**(@kstrafe)**](https://github.com/kstrafe) +- gustavderdrache [**(@gustavderdrache)**](https://github.com/gustavderdrache) +- Matt Sturgeon [**(@MattSturgeon)**](https://github.com/MattSturgeon) +- John Ericson [**(@Ericson2314)**](https://github.com/Ericson2314) +- Tristan Ross [**(@RossComputerGuy)**](https://github.com/RossComputerGuy) +- jayeshv [**(@jayeshv)**](https://github.com/jayeshv) +- Eelco Dolstra [**(@edolstra)**](https://github.com/edolstra) +- pennae [**(@pennae)**](https://github.com/pennae) +- Luc Perkins [**(@lucperkins)**](https://github.com/lucperkins) +- Cole Helbling [**(@cole-h)**](https://github.com/cole-h) +- Pol Dellaiera [**(@drupol)**](https://github.com/drupol) +- Sergei Zimmerman [**(@xokdvium)**](https://github.com/xokdvium) +- Seth Flynn [**(@getchoo)**](https://github.com/getchoo) +- Jonas Chevalier [**(@zimbatm)**](https://github.com/zimbatm) +- Stefan Boca [**(@stefanboca)**](https://github.com/stefanboca) +- Jeremy Fleischman [**(@jfly)**](https://github.com/jfly) +- Philipp Otterbein +- Raito Bezarius diff --git a/maintainers/data/release-credits-email-to-handle.json b/maintainers/data/release-credits-email-to-handle.json index c77fe4684..48e8685e6 100644 --- a/maintainers/data/release-credits-email-to-handle.json +++ b/maintainers/data/release-credits-email-to-handle.json @@ -166,5 +166,24 @@ "the-tumultuous-unicorn-of-darkness@gmx.com": "TheTumultuousUnicornOfDarkness", "dev@rodney.id.au": "rvl", "pe@pijul.org": "P-E-Meunier", - "yannik@floxdev.com": "ysndr" + "yannik@floxdev.com": "ysndr", + "73017521+egorkonovalov@users.noreply.github.com": "egorkonovalov", + "raito@lix.systems": null, + "nikita.nikita.krasnov@gmail.com": "synalice", + "lucperkins@gmail.com": "lucperkins", + "vladimir.cunat@nic.cz": "vcunat", + "walther@technowledgy.de": "wolfgangwalther", + "jayesh.mail@gmail.com": "jayeshv", + "samuli.thomasson@pm.me": "SimSaladin", + "kevin@stravers.net": "kstrafe", + "poperigby@mailbox.org": "poperigby", + "cole.helbling@determinate.systems": "cole-h", + "donottellmetonottellyou@gmail.com": "donottellmetonottellyou", + "getchoo@tuta.io": "getchoo", + "alex.ford@determinate.systems": "gustavderdrache", + "stefan.r.boca@gmail.com": "stefanboca", + "gwenn.lebihan7@gmail.com": "gwennlbh", + "hey@ewen.works": "gwennlbh", + "matt@sturgeon.me.uk": "MattSturgeon", + "pbsds@hotmail.com": "pbsds" } \ No newline at end of file diff --git a/maintainers/data/release-credits-handle-to-name.json b/maintainers/data/release-credits-handle-to-name.json index 734476078..a6352c44b 100644 --- a/maintainers/data/release-credits-handle-to-name.json +++ b/maintainers/data/release-credits-handle-to-name.json @@ -146,5 +146,21 @@ "ajlekcahdp4": "Alexander Romanov", "Valodim": "Vincent Breitmoser", "rvl": "Rodney Lorrimar", - "whatsthecraic": "Dean De Leo" + "whatsthecraic": "Dean De Leo", + "gwennlbh": "Gwenn Le Bihan", + "donottellmetonottellyou": "Jade Masker", + "kstrafe": null, + "synalice": "Nikita Krasnov", + "poperigby": "PopeRigby", + "MattSturgeon": "Matt Sturgeon", + "lucperkins": "Luc Perkins", + "gustavderdrache": null, + "SimSaladin": "Samuli Thomasson", + "getchoo": "Seth Flynn", + "stefanboca": "Stefan Boca", + "wolfgangwalther": "Wolfgang Walther", + "pbsds": "Peder Bergebakken Sundt", + "egorkonovalov": "Egor Konovalov", + "jayeshv": "jayeshv", + "vcunat": "Vladim\u00edr \u010cun\u00e1t" } \ No newline at end of file From 19c4e78d9726fa64bcb4060ea968eb885f0e95c7 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 7 Jul 2025 14:19:57 +0200 Subject: [PATCH 12/54] Typo --- doc/manual/source/release-notes/rl-2.30.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/doc/manual/source/release-notes/rl-2.30.md b/doc/manual/source/release-notes/rl-2.30.md index b205bad51..3111eddc6 100644 --- a/doc/manual/source/release-notes/rl-2.30.md +++ b/doc/manual/source/release-notes/rl-2.30.md @@ -38,8 +38,8 @@ - Non-flake inputs now contain a `sourceInfo` attribute [#13164](https://github.com/NixOS/nix/issues/13164) [#13170](https://github.com/NixOS/nix/pull/13170) - Flakes have always a `sourceInfo` attribute which describes the source of the flake. - The `sourceInfo.outPath` is often identical to the flake's `outPath`, however it can differ when the flake is located in a subdirectory of its source. + Flakes have always had a `sourceInfo` attribute which describes the source of the flake. + The `sourceInfo.outPath` is often identical to the flake's `outPath`. However, it can differ when the flake is located in a subdirectory of its source. Non-flake inputs (i.e. inputs with `flake = false`) can also be located at some path _within_ a wider source. This usually happens when defining a relative path input within the same source as the parent flake, e.g. `inputs.foo.url = ./some-file.nix`. From f5312492c1280f01914ee2a5bd025eed7b5305d9 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 7 Jul 2025 14:53:11 +0200 Subject: [PATCH 13/54] Add manual release notes --- doc/manual/source/release-notes/rl-2.30.md | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/doc/manual/source/release-notes/rl-2.30.md b/doc/manual/source/release-notes/rl-2.30.md index 3111eddc6..05e1d7fcb 100644 --- a/doc/manual/source/release-notes/rl-2.30.md +++ b/doc/manual/source/release-notes/rl-2.30.md @@ -1,5 +1,25 @@ # Release 2.30.0 (2025-07-07) +- Reduce the size of value from 24 to 16 bytes [#13407](https://github.com/NixOS/nix/pull/13407) + + This shaves off a very significant amount of memory used for evaluation (~20% percent reduction in maximum heap size and ~17% in total bytes). + +- `builtins.sort` uses PeekSort [#12623](https://github.com/NixOS/nix/pull/12623) + + Previously it used libstdc++'s `std::stable_sort()`. However, that implementation is not reliable if the user-supplied comparison function is not a strict weak ordering. + +- `nix repl` prints which variables were loaded [#11406](https://github.com/NixOS/nix/pull/11406) + + Instead of `Added variables` it now prints the first 10 variables that were added to the global scope. + +- `nix flake archive`: add `--no-check-sigs` option [#13277](https://github.com/NixOS/nix/pull/13277) + + This is useful when using `nix flake archive` with the destination set to a remote store. + +- Emit warnings for IFDs with `trace-import-from-derivation` option [#13279](https://github.com/NixOS/nix/pull/13279) + + While we have the setting `allow-import-from-derivation` to deny import-from-derivation (IFD), sometimes users would like to observe IFDs during CI processes to gradually phase out the idiom. The new setting `trace-import-from-derivation`, when set, logs a simple warning to the console. + - `build-dir` no longer defaults to `$TMPDIR` The directory in which temporary build directories are created no longer defaults From 2c0343ec51ef83c2cafda021e68046b35cb67c12 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 7 Jul 2025 15:00:49 +0200 Subject: [PATCH 14/54] # Contributors -> ## Contributors --- doc/manual/source/release-notes/rl-2.24.md | 2 +- doc/manual/source/release-notes/rl-2.25.md | 2 +- doc/manual/source/release-notes/rl-2.26.md | 2 +- doc/manual/source/release-notes/rl-2.27.md | 2 +- doc/manual/source/release-notes/rl-2.28.md | 2 +- doc/manual/source/release-notes/rl-2.29.md | 2 +- maintainers/release-notes | 2 +- 7 files changed, 7 insertions(+), 7 deletions(-) diff --git a/doc/manual/source/release-notes/rl-2.24.md b/doc/manual/source/release-notes/rl-2.24.md index bac0a5998..d4af3cb51 100644 --- a/doc/manual/source/release-notes/rl-2.24.md +++ b/doc/manual/source/release-notes/rl-2.24.md @@ -269,7 +269,7 @@ e.g. `--warn-large-path-threshold 100M`. -# Contributors +## Contributors This release was made possible by the following 43 contributors: diff --git a/doc/manual/source/release-notes/rl-2.25.md b/doc/manual/source/release-notes/rl-2.25.md index 29e3e509c..cfde8b1ef 100644 --- a/doc/manual/source/release-notes/rl-2.25.md +++ b/doc/manual/source/release-notes/rl-2.25.md @@ -77,7 +77,7 @@ `` is also known as the builtin derivation builder `builtin:fetchurl`. It's not to be confused with the evaluation-time function `builtins.fetchurl`, which was not affected by this issue. -# Contributors +## Contributors This release was made possible by the following 58 contributors: diff --git a/doc/manual/source/release-notes/rl-2.26.md b/doc/manual/source/release-notes/rl-2.26.md index d2a890eb6..0c3df828f 100644 --- a/doc/manual/source/release-notes/rl-2.26.md +++ b/doc/manual/source/release-notes/rl-2.26.md @@ -76,7 +76,7 @@ - Evaluation caching now works for dirty Git workdirs [#11992](https://github.com/NixOS/nix/pull/11992) -# Contributors +## Contributors This release was made possible by the following 45 contributors: diff --git a/doc/manual/source/release-notes/rl-2.27.md b/doc/manual/source/release-notes/rl-2.27.md index 3643f7476..34da62525 100644 --- a/doc/manual/source/release-notes/rl-2.27.md +++ b/doc/manual/source/release-notes/rl-2.27.md @@ -47,7 +47,7 @@ blake3-34P4p+iZXcbbyB1i4uoF7eWCGcZHjmaRn6Y7QdynLwU= ``` -# Contributors +## Contributors This release was made possible by the following 21 contributors: diff --git a/doc/manual/source/release-notes/rl-2.28.md b/doc/manual/source/release-notes/rl-2.28.md index 6da09546e..93ea2cfde 100644 --- a/doc/manual/source/release-notes/rl-2.28.md +++ b/doc/manual/source/release-notes/rl-2.28.md @@ -82,7 +82,7 @@ This completes the infrastructure overhaul for the [RFC 132](https://github.com/ Although this change is not as critical, we figured it would be good to do this API change at the same time, also. Also note that we try to keep the C API compatible, but we decided to break this function because it was young and likely not in widespread use yet. This frees up time to make important progress on the rest of the C API. -# Contributors +## Contributors This earlier-than-usual release was made possible by the following 16 contributors: diff --git a/doc/manual/source/release-notes/rl-2.29.md b/doc/manual/source/release-notes/rl-2.29.md index ad63fff2f..b59d6d6f0 100644 --- a/doc/manual/source/release-notes/rl-2.29.md +++ b/doc/manual/source/release-notes/rl-2.29.md @@ -111,7 +111,7 @@ This fact is counterbalanced by the fact that most of those changes are bug fixe This in particular prevents parts of GCC 14's diagnostics from being improperly filtered away. -# Contributors +## Contributors This release was made possible by the following 40 contributors: diff --git a/maintainers/release-notes b/maintainers/release-notes index 6586b22dc..5bb492227 100755 --- a/maintainers/release-notes +++ b/maintainers/release-notes @@ -157,7 +157,7 @@ section_title="Release $version_full ($DATE)" if ! $IS_PATCH; then echo - echo "# Contributors" + echo "## Contributors" echo VERSION=$version_full ./maintainers/release-credits fi From a492493d97d0d2beceb8fc6d20b4ea2d2459d2a5 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 7 Jul 2025 15:01:18 +0200 Subject: [PATCH 15/54] Rearrange release notes --- doc/manual/source/release-notes/rl-2.30.md | 66 ++++++++++++---------- 1 file changed, 37 insertions(+), 29 deletions(-) diff --git a/doc/manual/source/release-notes/rl-2.30.md b/doc/manual/source/release-notes/rl-2.30.md index 05e1d7fcb..c5b741a1f 100644 --- a/doc/manual/source/release-notes/rl-2.30.md +++ b/doc/manual/source/release-notes/rl-2.30.md @@ -1,24 +1,6 @@ # Release 2.30.0 (2025-07-07) -- Reduce the size of value from 24 to 16 bytes [#13407](https://github.com/NixOS/nix/pull/13407) - - This shaves off a very significant amount of memory used for evaluation (~20% percent reduction in maximum heap size and ~17% in total bytes). - -- `builtins.sort` uses PeekSort [#12623](https://github.com/NixOS/nix/pull/12623) - - Previously it used libstdc++'s `std::stable_sort()`. However, that implementation is not reliable if the user-supplied comparison function is not a strict weak ordering. - -- `nix repl` prints which variables were loaded [#11406](https://github.com/NixOS/nix/pull/11406) - - Instead of `Added variables` it now prints the first 10 variables that were added to the global scope. - -- `nix flake archive`: add `--no-check-sigs` option [#13277](https://github.com/NixOS/nix/pull/13277) - - This is useful when using `nix flake archive` with the destination set to a remote store. - -- Emit warnings for IFDs with `trace-import-from-derivation` option [#13279](https://github.com/NixOS/nix/pull/13279) - - While we have the setting `allow-import-from-derivation` to deny import-from-derivation (IFD), sometimes users would like to observe IFDs during CI processes to gradually phase out the idiom. The new setting `trace-import-from-derivation`, when set, logs a simple warning to the console. +## Backward-incompatible changes and deprecations - `build-dir` no longer defaults to `$TMPDIR` @@ -28,7 +10,7 @@ cause build impurities even when not used maliciously. We now default to `builds` in `NIX_STATE_DIR` (which is `/nix/var/nix/builds` in the default configuration). -- Deprecate manually making structured attrs with `__json = ...;` [#13220](https://github.com/NixOS/nix/pull/13220) +- Deprecate manually making structured attrs using the `__json` attribute [#13220](https://github.com/NixOS/nix/pull/13220) The proper way to create a derivation using [structured attrs] in the Nix language is by using `__structuredAttrs = true` with [`builtins.derivation`]. However, by exploiting how structured attrs are implementated, it has also been possible to create them by setting the `__json` environment variable to a serialized JSON string. @@ -37,6 +19,20 @@ [structured attrs]: @docroot@/language/advanced-attributes.md#adv-attr-structuredAttrs [`builtins.derivation`]: @docroot@/language/builtins.html#builtins-derivation +- Rename `nix profile install` to `nix profile add` [#13224](https://github.com/NixOS/nix/pull/13224) + + The command `nix profile install` has been renamed to `nix profile add` (though the former is still available as an alias). This is because the verb "add" is a better antonym for the verb "remove" (i.e. `nix profile remove`). Nix also does not have install hooks or general behavior often associated with "installing". + +## Performance improvements + +This release has a number performance improvements, in particular: + +- Reduce the size of value from 24 to 16 bytes [#13407](https://github.com/NixOS/nix/pull/13407) + + This shaves off a very significant amount of memory used for evaluation (~20% percent reduction in maximum heap size and ~17% in total bytes). + +## Features + - Add stack sampling evaluation profiler [#13220](https://github.com/NixOS/nix/pull/13220) Nix evaluator now supports stack sampling evaluation profiling via `--eval-profiler flamegraph` setting. @@ -48,14 +44,22 @@ Unlike existing `--trace-function-calls` this profiler includes the name of the function being called when it's available. +- `nix repl` prints which variables were loaded [#11406](https://github.com/NixOS/nix/pull/11406) + + Instead of `Added variables` it now prints the first 10 variables that were added to the global scope. + +- `nix flake archive`: Add `--no-check-sigs` option [#13277](https://github.com/NixOS/nix/pull/13277) + + This is useful when using `nix flake archive` with the destination set to a remote store. + +- Emit warnings for IFDs with `trace-import-from-derivation` option [#13279](https://github.com/NixOS/nix/pull/13279) + + While we have the setting `allow-import-from-derivation` to deny import-from-derivation (IFD), sometimes users would like to observe IFDs during CI processes to gradually phase out the idiom. The new setting `trace-import-from-derivation`, when set, logs a simple warning to the console. + - `json-log-path` setting [#13003](https://github.com/NixOS/nix/pull/13003) New setting `json-log-path` that sends a copy of all Nix log messages (in JSON format) to a file or Unix domain socket. -- Rename `nix profile install` to `nix profile add` [#13224](https://github.com/NixOS/nix/pull/13224) - - The command `nix profile install` has been renamed to `nix profile add` (though the former is still available as an alias). This is because the verb "add" is a better antonym for the verb "remove" (i.e. `nix profile remove`). Nix also does not have install hooks or general behavior often associated with "installing". - - Non-flake inputs now contain a `sourceInfo` attribute [#13164](https://github.com/NixOS/nix/issues/13164) [#13170](https://github.com/NixOS/nix/pull/13170) Flakes have always had a `sourceInfo` attribute which describes the source of the flake. @@ -70,13 +74,19 @@ This iterates on the work done in 2.26 to improve relative path support ([#10089](https://github.com/NixOS/nix/pull/10089)), and resolves a regression introduced in 2.28 relating to nested relative path inputs ([#13164](https://github.com/NixOS/nix/issues/13164)). +## Miscellaneous changes + +- `builtins.sort` uses PeekSort [#12623](https://github.com/NixOS/nix/pull/12623) + + Previously it used libstdc++'s `std::stable_sort()`. However, that implementation is not reliable if the user-supplied comparison function is not a strict weak ordering. + - Revert incomplete closure mixed download and build feature [#77](https://github.com/NixOS/nix/issues/77) [#12628](https://github.com/NixOS/nix/issues/12628) [#13176](https://github.com/NixOS/nix/pull/13176) Since Nix 1.3 (299141ecbd08bae17013226dbeae71e842b4fdd7 in 2013) Nix has attempted to mix together upstream fresh builds and downstream substitutions when remote substuters contain an "incomplete closure" (have some store objects, but not the store objects they reference). This feature is now removed. - Worst case, removing this feature could cause more building downstream, but it should not cause outright failures, since this is not happening for opaque store objects that we don't know how to build if we decide not to substitute. - In practice, however, we doubt even the more building is very likely to happen. + In the worst case, removing this feature could cause more building downstream, but it should not cause outright failures, since this is not happening for opaque store objects that we don't know how to build if we decide not to substitute. + In practice, however, we doubt even more building is very likely to happen. Remote stores that are missing dependencies in arbitrary ways (e.g. corruption) don't seem to be very common. On the contrary, when remote stores fail to implement the [closure property](@docroot@/store/store-object.md#closure-property), it is usually an *intentional* choice on the part of the remote store, because it wishes to serve as an "overlay" store over another store, such as `https://cache.nixos.org`. @@ -84,9 +94,7 @@ (In the future, we should make it easier for remote stores to indicate this to clients, to catch settings that won't work in general before a missing dependency is actually encountered.) - -# Contributors - +## Contributors This release was made possible by the following 32 contributors: From 8c71de202fc7b110cf1b7a28d20241fcab96f392 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 7 Jul 2025 15:49:12 +0200 Subject: [PATCH 16/54] Add link --- doc/manual/source/release-notes/rl-2.30.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/doc/manual/source/release-notes/rl-2.30.md b/doc/manual/source/release-notes/rl-2.30.md index c5b741a1f..cb9698846 100644 --- a/doc/manual/source/release-notes/rl-2.30.md +++ b/doc/manual/source/release-notes/rl-2.30.md @@ -35,13 +35,13 @@ This release has a number performance improvements, in particular: - Add stack sampling evaluation profiler [#13220](https://github.com/NixOS/nix/pull/13220) - Nix evaluator now supports stack sampling evaluation profiling via `--eval-profiler flamegraph` setting. - It collects collapsed call stack information to output file specified by + The Nix evaluator now supports [stack sampling evaluation profiling](@docroot@/advanced-topics/eval-profiler.md) via the `--eval-profiler flamegraph` setting. + It outputs collapsed call stack information to the file specified by `--eval-profile-file` (`nix.profile` by default) in a format directly consumable by `flamegraph.pl` and compatible tools like [speedscope](https://speedscope.app/). Sampling frequency can be configured via `--eval-profiler-frequency` (99 Hz by default). - Unlike existing `--trace-function-calls` this profiler includes the name of the function + Unlike the existing `--trace-function-calls`, this profiler includes the name of the function being called when it's available. - `nix repl` prints which variables were loaded [#11406](https://github.com/NixOS/nix/pull/11406) From 9e7655f440bf85c659042c8539ee65ab6f3c84eb Mon Sep 17 00:00:00 2001 From: Thomas Bereknyei Date: Mon, 7 Jul 2025 10:13:40 -0400 Subject: [PATCH 17/54] fix: make setuid tests use new build-dir location /nix/var/nix/builds --- tests/nixos/user-sandboxing/default.nix | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/tests/nixos/user-sandboxing/default.nix b/tests/nixos/user-sandboxing/default.nix index 028efd17f..3f6b575b0 100644 --- a/tests/nixos/user-sandboxing/default.nix +++ b/tests/nixos/user-sandboxing/default.nix @@ -104,15 +104,16 @@ in # Wait for the build to be ready # This is OK because it runs as root, so we can access everything - machine.wait_for_file("/tmp/nix-build-open-build-dir.drv-0/build/syncPoint") + machine.wait_until_succeeds("stat /nix/var/nix/builds/nix-build-open-build-dir.drv-*/build/syncPoint") + dir = machine.succeed("ls -d /nix/var/nix/builds/nix-build-open-build-dir.drv-*").strip() # But Alice shouldn't be able to access the build directory - machine.fail("su alice -c 'ls /tmp/nix-build-open-build-dir.drv-0/build'") - machine.fail("su alice -c 'touch /tmp/nix-build-open-build-dir.drv-0/build/bar'") - machine.fail("su alice -c 'cat /tmp/nix-build-open-build-dir.drv-0/build/foo'") + machine.fail(f"su alice -c 'ls {dir}/build'") + machine.fail(f"su alice -c 'touch {dir}/build/bar'") + machine.fail(f"su alice -c 'cat {dir}/build/foo'") # Tell the user to finish the build - machine.succeed("echo foo > /tmp/nix-build-open-build-dir.drv-0/build/syncPoint") + machine.succeed(f"echo foo > {dir}/build/syncPoint") with subtest("Being able to execute stuff as the build user doesn't give access to the build dir"): machine.succeed(r""" @@ -124,16 +125,17 @@ in args = [ (builtins.storePath "${create-hello-world}") ]; }' >&2 & """.strip()) - machine.wait_for_file("/tmp/nix-build-innocent.drv-0/build/syncPoint") + machine.wait_until_succeeds("stat /nix/var/nix/builds/nix-build-innocent.drv-*/build/syncPoint") + dir = machine.succeed("ls -d /nix/var/nix/builds/nix-build-innocent.drv-*").strip() # The build ran as `nixbld1` (which is the only build user on the # machine), but a process running as `nixbld1` outside the sandbox # shouldn't be able to touch the build directory regardless - machine.fail("su nixbld1 --shell ${pkgs.busybox-sandbox-shell}/bin/sh -c 'ls /tmp/nix-build-innocent.drv-0/build'") - machine.fail("su nixbld1 --shell ${pkgs.busybox-sandbox-shell}/bin/sh -c 'echo pwned > /tmp/nix-build-innocent.drv-0/build/result'") + machine.fail(f"su nixbld1 --shell ${pkgs.busybox-sandbox-shell}/bin/sh -c 'ls {dir}/build'") + machine.fail(f"su nixbld1 --shell ${pkgs.busybox-sandbox-shell}/bin/sh -c 'echo pwned > {dir}/build/result'") # Finish the build - machine.succeed("echo foo > /tmp/nix-build-innocent.drv-0/build/syncPoint") + machine.succeed(f"echo foo > {dir}/build/syncPoint") # Check that the build was not affected machine.succeed(r""" From 58e07c3291074af6fb3716c90a6b7b54bdd3de7c Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Mon, 7 Jul 2025 16:17:06 +0200 Subject: [PATCH 18/54] Sort contributors --- doc/manual/source/release-notes/rl-2.30.md | 58 +++++++++++----------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/doc/manual/source/release-notes/rl-2.30.md b/doc/manual/source/release-notes/rl-2.30.md index cb9698846..9c9e63acb 100644 --- a/doc/manual/source/release-notes/rl-2.30.md +++ b/doc/manual/source/release-notes/rl-2.30.md @@ -98,37 +98,37 @@ This release has a number performance improvements, in particular: This release was made possible by the following 32 contributors: -- Robert Hensing [**(@roberth)**](https://github.com/roberth) -- Jörg Thalheim [**(@Mic92)**](https://github.com/Mic92) -- Egor Konovalov [**(@egorkonovalov)**](https://github.com/egorkonovalov) -- PopeRigby [**(@poperigby)**](https://github.com/poperigby) -- Peder Bergebakken Sundt [**(@pbsds)**](https://github.com/pbsds) -- Farid Zakaria [**(@fzakaria)**](https://github.com/fzakaria) -- Gwenn Le Bihan [**(@gwennlbh)**](https://github.com/gwennlbh) -- Jade Masker [**(@donottellmetonottellyou)**](https://github.com/donottellmetonottellyou) -- Nikita Krasnov [**(@synalice)**](https://github.com/synalice) -- tomberek [**(@tomberek)**](https://github.com/tomberek) -- Wolfgang Walther [**(@wolfgangwalther)**](https://github.com/wolfgangwalther) -- Samuli Thomasson [**(@SimSaladin)**](https://github.com/SimSaladin) -- h0nIg [**(@h0nIg)**](https://github.com/h0nIg) -- Valentin Gagarin [**(@fricklerhandwerk)**](https://github.com/fricklerhandwerk) -- Vladimír Čunát [**(@vcunat)**](https://github.com/vcunat) -- Graham Christensen [**(@grahamc)**](https://github.com/grahamc) -- kstrafe [**(@kstrafe)**](https://github.com/kstrafe) -- gustavderdrache [**(@gustavderdrache)**](https://github.com/gustavderdrache) -- Matt Sturgeon [**(@MattSturgeon)**](https://github.com/MattSturgeon) -- John Ericson [**(@Ericson2314)**](https://github.com/Ericson2314) -- Tristan Ross [**(@RossComputerGuy)**](https://github.com/RossComputerGuy) -- jayeshv [**(@jayeshv)**](https://github.com/jayeshv) -- Eelco Dolstra [**(@edolstra)**](https://github.com/edolstra) -- pennae [**(@pennae)**](https://github.com/pennae) -- Luc Perkins [**(@lucperkins)**](https://github.com/lucperkins) - Cole Helbling [**(@cole-h)**](https://github.com/cole-h) +- Eelco Dolstra [**(@edolstra)**](https://github.com/edolstra) +- Egor Konovalov [**(@egorkonovalov)**](https://github.com/egorkonovalov) +- Farid Zakaria [**(@fzakaria)**](https://github.com/fzakaria) +- Graham Christensen [**(@grahamc)**](https://github.com/grahamc) +- gustavderdrache [**(@gustavderdrache)**](https://github.com/gustavderdrache) +- Gwenn Le Bihan [**(@gwennlbh)**](https://github.com/gwennlbh) +- h0nIg [**(@h0nIg)**](https://github.com/h0nIg) +- Jade Masker [**(@donottellmetonottellyou)**](https://github.com/donottellmetonottellyou) +- jayeshv [**(@jayeshv)**](https://github.com/jayeshv) +- Jeremy Fleischman [**(@jfly)**](https://github.com/jfly) +- John Ericson [**(@Ericson2314)**](https://github.com/Ericson2314) +- Jonas Chevalier [**(@zimbatm)**](https://github.com/zimbatm) +- Jörg Thalheim [**(@Mic92)**](https://github.com/Mic92) +- kstrafe [**(@kstrafe)**](https://github.com/kstrafe) +- Luc Perkins [**(@lucperkins)**](https://github.com/lucperkins) +- Matt Sturgeon [**(@MattSturgeon)**](https://github.com/MattSturgeon) +- Nikita Krasnov [**(@synalice)**](https://github.com/synalice) +- Peder Bergebakken Sundt [**(@pbsds)**](https://github.com/pbsds) +- pennae [**(@pennae)**](https://github.com/pennae) +- Philipp Otterbein - Pol Dellaiera [**(@drupol)**](https://github.com/drupol) +- PopeRigby [**(@poperigby)**](https://github.com/poperigby) +- Raito Bezarius +- Robert Hensing [**(@roberth)**](https://github.com/roberth) +- Samuli Thomasson [**(@SimSaladin)**](https://github.com/SimSaladin) - Sergei Zimmerman [**(@xokdvium)**](https://github.com/xokdvium) - Seth Flynn [**(@getchoo)**](https://github.com/getchoo) -- Jonas Chevalier [**(@zimbatm)**](https://github.com/zimbatm) - Stefan Boca [**(@stefanboca)**](https://github.com/stefanboca) -- Jeremy Fleischman [**(@jfly)**](https://github.com/jfly) -- Philipp Otterbein -- Raito Bezarius +- tomberek [**(@tomberek)**](https://github.com/tomberek) +- Tristan Ross [**(@RossComputerGuy)**](https://github.com/RossComputerGuy) +- Valentin Gagarin [**(@fricklerhandwerk)**](https://github.com/fricklerhandwerk) +- Vladimír Čunát [**(@vcunat)**](https://github.com/vcunat) +- Wolfgang Walther [**(@wolfgangwalther)**](https://github.com/wolfgangwalther) From 9f8df6878faec8d0992d64f7a41f90adf6003896 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Mon, 7 Jul 2025 16:13:53 +0200 Subject: [PATCH 19/54] doc: Add more links Mostly in the 2.30 release notes --- doc/manual/source/release-notes/rl-2.30.md | 53 +++++++++++++------ src/libexpr/include/nix/expr/eval-settings.hh | 2 + 2 files changed, 38 insertions(+), 17 deletions(-) diff --git a/doc/manual/source/release-notes/rl-2.30.md b/doc/manual/source/release-notes/rl-2.30.md index 9c9e63acb..34d3e5bab 100644 --- a/doc/manual/source/release-notes/rl-2.30.md +++ b/doc/manual/source/release-notes/rl-2.30.md @@ -2,7 +2,7 @@ ## Backward-incompatible changes and deprecations -- `build-dir` no longer defaults to `$TMPDIR` +- [`build-dir`] no longer defaults to `$TMPDIR` The directory in which temporary build directories are created no longer defaults to `TMPDIR` or `/tmp`, to avoid builders making their directories @@ -19,9 +19,9 @@ [structured attrs]: @docroot@/language/advanced-attributes.md#adv-attr-structuredAttrs [`builtins.derivation`]: @docroot@/language/builtins.html#builtins-derivation -- Rename `nix profile install` to `nix profile add` [#13224](https://github.com/NixOS/nix/pull/13224) +- Rename `nix profile install` to [`nix profile add`] [#13224](https://github.com/NixOS/nix/pull/13224) - The command `nix profile install` has been renamed to `nix profile add` (though the former is still available as an alias). This is because the verb "add" is a better antonym for the verb "remove" (i.e. `nix profile remove`). Nix also does not have install hooks or general behavior often associated with "installing". + The command `nix profile install` has been renamed to [`nix profile add`] (though the former is still available as an alias). This is because the verb "add" is a better antonym for the verb "remove" (i.e. `nix profile remove`). Nix also does not have install hooks or general behavior often associated with "installing". ## Performance improvements @@ -33,39 +33,39 @@ This release has a number performance improvements, in particular: ## Features -- Add stack sampling evaluation profiler [#13220](https://github.com/NixOS/nix/pull/13220) +- Add [stack sampling evaluation profiler] [#13220](https://github.com/NixOS/nix/pull/13220) - The Nix evaluator now supports [stack sampling evaluation profiling](@docroot@/advanced-topics/eval-profiler.md) via the `--eval-profiler flamegraph` setting. + The Nix evaluator now supports [stack sampling evaluation profiling](@docroot@/advanced-topics/eval-profiler.md) via the [`--eval-profiler flamegraph`] setting. It outputs collapsed call stack information to the file specified by - `--eval-profile-file` (`nix.profile` by default) in a format directly consumable + [`--eval-profile-file`] (`nix.profile` by default) in a format directly consumable by `flamegraph.pl` and compatible tools like [speedscope](https://speedscope.app/). - Sampling frequency can be configured via `--eval-profiler-frequency` (99 Hz by default). + Sampling frequency can be configured via [`--eval-profiler-frequency`] (99 Hz by default). - Unlike the existing `--trace-function-calls`, this profiler includes the name of the function + Unlike the existing [`--trace-function-calls`], this profiler includes the name of the function being called when it's available. -- `nix repl` prints which variables were loaded [#11406](https://github.com/NixOS/nix/pull/11406) +- [`nix repl`] prints which variables were loaded [#11406](https://github.com/NixOS/nix/pull/11406) Instead of `Added variables` it now prints the first 10 variables that were added to the global scope. -- `nix flake archive`: Add `--no-check-sigs` option [#13277](https://github.com/NixOS/nix/pull/13277) +- `nix flake archive`: Add [`--no-check-sigs`] option [#13277](https://github.com/NixOS/nix/pull/13277) - This is useful when using `nix flake archive` with the destination set to a remote store. + This is useful when using [`nix flake archive`] with the destination set to a remote store. -- Emit warnings for IFDs with `trace-import-from-derivation` option [#13279](https://github.com/NixOS/nix/pull/13279) +- Emit warnings for IFDs with [`trace-import-from-derivation`] option [#13279](https://github.com/NixOS/nix/pull/13279) - While we have the setting `allow-import-from-derivation` to deny import-from-derivation (IFD), sometimes users would like to observe IFDs during CI processes to gradually phase out the idiom. The new setting `trace-import-from-derivation`, when set, logs a simple warning to the console. + While we have the setting [`allow-import-from-derivation`] to deny import-from-derivation (IFD), sometimes users would like to observe IFDs during CI processes to gradually phase out the idiom. The new setting `trace-import-from-derivation`, when set, logs a simple warning to the console. - `json-log-path` setting [#13003](https://github.com/NixOS/nix/pull/13003) - New setting `json-log-path` that sends a copy of all Nix log messages (in JSON format) to a file or Unix domain socket. + New setting [`json-log-path`] that sends a copy of all Nix log messages (in JSON format) to a file or Unix domain socket. - Non-flake inputs now contain a `sourceInfo` attribute [#13164](https://github.com/NixOS/nix/issues/13164) [#13170](https://github.com/NixOS/nix/pull/13170) Flakes have always had a `sourceInfo` attribute which describes the source of the flake. The `sourceInfo.outPath` is often identical to the flake's `outPath`. However, it can differ when the flake is located in a subdirectory of its source. - Non-flake inputs (i.e. inputs with `flake = false`) can also be located at some path _within_ a wider source. + Non-flake inputs (i.e. inputs with [`flake = false`]) can also be located at some path _within_ a wider source. This usually happens when defining a relative path input within the same source as the parent flake, e.g. `inputs.foo.url = ./some-file.nix`. Such relative inputs will now inherit their parent's `sourceInfo`. @@ -76,13 +76,13 @@ This release has a number performance improvements, in particular: ## Miscellaneous changes -- `builtins.sort` uses PeekSort [#12623](https://github.com/NixOS/nix/pull/12623) +- [`builtins.sort`] uses PeekSort [#12623](https://github.com/NixOS/nix/pull/12623) Previously it used libstdc++'s `std::stable_sort()`. However, that implementation is not reliable if the user-supplied comparison function is not a strict weak ordering. - Revert incomplete closure mixed download and build feature [#77](https://github.com/NixOS/nix/issues/77) [#12628](https://github.com/NixOS/nix/issues/12628) [#13176](https://github.com/NixOS/nix/pull/13176) - Since Nix 1.3 (299141ecbd08bae17013226dbeae71e842b4fdd7 in 2013) Nix has attempted to mix together upstream fresh builds and downstream substitutions when remote substuters contain an "incomplete closure" (have some store objects, but not the store objects they reference). + Since Nix 1.3 ([commit `299141e`] in 2013) Nix has attempted to mix together upstream fresh builds and downstream substitutions when remote substuters contain an "incomplete closure" (have some store objects, but not the store objects they reference). This feature is now removed. In the worst case, removing this feature could cause more building downstream, but it should not cause outright failures, since this is not happening for opaque store objects that we don't know how to build if we decide not to substitute. @@ -132,3 +132,22 @@ This release was made possible by the following 32 contributors: - Valentin Gagarin [**(@fricklerhandwerk)**](https://github.com/fricklerhandwerk) - Vladimír Čunát [**(@vcunat)**](https://github.com/vcunat) - Wolfgang Walther [**(@wolfgangwalther)**](https://github.com/wolfgangwalther) + + +[stack sampling evaluation profiler]: @docroot@/advanced-topics/eval-profiler.md +[`--eval-profiler`]: @docroot@/command-ref/conf-file.md#conf-eval-profiler +[`--eval-profiler flamegraph`]: @docroot@/command-ref/conf-file.md#conf-eval-profiler +[`--trace-function-calls`]: @docroot@/command-ref/conf-file.md#conf-trace-function-calls +[`--eval-profile-file`]: @docroot@/command-ref/conf-file.md#conf-eval-profile-file +[`--eval-profiler-frequency`]: @docroot@/command-ref/conf-file.md#conf-eval-profiler-frequency +[`build-dir`]: @docroot@/command-ref/conf-file.md#conf-build-dir +[`nix profile add`]: @docroot@/command-ref/new-cli/nix3-profile-add.md +[`nix repl`]: @docroot@/command-ref/new-cli/nix3-repl.md +[`nix flake archive`]: @docroot@/command-ref/new-cli/nix3-flake-archive.md +[`json-log-path`]: @docroot@/command-ref/conf-file.md#conf-json-log-path +[`trace-import-from-derivation`]: @docroot@/command-ref/conf-file.md#conf-trace-import-from-derivation +[`allow-import-from-derivation`]: @docroot@/command-ref/conf-file.md#conf-allow-import-from-derivation +[`builtins.sort`]: @docroot@/language/builtins.md#builtins-sort +[`flake = false`]: @docroot@/command-ref/new-cli/nix3-flake.md?highlight=false#flake-inputs +[`--no-check-sigs`]: @docroot@/command-ref/new-cli/nix3-flake-archive.md#opt-no-check-sigs +[commit `299141e`]: https://github.com/NixOS/nix/commit/299141ecbd08bae17013226dbeae71e842b4fdd7 diff --git a/src/libexpr/include/nix/expr/eval-settings.hh b/src/libexpr/include/nix/expr/eval-settings.hh index f88f13874..eee3b0f0e 100644 --- a/src/libexpr/include/nix/expr/eval-settings.hh +++ b/src/libexpr/include/nix/expr/eval-settings.hh @@ -211,6 +211,8 @@ struct EvalSettings : Config * `flamegraph` stack sampling profiler. Outputs folded format, one line per stack (suitable for `flamegraph.pl` and compatible tools). Use [`eval-profile-file`](#conf-eval-profile-file) to specify where the profile is saved. + + See [Using the `eval-profiler`](@docroot@/advanced-topics/eval-profiler.md). )"}; Setting evalProfileFile{this, "nix.profile", "eval-profile-file", From 87299e466daca97fd48d3d446bb587e4f9d46d9a Mon Sep 17 00:00:00 2001 From: John Soo Date: Mon, 7 Jul 2025 11:14:12 -0600 Subject: [PATCH 20/54] installers, tests: remove --preserve=mode from cp invocations -p preserves xattrs and acls which can be incompatible between filesystems Unfortunately keep -p on darwin because the bsd coreutils do not support --preserve. Fixes #13426 --- scripts/install-multi-user.sh | 9 +++++++-- scripts/install-nix-from-tarball.sh | 6 +++++- tests/nixos/github-flakes.nix | 2 +- tests/nixos/sourcehut-flakes.nix | 2 +- tests/nixos/tarball-flakes.nix | 2 +- 5 files changed, 15 insertions(+), 6 deletions(-) diff --git a/scripts/install-multi-user.sh b/scripts/install-multi-user.sh index f051ccc46..e9ddfc014 100644 --- a/scripts/install-multi-user.sh +++ b/scripts/install-multi-user.sh @@ -834,8 +834,13 @@ install_from_extracted_nix() { ( cd "$EXTRACTED_NIX_PATH" - _sudo "to copy the basic Nix files to the new store at $NIX_ROOT/store" \ - cp -RPp ./store/* "$NIX_ROOT/store/" + if is_os_darwin; then + _sudo "to copy the basic Nix files to the new store at $NIX_ROOT/store" \ + cp -RPp ./store/* "$NIX_ROOT/store/" + else + _sudo "to copy the basic Nix files to the new store at $NIX_ROOT/store" \ + cp -RP --preserve=ownership,timestamps ./store/* "$NIX_ROOT/store/" + fi _sudo "to make the new store non-writable at $NIX_ROOT/store" \ chmod -R ugo-w "$NIX_ROOT/store/" diff --git a/scripts/install-nix-from-tarball.sh b/scripts/install-nix-from-tarball.sh index 8d127a9c5..ec3264793 100644 --- a/scripts/install-nix-from-tarball.sh +++ b/scripts/install-nix-from-tarball.sh @@ -167,7 +167,11 @@ for i in $(cd "$self/store" >/dev/null && echo ./*); do rm -rf "$i_tmp" fi if ! [ -e "$dest/store/$i" ]; then - cp -RPp "$self/store/$i" "$i_tmp" + if [ "$(uname -s)" = "Darwin" ]; then + cp -RPp "$self/store/$i" "$i_tmp" + else + cp -RP --preserve=ownership,timestamps "$self/store/$i" "$i_tmp" + fi chmod -R a-w "$i_tmp" chmod +w "$i_tmp" mv "$i_tmp" "$dest/store/$i" diff --git a/tests/nixos/github-flakes.nix b/tests/nixos/github-flakes.nix index 06142c2ef..91fd6b062 100644 --- a/tests/nixos/github-flakes.nix +++ b/tests/nixos/github-flakes.nix @@ -81,7 +81,7 @@ let mkdir -p $out/archive dir=NixOS-nixpkgs-${nixpkgs.shortRev} - cp -prd ${nixpkgs} $dir + cp -rd --preserve=ownership,timestamps ${nixpkgs} $dir # Set the correct timestamp in the tarball. find $dir -print0 | xargs -0 touch -h -t ${builtins.substring 0 12 nixpkgs.lastModifiedDate}.${ builtins.substring 12 2 nixpkgs.lastModifiedDate diff --git a/tests/nixos/sourcehut-flakes.nix b/tests/nixos/sourcehut-flakes.nix index 61670ccf3..3f05130d6 100644 --- a/tests/nixos/sourcehut-flakes.nix +++ b/tests/nixos/sourcehut-flakes.nix @@ -48,7 +48,7 @@ let nixpkgs-repo = pkgs.runCommand "nixpkgs-flake" { } '' dir=NixOS-nixpkgs-${nixpkgs.shortRev} - cp -prd ${nixpkgs} $dir + cp -rd --preserve=ownership,timestamps ${nixpkgs} $dir # Set the correct timestamp in the tarball. find $dir -print0 | xargs -0 touch -h -t ${builtins.substring 0 12 nixpkgs.lastModifiedDate}.${ diff --git a/tests/nixos/tarball-flakes.nix b/tests/nixos/tarball-flakes.nix index 7b3638b64..26c20cb1a 100644 --- a/tests/nixos/tarball-flakes.nix +++ b/tests/nixos/tarball-flakes.nix @@ -13,7 +13,7 @@ let set -x dir=nixpkgs-${nixpkgs.shortRev} - cp -prd ${nixpkgs} $dir + cp -rd --preserve=ownership,timestamps ${nixpkgs} $dir # Set the correct timestamp in the tarball. find $dir -print0 | xargs -0 touch -h -t ${builtins.substring 0 12 nixpkgs.lastModifiedDate}.${ builtins.substring 12 2 nixpkgs.lastModifiedDate From 723903da3cb4cac844dbfe1d7865080c868a4ad7 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 8 Jul 2025 16:14:50 +0200 Subject: [PATCH 21/54] Bump version --- .version | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.version b/.version index 6a6900382..bafceb320 100644 --- a/.version +++ b/.version @@ -1 +1 @@ -2.30.0 +2.31.0 From 4fa99d743ea365681011c56f1901f6d0c2ce83ab Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 8 Jul 2025 16:15:45 +0200 Subject: [PATCH 22/54] release-process.md: Remove unnecessary step --- maintainers/release-process.md | 4 ---- 1 file changed, 4 deletions(-) diff --git a/maintainers/release-process.md b/maintainers/release-process.md index f2c61302b..37b38fb9f 100644 --- a/maintainers/release-process.md +++ b/maintainers/release-process.md @@ -39,10 +39,6 @@ release: * Proof-read / edit / rearrange the release notes if needed. Breaking changes and highlights should go to the top. -* Run `maintainers/release-credits` to make sure the credits script works - and produces a sensible output. Some emails might not automatically map to - a GitHub handle. - * Push. ```console From 06665e27f48ca04ac877b2b09749fbdc3de0a7eb Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 8 Jul 2025 16:18:53 +0200 Subject: [PATCH 23/54] Update .mergify.yml --- .mergify.yml | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/.mergify.yml b/.mergify.yml index 8941711a9..f49144113 100644 --- a/.mergify.yml +++ b/.mergify.yml @@ -150,3 +150,14 @@ pull_request_rules: labels: - automatic backport - merge-queue + + - name: backport patches to 2.30 + conditions: + - label=backport 2.30-maintenance + actions: + backport: + branches: + - "2.30-maintenance" + labels: + - automatic backport + - merge-queue From a16491375a55ee257ecce84c434750744247abd0 Mon Sep 17 00:00:00 2001 From: Connor Baker Date: Tue, 8 Jul 2025 12:59:16 -0700 Subject: [PATCH 24/54] globals.hh: fix broken link to nspawn example The substitution included the `.` at the end of the URL, breaking it. --- src/libstore/include/nix/store/globals.hh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/include/nix/store/globals.hh b/src/libstore/include/nix/store/globals.hh index 0ac689b55..2e67c7bbf 100644 --- a/src/libstore/include/nix/store/globals.hh +++ b/src/libstore/include/nix/store/globals.hh @@ -873,7 +873,7 @@ public: On Linux, Nix can run builds in a user namespace where they run as root (UID 0) and have 65,536 UIDs available. This is primarily useful for running containers such as `systemd-nspawn` inside a Nix build. For an example, see [`tests/systemd-nspawn/nix`][nspawn]. - [nspawn]: https://github.com/NixOS/nix/blob/67bcb99700a0da1395fa063d7c6586740b304598/tests/systemd-nspawn.nix. + [nspawn]: https://github.com/NixOS/nix/blob/67bcb99700a0da1395fa063d7c6586740b304598/tests/systemd-nspawn.nix Included by default on Linux if the [`auto-allocate-uids`](#conf-auto-allocate-uids) setting is enabled. )", From 8a1f471b6607e4626e2cd8ca1e02401578e0044d Mon Sep 17 00:00:00 2001 From: h0nIg Date: Wed, 9 Jul 2025 09:30:11 +0200 Subject: [PATCH 25/54] docker: fix nixConf --- docker.nix | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docker.nix b/docker.nix index c6e8e478e..2addd0458 100644 --- a/docker.nix +++ b/docker.nix @@ -184,11 +184,11 @@ let } " = "; }; - nixConfContents = toConf { + nixConfContents = toConf ({ sandbox = false; build-users-group = "nixbld"; trusted-public-keys = [ "cache.nixos.org-1:6NCHdD59X431o0gWypbMrAURkbJ16ZPMQFGspcDShjY=" ]; - }; + } // nixConf); userHome = if uid == 0 then "/root" else "/home/${uname}"; From 9857c0bb52cfb62f324ce598214f20cc3521e3a8 Mon Sep 17 00:00:00 2001 From: h0nIg Date: Wed, 9 Jul 2025 09:34:50 +0200 Subject: [PATCH 26/54] docker: fix nixConf - fmt --- docker.nix | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/docker.nix b/docker.nix index 2addd0458..f59492025 100644 --- a/docker.nix +++ b/docker.nix @@ -184,11 +184,14 @@ let } " = "; }; - nixConfContents = toConf ({ - sandbox = false; - build-users-group = "nixbld"; - trusted-public-keys = [ "cache.nixos.org-1:6NCHdD59X431o0gWypbMrAURkbJ16ZPMQFGspcDShjY=" ]; - } // nixConf); + nixConfContents = toConf ( + { + sandbox = false; + build-users-group = "nixbld"; + trusted-public-keys = [ "cache.nixos.org-1:6NCHdD59X431o0gWypbMrAURkbJ16ZPMQFGspcDShjY=" ]; + } + // nixConf + ); userHome = if uid == 0 then "/root" else "/home/${uname}"; From 95437b90fc68bd3fff5a47bd4ac6e5186eb51a00 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 9 Jul 2025 17:00:49 +0200 Subject: [PATCH 27/54] lockFlake(): When updating a lock, respect the input's lock file --- src/libflake/flake.cc | 10 +++----- tests/functional/flakes/flakes.sh | 38 +++++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 7 deletions(-) diff --git a/src/libflake/flake.cc b/src/libflake/flake.cc index 322abaa4a..7a11e6047 100644 --- a/src/libflake/flake.cc +++ b/src/libflake/flake.cc @@ -715,16 +715,12 @@ LockedFlake lockFlake( Finally cleanup([&]() { parents.pop_back(); }); /* Recursively process the inputs of this - flake. Also, unless we already have this flake - in the top-level lock file, use this flake's - own lock file. */ + flake, using its own lock file. */ nodePaths.emplace(childNode, inputFlake.path.parent()); computeLocks( inputFlake.inputs, childNode, inputAttrPath, - oldLock - ? std::dynamic_pointer_cast(oldLock) - : readLockFile(state.fetchSettings, inputFlake.lockFilePath()).root.get_ptr(), - oldLock ? followsPrefix : inputAttrPath, + readLockFile(state.fetchSettings, inputFlake.lockFilePath()).root.get_ptr(), + inputAttrPath, inputFlake.path, false); } diff --git a/tests/functional/flakes/flakes.sh b/tests/functional/flakes/flakes.sh index ce695a6cb..7fd9dc9b5 100755 --- a/tests/functional/flakes/flakes.sh +++ b/tests/functional/flakes/flakes.sh @@ -432,3 +432,41 @@ nix flake metadata "$flake2Dir" --reference-lock-file $TEST_ROOT/flake2-overridd # reference-lock-file can only be used if allow-dirty is set. expectStderr 1 nix flake metadata "$flake2Dir" --no-allow-dirty --reference-lock-file $TEST_ROOT/flake2-overridden.lock + +# After changing an input (flake2 from newFlake2Rev to prevFlake2Rev), we should have the transitive inputs locked by revision $prevFlake2Rev of flake2. +prevFlake1Rev=$(nix flake metadata --json "$flake1Dir" | jq -r .revision) +prevFlake2Rev=$(nix flake metadata --json "$flake2Dir" | jq -r .revision) + +echo "# bla" >> "$flake1Dir/flake.nix" +git -C "$flake1Dir" commit flake.nix -m 'bla' + +nix flake update --flake "$flake2Dir" +git -C "$flake2Dir" commit flake.lock -m 'bla' + +newFlake1Rev=$(nix flake metadata --json "$flake1Dir" | jq -r .revision) +newFlake2Rev=$(nix flake metadata --json "$flake2Dir" | jq -r .revision) + +cat > "$flake3Dir/flake.nix" < "$flake3Dir/flake.nix" < Date: Thu, 10 Jul 2025 11:41:32 +0200 Subject: [PATCH 28/54] fetchClosure: Fix gcc warning MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes: [261/394] Linking target src/libexpr/libnixexpr.so In function ‘copy’, inlined from ‘__ct ’ at /nix/store/24sdvjs6rfqs69d21gdn437mb3vc0svh-gcc-14.2.1.20250322/include/c++/14.2.1.20250322/bits/basic_string.h:688:23, inlined from ‘operator+’ at /nix/store/24sdvjs6rfqs69d21gdn437mb3vc0svh-gcc-14.2.1.20250322/include/c++/14.2.1.20250322/bits/basic_string.h:3735:43, inlined from ‘operator()’ at ../src/libexpr/primops/fetchClosure.cc:127:58, inlined from ‘prim_fetchClosure’ at ../src/libexpr/primops/fetchClosure.cc:132:88: /nix/store/24sdvjs6rfqs69d21gdn437mb3vc0svh-gcc-14.2.1.20250322/include/c++/14.2.1.20250322/bits/char_traits.h:427:56: warning: ‘__builtin_memcpy’ writing 74 bytes into a region of size 16 overflows the destination [-Wstringop-overflow=] 427 | return static_cast(__builtin_memcpy(__s1, __s2, __n)); | ^ ../src/libexpr/primops/fetchClosure.cc: In function ‘prim_fetchClosure’: ../src/libexpr/primops/fetchClosure.cc:132:88: note: at offset 16 into destination object ‘’ of size 32 132 | fromPath = state.coerceToStorePath(attr.pos, *attr.value, context, attrHint()); | ^ --- src/libexpr/primops/fetchClosure.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libexpr/primops/fetchClosure.cc b/src/libexpr/primops/fetchClosure.cc index ea6145f6f..4be4dac8f 100644 --- a/src/libexpr/primops/fetchClosure.cc +++ b/src/libexpr/primops/fetchClosure.cc @@ -124,7 +124,7 @@ static void prim_fetchClosure(EvalState & state, const PosIdx pos, Value * * arg for (auto & attr : *args[0]->attrs()) { const auto & attrName = state.symbols[attr.name]; auto attrHint = [&]() -> std::string { - return "while evaluating the '" + attrName + "' attribute passed to builtins.fetchClosure"; + return fmt("while evaluating the attribute '%s' passed to builtins.fetchClosure", attrName); }; if (attrName == "fromPath") { From 74a144ce9831c65371f7482fc6ae748872df679d Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 10 Jul 2025 11:53:36 +0200 Subject: [PATCH 29/54] Require Boost 1.81.0 or higher Note: this version of Boost was released in December 2022. --- src/libexpr/include/nix/expr/symbol-table.hh | 14 +------------- src/libexpr/meson.build | 1 + src/libutil/serialise.cc | 4 ---- 3 files changed, 2 insertions(+), 17 deletions(-) diff --git a/src/libexpr/include/nix/expr/symbol-table.hh b/src/libexpr/include/nix/expr/symbol-table.hh index 20a05a09d..1249bdb88 100644 --- a/src/libexpr/include/nix/expr/symbol-table.hh +++ b/src/libexpr/include/nix/expr/symbol-table.hh @@ -7,12 +7,7 @@ #include "nix/util/error.hh" #include -#define USE_FLAT_SYMBOL_SET (BOOST_VERSION >= 108100) -#if USE_FLAT_SYMBOL_SET -# include -#else -# include -#endif +#include namespace nix { @@ -214,12 +209,7 @@ private: * Transparent lookup of string view for a pointer to a ChunkedVector entry -> return offset into the store. * ChunkedVector references are never invalidated. */ -#if USE_FLAT_SYMBOL_SET boost::unordered_flat_set symbols{SymbolStr::chunkSize}; -#else - using SymbolValueAlloc = std::pmr::polymorphic_allocator; - boost::unordered_set symbols{SymbolStr::chunkSize, {&buffer}}; -#endif public: @@ -287,5 +277,3 @@ struct std::hash return std::hash{}(s.id); } }; - -#undef USE_FLAT_SYMBOL_SET diff --git a/src/libexpr/meson.build b/src/libexpr/meson.build index f5adafae0..533030359 100644 --- a/src/libexpr/meson.build +++ b/src/libexpr/meson.build @@ -41,6 +41,7 @@ boost = dependency( 'boost', modules : ['container', 'context'], include_type: 'system', + version: '>=1.81.0' ) # boost is a public dependency, but not a pkg-config dependency unfortunately, so we # put in `deps_other`. diff --git a/src/libutil/serialise.cc b/src/libutil/serialise.cc index 55397c6d4..a74531582 100644 --- a/src/libutil/serialise.cc +++ b/src/libutil/serialise.cc @@ -194,10 +194,6 @@ size_t StringSource::read(char * data, size_t len) } -#if BOOST_VERSION >= 106300 && BOOST_VERSION < 106600 -#error Coroutines are broken in this version of Boost! -#endif - std::unique_ptr sourceToSink(std::function fun) { struct SourceToSink : FinishSink From ca9f2028b020447559d509252c31a145f92dfaff Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 10 Jul 2025 12:27:17 +0200 Subject: [PATCH 30/54] Simplify SymbolTable::create() --- src/libexpr/include/nix/expr/symbol-table.hh | 14 +------------- src/libexpr/meson.build | 2 +- 2 files changed, 2 insertions(+), 14 deletions(-) diff --git a/src/libexpr/include/nix/expr/symbol-table.hh b/src/libexpr/include/nix/expr/symbol-table.hh index 1249bdb88..4dedf3d91 100644 --- a/src/libexpr/include/nix/expr/symbol-table.hh +++ b/src/libexpr/include/nix/expr/symbol-table.hh @@ -220,19 +220,7 @@ public: // Most symbols are looked up more than once, so we trade off insertion performance // for lookup performance. // FIXME: make this thread-safe. - return [&](T && key) -> Symbol { - if constexpr (requires { symbols.insert(key); }) { - auto [it, _] = symbols.insert(key); - return Symbol(*it); - } else { - auto it = symbols.find(key); - if (it != symbols.end()) - return Symbol(*it); - - it = symbols.emplace(key).first; - return Symbol(*it); - } - }(SymbolStr::Key{store, s, stringAlloc}); + return Symbol(*symbols.insert(SymbolStr::Key{store, s, stringAlloc}).first); } std::vector resolve(const std::vector & symbols) const diff --git a/src/libexpr/meson.build b/src/libexpr/meson.build index 533030359..43e4b9c98 100644 --- a/src/libexpr/meson.build +++ b/src/libexpr/meson.build @@ -41,7 +41,7 @@ boost = dependency( 'boost', modules : ['container', 'context'], include_type: 'system', - version: '>=1.81.0' + version: '>=1.82.0' ) # boost is a public dependency, but not a pkg-config dependency unfortunately, so we # put in `deps_other`. From fc03b89ff4d6bdd47eb128385fa997a660c03c32 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Thu, 10 Jul 2025 17:50:43 +0200 Subject: [PATCH 31/54] Fix lessThan doc --- src/libexpr/primops.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index f9f834a62..99ca19d7e 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -4122,9 +4122,9 @@ static RegisterPrimOp primop_lessThan({ .name = "__lessThan", .args = {"e1", "e2"}, .doc = R"( - Return `true` if the number *e1* is less than the number *e2*, and - `false` otherwise. Evaluation aborts if either *e1* or *e2* does not - evaluate to a number. + Return `true` if the value *e1* is less than the value *e2*, and `false` otherwise. + Evaluation aborts if either *e1* or *e2* does not evaluate to a number, string or path. + Furthermore, it aborts if *e2* does not match *e1*'s type according to the aforementioned classification of number, string or path. )", .fun = prim_lessThan, }); From a17f377f69c33e5aa553a9098974af7972f2662f Mon Sep 17 00:00:00 2001 From: Elliot Cameron <130508846+de11n@users.noreply.github.com> Date: Thu, 10 Jul 2025 16:19:43 -0400 Subject: [PATCH 32/54] Fix documentation for GC w.r.t. symlinks --- .../source/package-management/garbage-collector-roots.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/doc/manual/source/package-management/garbage-collector-roots.md b/doc/manual/source/package-management/garbage-collector-roots.md index 30c5b7f8d..925a33162 100644 --- a/doc/manual/source/package-management/garbage-collector-roots.md +++ b/doc/manual/source/package-management/garbage-collector-roots.md @@ -12,7 +12,7 @@ $ ln -s /nix/store/d718ef...-foo /nix/var/nix/gcroots/bar That is, after this command, the garbage collector will not remove `/nix/store/d718ef...-foo` or any of its dependencies. -Subdirectories of `prefix/nix/var/nix/gcroots` are also searched for -symlinks. Symlinks to non-store paths are followed and searched for -roots, but symlinks to non-store paths *inside* the paths reached in -that way are not followed to prevent infinite recursion. +Subdirectories of `prefix/nix/var/nix/gcroots` are searched +recursively. Symlinks to store paths count as roots. Symlinks to +non-store paths are ignored, unless the non-store path is itself a +symlink to a store path. \ No newline at end of file From 6e78cc90d3415694ec15bd273b47d21bb1be96ad Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Fri, 11 Jul 2025 20:20:48 +0300 Subject: [PATCH 33/54] libexpr: Fix invalid handling of errors for imported functions c39cc004043b95d55a0c2c2bdba58d6d3e0db846 has added assertions for all Value accesses and the following case has started failing with an `unreachable`: (/tmp/fun.nix): ```nix {a}: a ``` ``` $ nix eval --impure --expr 'import /tmp/fun.nix {a="a";b="b";}' ``` This would crash: ``` terminating due to unexpected unrecoverable internal error: Unexpected condition in getStorage at ../include/nix/expr/value.hh:844 ``` This is not a regression, but rather surfaces an existing problem, which previously was left undiagnosed. In the case of an import `fun` is the `import` primOp, so that read is invalid and previously this resulted in an access into an inactive union member, which is UB. The correct thing to use is `vCur`. Identical problem also affected the case of a missing argument. Add previously failing test cases to the functional/lang test suite. Fixes #13448. --- src/libexpr/eval.cc | 4 ++-- .../lang/eval-fail-missing-arg-import.err.exp | 12 ++++++++++++ .../lang/eval-fail-missing-arg-import.nix | 1 + .../lang/eval-fail-undeclared-arg-import.err.exp | 13 +++++++++++++ .../lang/eval-fail-undeclared-arg-import.nix | 4 ++++ .../lang/non-eval-trivial-lambda-formals.nix | 1 + 6 files changed, 33 insertions(+), 2 deletions(-) create mode 100644 tests/functional/lang/eval-fail-missing-arg-import.err.exp create mode 100644 tests/functional/lang/eval-fail-missing-arg-import.nix create mode 100644 tests/functional/lang/eval-fail-undeclared-arg-import.err.exp create mode 100644 tests/functional/lang/eval-fail-undeclared-arg-import.nix create mode 100644 tests/functional/lang/non-eval-trivial-lambda-formals.nix diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 1321e00a5..47cc35daa 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -1602,7 +1602,7 @@ void EvalState::callFunction(Value & fun, std::span args, Value & vRes, symbols[i.name]) .atPos(lambda.pos) .withTrace(pos, "from call site") - .withFrame(*fun.lambda().env, lambda) + .withFrame(*vCur.lambda().env, lambda) .debugThrow(); } env2.values[displ++] = i.def->maybeThunk(*this, env2); @@ -1629,7 +1629,7 @@ void EvalState::callFunction(Value & fun, std::span args, Value & vRes, .atPos(lambda.pos) .withTrace(pos, "from call site") .withSuggestions(suggestions) - .withFrame(*fun.lambda().env, lambda) + .withFrame(*vCur.lambda().env, lambda) .debugThrow(); } unreachable(); diff --git a/tests/functional/lang/eval-fail-missing-arg-import.err.exp b/tests/functional/lang/eval-fail-missing-arg-import.err.exp new file mode 100644 index 000000000..45774f003 --- /dev/null +++ b/tests/functional/lang/eval-fail-missing-arg-import.err.exp @@ -0,0 +1,12 @@ +error: + … from call site + at /pwd/lang/eval-fail-missing-arg-import.nix:1:1: + 1| import ./non-eval-trivial-lambda-formals.nix { } + | ^ + 2| + + error: function 'anonymous lambda' called without required argument 'a' + at /pwd/lang/non-eval-trivial-lambda-formals.nix:1:1: + 1| { a }: a + | ^ + 2| diff --git a/tests/functional/lang/eval-fail-missing-arg-import.nix b/tests/functional/lang/eval-fail-missing-arg-import.nix new file mode 100644 index 000000000..7cb33f2b5 --- /dev/null +++ b/tests/functional/lang/eval-fail-missing-arg-import.nix @@ -0,0 +1 @@ +import ./non-eval-trivial-lambda-formals.nix { } diff --git a/tests/functional/lang/eval-fail-undeclared-arg-import.err.exp b/tests/functional/lang/eval-fail-undeclared-arg-import.err.exp new file mode 100644 index 000000000..ca797d3ec --- /dev/null +++ b/tests/functional/lang/eval-fail-undeclared-arg-import.err.exp @@ -0,0 +1,13 @@ +error: + … from call site + at /pwd/lang/eval-fail-undeclared-arg-import.nix:1:1: + 1| import ./non-eval-trivial-lambda-formals.nix { + | ^ + 2| a = "a"; + + error: function 'anonymous lambda' called with unexpected argument 'b' + at /pwd/lang/non-eval-trivial-lambda-formals.nix:1:1: + 1| { a }: a + | ^ + 2| + Did you mean a? diff --git a/tests/functional/lang/eval-fail-undeclared-arg-import.nix b/tests/functional/lang/eval-fail-undeclared-arg-import.nix new file mode 100644 index 000000000..e8454c725 --- /dev/null +++ b/tests/functional/lang/eval-fail-undeclared-arg-import.nix @@ -0,0 +1,4 @@ +import ./non-eval-trivial-lambda-formals.nix { + a = "a"; + b = "b"; +} diff --git a/tests/functional/lang/non-eval-trivial-lambda-formals.nix b/tests/functional/lang/non-eval-trivial-lambda-formals.nix new file mode 100644 index 000000000..46a7ea4f4 --- /dev/null +++ b/tests/functional/lang/non-eval-trivial-lambda-formals.nix @@ -0,0 +1 @@ +{ a }: a From e2ef2cfcbc83ea01308ee64c38a58707ab23dec3 Mon Sep 17 00:00:00 2001 From: gustavderdrache Date: Fri, 11 Jul 2025 18:00:26 -0400 Subject: [PATCH 34/54] Address ifdef problem with macOS/BSD sandboxing --- src/libstore/unix/user-lock.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/unix/user-lock.cc b/src/libstore/unix/user-lock.cc index 6a07cb7cc..f5d164e5b 100644 --- a/src/libstore/unix/user-lock.cc +++ b/src/libstore/unix/user-lock.cc @@ -197,7 +197,7 @@ bool useBuildUsers() #ifdef __linux__ static bool b = (settings.buildUsersGroup != "" || settings.autoAllocateUids) && isRootUser(); return b; - #elif defined(__APPLE__) && defined(__FreeBSD__) + #elif defined(__APPLE__) || defined(__FreeBSD__) static bool b = settings.buildUsersGroup != "" && isRootUser(); return b; #else From 8e5814d972642def9842fba3f8a6116f6b9e5c96 Mon Sep 17 00:00:00 2001 From: gustavderdrache Date: Fri, 11 Jul 2025 18:38:51 -0400 Subject: [PATCH 35/54] CI: Roll nix version to 2.29.1 This works around the macOS issue that the prior commit addresses. --- .github/workflows/ci.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 29cb33f56..ac749bc3f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -14,6 +14,8 @@ jobs: with: fetch-depth: 0 - uses: cachix/install-nix-action@v31 + with: + install_url: "https://releases.nixos.org/nix/nix-2.29.1/install" - run: nix --experimental-features 'nix-command flakes' flake show --all-systems --json tests: @@ -36,6 +38,7 @@ jobs: fetch-depth: 0 - uses: cachix/install-nix-action@v31 with: + install_url: "https://releases.nixos.org/nix/nix-2.29.1/install" # The sandbox would otherwise be disabled by default on Darwin extra_nix_config: | sandbox = true From a48632f2e00ccab40c94d498b10591beb76ec6bf Mon Sep 17 00:00:00 2001 From: Justin Bailey Date: Fri, 11 Jul 2025 17:30:03 -0700 Subject: [PATCH 36/54] Better Handling for Expired Credentials When AWS credentials expired, in some scenarios they led to the nix process aborting with an error similar to ' Unable to parse ExceptionName: ExpiredToken'. This change updates the S3 handling code such that those errors are treated like 403s or 404s. Closes #13459 --- src/libstore/s3-binary-cache-store.cc | 21 ++++++++++++++++++--- 1 file changed, 18 insertions(+), 3 deletions(-) diff --git a/src/libstore/s3-binary-cache-store.cc b/src/libstore/s3-binary-cache-store.cc index 618112d1c..b1cba3358 100644 --- a/src/libstore/s3-binary-cache-store.cc +++ b/src/libstore/s3-binary-cache-store.cc @@ -37,10 +37,11 @@ namespace nix { struct S3Error : public Error { Aws::S3::S3Errors err; + Aws::String exceptionName; template - S3Error(Aws::S3::S3Errors err, const Args & ... args) - : Error(args...), err(err) { }; + S3Error(Aws::S3::S3Errors err, Aws::String exceptionName, const Args & ... args) + : Error(args...), err(err), exceptionName(exceptionName) { }; }; /* Helper: given an Outcome, return R in case of success, or @@ -51,6 +52,7 @@ R && checkAws(std::string_view s, Aws::Utils::Outcome && outcome) if (!outcome.IsSuccess()) throw S3Error( outcome.GetError().GetErrorType(), + outcome.GetError().GetExceptionName(), fmt( "%s: %s (request id: %s)", s, @@ -226,7 +228,13 @@ S3Helper::FileTransferResult S3Helper::getObject( } catch (S3Error & e) { if ((e.err != Aws::S3::S3Errors::NO_SUCH_KEY) && - (e.err != Aws::S3::S3Errors::ACCESS_DENIED)) throw; + (e.err != Aws::S3::S3Errors::ACCESS_DENIED) && + // Expired tokens are not really an error, more of a caching problem. Should be treated same as 403. + // + // AWS unwilling to provide a specific error type for the situation (https://github.com/aws/aws-sdk-cpp/issues/1843) + // so use this hack + (e.exceptionName != "ExpiredToken") + ) throw; } auto now2 = std::chrono::steady_clock::now(); @@ -325,15 +333,22 @@ struct S3BinaryCacheStoreImpl : virtual S3BinaryCacheStore { stats.head++; + // error: AWS error fetching 'vjgpmfn7s6vkynymnk8jfx2fcxnsbd6b.narinfo': Unable to parse ExceptionName: ExpiredToken Message: The provided token has expired. auto res = s3Helper.client->HeadObject( Aws::S3::Model::HeadObjectRequest() .WithBucket(config->bucketName) .WithKey(path)); + printError("Checking for file"); + if (!res.IsSuccess()) { auto & error = res.GetError(); if (error.GetErrorType() == Aws::S3::S3Errors::RESOURCE_NOT_FOUND || error.GetErrorType() == Aws::S3::S3Errors::NO_SUCH_KEY + // Expired tokens are not really an error, more of a caching problem. Should be treated same as 403. + // AWS unwilling to provide a specific error type for the situation (https://github.com/aws/aws-sdk-cpp/issues/1843) + // so use this hack + || (error.GetErrorType() == Aws::S3::S3Errors::UNKNOWN && error.GetExceptionName() == "ExpiredToken") // If bucket listing is disabled, 404s turn into 403s || error.GetErrorType() == Aws::S3::S3Errors::ACCESS_DENIED) return false; From 22d6969d669d492d4948967f76432c1c6a74efd5 Mon Sep 17 00:00:00 2001 From: m4dc4p Date: Sat, 12 Jul 2025 08:05:52 -0700 Subject: [PATCH 37/54] Update src/libstore/s3-binary-cache-store.cc Co-authored-by: Eelco Dolstra --- src/libstore/s3-binary-cache-store.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libstore/s3-binary-cache-store.cc b/src/libstore/s3-binary-cache-store.cc index b1cba3358..f3d029c5e 100644 --- a/src/libstore/s3-binary-cache-store.cc +++ b/src/libstore/s3-binary-cache-store.cc @@ -333,7 +333,6 @@ struct S3BinaryCacheStoreImpl : virtual S3BinaryCacheStore { stats.head++; - // error: AWS error fetching 'vjgpmfn7s6vkynymnk8jfx2fcxnsbd6b.narinfo': Unable to parse ExceptionName: ExpiredToken Message: The provided token has expired. auto res = s3Helper.client->HeadObject( Aws::S3::Model::HeadObjectRequest() .WithBucket(config->bucketName) From f786c0b8d1ae75a4ecdd83332ec1f8d6da45b0f3 Mon Sep 17 00:00:00 2001 From: m4dc4p Date: Sat, 12 Jul 2025 08:06:09 -0700 Subject: [PATCH 38/54] Update src/libstore/s3-binary-cache-store.cc Co-authored-by: Eelco Dolstra --- src/libstore/s3-binary-cache-store.cc | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/libstore/s3-binary-cache-store.cc b/src/libstore/s3-binary-cache-store.cc index f3d029c5e..cbb47c063 100644 --- a/src/libstore/s3-binary-cache-store.cc +++ b/src/libstore/s3-binary-cache-store.cc @@ -338,8 +338,6 @@ struct S3BinaryCacheStoreImpl : virtual S3BinaryCacheStore .WithBucket(config->bucketName) .WithKey(path)); - printError("Checking for file"); - if (!res.IsSuccess()) { auto & error = res.GetError(); if (error.GetErrorType() == Aws::S3::S3Errors::RESOURCE_NOT_FOUND From 5cd94436f526976950fef72c4d856347107162dc Mon Sep 17 00:00:00 2001 From: Emily Date: Fri, 27 Jun 2025 14:42:07 +0100 Subject: [PATCH 39/54] libstore: fix Unix sockets in the build directory on sandboxed macOS MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We’re already allowing `/tmp` anyway, so this should be harmless, and it fixes a regression in the default configuration caused by moving the build directories out of `temp-dir`. (For instance, that broke the Lix `guessOrInventPath.sockets` test.) Note that removing `/tmp` breaks quite a few builds, so although it may be a good idea in general it would require work on the Nixpkgs side. Fixes: 749afbbe99fd7b45f828b72628252feba9241362 Change-Id: I6a6a69645f429bc50d4cb24283feda3d3091f534 (This is a cherry-pick of commit d1db3e5fa3faa43b3d2f2e2e843e9cfc1e6e1b71) Lix patch: https://gerrit.lix.systems/c/lix/+/3500 --- src/libstore/unix/build/darwin-derivation-builder.cc | 2 ++ src/libstore/unix/build/sandbox-defaults.sb | 6 ++++-- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/src/libstore/unix/build/darwin-derivation-builder.cc b/src/libstore/unix/build/darwin-derivation-builder.cc index 5e06dbe55..3985498c1 100644 --- a/src/libstore/unix/build/darwin-derivation-builder.cc +++ b/src/libstore/unix/build/darwin-derivation-builder.cc @@ -160,6 +160,8 @@ struct DarwinDerivationBuilder : DerivationBuilderImpl if (getEnv("_NIX_TEST_NO_SANDBOX") != "1") { Strings sandboxArgs; + sandboxArgs.push_back("_NIX_BUILD_TOP"); + sandboxArgs.push_back(tmpDir); sandboxArgs.push_back("_GLOBAL_TMP_DIR"); sandboxArgs.push_back(globalTmpDir); if (drvOptions.allowLocalNetworking) { diff --git a/src/libstore/unix/build/sandbox-defaults.sb b/src/libstore/unix/build/sandbox-defaults.sb index 15cd6daf5..dd6a064c1 100644 --- a/src/libstore/unix/build/sandbox-defaults.sb +++ b/src/libstore/unix/build/sandbox-defaults.sb @@ -29,12 +29,14 @@ R""( ; Allow getpwuid. (allow mach-lookup (global-name "com.apple.system.opendirectoryd.libinfo")) -; Access to /tmp. +; Access to /tmp and the build directory. ; The network-outbound/network-inbound ones are for unix domain sockets, which ; we allow access to in TMPDIR (but if we allow them more broadly, you could in ; theory escape the sandbox) (allow file* process-exec network-outbound network-inbound - (literal "/tmp") (subpath TMPDIR)) + (literal "/tmp") + (subpath TMPDIR) + (subpath (param "_NIX_BUILD_TOP"))) ; Some packages like to read the system version. (allow file-read* From 3e9a100bdf64664bea84aa9eb8ae620945d2651b Mon Sep 17 00:00:00 2001 From: Pol Dellaiera Date: Sun, 13 Jul 2025 22:49:06 +0200 Subject: [PATCH 40/54] docker: set default parameters values --- docker.nix | 44 ++++++++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/docker.nix b/docker.nix index f59492025..410e4a178 100644 --- a/docker.nix +++ b/docker.nix @@ -1,10 +1,10 @@ { # Core dependencies - pkgs, - lib, - dockerTools, - runCommand, - buildPackages, + pkgs ? import { }, + lib ? pkgs.lib, + dockerTools ? pkgs.dockerTools, + runCommand ? pkgs.runCommand, + buildPackages ? pkgs.buildPackages, # Image configuration name ? "nix", tag ? "latest", @@ -28,24 +28,24 @@ }, Cmd ? [ (lib.getExe bashInteractive) ], # Default Packages - nix, - bashInteractive, - coreutils-full, - gnutar, - gzip, - gnugrep, - which, - curl, - less, - wget, - man, - cacert, - findutils, - iana-etc, - gitMinimal, - openssh, + nix ? pkgs.nix, + bashInteractive ? pkgs.bashInteractive, + coreutils-full ? pkgs.coreutils-full, + gnutar ? pkgs.gnutar, + gzip ? pkgs.gzip, + gnugrep ? pkgs.gnugrep, + which ? pkgs.which, + curl ? pkgs.curl, + less ? pkgs.less, + wget ? pkgs.wget, + man ? pkgs.man, + cacert ? pkgs.cacert, + findutils ? pkgs.findutils, + iana-etc ? pkgs.iana-etc, + gitMinimal ? pkgs.gitMinimal, + openssh ? pkgs.openssh, # Other dependencies - shadow, + shadow ? pkgs.shadow, }: let defaultPkgs = [ From 04f6974d2c47ae3cc44733adb707107a675e2c92 Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Sun, 13 Jul 2025 15:21:01 +0300 Subject: [PATCH 41/54] ci: Dogfood Nix from master --- .../actions/install-nix-action/action.yaml | 50 +++++++++++++++++++ .github/workflows/ci.yml | 14 ++++-- 2 files changed, 59 insertions(+), 5 deletions(-) create mode 100644 .github/actions/install-nix-action/action.yaml diff --git a/.github/actions/install-nix-action/action.yaml b/.github/actions/install-nix-action/action.yaml new file mode 100644 index 000000000..28103f589 --- /dev/null +++ b/.github/actions/install-nix-action/action.yaml @@ -0,0 +1,50 @@ +name: "Install Nix" +description: "Helper action for installing Nix with support for dogfooding from master" +inputs: + dogfood: + description: "Whether to use Nix installed from the latest artifact from master branch" + required: true # Be explicit about the fact that we are using unreleased artifacts + extra_nix_config: + description: "Gets appended to `/etc/nix/nix.conf` if passed." + install_url: + description: "URL of the Nix installer" + required: false + default: "https://releases.nixos.org/nix/nix-2.30.1/install" + github_token: + description: "Github token" + required: true +runs: + using: "composite" + steps: + - name: "Download nix install artifact from master" + shell: bash + id: download-nix-installer + if: ${{ inputs.dogfood }} + run: | + RUN_ID=$(gh run list --repo "$DOGFOOD_REPO" --workflow ci.yml --branch master --status success --json databaseId --jq ".[0].databaseId") + + if [ "$RUNNER_OS" == "Linux" ]; then + INSTALLER_ARTIFACT="installer-linux" + elif [ "$RUNNER_OS" == "macOS" ]; then + INSTALLER_ARTIFACT="installer-darwin" + else + echo "::error ::Unsupported RUNNER_OS: $RUNNER_OS" + exit 1 + fi + + INSTALLER_DOWNLOAD_DIR="$GITHUB_WORKSPACE/$INSTALLER_ARTIFACT" + mkdir -p "$INSTALLER_DOWNLOAD_DIR" + + gh run download "$RUN_ID" --repo "$DOGFOOD_REPO" -n "$INSTALLER_ARTIFACT" -D "$INSTALLER_DOWNLOAD_DIR" + echo "installer-path=file://$INSTALLER_DOWNLOAD_DIR" >> "$GITHUB_OUTPUT" + + echo "::notice ::Dogfooding Nix installer from master (https://github.com/$DOGFOOD_REPO/actions/runs/$RUN_ID)" + env: + GH_TOKEN: ${{ inputs.github_token }} + DOGFOOD_REPO: "NixOS/nix" + - uses: cachix/install-nix-action@c134e4c9e34bac6cab09cf239815f9339aaaf84e # v31.5.1 + with: + # Ternary operator in GHA: https://www.github.com/actions/runner/issues/409#issuecomment-752775072 + install_url: ${{ inputs.dogfood && format('{0}/install', steps.download-nix-installer.outputs.installer-path) || inputs.install_url }} + install_options: ${{ inputs.dogfood && format('--tarball-url-prefix {0}', steps.download-nix-installer.outputs.installer-path) || '' }} + extra_nix_config: ${{ inputs.extra_nix_config }} diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ac749bc3f..2531ee020 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -13,10 +13,13 @@ jobs: - uses: actions/checkout@v4 with: fetch-depth: 0 - - uses: cachix/install-nix-action@v31 + - uses: ./.github/actions/install-nix-action with: - install_url: "https://releases.nixos.org/nix/nix-2.29.1/install" - - run: nix --experimental-features 'nix-command flakes' flake show --all-systems --json + dogfood: true + extra_nix_config: + experimental-features = nix-command flakes + github_token: ${{ secrets.GITHUB_TOKEN }} + - run: nix flake show --all-systems --json tests: strategy: @@ -36,9 +39,10 @@ jobs: - uses: actions/checkout@v4 with: fetch-depth: 0 - - uses: cachix/install-nix-action@v31 + - uses: ./.github/actions/install-nix-action with: - install_url: "https://releases.nixos.org/nix/nix-2.29.1/install" + github_token: ${{ secrets.GITHUB_TOKEN }} + dogfood: true # The sandbox would otherwise be disabled by default on Darwin extra_nix_config: | sandbox = true From 3b3c02160dce1110ed9856aa6234fd37fa5c9347 Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Sun, 13 Jul 2025 16:05:05 +0300 Subject: [PATCH 42/54] ci: Dogfood nix from master for `vm_tests` and `flake_regressions` This should provide more coverage for the build from master that is being dogfooded. --- .github/workflows/ci.yml | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 2531ee020..da6f35907 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -182,7 +182,12 @@ jobs: runs-on: ubuntu-24.04 steps: - uses: actions/checkout@v4 - - uses: DeterminateSystems/nix-installer-action@main + - uses: ./.github/actions/install-nix-action + with: + dogfood: true + extra_nix_config: + experimental-features = nix-command flakes + github_token: ${{ secrets.GITHUB_TOKEN }} - uses: DeterminateSystems/magic-nix-cache-action@main - run: | nix build -L \ @@ -208,6 +213,11 @@ jobs: with: repository: NixOS/flake-regressions-data path: flake-regressions/tests - - uses: DeterminateSystems/nix-installer-action@main + - uses: ./.github/actions/install-nix-action + with: + dogfood: true + extra_nix_config: + experimental-features = nix-command flakes + github_token: ${{ secrets.GITHUB_TOKEN }} - uses: DeterminateSystems/magic-nix-cache-action@main - run: nix build -L --out-link ./new-nix && PATH=$(pwd)/new-nix/bin:$PATH MAX_FLAKES=25 flake-regressions/eval-all.sh From 6abc29bba526c426bdaf4477d0740d877ac53294 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 15 Jul 2025 15:17:33 +0200 Subject: [PATCH 43/54] Move boost version check to libutil --- src/libexpr/meson.build | 1 - src/libutil/meson.build | 1 + 2 files changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libexpr/meson.build b/src/libexpr/meson.build index 43e4b9c98..f5adafae0 100644 --- a/src/libexpr/meson.build +++ b/src/libexpr/meson.build @@ -41,7 +41,6 @@ boost = dependency( 'boost', modules : ['container', 'context'], include_type: 'system', - version: '>=1.82.0' ) # boost is a public dependency, but not a pkg-config dependency unfortunately, so we # put in `deps_other`. diff --git a/src/libutil/meson.build b/src/libutil/meson.build index f5ad2b1f6..f48c8f3d7 100644 --- a/src/libutil/meson.build +++ b/src/libutil/meson.build @@ -59,6 +59,7 @@ boost = dependency( 'boost', modules : ['context', 'coroutine', 'iostreams'], include_type: 'system', + version: '>=1.82.0' ) # boost is a public dependency, but not a pkg-config dependency unfortunately, so we # put in `deps_other`. From fde606887430344dd91dbbf907b8cc3bad1f2125 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 15 Jul 2025 18:09:06 +0200 Subject: [PATCH 44/54] Improve rendering of ignored exceptions Instead of error (ignored): error: SQLite database '...' is busy we now get error (ignored): SQLite database '...' is busy --- src/libutil/util.cc | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/libutil/util.cc b/src/libutil/util.cc index c9cc80fef..23dafe8c9 100644 --- a/src/libutil/util.cc +++ b/src/libutil/util.cc @@ -190,8 +190,10 @@ void ignoreExceptionInDestructor(Verbosity lvl) try { try { throw; + } catch (Error & e) { + printMsg(lvl, ANSI_RED "error (ignored):" ANSI_NORMAL " %s", e.info().msg); } catch (std::exception & e) { - printMsg(lvl, "error (ignored): %1%", e.what()); + printMsg(lvl, ANSI_RED "error (ignored):" ANSI_NORMAL " %s", e.what()); } } catch (...) { } } @@ -202,8 +204,10 @@ void ignoreExceptionExceptInterrupt(Verbosity lvl) throw; } catch (const Interrupted & e) { throw; + } catch (Error & e) { + printMsg(lvl, ANSI_RED "error (ignored):" ANSI_NORMAL " %s", e.info().msg); } catch (std::exception & e) { - printMsg(lvl, "error (ignored): %1%", e.what()); + printMsg(lvl, ANSI_RED "error (ignored):" ANSI_NORMAL " %s", e.what()); } } From 7b2f24d68837b178015020f577a4b3abe2d99b92 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 15 Jul 2025 18:10:07 +0200 Subject: [PATCH 45/54] Improve handleSQLiteBusy() message Closes https://github.com/NixOS/nix/pull/10319. --- src/libstore/sqlite.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libstore/sqlite.cc b/src/libstore/sqlite.cc index 55b967ed6..c3fb1f413 100644 --- a/src/libstore/sqlite.cc +++ b/src/libstore/sqlite.cc @@ -250,7 +250,7 @@ void handleSQLiteBusy(const SQLiteBusy & e, time_t & nextWarning) if (now > nextWarning) { nextWarning = now + 10; logWarning({ - .msg = HintFmt(e.what()) + .msg = e.info().msg }); } From 8e8416387c935f2dec5bd24ceac3e3553cae0d59 Mon Sep 17 00:00:00 2001 From: Farid Zakaria Date: Fri, 11 Jul 2025 09:34:06 -0700 Subject: [PATCH 46/54] Add error message when git returns non-0 for fetch Users have complained that fetchGit is flaky however the culprit is likely that `git fetch` was unable itself to download the repository for whatever reason (i.e. poor network etc..) Nothing was checking the status of `git fetch` and the error message that would eventually surface to the users were that the commit was not found. Add explicit error checking for status code from `git fetch` and return a message earlier on to indicate that the failure was from that point. fixes #10431 --- src/libfetchers/git-utils.cc | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/libfetchers/git-utils.cc b/src/libfetchers/git-utils.cc index 9fe271fe8..563c2180d 100644 --- a/src/libfetchers/git-utils.cc +++ b/src/libfetchers/git-utils.cc @@ -545,7 +545,7 @@ struct GitRepoImpl : GitRepo, std::enable_shared_from_this append(gitArgs, {"--depth", "1"}); append(gitArgs, {std::string("--"), url, refspec}); - runProgram(RunOptions { + auto [status, output] = runProgram(RunOptions { .program = "git", .lookupPath = true, // FIXME: git stderr messes up our progress indicator, so @@ -554,6 +554,10 @@ struct GitRepoImpl : GitRepo, std::enable_shared_from_this .input = {}, .isInteractive = true }); + + if (status > 0) { + throw Error("Failed to fetch git repository %s: %s", url, output); + } } void verifyCommit( From fb6f494d35656556465f05db9f4ee8dbdb1d3bf5 Mon Sep 17 00:00:00 2001 From: Farid Zakaria Date: Fri, 11 Jul 2025 09:39:34 -0700 Subject: [PATCH 47/54] merge stderr to stdout so we can emit it --- src/libfetchers/git-utils.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libfetchers/git-utils.cc b/src/libfetchers/git-utils.cc index 563c2180d..d76f6879d 100644 --- a/src/libfetchers/git-utils.cc +++ b/src/libfetchers/git-utils.cc @@ -552,11 +552,12 @@ struct GitRepoImpl : GitRepo, std::enable_shared_from_this // we're using --quiet for now. Should process its stderr. .args = gitArgs, .input = {}, + .mergeStderrToStdout = true, .isInteractive = true }); - + if (status > 0) { - throw Error("Failed to fetch git repository %s: %s", url, output); + throw Error("Failed to fetch git repository %s : %s", url, output); } } From a4f548fed1b6fe7c7f1882c3214175d4147ddfe4 Mon Sep 17 00:00:00 2001 From: Farid Zakaria Date: Fri, 11 Jul 2025 13:28:04 -0700 Subject: [PATCH 48/54] Fix FetchGit test --- src/libfetchers/git.cc | 14 +++++++++++-- tests/functional/fetchGit.sh | 39 +++++++++++++++++++----------------- 2 files changed, 33 insertions(+), 20 deletions(-) diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index cf255c001..88fe2e83d 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -444,7 +444,11 @@ struct GitInputScheme : InputScheme // repo, treat as a remote URI to force a clone. static bool forceHttp = getEnv("_NIX_FORCE_HTTP") == "1"; // for testing auto url = parseURL(getStrAttr(input.attrs, "url")); - bool isBareRepository = url.scheme == "file" && !pathExists(url.path + "/.git"); + + // Why are we checking for bare repository? + // well if it's a bare repository we want to force a git fetch rather than copying the folder + bool isBareRepository = url.scheme == "file" && pathExists(url.path) && + !pathExists(url.path + "/.git"); // // FIXME: here we turn a possibly relative path into an absolute path. // This allows relative git flake inputs to be resolved against the @@ -462,6 +466,12 @@ struct GitInputScheme : InputScheme "See https://github.com/NixOS/nix/issues/12281 for details.", url); } + + // If we don't check here for the path existence, then we can give libgit2 any directory + // and it will initialize them as git directories. + if (!pathExists(url.path)) { + throw Error("The path '%s' does not exist.", url.path); + } repoInfo.location = std::filesystem::absolute(url.path); } else { if (url.scheme == "file") @@ -599,7 +609,7 @@ struct GitInputScheme : InputScheme ? cacheDir / ref : cacheDir / "refs/heads" / ref; - bool doFetch; + bool doFetch = false; time_t now = time(0); /* If a rev was specified, we need to fetch if it's not in the diff --git a/tests/functional/fetchGit.sh b/tests/functional/fetchGit.sh index a41aa35c0..dc5d8f818 100755 --- a/tests/functional/fetchGit.sh +++ b/tests/functional/fetchGit.sh @@ -53,6 +53,27 @@ rm -rf $TEST_HOME/.cache/nix path=$(nix eval --impure --raw --expr "(builtins.fetchGit file://$repo).outPath") [[ $(cat $path/hello) = world ]] +# Fetch again. This should be cached. +# NOTE: This has to be done before the test case below which tries to pack-refs +# the reason being that the lookup on the cache uses the ref-file `/refs/heads/master` +# which does not exist after packing. +mv $repo ${repo}-tmp +path2=$(nix eval --impure --raw --expr "(builtins.fetchGit file://$repo).outPath") +[[ $path = $path2 ]] + +[[ $(nix eval --impure --expr "(builtins.fetchGit file://$repo).revCount") = 2 ]] +[[ $(nix eval --impure --raw --expr "(builtins.fetchGit file://$repo).rev") = $rev2 ]] +[[ $(nix eval --impure --raw --expr "(builtins.fetchGit file://$repo).shortRev") = ${rev2:0:7} ]] + +# Fetching with a explicit hash should succeed. +path2=$(nix eval --refresh --raw --expr "(builtins.fetchGit { url = file://$repo; rev = \"$rev2\"; }).outPath") +[[ $path = $path2 ]] + +path2=$(nix eval --refresh --raw --expr "(builtins.fetchGit { url = file://$repo; rev = \"$rev1\"; }).outPath") +[[ $(cat $path2/hello) = utrecht ]] + +mv ${repo}-tmp $repo + # Fetch when the cache has packed-refs # Regression test of #8822 git -C $TEST_HOME/.cache/nix/gitv3/*/ pack-refs --all @@ -83,24 +104,6 @@ path2=$(nix eval --raw --expr "(builtins.fetchGit { url = file://$repo; rev = \" # But without a hash, it fails. expectStderr 1 nix eval --expr 'builtins.fetchGit "file:///foo"' | grepQuiet "'fetchGit' doesn't fetch unlocked input" -# Fetch again. This should be cached. -mv $repo ${repo}-tmp -path2=$(nix eval --impure --raw --expr "(builtins.fetchGit file://$repo).outPath") -[[ $path = $path2 ]] - -[[ $(nix eval --impure --expr "(builtins.fetchGit file://$repo).revCount") = 2 ]] -[[ $(nix eval --impure --raw --expr "(builtins.fetchGit file://$repo).rev") = $rev2 ]] -[[ $(nix eval --impure --raw --expr "(builtins.fetchGit file://$repo).shortRev") = ${rev2:0:7} ]] - -# Fetching with a explicit hash should succeed. -path2=$(nix eval --refresh --raw --expr "(builtins.fetchGit { url = file://$repo; rev = \"$rev2\"; }).outPath") -[[ $path = $path2 ]] - -path2=$(nix eval --refresh --raw --expr "(builtins.fetchGit { url = file://$repo; rev = \"$rev1\"; }).outPath") -[[ $(cat $path2/hello) = utrecht ]] - -mv ${repo}-tmp $repo - # Using a clean working tree should produce the same result. path2=$(nix eval --impure --raw --expr "(builtins.fetchGit $repo).outPath") [[ $path = $path2 ]] From 196c21c5a0e2f9b3149689ea36789cf0478d893a Mon Sep 17 00:00:00 2001 From: Farid Zakaria Date: Wed, 16 Jul 2025 21:09:59 -0700 Subject: [PATCH 49/54] Add helpful messages when file:// used as tarball When `file://` is used accidentally in a flake as the source it is expected to be a tarball by default. Add some friendlier error messages to either inform the user this is not in fact a tarball or if it's a git directory, let them know they can use `git+file`. fixes #12935 --- src/libfetchers/tarball.cc | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/src/libfetchers/tarball.cc b/src/libfetchers/tarball.cc index b0822cc33..59316eabd 100644 --- a/src/libfetchers/tarball.cc +++ b/src/libfetchers/tarball.cc @@ -111,6 +111,25 @@ static DownloadTarballResult downloadTarball_( const Headers & headers, const std::string & displayPrefix) { + + // Some friendly error messages for common mistakes. + // Namely lets catch when the url is a local file path, but + // it is not in fact a tarball. + if (url.rfind("file://", 0) == 0) { + // Remove "file://" prefix to get the local file path + std::string localPath = url.substr(7); + if (!std::filesystem::exists(localPath)) { + throw Error("tarball '%s' does not exist.", localPath); + } + if (std::filesystem::is_directory(localPath)) { + if (std::filesystem::exists(localPath + "/.git")) { + throw Error( + "tarball '%s' is a git repository, not a tarball. Please use `git+file` as the scheme.", localPath); + } + throw Error("tarball '%s' is a directory, not a file.", localPath); + } + } + Cache::Key cacheKey{"tarball", {{"url", url}}}; auto cached = settings.getCache()->lookupExpired(cacheKey); From 6681933643e4e80617d3fdc1cb2ea2358acc9a19 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Thu, 17 Jul 2025 12:26:50 +0200 Subject: [PATCH 50/54] Fix Windows header inclusions for clang-tidy Move windows-error.hh includes inside _WIN32 guards to prevent clang-tidy errors when analyzing these files on non-Windows platforms. --- src/libutil/windows/windows-async-pipe.cc | 5 +++-- src/libutil/windows/windows-error.cc | 3 +-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/libutil/windows/windows-async-pipe.cc b/src/libutil/windows/windows-async-pipe.cc index d47930a1b..da47c37a8 100644 --- a/src/libutil/windows/windows-async-pipe.cc +++ b/src/libutil/windows/windows-async-pipe.cc @@ -1,7 +1,8 @@ -#include "nix/util/windows-async-pipe.hh" -#include "nix/util/windows-error.hh" + #ifdef _WIN32 +# include "nix/util/windows-async-pipe.hh" +# include "nix/util/windows-error.hh" namespace nix::windows { diff --git a/src/libutil/windows/windows-error.cc b/src/libutil/windows/windows-error.cc index 1e7aff830..0761bdfd5 100644 --- a/src/libutil/windows/windows-error.cc +++ b/src/libutil/windows/windows-error.cc @@ -1,6 +1,5 @@ -#include "nix/util/windows-error.hh" - #ifdef _WIN32 +#include "nix/util/windows-error.hh" #include #define WIN32_LEAN_AND_MEAN #include From f12f96bcbbd5edba64c71e1ea8e45a388490d80f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Thu, 17 Jul 2025 11:17:59 +0200 Subject: [PATCH 51/54] Fix virtual method calls during construction in S3BinaryCacheStoreImpl Move init() call from constructor to openStore() method to avoid calling virtual methods during object construction. This prevents undefined behavior when virtual methods are called before the object is fully constructed. --- src/libstore/s3-binary-cache-store.cc | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/libstore/s3-binary-cache-store.cc b/src/libstore/s3-binary-cache-store.cc index cbb47c063..9bb47a010 100644 --- a/src/libstore/s3-binary-cache-store.cc +++ b/src/libstore/s3-binary-cache-store.cc @@ -289,8 +289,6 @@ struct S3BinaryCacheStoreImpl : virtual S3BinaryCacheStore , s3Helper(config->profile, config->region, config->scheme, config->endpoint) { diskCache = getNarInfoDiskCache(); - - init(); } std::string getUri() override @@ -597,10 +595,12 @@ struct S3BinaryCacheStoreImpl : virtual S3BinaryCacheStore ref S3BinaryCacheStoreImpl::Config::openStore() const { - return make_ref(ref{ + auto store = make_ref(ref{ // FIXME we shouldn't actually need a mutable config std::const_pointer_cast(shared_from_this()) }); + store->init(); + return store; } static RegisterStoreImplementation regS3BinaryCacheStore; From 44963da7872821983a1b28e79a31c9ea305d0830 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=B6rg=20Thalheim?= Date: Thu, 17 Jul 2025 11:15:51 +0200 Subject: [PATCH 52/54] Fix virtual method calls during construction in LocalBinaryCacheStore Move init() call from constructor to openStore() method to avoid calling virtual methods during object construction. This prevents undefined behavior when virtual methods are called before the object is fully constructed. --- src/libstore/local-binary-cache-store.cc | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libstore/local-binary-cache-store.cc b/src/libstore/local-binary-cache-store.cc index 2f23135fa..03a9bd055 100644 --- a/src/libstore/local-binary-cache-store.cc +++ b/src/libstore/local-binary-cache-store.cc @@ -39,7 +39,6 @@ struct LocalBinaryCacheStore : , BinaryCacheStore{*config} , config{config} { - init(); } void init() override; @@ -126,10 +125,12 @@ StringSet LocalBinaryCacheStoreConfig::uriSchemes() } ref LocalBinaryCacheStoreConfig::openStore() const { - return make_ref(ref{ + auto store = make_ref(ref{ // FIXME we shouldn't actually need a mutable config std::const_pointer_cast(shared_from_this()) }); + store->init(); + return store; } static RegisterStoreImplementation regLocalBinaryCacheStore; From d678b071d69569786db4a4cc8110ee0cd4496e2f Mon Sep 17 00:00:00 2001 From: Oleksandr Knyshuk Date: Thu, 17 Jul 2025 17:26:56 +0200 Subject: [PATCH 53/54] Make nix help shell work by handling aliases properly Previously, `nix help shell` failed with "Nix has no subcommand 'shell'" despite `nix shell --help` working correctly. This happened because the `shell` command is actually an alias for `env shell`, and the help system wasn't resolving aliases when looking up documentation. This patch modifies the `showHelp` function to check for and resolve aliases before generating the manpage name, ensuring that shorthand commands like `shell` get proper help documentation. Closes: #13431 --- src/nix/main.cc | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/nix/main.cc b/src/nix/main.cc index 6144f746f..502e04e60 100644 --- a/src/nix/main.cc +++ b/src/nix/main.cc @@ -212,6 +212,14 @@ struct NixArgs : virtual MultiCommand, virtual MixCommonArgs, virtual RootArgs lowdown. */ static void showHelp(std::vector subcommand, NixArgs & toplevel) { + // Check for aliases if subcommand has exactly one element + if (subcommand.size() == 1) { + auto alias = toplevel.aliases.find(subcommand[0]); + if (alias != toplevel.aliases.end()) { + subcommand = alias->second.replacement; + } + } + auto mdName = subcommand.empty() ? "nix" : fmt("nix3-%s", concatStringsSep("-", subcommand)); evalSettings.restrictEval = false; From cfb8a318853d95d5a5346c1ad5a975c0c07fed26 Mon Sep 17 00:00:00 2001 From: Oleksandr Knyshuk Date: Thu, 17 Jul 2025 21:58:15 +0200 Subject: [PATCH 54/54] Require rsync in nix-manual meson.build Closes: #13313 --- doc/manual/meson.build | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/doc/manual/meson.build b/doc/manual/meson.build index 6fe2374a7..0779cd267 100644 --- a/doc/manual/meson.build +++ b/doc/manual/meson.build @@ -8,6 +8,7 @@ nix = find_program('nix', native : true) mdbook = find_program('mdbook', native : true) bash = find_program('bash', native : true) +rsync = find_program('rsync', required: true, native: true) pymod = import('python') python = pymod.find_installation('python3') @@ -84,7 +85,7 @@ manual = custom_target( @0@ @INPUT0@ @CURRENT_SOURCE_DIR@ > @DEPFILE@ @0@ @INPUT1@ summary @2@ < @CURRENT_SOURCE_DIR@/source/SUMMARY.md.in > @2@/source/SUMMARY.md sed -e 's|@version@|@3@|g' < @INPUT2@ > @2@/book.toml - rsync -r --include='*.md' @CURRENT_SOURCE_DIR@/ @2@/ + @4@ -r --include='*.md' @CURRENT_SOURCE_DIR@/ @2@/ (cd @2@; RUST_LOG=warn @1@ build -d @2@ 3>&2 2>&1 1>&3) | { grep -Fv "because fragment resolution isn't implemented" || :; } 3>&2 2>&1 1>&3 rm -rf @2@/manual mv @2@/html @2@/manual @@ -94,6 +95,7 @@ manual = custom_target( mdbook.full_path(), meson.current_build_dir(), meson.project_version(), + rsync.full_path(), ), ], input : [