diff options
Diffstat (limited to 'src')
114 files changed, 852 insertions, 405 deletions
diff --git a/src/build-remote/build-remote.cc b/src/build-remote/build-remote.cc index 2450e80c2..3c7af067b 100644 --- a/src/build-remote/build-remote.cc +++ b/src/build-remote/build-remote.cc @@ -1,11 +1,7 @@ -#include <cstdlib> -#include <cstring> #include <algorithm> #include <set> #include <memory> -#include <string_view> #include <tuple> -#include <iomanip> #if __APPLE__ #include <sys/time.h> #endif @@ -18,6 +14,7 @@ #include "build-result.hh" #include "store-api.hh" #include "derivations.hh" +#include "strings.hh" #include "local-store.hh" #include "legacy.hh" #include "experimental-features.hh" diff --git a/src/libcmd/built-path.hh b/src/libcmd/built-path.hh index da87a33b0..555adb04f 100644 --- a/src/libcmd/built-path.hh +++ b/src/libcmd/built-path.hh @@ -20,13 +20,15 @@ struct SingleBuiltPathBuilt { DECLARE_CMP(SingleBuiltPathBuilt); }; -using _SingleBuiltPathRaw = std::variant< +namespace built_path::detail { +using SingleBuiltPathRaw = std::variant< DerivedPathOpaque, SingleBuiltPathBuilt >; +} -struct SingleBuiltPath : _SingleBuiltPathRaw { - using Raw = _SingleBuiltPathRaw; +struct SingleBuiltPath : built_path::detail::SingleBuiltPathRaw { + using Raw = built_path::detail::SingleBuiltPathRaw; using Raw::Raw; using Opaque = DerivedPathOpaque; @@ -65,17 +67,19 @@ struct BuiltPathBuilt { DECLARE_CMP(BuiltPathBuilt); }; -using _BuiltPathRaw = std::variant< +namespace built_path::detail { +using BuiltPathRaw = std::variant< DerivedPath::Opaque, BuiltPathBuilt >; +} /** * A built path. Similar to a DerivedPath, but enriched with the corresponding * output path(s). */ -struct BuiltPath : _BuiltPathRaw { - using Raw = _BuiltPathRaw; +struct BuiltPath : built_path::detail::BuiltPathRaw { + using Raw = built_path::detail::BuiltPathRaw; using Raw::Raw; using Opaque = DerivedPathOpaque; diff --git a/src/libcmd/editor-for.cc b/src/libcmd/editor-for.cc index 67653d9c9..868153e90 100644 --- a/src/libcmd/editor-for.cc +++ b/src/libcmd/editor-for.cc @@ -1,6 +1,7 @@ #include "editor-for.hh" #include "environment-variables.hh" #include "source-path.hh" +#include "strings.hh" namespace nix { diff --git a/src/libexpr/eval-error.hh b/src/libexpr/eval-error.hh index 19540d612..7114d392f 100644 --- a/src/libexpr/eval-error.hh +++ b/src/libexpr/eval-error.hh @@ -1,9 +1,8 @@ #pragma once ///@file -#include <algorithm> - #include "error.hh" +#include "types.hh" #include "pos-idx.hh" namespace nix { diff --git a/src/libexpr/eval-inline.hh b/src/libexpr/eval-inline.hh index 4631b71eb..30badb93f 100644 --- a/src/libexpr/eval-inline.hh +++ b/src/libexpr/eval-inline.hh @@ -31,7 +31,7 @@ Value * EvalState::allocValue() #endif nrValues++; - return (Value *) p; + return static_cast<Value *>(p); } @@ -54,10 +54,10 @@ Env & EvalState::allocEnv(size_t size) void * p = *env1AllocCache; *env1AllocCache = GC_NEXT(p); GC_NEXT(p) = nullptr; - env = (Env *) p; + env = static_cast<Env *>(p); } else #endif - env = (Env *) gcAllocBytes(sizeof(Env) + size * sizeof(Value *)); + env = static_cast<Env *>(gcAllocBytes(sizeof(Env) + size * sizeof(Value *))); /* We assume that env->values has been cleared by the allocator; maybeThunk() and lookupVar fromWith expect this. */ diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 227cd254a..a87383f7f 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -19,6 +19,7 @@ #include "gc-small-vector.hh" #include "fetch-to-store.hh" #include "flake/flakeref.hh" +#include "exit.hh" #include <algorithm> #include <iostream> diff --git a/src/libexpr/eval.hh b/src/libexpr/eval.hh index eab1f22ef..ba1e4a820 100644 --- a/src/libexpr/eval.hh +++ b/src/libexpr/eval.hh @@ -12,6 +12,7 @@ #include "experimental-features.hh" #include "search-path.hh" #include "repl-exit-status.hh" +#include "backed-string-view.hh" #include <map> #include <optional> @@ -791,4 +792,4 @@ static constexpr std::string_view corepkgsPrefix{"/__corepkgs__/"}; } -#include "eval-inline.hh" +#include "eval-inline.hh" // IWYU pragma: keep diff --git a/src/libexpr/gc-alloc.hh b/src/libexpr/gc-alloc.hh index fc034045f..afdd7eeb0 100644 --- a/src/libexpr/gc-alloc.hh +++ b/src/libexpr/gc-alloc.hh @@ -120,6 +120,7 @@ inline T * gcAllocType(size_t howMany = 1) // However, people can and do request zero sized allocations, so we need // to check that neither of our multiplicands were zero before complaining // about it. + // NOLINTNEXTLINE(bugprone-sizeof-expression): yeah we only seem to alloc pointers with this. the calculation *is* correct though! auto checkedSz = checked::Checked<size_t>(howMany) * sizeof(T); size_t sz = checkedSz.valueWrapping(); if (checkedSz.overflowed()) { diff --git a/src/libexpr/nixexpr.cc b/src/libexpr/nixexpr.cc index 08d4b279b..68da254e2 100644 --- a/src/libexpr/nixexpr.cc +++ b/src/libexpr/nixexpr.cc @@ -11,6 +11,7 @@ namespace nix { ExprBlackHole eBlackHole; +Expr *eBlackHoleAddr = &eBlackHole; // FIXME: remove, because *symbols* are abstract and do not have a single // textual representation; see printIdentifier() diff --git a/src/libexpr/nixexpr.hh b/src/libexpr/nixexpr.hh index 52f254813..d16281c39 100644 --- a/src/libexpr/nixexpr.hh +++ b/src/libexpr/nixexpr.hh @@ -11,6 +11,7 @@ #include "eval-error.hh" #include "pos-idx.hh" #include "pos-table.hh" +#include "strings.hh" namespace nix { diff --git a/src/libexpr/value.hh b/src/libexpr/value.hh index 57485aa0a..e38b11cbf 100644 --- a/src/libexpr/value.hh +++ b/src/libexpr/value.hh @@ -136,7 +136,9 @@ class ExternalValueBase std::ostream & operator << (std::ostream & str, const ExternalValueBase & v); -extern ExprBlackHole eBlackHole; +/** This is just the address of eBlackHole. It exists because eBlackHole has an + * incomplete type at usage sites so is not possible to cast. */ +extern Expr *eBlackHoleAddr; struct NewValueAs { @@ -196,6 +198,7 @@ private: public: // Discount `using NewValueAs::*;` +// NOLINTNEXTLINE(bugprone-macro-parentheses) #define USING_VALUETYPE(name) using name = NewValueAs::name USING_VALUETYPE(integer_t); USING_VALUETYPE(floating_t); @@ -473,7 +476,7 @@ public: /// Constructs an evil thunk, whose evaluation represents infinite recursion. explicit Value(blackhole_t) : internalType(tThunk) - , thunk({ .env = nullptr, .expr = reinterpret_cast<Expr *>(&eBlackHole) }) + , thunk({ .env = nullptr, .expr = eBlackHoleAddr }) { } Value(Value const & rhs) = default; @@ -513,7 +516,10 @@ public: // type() == nThunk inline bool isThunk() const { return internalType == tThunk; }; inline bool isApp() const { return internalType == tApp; }; - inline bool isBlackhole() const; + inline bool isBlackhole() const + { + return internalType == tThunk && thunk.expr == eBlackHoleAddr; + } // type() == nFunction inline bool isLambda() const { return internalType == tLambda; }; @@ -669,11 +675,6 @@ public: void mkStringMove(const char * s, const NixStringContext & context); - inline void mkString(const Symbol & s) - { - mkString(((const std::string &) s).c_str()); - } - void mkPath(const SourcePath & path); inline void mkPath(const char * path) @@ -732,7 +733,11 @@ public: lambda.fun = f; } - inline void mkBlackhole(); + inline void mkBlackhole() + { + internalType = tThunk; + thunk.expr = eBlackHoleAddr; + } void mkPrimOp(PrimOp * p); @@ -832,18 +837,6 @@ public: } }; - -bool Value::isBlackhole() const -{ - return internalType == tThunk && thunk.expr == (Expr*) &eBlackHole; -} - -void Value::mkBlackhole() -{ - internalType = tThunk; - thunk.expr = (Expr*) &eBlackHole; -} - using ValueVector = GcVector<Value *>; using ValueMap = GcMap<Symbol, Value *>; using ValueVectorMap = std::map<Symbol, ValueVector>; diff --git a/src/libfetchers/fetchers.hh b/src/libfetchers/fetchers.hh index 40f2b6294..f778908fb 100644 --- a/src/libfetchers/fetchers.hh +++ b/src/libfetchers/fetchers.hh @@ -7,6 +7,8 @@ #include "path.hh" #include "attrs.hh" #include "url.hh" +#include "ref.hh" +#include "strings.hh" #include <memory> diff --git a/src/libmain/progress-bar.cc b/src/libmain/progress-bar.cc index f0270df04..cdb15d8c7 100644 --- a/src/libmain/progress-bar.cc +++ b/src/libmain/progress-bar.cc @@ -1,8 +1,9 @@ #include "progress-bar.hh" +#include "file-system.hh" #include "sync.hh" -#include "store-api.hh" #include "names.hh" #include "terminal.hh" +#include "strings.hh" #include <map> #include <thread> diff --git a/src/libmain/shared.cc b/src/libmain/shared.cc index 018e34509..bc9548e09 100644 --- a/src/libmain/shared.cc +++ b/src/libmain/shared.cc @@ -6,6 +6,8 @@ #include "loggers.hh" #include "current-process.hh" #include "terminal.hh" +#include "strings.hh" +#include "exit.hh" #include <algorithm> #include <exception> diff --git a/src/libmain/shared.hh b/src/libmain/shared.hh index d7d872319..b41efe567 100644 --- a/src/libmain/shared.hh +++ b/src/libmain/shared.hh @@ -7,12 +7,10 @@ #include "path.hh" #include "derived-path.hh" #include "processes.hh" -#include "exit.hh" +#include "strings.hh" #include <signal.h> -#include <locale> - namespace nix { diff --git a/src/libstore/binary-cache-store.cc b/src/libstore/binary-cache-store.cc index fc0569a66..d4197b3df 100644 --- a/src/libstore/binary-cache-store.cc +++ b/src/libstore/binary-cache-store.cc @@ -10,6 +10,7 @@ #include "nar-accessor.hh" #include "thread-pool.hh" #include "signals.hh" +#include "strings.hh" #include <chrono> #include <regex> diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc index f2c8ccc5f..b59033bae 100644 --- a/src/libstore/build/derivation-goal.cc +++ b/src/libstore/build/derivation-goal.cc @@ -9,6 +9,7 @@ #include "logging-json.hh" #include "substitution-goal.hh" #include "drv-output-substitution-goal.hh" +#include "strings.hh" #include <fstream> #include <sys/types.h> @@ -57,8 +58,8 @@ namespace nix { DerivationGoal::DerivationGoal(const StorePath & drvPath, - const OutputsSpec & wantedOutputs, Worker & worker, BuildMode buildMode) - : Goal(worker) + const OutputsSpec & wantedOutputs, Worker & worker, bool isDependency, BuildMode buildMode) + : Goal(worker, isDependency) , useDerivation(true) , drvPath(drvPath) , wantedOutputs(wantedOutputs) @@ -70,13 +71,13 @@ DerivationGoal::DerivationGoal(const StorePath & drvPath, DerivedPath::Built { makeConstantStorePathRef(drvPath), wantedOutputs }.to_string(worker.store)); trace("created"); - mcExpectedBuilds = std::make_unique<MaintainCount<uint64_t>>(worker.expectedBuilds); + mcExpectedBuilds = worker.expectedBuilds.addTemporarily(1); } DerivationGoal::DerivationGoal(const StorePath & drvPath, const BasicDerivation & drv, - const OutputsSpec & wantedOutputs, Worker & worker, BuildMode buildMode) - : Goal(worker) + const OutputsSpec & wantedOutputs, Worker & worker, bool isDependency, BuildMode buildMode) + : Goal(worker, isDependency) , useDerivation(false) , drvPath(drvPath) , wantedOutputs(wantedOutputs) @@ -90,7 +91,7 @@ DerivationGoal::DerivationGoal(const StorePath & drvPath, const BasicDerivation DerivedPath::Built { makeConstantStorePathRef(drvPath), drv.outputNames() }.to_string(worker.store)); trace("created"); - mcExpectedBuilds = std::make_unique<MaintainCount<uint64_t>>(worker.expectedBuilds); + mcExpectedBuilds = worker.expectedBuilds.addTemporarily(1); /* Prevent the .chroot directory from being garbage-collected. (See isActiveTempFile() in gc.cc.) */ @@ -169,7 +170,7 @@ Goal::WorkResult DerivationGoal::getDerivation(bool inBuildSlot) state = &DerivationGoal::loadDerivation; - return WaitForGoals{{worker.makePathSubstitutionGoal(drvPath)}}; + return WaitForGoals{{worker.goalFactory().makePathSubstitutionGoal(drvPath)}}; } @@ -267,14 +268,14 @@ Goal::WorkResult DerivationGoal::haveDerivation(bool inBuildSlot) if (!status.wanted) continue; if (!status.known) result.goals.insert( - worker.makeDrvOutputSubstitutionGoal( + worker.goalFactory().makeDrvOutputSubstitutionGoal( DrvOutput{status.outputHash, outputName}, buildMode == bmRepair ? Repair : NoRepair ) ); else { auto * cap = getDerivationCA(*drv); - result.goals.insert(worker.makePathSubstitutionGoal( + result.goals.insert(worker.goalFactory().makePathSubstitutionGoal( status.known->path, buildMode == bmRepair ? Repair : NoRepair, cap ? std::optional { *cap } : std::nullopt)); @@ -373,7 +374,7 @@ Goal::WorkResult DerivationGoal::gaveUpOnSubstitution(bool inBuildSlot) addWaiteeDerivedPath = [&](ref<SingleDerivedPath> inputDrv, const DerivedPathMap<StringSet>::ChildNode & inputNode) { if (!inputNode.value.empty()) - result.goals.insert(worker.makeGoal( + result.goals.insert(worker.goalFactory().makeGoal( DerivedPath::Built { .drvPath = inputDrv, .outputs = inputNode.value, @@ -418,7 +419,7 @@ Goal::WorkResult DerivationGoal::gaveUpOnSubstitution(bool inBuildSlot) if (!settings.useSubstitutes) throw Error("dependency '%s' of '%s' does not exist, and substitution is disabled", worker.store.printStorePath(i), worker.store.printStorePath(drvPath)); - result.goals.insert(worker.makePathSubstitutionGoal(i)); + result.goals.insert(worker.goalFactory().makePathSubstitutionGoal(i)); } if (result.goals.empty()) {/* to prevent hang (no wake-up event) */ @@ -474,9 +475,9 @@ Goal::WorkResult DerivationGoal::repairClosure() worker.store.printStorePath(i), worker.store.printStorePath(drvPath)); auto drvPath2 = outputsToDrv.find(i); if (drvPath2 == outputsToDrv.end()) - result.goals.insert(worker.makePathSubstitutionGoal(i, Repair)); + result.goals.insert(worker.goalFactory().makePathSubstitutionGoal(i, Repair)); else - result.goals.insert(worker.makeGoal( + result.goals.insert(worker.goalFactory().makeGoal( DerivedPath::Built { .drvPath = makeConstantStorePathRef(drvPath2->second), .outputs = OutputsSpec::All { }, @@ -579,7 +580,7 @@ Goal::WorkResult DerivationGoal::inputsRealised(bool inBuildSlot) worker.store.printStorePath(pathResolved), }); - resolvedDrvGoal = worker.makeDerivationGoal( + resolvedDrvGoal = worker.goalFactory().makeDerivationGoal( pathResolved, wantedOutputs, buildMode); state = &DerivationGoal::resolvedFinished; @@ -648,7 +649,7 @@ Goal::WorkResult DerivationGoal::inputsRealised(bool inBuildSlot) slot to become available, since we don't need one if there is a build hook. */ state = &DerivationGoal::tryToBuild; - return ContinueImmediately{}; + return tryToBuild(inBuildSlot); } void DerivationGoal::started() @@ -661,7 +662,7 @@ void DerivationGoal::started() if (hook) msg += fmt(" on '%s'", machineName); act = std::make_unique<Activity>(*logger, lvlInfo, actBuild, msg, Logger::Fields{worker.store.printStorePath(drvPath), hook ? machineName : "", 1, 1}); - mcRunningBuilds = std::make_unique<MaintainCount<uint64_t>>(worker.runningBuilds); + mcRunningBuilds = worker.runningBuilds.addTemporarily(1); } Goal::WorkResult DerivationGoal::tryToBuild(bool inBuildSlot) @@ -771,7 +772,7 @@ Goal::WorkResult DerivationGoal::tryToBuild(bool inBuildSlot) actLock.reset(); state = &DerivationGoal::tryLocalBuild; - return ContinueImmediately{}; + return tryLocalBuild(inBuildSlot); } Goal::WorkResult DerivationGoal::tryLocalBuild(bool inBuildSlot) { @@ -1543,8 +1544,13 @@ Goal::Finished DerivationGoal::done( fs << worker.store.printStorePath(drvPath) << "\t" << buildResult.toString() << std::endl; } + if (ex && isDependency) { + logError(ex->info()); + } + return Finished{ - .result = buildResult.success() ? ecSuccess : ecFailed, + .exitCode = buildResult.success() ? ecSuccess : ecFailed, + .result = buildResult, .ex = ex ? std::make_shared<Error>(std::move(*ex)) : nullptr, .permanentFailure = buildResult.status == BuildResult::PermanentFailure, .timedOut = buildResult.status == BuildResult::TimedOut, diff --git a/src/libstore/build/derivation-goal.hh b/src/libstore/build/derivation-goal.hh index c9638c2b6..bf4a3da93 100644 --- a/src/libstore/build/derivation-goal.hh +++ b/src/libstore/build/derivation-goal.hh @@ -1,6 +1,7 @@ #pragma once ///@file +#include "notifying-counter.hh" #include "parsed-derivations.hh" #include "lock.hh" #include "outputs-spec.hh" @@ -217,7 +218,7 @@ struct DerivationGoal : public Goal BuildMode buildMode; - std::unique_ptr<MaintainCount<uint64_t>> mcExpectedBuilds, mcRunningBuilds; + NotifyingCounter<uint64_t>::Bump mcExpectedBuilds, mcRunningBuilds; std::unique_ptr<Activity> act; @@ -234,10 +235,10 @@ struct DerivationGoal : public Goal std::string machineName; DerivationGoal(const StorePath & drvPath, - const OutputsSpec & wantedOutputs, Worker & worker, + const OutputsSpec & wantedOutputs, Worker & worker, bool isDependency, BuildMode buildMode = bmNormal); DerivationGoal(const StorePath & drvPath, const BasicDerivation & drv, - const OutputsSpec & wantedOutputs, Worker & worker, + const OutputsSpec & wantedOutputs, Worker & worker, bool isDependency, BuildMode buildMode = bmNormal); virtual ~DerivationGoal() noexcept(false); diff --git a/src/libstore/build/drv-output-substitution-goal.cc b/src/libstore/build/drv-output-substitution-goal.cc index e6c7524e9..d7a7d257c 100644 --- a/src/libstore/build/drv-output-substitution-goal.cc +++ b/src/libstore/build/drv-output-substitution-goal.cc @@ -1,4 +1,5 @@ #include "drv-output-substitution-goal.hh" +#include "build-result.hh" #include "finally.hh" #include "worker.hh" #include "substitution-goal.hh" @@ -9,9 +10,10 @@ namespace nix { DrvOutputSubstitutionGoal::DrvOutputSubstitutionGoal( const DrvOutput & id, Worker & worker, + bool isDependency, RepairFlag repair, std::optional<ContentAddress> ca) - : Goal(worker) + : Goal(worker, isDependency) , id(id) { state = &DrvOutputSubstitutionGoal::init; @@ -26,7 +28,7 @@ Goal::WorkResult DrvOutputSubstitutionGoal::init(bool inBuildSlot) /* If the derivation already exists, we’re done */ if (worker.store.queryRealisation(id)) { - return Finished{ecSuccess}; + return Finished{ecSuccess, std::move(buildResult)}; } subs = settings.useSubstitutes ? getDefaultSubstituters() : std::list<ref<Store>>(); @@ -41,8 +43,7 @@ Goal::WorkResult DrvOutputSubstitutionGoal::tryNext(bool inBuildSlot) return WaitForSlot{}; } - maintainRunningSubstitutions = - std::make_unique<MaintainCount<uint64_t>>(worker.runningSubstitutions); + maintainRunningSubstitutions = worker.runningSubstitutions.addTemporarily(1); if (subs.size() == 0) { /* None left. Terminate this goal and let someone else deal @@ -56,7 +57,7 @@ Goal::WorkResult DrvOutputSubstitutionGoal::tryNext(bool inBuildSlot) /* Hack: don't indicate failure if there were no substituters. In that case the calling derivation should just do a build. */ - return Finished{substituterFailed ? ecFailed : ecNoSubstituters}; + return Finished{substituterFailed ? ecFailed : ecNoSubstituters, std::move(buildResult)}; } sub = subs.front(); @@ -111,11 +112,11 @@ Goal::WorkResult DrvOutputSubstitutionGoal::realisationFetched(bool inBuildSlot) ); return tryNext(inBuildSlot); } - result.goals.insert(worker.makeDrvOutputSubstitutionGoal(depId)); + result.goals.insert(worker.goalFactory().makeDrvOutputSubstitutionGoal(depId)); } } - result.goals.insert(worker.makePathSubstitutionGoal(outputInfo->outPath)); + result.goals.insert(worker.goalFactory().makePathSubstitutionGoal(outputInfo->outPath)); if (result.goals.empty()) { return outPathValid(inBuildSlot); @@ -133,7 +134,8 @@ Goal::WorkResult DrvOutputSubstitutionGoal::outPathValid(bool inBuildSlot) if (nrFailed > 0) { debug("The output path of the derivation output '%s' could not be substituted", id.to_string()); return Finished{ - nrNoSubstituters > 0 || nrIncompleteClosure > 0 ? ecIncompleteClosure : ecFailed + nrNoSubstituters > 0 || nrIncompleteClosure > 0 ? ecIncompleteClosure : ecFailed, + std::move(buildResult), }; } @@ -144,7 +146,7 @@ Goal::WorkResult DrvOutputSubstitutionGoal::outPathValid(bool inBuildSlot) Goal::WorkResult DrvOutputSubstitutionGoal::finished() { trace("finished"); - return Finished{ecSuccess}; + return Finished{ecSuccess, std::move(buildResult)}; } std::string DrvOutputSubstitutionGoal::key() diff --git a/src/libstore/build/drv-output-substitution-goal.hh b/src/libstore/build/drv-output-substitution-goal.hh index a660a4f3e..8de4d45dd 100644 --- a/src/libstore/build/drv-output-substitution-goal.hh +++ b/src/libstore/build/drv-output-substitution-goal.hh @@ -1,6 +1,7 @@ #pragma once ///@file +#include "notifying-counter.hh" #include "store-api.hh" #include "goal.hh" #include "realisation.hh" @@ -40,7 +41,7 @@ class DrvOutputSubstitutionGoal : public Goal { */ std::shared_ptr<Store> sub; - std::unique_ptr<MaintainCount<uint64_t>> maintainRunningSubstitutions; + NotifyingCounter<uint64_t>::Bump maintainRunningSubstitutions; struct DownloadState { @@ -56,7 +57,13 @@ class DrvOutputSubstitutionGoal : public Goal { bool substituterFailed = false; public: - DrvOutputSubstitutionGoal(const DrvOutput& id, Worker & worker, RepairFlag repair = NoRepair, std::optional<ContentAddress> ca = std::nullopt); + DrvOutputSubstitutionGoal( + const DrvOutput & id, + Worker & worker, + bool isDependency, + RepairFlag repair = NoRepair, + std::optional<ContentAddress> ca = std::nullopt + ); typedef WorkResult (DrvOutputSubstitutionGoal::*GoalState)(bool inBuildSlot); GoalState state; diff --git a/src/libstore/build/entry-points.cc b/src/libstore/build/entry-points.cc index f52f2876f..a5bb05b24 100644 --- a/src/libstore/build/entry-points.cc +++ b/src/libstore/build/entry-points.cc @@ -2,6 +2,7 @@ #include "substitution-goal.hh" #include "derivation-goal.hh" #include "local-store.hh" +#include "strings.hh" namespace nix { @@ -9,11 +10,12 @@ void Store::buildPaths(const std::vector<DerivedPath> & reqs, BuildMode buildMod { Worker worker(*this, evalStore ? *evalStore : *this); - Goals goals; - for (auto & br : reqs) - goals.insert(worker.makeGoal(br, buildMode)); - - worker.run(goals); + auto goals = worker.run([&](GoalFactory & gf) { + Goals goals; + for (auto & br : reqs) + goals.insert(gf.makeGoal(br, buildMode)); + return goals; + }); StringSet failed; std::shared_ptr<Error> ex; @@ -47,17 +49,17 @@ std::vector<KeyedBuildResult> Store::buildPathsWithResults( std::shared_ptr<Store> evalStore) { Worker worker(*this, evalStore ? *evalStore : *this); - - Goals goals; std::vector<std::pair<const DerivedPath &, GoalPtr>> state; - for (const auto & req : reqs) { - auto goal = worker.makeGoal(req, buildMode); - goals.insert(goal); - state.push_back({req, goal}); - } - - worker.run(goals); + auto goals = worker.run([&](GoalFactory & gf) { + Goals goals; + for (const auto & req : reqs) { + auto goal = gf.makeGoal(req, buildMode); + goals.insert(goal); + state.push_back({req, goal}); + } + return goals; + }); std::vector<KeyedBuildResult> results; @@ -71,10 +73,12 @@ BuildResult Store::buildDerivation(const StorePath & drvPath, const BasicDerivat BuildMode buildMode) { Worker worker(*this, *this); - auto goal = worker.makeBasicDerivationGoal(drvPath, drv, OutputsSpec::All {}, buildMode); try { - worker.run(Goals{goal}); + auto goals = worker.run([&](GoalFactory & gf) -> Goals { + return Goals{gf.makeBasicDerivationGoal(drvPath, drv, OutputsSpec::All{}, buildMode)}; + }); + auto goal = *goals.begin(); return goal->buildResult.restrictTo(DerivedPath::Built { .drvPath = makeConstantStorePathRef(drvPath), .outputs = OutputsSpec::All {}, @@ -94,10 +98,10 @@ void Store::ensurePath(const StorePath & path) if (isValidPath(path)) return; Worker worker(*this, *this); - GoalPtr goal = worker.makePathSubstitutionGoal(path); - Goals goals = {goal}; - worker.run(goals); + auto goals = + worker.run([&](GoalFactory & gf) { return Goals{gf.makePathSubstitutionGoal(path)}; }); + auto goal = *goals.begin(); if (goal->exitCode != Goal::ecSuccess) { if (goal->ex) { @@ -112,23 +116,27 @@ void Store::ensurePath(const StorePath & path) void Store::repairPath(const StorePath & path) { Worker worker(*this, *this); - GoalPtr goal = worker.makePathSubstitutionGoal(path, Repair); - Goals goals = {goal}; - worker.run(goals); + auto goals = worker.run([&](GoalFactory & gf) { + return Goals{gf.makePathSubstitutionGoal(path, Repair)}; + }); + auto goal = *goals.begin(); if (goal->exitCode != Goal::ecSuccess) { /* Since substituting the path didn't work, if we have a valid deriver, then rebuild the deriver. */ auto info = queryPathInfo(path); if (info->deriver && isValidPath(*info->deriver)) { - goals.clear(); - goals.insert(worker.makeGoal(DerivedPath::Built { - .drvPath = makeConstantStorePathRef(*info->deriver), - // FIXME: Should just build the specific output we need. - .outputs = OutputsSpec::All { }, - }, bmRepair)); - worker.run(goals); + worker.run([&](GoalFactory & gf) { + return Goals{gf.makeGoal( + DerivedPath::Built{ + .drvPath = makeConstantStorePathRef(*info->deriver), + // FIXME: Should just build the specific output we need. + .outputs = OutputsSpec::All{}, + }, + bmRepair + )}; + }); } else throw Error(worker.failingExitStatus(), "cannot repair path '%s'", printStorePath(path)); } diff --git a/src/libstore/build/goal.hh b/src/libstore/build/goal.hh index 1bfea8231..502ba2a7d 100644 --- a/src/libstore/build/goal.hh +++ b/src/libstore/build/goal.hh @@ -61,6 +61,13 @@ struct Goal Worker & worker; /** + * Whether this goal is only a dependency of other goals. Toplevel + * goals that are also dependencies of other toplevel goals do not + * set this, only goals that are exclusively dependencies do this. + */ + const bool isDependency; + + /** * Goals that this goal is waiting for. */ Goals waitees; @@ -117,7 +124,8 @@ public: bool inBuildSlot; }; struct [[nodiscard]] Finished { - ExitCode result; + ExitCode exitCode; + BuildResult result; std::shared_ptr<Error> ex; bool permanentFailure = false; bool timedOut = false; @@ -143,8 +151,9 @@ public: */ std::shared_ptr<Error> ex; - explicit Goal(Worker & worker) + explicit Goal(Worker & worker, bool isDependency) : worker(worker) + , isDependency(isDependency) { } virtual ~Goal() noexcept(false) diff --git a/src/libstore/build/hook-instance.cc b/src/libstore/build/hook-instance.cc index d5da80c74..f91a904cc 100644 --- a/src/libstore/build/hook-instance.cc +++ b/src/libstore/build/hook-instance.cc @@ -2,6 +2,7 @@ #include "file-system.hh" #include "globals.hh" #include "hook-instance.hh" +#include "strings.hh" namespace nix { diff --git a/src/libstore/build/local-derivation-goal.cc b/src/libstore/build/local-derivation-goal.cc index 1571627d6..7553f1e79 100644 --- a/src/libstore/build/local-derivation-goal.cc +++ b/src/libstore/build/local-derivation-goal.cc @@ -1,15 +1,12 @@ #include "local-derivation-goal.hh" #include "indirect-root-store.hh" -#include "hook-instance.hh" #include "machines.hh" #include "store-api.hh" #include "worker.hh" #include "builtins.hh" #include "builtins/buildenv.hh" #include "path-references.hh" -#include "finally.hh" #include "archive.hh" -#include "compression.hh" #include "daemon.hh" #include "topo-sort.hh" #include "json-utils.hh" @@ -19,6 +16,7 @@ #include "child.hh" #include "unix-domain-socket.hh" #include "mount.hh" +#include "strings.hh" #include <regex> #include <queue> diff --git a/src/libstore/build/local-derivation-goal.hh b/src/libstore/build/local-derivation-goal.hh index f17685af8..37a96b4d1 100644 --- a/src/libstore/build/local-derivation-goal.hh +++ b/src/libstore/build/local-derivation-goal.hh @@ -186,6 +186,7 @@ struct LocalDerivationGoal : public DerivationGoal const StorePath & drvPath, const OutputsSpec & wantedOutputs, Worker & worker, + bool isDependency, BuildMode buildMode ); @@ -198,6 +199,7 @@ struct LocalDerivationGoal : public DerivationGoal const BasicDerivation & drv, const OutputsSpec & wantedOutputs, Worker & worker, + bool isDependency, BuildMode buildMode ); diff --git a/src/libstore/build/substitution-goal.cc b/src/libstore/build/substitution-goal.cc index d84b65a53..33715ea6b 100644 --- a/src/libstore/build/substitution-goal.cc +++ b/src/libstore/build/substitution-goal.cc @@ -6,8 +6,14 @@ namespace nix { -PathSubstitutionGoal::PathSubstitutionGoal(const StorePath & storePath, Worker & worker, RepairFlag repair, std::optional<ContentAddress> ca) - : Goal(worker) +PathSubstitutionGoal::PathSubstitutionGoal( + const StorePath & storePath, + Worker & worker, + bool isDependency, + RepairFlag repair, + std::optional<ContentAddress> ca +) + : Goal(worker, isDependency) , storePath(storePath) , repair(repair) , ca(ca) @@ -15,7 +21,7 @@ PathSubstitutionGoal::PathSubstitutionGoal(const StorePath & storePath, Worker & state = &PathSubstitutionGoal::init; name = fmt("substitution of '%s'", worker.store.printStorePath(this->storePath)); trace("created"); - maintainExpectedSubstitutions = std::make_unique<MaintainCount<uint64_t>>(worker.expectedSubstitutions); + maintainExpectedSubstitutions = worker.expectedSubstitutions.addTemporarily(1); } @@ -35,7 +41,7 @@ Goal::Finished PathSubstitutionGoal::done( debug(*errorMsg); buildResult.errorMsg = *errorMsg; } - return Finished{result}; + return Finished{result, std::move(buildResult)}; } @@ -133,11 +139,11 @@ Goal::WorkResult PathSubstitutionGoal::tryNext(bool inBuildSlot) /* Update the total expected download size. */ auto narInfo = std::dynamic_pointer_cast<const NarInfo>(info); - maintainExpectedNar = std::make_unique<MaintainCount<uint64_t>>(worker.expectedNarSize, info->narSize); + maintainExpectedNar = worker.expectedNarSize.addTemporarily(info->narSize); maintainExpectedDownload = narInfo && narInfo->fileSize - ? std::make_unique<MaintainCount<uint64_t>>(worker.expectedDownloadSize, narInfo->fileSize) + ? worker.expectedDownloadSize.addTemporarily(narInfo->fileSize) : nullptr; /* Bail out early if this substituter lacks a valid @@ -155,7 +161,7 @@ Goal::WorkResult PathSubstitutionGoal::tryNext(bool inBuildSlot) WaitForGoals result; for (auto & i : info->references) if (i != storePath) /* ignore self-references */ - result.goals.insert(worker.makePathSubstitutionGoal(i)); + result.goals.insert(worker.goalFactory().makePathSubstitutionGoal(i)); if (result.goals.empty()) {/* to prevent hang (no wake-up event) */ return referencesValid(inBuildSlot); @@ -182,7 +188,7 @@ Goal::WorkResult PathSubstitutionGoal::referencesValid(bool inBuildSlot) assert(worker.store.isValidPath(i)); state = &PathSubstitutionGoal::tryToRun; - return ContinueImmediately{}; + return tryToRun(inBuildSlot); } @@ -194,7 +200,7 @@ Goal::WorkResult PathSubstitutionGoal::tryToRun(bool inBuildSlot) return WaitForSlot{}; } - maintainRunningSubstitutions = std::make_unique<MaintainCount<uint64_t>>(worker.runningSubstitutions); + maintainRunningSubstitutions = worker.runningSubstitutions.addTemporarily(1); outPipe.create(); @@ -250,7 +256,7 @@ Goal::WorkResult PathSubstitutionGoal::finished(bool inBuildSlot) /* Try the next substitute. */ state = &PathSubstitutionGoal::tryNext; - return ContinueImmediately{}; + return tryNext(inBuildSlot); } worker.markContentsGood(storePath); @@ -262,13 +268,10 @@ Goal::WorkResult PathSubstitutionGoal::finished(bool inBuildSlot) maintainExpectedSubstitutions.reset(); worker.doneSubstitutions++; - if (maintainExpectedDownload) { - auto fileSize = maintainExpectedDownload->delta; - maintainExpectedDownload.reset(); - worker.doneDownloadSize += fileSize; - } + worker.doneDownloadSize += maintainExpectedDownload.delta(); + maintainExpectedDownload.reset(); - worker.doneNarSize += maintainExpectedNar->delta; + worker.doneNarSize += maintainExpectedNar.delta(); maintainExpectedNar.reset(); return done(ecSuccess, BuildResult::Substituted); diff --git a/src/libstore/build/substitution-goal.hh b/src/libstore/build/substitution-goal.hh index e546fc06f..9c7e6f470 100644 --- a/src/libstore/build/substitution-goal.hh +++ b/src/libstore/build/substitution-goal.hh @@ -2,6 +2,7 @@ ///@file #include "lock.hh" +#include "notifying-counter.hh" #include "store-api.hh" #include "goal.hh" @@ -63,7 +64,7 @@ struct PathSubstitutionGoal : public Goal */ Path destPath; - std::unique_ptr<MaintainCount<uint64_t>> maintainExpectedSubstitutions, + NotifyingCounter<uint64_t>::Bump maintainExpectedSubstitutions, maintainRunningSubstitutions, maintainExpectedNar, maintainExpectedDownload; typedef WorkResult (PathSubstitutionGoal::*GoalState)(bool inBuildSlot); @@ -80,7 +81,13 @@ struct PathSubstitutionGoal : public Goal std::optional<std::string> errorMsg = {}); public: - PathSubstitutionGoal(const StorePath & storePath, Worker & worker, RepairFlag repair = NoRepair, std::optional<ContentAddress> ca = std::nullopt); + PathSubstitutionGoal( + const StorePath & storePath, + Worker & worker, + bool isDependency, + RepairFlag repair = NoRepair, + std::optional<ContentAddress> ca = std::nullopt + ); ~PathSubstitutionGoal(); Finished timedOut(Error && ex) override { abort(); }; diff --git a/src/libstore/build/worker.cc b/src/libstore/build/worker.cc index 1b4633e64..f619d574d 100644 --- a/src/libstore/build/worker.cc +++ b/src/libstore/build/worker.cc @@ -1,5 +1,6 @@ #include "charptr-cast.hh" #include "worker.hh" +#include "finally.hh" #include "substitution-goal.hh" #include "drv-output-substitution-goal.hh" #include "local-derivation-goal.hh" @@ -59,22 +60,38 @@ std::shared_ptr<DerivationGoal> Worker::makeDerivationGoalCommon( std::shared_ptr<DerivationGoal> Worker::makeDerivationGoal(const StorePath & drvPath, const OutputsSpec & wantedOutputs, BuildMode buildMode) { - return makeDerivationGoalCommon(drvPath, wantedOutputs, [&]() -> std::shared_ptr<DerivationGoal> { - return !dynamic_cast<LocalStore *>(&store) - ? std::make_shared<DerivationGoal>(drvPath, wantedOutputs, *this, buildMode) - : LocalDerivationGoal::makeLocalDerivationGoal(drvPath, wantedOutputs, *this, buildMode); - }); + return makeDerivationGoalCommon( + drvPath, + wantedOutputs, + [&]() -> std::shared_ptr<DerivationGoal> { + return !dynamic_cast<LocalStore *>(&store) + ? std::make_shared<DerivationGoal>( + drvPath, wantedOutputs, *this, running, buildMode + ) + : LocalDerivationGoal::makeLocalDerivationGoal( + drvPath, wantedOutputs, *this, running, buildMode + ); + } + ); } std::shared_ptr<DerivationGoal> Worker::makeBasicDerivationGoal(const StorePath & drvPath, const BasicDerivation & drv, const OutputsSpec & wantedOutputs, BuildMode buildMode) { - return makeDerivationGoalCommon(drvPath, wantedOutputs, [&]() -> std::shared_ptr<DerivationGoal> { - return !dynamic_cast<LocalStore *>(&store) - ? std::make_shared<DerivationGoal>(drvPath, drv, wantedOutputs, *this, buildMode) - : LocalDerivationGoal::makeLocalDerivationGoal(drvPath, drv, wantedOutputs, *this, buildMode); - }); + return makeDerivationGoalCommon( + drvPath, + wantedOutputs, + [&]() -> std::shared_ptr<DerivationGoal> { + return !dynamic_cast<LocalStore *>(&store) + ? std::make_shared<DerivationGoal>( + drvPath, drv, wantedOutputs, *this, running, buildMode + ) + : LocalDerivationGoal::makeLocalDerivationGoal( + drvPath, drv, wantedOutputs, *this, running, buildMode + ); + } + ); } @@ -83,7 +100,7 @@ std::shared_ptr<PathSubstitutionGoal> Worker::makePathSubstitutionGoal(const Sto std::weak_ptr<PathSubstitutionGoal> & goal_weak = substitutionGoals[path]; auto goal = goal_weak.lock(); // FIXME if (!goal) { - goal = std::make_shared<PathSubstitutionGoal>(path, *this, repair, ca); + goal = std::make_shared<PathSubstitutionGoal>(path, *this, running, repair, ca); goal_weak = goal; wakeUp(goal); } @@ -96,7 +113,7 @@ std::shared_ptr<DrvOutputSubstitutionGoal> Worker::makeDrvOutputSubstitutionGoal std::weak_ptr<DrvOutputSubstitutionGoal> & goal_weak = drvOutputSubstitutionGoals[id]; auto goal = goal_weak.lock(); // FIXME if (!goal) { - goal = std::make_shared<DrvOutputSubstitutionGoal>(id, *this, repair, ca); + goal = std::make_shared<DrvOutputSubstitutionGoal>(id, *this, running, repair, ca); goal_weak = goal; wakeUp(goal); } @@ -139,20 +156,14 @@ void Worker::goalFinished(GoalPtr goal, Goal::Finished & f) { goal->trace("done"); assert(!goal->exitCode.has_value()); - goal->exitCode = f.result; + goal->exitCode = f.exitCode; + goal->ex = f.ex; permanentFailure |= f.permanentFailure; timedOut |= f.timedOut; hashMismatch |= f.hashMismatch; checkMismatch |= f.checkMismatch; - if (f.ex) { - if (!goal->waiters.empty()) - logError(f.ex->info()); - else - goal->ex = f.ex; - } - for (auto & i : goal->waiters) { if (GoalPtr waiting = i.lock()) { assert(waiting->waitees.count(goal)); @@ -160,11 +171,11 @@ void Worker::goalFinished(GoalPtr goal, Goal::Finished & f) waiting->trace(fmt("waitee '%s' done; %d left", goal->name, waiting->waitees.size())); - if (f.result != Goal::ecSuccess) ++waiting->nrFailed; - if (f.result == Goal::ecNoSubstituters) ++waiting->nrNoSubstituters; - if (f.result == Goal::ecIncompleteClosure) ++waiting->nrIncompleteClosure; + if (f.exitCode != Goal::ecSuccess) ++waiting->nrFailed; + if (f.exitCode == Goal::ecNoSubstituters) ++waiting->nrNoSubstituters; + if (f.exitCode == Goal::ecIncompleteClosure) ++waiting->nrIncompleteClosure; - if (waiting->waitees.empty() || (f.result == Goal::ecFailed && !settings.keepGoing)) { + if (waiting->waitees.empty() || (f.exitCode == Goal::ecFailed && !settings.keepGoing)) { /* If we failed and keepGoing is not set, we remove all remaining waitees. */ for (auto & i : waiting->waitees) { @@ -309,26 +320,38 @@ void Worker::waitForAWhile(GoalPtr goal) } -void Worker::run(const Goals & _topGoals) +void Worker::updateStatistics() { - std::vector<nix::DerivedPath> topPaths; - - for (auto & i : _topGoals) { - topGoals.insert(i); - if (auto goal = dynamic_cast<DerivationGoal *>(i.get())) { - topPaths.push_back(DerivedPath::Built { - .drvPath = makeConstantStorePathRef(goal->drvPath), - .outputs = goal->wantedOutputs, - }); - } else if (auto goal = dynamic_cast<PathSubstitutionGoal *>(i.get())) { - topPaths.push_back(DerivedPath::Opaque{goal->storePath}); - } + // only update progress info while running. this notably excludes updating + // progress info while destroying, which causes the progress bar to assert + if (running && statisticsOutdated) { + actDerivations.progress( + doneBuilds, expectedBuilds + doneBuilds, runningBuilds, failedBuilds + ); + actSubstitutions.progress( + doneSubstitutions, + expectedSubstitutions + doneSubstitutions, + runningSubstitutions, + failedSubstitutions + ); + act.setExpected(actFileTransfer, expectedDownloadSize + doneDownloadSize); + act.setExpected(actCopyPath, expectedNarSize + doneNarSize); + + statisticsOutdated = false; } +} + +Goals Worker::run(std::function<Goals (GoalFactory &)> req) +{ + auto _topGoals = req(goalFactory()); + + assert(!running); + running = true; + Finally const _stop([&] { running = false; }); - /* Call queryMissing() to efficiently query substitutes. */ - StorePathSet willBuild, willSubstitute, unknown; - uint64_t downloadSize, narSize; - store.queryMissing(topPaths, willBuild, willSubstitute, unknown, downloadSize, narSize); + updateStatistics(); + + topGoals = _topGoals; debug("entered goal loop"); @@ -357,18 +380,7 @@ void Worker::run(const Goals & _topGoals) ? nrSubstitutions < std::max(1U, (unsigned int) settings.maxSubstitutionJobs) : nrLocalBuilds < settings.maxBuildJobs; handleWorkResult(goal, goal->work(inSlot)); - - actDerivations.progress( - doneBuilds, expectedBuilds + doneBuilds, runningBuilds, failedBuilds - ); - actSubstitutions.progress( - doneSubstitutions, - expectedSubstitutions + doneSubstitutions, - runningSubstitutions, - failedSubstitutions - ); - act.setExpected(actFileTransfer, expectedDownloadSize + doneDownloadSize); - act.setExpected(actCopyPath, expectedNarSize + doneNarSize); + updateStatistics(); if (topGoals.empty()) break; // stuff may have been cancelled } @@ -390,6 +402,8 @@ void Worker::run(const Goals & _topGoals) assert(!settings.keepGoing || awake.empty()); assert(!settings.keepGoing || wantingToBuild.empty()); assert(!settings.keepGoing || children.empty()); + + return _topGoals; } void Worker::waitForInput() diff --git a/src/libstore/build/worker.hh b/src/libstore/build/worker.hh index 7abb966f9..9a6ed8449 100644 --- a/src/libstore/build/worker.hh +++ b/src/libstore/build/worker.hh @@ -1,6 +1,7 @@ #pragma once ///@file +#include "notifying-counter.hh" #include "types.hh" #include "lock.hh" #include "store-api.hh" @@ -40,13 +41,62 @@ struct Child /* Forward definition. */ struct HookInstance; +class GoalFactory +{ +public: + virtual std::shared_ptr<DerivationGoal> makeDerivationGoal( + const StorePath & drvPath, const OutputsSpec & wantedOutputs, BuildMode buildMode = bmNormal + ) = 0; + virtual std::shared_ptr<DerivationGoal> makeBasicDerivationGoal( + const StorePath & drvPath, + const BasicDerivation & drv, + const OutputsSpec & wantedOutputs, + BuildMode buildMode = bmNormal + ) = 0; + + /** + * @ref SubstitutionGoal "substitution goal" + */ + virtual std::shared_ptr<PathSubstitutionGoal> makePathSubstitutionGoal( + const StorePath & storePath, + RepairFlag repair = NoRepair, + std::optional<ContentAddress> ca = std::nullopt + ) = 0; + virtual std::shared_ptr<DrvOutputSubstitutionGoal> makeDrvOutputSubstitutionGoal( + const DrvOutput & id, + RepairFlag repair = NoRepair, + std::optional<ContentAddress> ca = std::nullopt + ) = 0; + + /** + * Make a goal corresponding to the `DerivedPath`. + * + * It will be a `DerivationGoal` for a `DerivedPath::Built` or + * a `SubstitutionGoal` for a `DerivedPath::Opaque`. + */ + virtual GoalPtr makeGoal(const DerivedPath & req, BuildMode buildMode = bmNormal) = 0; +}; + +// elaborate hoax to let goals access factory methods while hiding them from the public +class WorkerBase : protected GoalFactory +{ + friend struct DerivationGoal; + friend struct PathSubstitutionGoal; + friend class DrvOutputSubstitutionGoal; + +protected: + GoalFactory & goalFactory() { return *this; } +}; + /** * The worker class. */ -class Worker +class Worker : public WorkerBase { private: + bool running = false; + /* Note: the worker should only have strong pointers to the top-level goals. */ @@ -164,6 +214,21 @@ private: void childStarted(GoalPtr goal, const std::set<int> & fds, bool inBuildSlot); + /** + * Pass current stats counters to the logger for progress bar updates. + */ + void updateStatistics(); + + bool statisticsOutdated = true; + + /** + * Mark statistics as outdated, such that `updateStatistics` will be called. + */ + void updateStatisticsLater() + { + statisticsOutdated = true; + } + public: const Activity act; @@ -185,19 +250,19 @@ public: HookState hook; - uint64_t expectedBuilds = 0; - uint64_t doneBuilds = 0; - uint64_t failedBuilds = 0; - uint64_t runningBuilds = 0; + NotifyingCounter<uint64_t> expectedBuilds{[this] { updateStatisticsLater(); }}; + NotifyingCounter<uint64_t> doneBuilds{[this] { updateStatisticsLater(); }}; + NotifyingCounter<uint64_t> failedBuilds{[this] { updateStatisticsLater(); }}; + NotifyingCounter<uint64_t> runningBuilds{[this] { updateStatisticsLater(); }}; - uint64_t expectedSubstitutions = 0; - uint64_t doneSubstitutions = 0; - uint64_t failedSubstitutions = 0; - uint64_t runningSubstitutions = 0; - uint64_t expectedDownloadSize = 0; - uint64_t doneDownloadSize = 0; - uint64_t expectedNarSize = 0; - uint64_t doneNarSize = 0; + NotifyingCounter<uint64_t> expectedSubstitutions{[this] { updateStatisticsLater(); }}; + NotifyingCounter<uint64_t> doneSubstitutions{[this] { updateStatisticsLater(); }}; + NotifyingCounter<uint64_t> failedSubstitutions{[this] { updateStatisticsLater(); }}; + NotifyingCounter<uint64_t> runningSubstitutions{[this] { updateStatisticsLater(); }}; + NotifyingCounter<uint64_t> expectedDownloadSize{[this] { updateStatisticsLater(); }}; + NotifyingCounter<uint64_t> doneDownloadSize{[this] { updateStatisticsLater(); }}; + NotifyingCounter<uint64_t> expectedNarSize{[this] { updateStatisticsLater(); }}; + NotifyingCounter<uint64_t> doneNarSize{[this] { updateStatisticsLater(); }}; Worker(Store & store, Store & evalStore); ~Worker(); @@ -213,19 +278,18 @@ private: std::shared_ptr<DerivationGoal> makeDerivationGoalCommon( const StorePath & drvPath, const OutputsSpec & wantedOutputs, std::function<std::shared_ptr<DerivationGoal>()> mkDrvGoal); -public: std::shared_ptr<DerivationGoal> makeDerivationGoal( const StorePath & drvPath, - const OutputsSpec & wantedOutputs, BuildMode buildMode = bmNormal); + const OutputsSpec & wantedOutputs, BuildMode buildMode = bmNormal) override; std::shared_ptr<DerivationGoal> makeBasicDerivationGoal( const StorePath & drvPath, const BasicDerivation & drv, - const OutputsSpec & wantedOutputs, BuildMode buildMode = bmNormal); + const OutputsSpec & wantedOutputs, BuildMode buildMode = bmNormal) override; /** * @ref SubstitutionGoal "substitution goal" */ - std::shared_ptr<PathSubstitutionGoal> makePathSubstitutionGoal(const StorePath & storePath, RepairFlag repair = NoRepair, std::optional<ContentAddress> ca = std::nullopt); - std::shared_ptr<DrvOutputSubstitutionGoal> makeDrvOutputSubstitutionGoal(const DrvOutput & id, RepairFlag repair = NoRepair, std::optional<ContentAddress> ca = std::nullopt); + std::shared_ptr<PathSubstitutionGoal> makePathSubstitutionGoal(const StorePath & storePath, RepairFlag repair = NoRepair, std::optional<ContentAddress> ca = std::nullopt) override; + std::shared_ptr<DrvOutputSubstitutionGoal> makeDrvOutputSubstitutionGoal(const DrvOutput & id, RepairFlag repair = NoRepair, std::optional<ContentAddress> ca = std::nullopt) override; /** * Make a goal corresponding to the `DerivedPath`. @@ -233,8 +297,9 @@ public: * It will be a `DerivationGoal` for a `DerivedPath::Built` or * a `SubstitutionGoal` for a `DerivedPath::Opaque`. */ - GoalPtr makeGoal(const DerivedPath & req, BuildMode buildMode = bmNormal); + GoalPtr makeGoal(const DerivedPath & req, BuildMode buildMode = bmNormal) override; +public: /** * Unregisters a running child process. */ @@ -243,7 +308,7 @@ public: /** * Loop until the specified top-level goals have finished. */ - void run(const Goals & topGoals); + Goals run(std::function<Goals (GoalFactory &)> req); /*** * The exit status in case of failure. diff --git a/src/libstore/builtins/buildenv.cc b/src/libstore/builtins/buildenv.cc index 61c729d27..9fe5f6660 100644 --- a/src/libstore/builtins/buildenv.cc +++ b/src/libstore/builtins/buildenv.cc @@ -1,4 +1,5 @@ #include "buildenv.hh" +#include "strings.hh" #include <sys/stat.h> #include <sys/types.h> diff --git a/src/libstore/builtins/fetchurl.cc b/src/libstore/builtins/fetchurl.cc index 4049d1c6c..062ecdc14 100644 --- a/src/libstore/builtins/fetchurl.cc +++ b/src/libstore/builtins/fetchurl.cc @@ -3,6 +3,7 @@ #include "store-api.hh" #include "archive.hh" #include "compression.hh" +#include "strings.hh" namespace nix { diff --git a/src/libstore/common-protocol-impl.hh b/src/libstore/common-protocol-impl.hh index fd1387e95..387f848ed 100644 --- a/src/libstore/common-protocol-impl.hh +++ b/src/libstore/common-protocol-impl.hh @@ -20,6 +20,7 @@ namespace nix { { \ return LengthPrefixedProtoHelper<CommonProto, T >::read(store, conn); \ } \ + /* NOLINTNEXTLINE(bugprone-macro-parentheses) */ \ TEMPLATE [[nodiscard]] WireFormatGenerator CommonProto::Serialise< T >::write(const Store & store, CommonProto::WriteConn conn, const T & t) \ { \ return LengthPrefixedProtoHelper<CommonProto, T >::write(store, conn, t); \ diff --git a/src/libstore/content-address.cc b/src/libstore/content-address.cc index 6aa6d598d..dae311609 100644 --- a/src/libstore/content-address.cc +++ b/src/libstore/content-address.cc @@ -1,6 +1,7 @@ #include "args.hh" #include "content-address.hh" #include "split.hh" +#include "strings.hh" namespace nix { diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 5ac9cd2ef..a9239197b 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -2,7 +2,7 @@ #include "monitor-fd.hh" #include "worker-protocol.hh" #include "worker-protocol-impl.hh" -#include "build-result.hh" +#include "build-result.hh" // IWYU pragma: keep #include "store-api.hh" #include "store-cast.hh" #include "gc-store.hh" @@ -12,6 +12,7 @@ #include "finally.hh" #include "archive.hh" #include "derivations.hh" +#include "strings.hh" #include "args.hh" #include <sstream> diff --git a/src/libstore/derivations.cc b/src/libstore/derivations.cc index 7f41e6865..93baa9f20 100644 --- a/src/libstore/derivations.cc +++ b/src/libstore/derivations.cc @@ -3,11 +3,12 @@ #include "store-api.hh" #include "globals.hh" #include "types.hh" -#include "split.hh" #include "common-protocol.hh" #include "common-protocol-impl.hh" -#include "fs-accessor.hh" #include "json-utils.hh" +#include "strings.hh" +#include "backed-string-view.hh" + #include <boost/container/small_vector.hpp> #include <nlohmann/json.hpp> diff --git a/src/libstore/derived-path.hh b/src/libstore/derived-path.hh index c87cf2004..b75415aec 100644 --- a/src/libstore/derived-path.hh +++ b/src/libstore/derived-path.hh @@ -5,6 +5,7 @@ #include "path.hh" #include "outputs-spec.hh" #include "comparator.hh" +#include "ref.hh" #include <variant> @@ -78,10 +79,12 @@ struct SingleDerivedPathBuilt { DECLARE_CMP(SingleDerivedPathBuilt); }; -using _SingleDerivedPathRaw = std::variant< +namespace derived_path::detail { +using SingleDerivedPathRaw = std::variant< DerivedPathOpaque, SingleDerivedPathBuilt >; +} /** * A "derived path" is a very simple sort of expression (not a Nix @@ -94,8 +97,8 @@ using _SingleDerivedPathRaw = std::variant< * - built, in which case it is a pair of a derivation path and an * output name. */ -struct SingleDerivedPath : _SingleDerivedPathRaw { - using Raw = _SingleDerivedPathRaw; +struct SingleDerivedPath : derived_path::detail::SingleDerivedPathRaw { + using Raw = derived_path::detail::SingleDerivedPathRaw; using Raw::Raw; using Opaque = DerivedPathOpaque; @@ -201,10 +204,12 @@ struct DerivedPathBuilt { DECLARE_CMP(DerivedPathBuilt); }; -using _DerivedPathRaw = std::variant< +namespace derived_path::detail { +using DerivedPathRaw = std::variant< DerivedPathOpaque, DerivedPathBuilt >; +} /** * A "derived path" is a very simple sort of expression that evaluates @@ -216,8 +221,8 @@ using _DerivedPathRaw = std::variant< * - built, in which case it is a pair of a derivation path and some * output names. */ -struct DerivedPath : _DerivedPathRaw { - using Raw = _DerivedPathRaw; +struct DerivedPath : derived_path::detail::DerivedPathRaw { + using Raw = derived_path::detail::DerivedPathRaw; using Raw::Raw; using Opaque = DerivedPathOpaque; diff --git a/src/libstore/dummy-store.cc b/src/libstore/dummy-store.cc index e16f87e4b..f14b7ddd1 100644 --- a/src/libstore/dummy-store.cc +++ b/src/libstore/dummy-store.cc @@ -1,3 +1,4 @@ +#include "dummy-store.hh" #include "store-api.hh" namespace nix { @@ -73,6 +74,8 @@ struct DummyStore : public virtual DummyStoreConfig, public virtual Store { unsupported("getFSAccessor"); } }; -static RegisterStoreImplementation<DummyStore, DummyStoreConfig> regDummyStore; +void registerDummyStore() { + StoreImplementations::add<DummyStore, DummyStoreConfig>(); +} } diff --git a/src/libstore/dummy-store.hh b/src/libstore/dummy-store.hh new file mode 100644 index 000000000..355c011f8 --- /dev/null +++ b/src/libstore/dummy-store.hh @@ -0,0 +1,8 @@ +#pragma once +///@file + +namespace nix { + +void registerDummyStore(); + +} diff --git a/src/libstore/filetransfer.cc b/src/libstore/filetransfer.cc index 11c8a755c..f3e8a5312 100644 --- a/src/libstore/filetransfer.cc +++ b/src/libstore/filetransfer.cc @@ -5,6 +5,7 @@ #include "s3.hh" #include "signals.hh" #include "compression.hh" +#include "strings.hh" #if ENABLE_S3 #include <aws/core/client/ClientConfiguration.h> diff --git a/src/libstore/filetransfer.hh b/src/libstore/filetransfer.hh index b2ee66312..4352cb81b 100644 --- a/src/libstore/filetransfer.hh +++ b/src/libstore/filetransfer.hh @@ -2,6 +2,7 @@ ///@file #include "box_ptr.hh" +#include "ref.hh" #include "logging.hh" #include "serialise.hh" #include "types.hh" diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc index 7b4a12dc9..d5903d01e 100644 --- a/src/libstore/gc.cc +++ b/src/libstore/gc.cc @@ -5,6 +5,7 @@ #include "signals.hh" #include "finally.hh" #include "unix-domain-socket.hh" +#include "strings.hh" #include <queue> #include <regex> diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc index 29ec60105..c114e22dc 100644 --- a/src/libstore/globals.cc +++ b/src/libstore/globals.cc @@ -33,6 +33,16 @@ #include <sys/sysctl.h> #endif +// All built-in store implementations. +#include "dummy-store.hh" +#include "http-binary-cache-store.hh" +#include "legacy-ssh-store.hh" +#include "local-binary-cache-store.hh" +#include "local-store.hh" +#include "s3-binary-cache-store.hh" +#include "ssh-store.hh" +#include "uds-remote-store.hh" + namespace nix { @@ -397,6 +407,17 @@ static void preloadNSS() }); } +static void registerStoreImplementations() { + registerDummyStore(); + registerHttpBinaryCacheStore(); + registerLegacySSHStore(); + registerLocalBinaryCacheStore(); + registerLocalStore(); + registerS3BinaryCacheStore(); + registerSSHStore(); + registerUDSRemoteStore(); +} + static bool initLibStoreDone = false; void assertLibStoreInitialized() { @@ -434,6 +455,8 @@ void initLibStore() { unsetenv("TMPDIR"); #endif + registerStoreImplementations(); + initLibStoreDone = true; } diff --git a/src/libstore/http-binary-cache-store.cc b/src/libstore/http-binary-cache-store.cc index 7ceea716a..c68faf552 100644 --- a/src/libstore/http-binary-cache-store.cc +++ b/src/libstore/http-binary-cache-store.cc @@ -1,3 +1,4 @@ +#include "http-binary-cache-store.hh" #include "binary-cache-store.hh" #include "filetransfer.hh" #include "globals.hh" @@ -194,6 +195,8 @@ protected: } }; -static RegisterStoreImplementation<HttpBinaryCacheStore, HttpBinaryCacheStoreConfig> regHttpBinaryCacheStore; +void registerHttpBinaryCacheStore() { + StoreImplementations::add<HttpBinaryCacheStore, HttpBinaryCacheStoreConfig>(); +} } diff --git a/src/libstore/http-binary-cache-store.hh b/src/libstore/http-binary-cache-store.hh new file mode 100644 index 000000000..097c5480b --- /dev/null +++ b/src/libstore/http-binary-cache-store.hh @@ -0,0 +1,8 @@ +#pragma once +///@file + +namespace nix { + +void registerHttpBinaryCacheStore(); + +} diff --git a/src/libstore/legacy-ssh-store.cc b/src/libstore/legacy-ssh-store.cc index 5650282d6..4433b411d 100644 --- a/src/libstore/legacy-ssh-store.cc +++ b/src/libstore/legacy-ssh-store.cc @@ -1,4 +1,4 @@ -#include "ssh-store-config.hh" +#include "legacy-ssh-store.hh" #include "archive.hh" #include "pool.hh" #include "remote-store.hh" @@ -8,6 +8,8 @@ #include "store-api.hh" #include "path-with-outputs.hh" #include "ssh.hh" +#include "ssh-store.hh" +#include "strings.hh" #include "derivations.hh" namespace nix { @@ -412,6 +414,8 @@ public: { unsupported("queryRealisation"); } }; -static RegisterStoreImplementation<LegacySSHStore, LegacySSHStoreConfig> regLegacySSHStore; +void registerLegacySSHStore() { + StoreImplementations::add<LegacySSHStore, LegacySSHStoreConfig>(); +} } diff --git a/src/libstore/legacy-ssh-store.hh b/src/libstore/legacy-ssh-store.hh new file mode 100644 index 000000000..76298d8d9 --- /dev/null +++ b/src/libstore/legacy-ssh-store.hh @@ -0,0 +1,8 @@ +#pragma once +///@file + +namespace nix { + +void registerLegacySSHStore(); + +} diff --git a/src/libstore/length-prefixed-protocol-helper.hh b/src/libstore/length-prefixed-protocol-helper.hh index 1475d2690..423bc77b8 100644 --- a/src/libstore/length-prefixed-protocol-helper.hh +++ b/src/libstore/length-prefixed-protocol-helper.hh @@ -61,9 +61,9 @@ template<class Inner, typename... Ts> LENGTH_PREFIXED_PROTO_HELPER(Inner, std::tuple<Ts...>); template<class Inner, typename K, typename V> -#define _X std::map<K, V> -LENGTH_PREFIXED_PROTO_HELPER(Inner, _X); -#undef _X +#define DONT_SUBSTITUTE_KV_TYPE std::map<K, V> +LENGTH_PREFIXED_PROTO_HELPER(Inner, DONT_SUBSTITUTE_KV_TYPE); +#undef DONT_SUBSTITUTE_KV_TYPE template<class Inner, typename T> std::vector<T> diff --git a/src/libstore/local-binary-cache-store.cc b/src/libstore/local-binary-cache-store.cc index 99dd0e0f6..2f6a092e1 100644 --- a/src/libstore/local-binary-cache-store.cc +++ b/src/libstore/local-binary-cache-store.cc @@ -1,3 +1,4 @@ +#include "local-binary-cache-store.hh" #include "binary-cache-store.hh" #include "globals.hh" #include "nar-info-disk-cache.hh" @@ -124,6 +125,8 @@ std::set<std::string> LocalBinaryCacheStore::uriSchemes() return {"file"}; } -static RegisterStoreImplementation<LocalBinaryCacheStore, LocalBinaryCacheStoreConfig> regLocalBinaryCacheStore; +void registerLocalBinaryCacheStore() { + StoreImplementations::add<LocalBinaryCacheStore, LocalBinaryCacheStoreConfig>(); +} } diff --git a/src/libstore/local-binary-cache-store.hh b/src/libstore/local-binary-cache-store.hh new file mode 100644 index 000000000..727d60cb3 --- /dev/null +++ b/src/libstore/local-binary-cache-store.hh @@ -0,0 +1,8 @@ +#pragma once +///@file + +namespace nix { + +void registerLocalBinaryCacheStore(); + +} diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc index 4c8e2ea2f..49991e38a 100644 --- a/src/libstore/local-store.cc +++ b/src/libstore/local-store.cc @@ -10,6 +10,7 @@ #include "signals.hh" #include "finally.hh" #include "compression.hh" +#include "strings.hh" #include <algorithm> #include <cstring> diff --git a/src/libstore/local-store.hh b/src/libstore/local-store.hh index f6b553615..1913aa192 100644 --- a/src/libstore/local-store.hh +++ b/src/libstore/local-store.hh @@ -421,4 +421,7 @@ void canonicaliseTimestampAndPermissions(const Path & path); MakeError(PathInUse, Error); +// Implemented by the relevant platform/ module being used. +void registerLocalStore(); + } diff --git a/src/libstore/lock.hh b/src/libstore/lock.hh index 1c268e1fb..ac5ff061b 100644 --- a/src/libstore/lock.hh +++ b/src/libstore/lock.hh @@ -1,11 +1,10 @@ #pragma once ///@file -#include "types.hh" - -#include <optional> +#include <memory> #include <sys/types.h> +#include <vector> namespace nix { diff --git a/src/libstore/machines.hh b/src/libstore/machines.hh index 8516409d4..56d9bed30 100644 --- a/src/libstore/machines.hh +++ b/src/libstore/machines.hh @@ -1,7 +1,9 @@ #pragma once ///@file -#include "types.hh" +#include "ref.hh" +#include <set> +#include <vector> namespace nix { diff --git a/src/libstore/make-content-addressed.cc b/src/libstore/make-content-addressed.cc index abb6e9889..0c592ce22 100644 --- a/src/libstore/make-content-addressed.cc +++ b/src/libstore/make-content-addressed.cc @@ -1,5 +1,6 @@ #include "make-content-addressed.hh" #include "references.hh" +#include "strings.hh" namespace nix { diff --git a/src/libstore/meson.build b/src/libstore/meson.build index 5416bd2b5..74f5cd04e 100644 --- a/src/libstore/meson.build +++ b/src/libstore/meson.build @@ -118,12 +118,16 @@ libstore_headers = files( 'derived-path-map.hh', 'derived-path.hh', 'downstream-placeholder.hh', + 'dummy-store.hh', 'filetransfer.hh', 'fs-accessor.hh', 'gc-store.hh', 'globals.hh', + 'http-binary-cache-store.hh', 'indirect-root-store.hh', + 'legacy-ssh-store.hh', 'length-prefixed-protocol-helper.hh', + 'local-binary-cache-store.hh', 'local-fs-store.hh', 'local-store.hh', 'lock.hh', @@ -152,8 +156,8 @@ libstore_headers = files( 'serve-protocol-impl.hh', 'serve-protocol.hh', 'sqlite.hh', - 'ssh-store-config.hh', 'ssh.hh', + 'ssh-store.hh', 'store-api.hh', 'store-cast.hh', 'uds-remote-store.hh', diff --git a/src/libstore/misc.cc b/src/libstore/misc.cc index a63a28482..7ea7c189a 100644 --- a/src/libstore/misc.cc +++ b/src/libstore/misc.cc @@ -1,12 +1,12 @@ #include "derivations.hh" #include "parsed-derivations.hh" #include "globals.hh" -#include "local-store.hh" #include "store-api.hh" #include "thread-pool.hh" #include "topo-sort.hh" #include "closure.hh" #include "filetransfer.hh" +#include "strings.hh" namespace nix { diff --git a/src/libstore/nar-accessor.hh b/src/libstore/nar-accessor.hh index 5e19bd3c7..4299daf1b 100644 --- a/src/libstore/nar-accessor.hh +++ b/src/libstore/nar-accessor.hh @@ -1,10 +1,12 @@ #pragma once ///@file +#include "fs-accessor.hh" +#include "ref.hh" + #include <functional> #include <nlohmann/json_fwd.hpp> -#include "fs-accessor.hh" namespace nix { diff --git a/src/libstore/nar-info-disk-cache.cc b/src/libstore/nar-info-disk-cache.cc index 5c0bb17b9..c83a8fbfb 100644 --- a/src/libstore/nar-info-disk-cache.cc +++ b/src/libstore/nar-info-disk-cache.cc @@ -4,6 +4,7 @@ #include "sqlite.hh" #include "globals.hh" #include "users.hh" +#include "strings.hh" #include <sqlite3.h> #include <nlohmann/json.hpp> diff --git a/src/libstore/nar-info.cc b/src/libstore/nar-info.cc index e557b4677..0434873df 100644 --- a/src/libstore/nar-info.cc +++ b/src/libstore/nar-info.cc @@ -1,4 +1,4 @@ -#include "globals.hh" +#include "strings.hh" #include "nar-info.hh" #include "store-api.hh" diff --git a/src/libstore/optimise-store.cc b/src/libstore/optimise-store.cc index 9c871b78f..c60e5a85d 100644 --- a/src/libstore/optimise-store.cc +++ b/src/libstore/optimise-store.cc @@ -1,6 +1,7 @@ #include "local-store.hh" #include "globals.hh" #include "signals.hh" +#include "strings.hh" #include <cstring> #include <sys/types.h> diff --git a/src/libstore/parsed-derivations.cc b/src/libstore/parsed-derivations.cc index 992a79c6e..1b2ec914d 100644 --- a/src/libstore/parsed-derivations.cc +++ b/src/libstore/parsed-derivations.cc @@ -1,4 +1,5 @@ #include "parsed-derivations.hh" +#include "strings.hh" #include <nlohmann/json.hpp> #include <regex> diff --git a/src/libstore/path-info.cc b/src/libstore/path-info.cc index 4dc2823ce..54519a867 100644 --- a/src/libstore/path-info.cc +++ b/src/libstore/path-info.cc @@ -1,5 +1,6 @@ #include "path-info.hh" #include "store-api.hh" +#include "strings.hh" namespace nix { diff --git a/src/libstore/path-with-outputs.cc b/src/libstore/path-with-outputs.cc index af6837370..c49531acb 100644 --- a/src/libstore/path-with-outputs.cc +++ b/src/libstore/path-with-outputs.cc @@ -1,7 +1,6 @@ #include "path-with-outputs.hh" #include "store-api.hh" - -#include <regex> +#include "strings.hh" namespace nix { diff --git a/src/libstore/path.cc b/src/libstore/path.cc index d4b5fc0dc..82ed20285 100644 --- a/src/libstore/path.cc +++ b/src/libstore/path.cc @@ -112,3 +112,13 @@ PathSet Store::printStorePathSet(const StorePathSet & paths) const } } + +std::size_t std::hash<nix::StorePath>::operator()(const nix::StorePath & path) const noexcept +{ + // It's already a cryptographic hash of 160 bits (assuming that nobody gives us bogus ones...), so just parse it. + auto h = nix::Hash::parseNonSRIUnprefixed(path.hashPart(), nix::HashType::SHA1); + // This need not be stable across machines, so bit casting the start of it is fine. + size_t r; + memcpy(&r, h.hash, sizeof(r)); + return r; +} diff --git a/src/libstore/path.hh b/src/libstore/path.hh index 4ca6747b3..09b4710c1 100644 --- a/src/libstore/path.hh +++ b/src/libstore/path.hh @@ -2,8 +2,9 @@ ///@file #include <string_view> +#include <string> -#include "types.hh" +#include "types.hh" // IWYU pragma: keep namespace nix { @@ -89,10 +90,7 @@ const std::string drvExtension = ".drv"; namespace std { template<> struct hash<nix::StorePath> { - std::size_t operator()(const nix::StorePath & path) const noexcept - { - return * (std::size_t *) path.to_string().data(); - } + std::size_t operator()(const nix::StorePath & path) const noexcept; }; } diff --git a/src/libstore/pathlocks.hh b/src/libstore/pathlocks.hh index 7fcfa2e40..d06d031b5 100644 --- a/src/libstore/pathlocks.hh +++ b/src/libstore/pathlocks.hh @@ -49,8 +49,12 @@ struct FdLock ~FdLock() { - if (acquired) - lockFile(fd, ltNone, false); + try { + if (acquired) + lockFile(fd, ltNone, false); + } catch (SysError &) { + ignoreException(); + } } }; diff --git a/src/libstore/platform.cc b/src/libstore/platform.cc index 72757e39b..f2c023c82 100644 --- a/src/libstore/platform.cc +++ b/src/libstore/platform.cc @@ -29,17 +29,18 @@ std::shared_ptr<LocalDerivationGoal> LocalDerivationGoal::makeLocalDerivationGoa const StorePath & drvPath, const OutputsSpec & wantedOutputs, Worker & worker, + bool isDependency, BuildMode buildMode ) { #if __linux__ - return std::make_shared<LinuxLocalDerivationGoal>(drvPath, wantedOutputs, worker, buildMode); + return std::make_shared<LinuxLocalDerivationGoal>(drvPath, wantedOutputs, worker, isDependency, buildMode); #elif __APPLE__ - return std::make_shared<DarwinLocalDerivationGoal>(drvPath, wantedOutputs, worker, buildMode); + return std::make_shared<DarwinLocalDerivationGoal>(drvPath, wantedOutputs, worker, isDependency, buildMode); #elif __FreeBSD__ - return std::make_shared<FreeBSDLocalDerivationGoal>(drvPath, wantedOutputs, worker, buildMode); + return std::make_shared<FreeBSDLocalDerivationGoal>(drvPath, wantedOutputs, worker, isDependency, buildMode); #else - return std::make_shared<FallbackLocalDerivationGoal>(drvPath, wantedOutputs, worker, buildMode); + return std::make_shared<FallbackLocalDerivationGoal>(drvPath, wantedOutputs, worker, isDependency, buildMode); #endif } @@ -48,24 +49,25 @@ std::shared_ptr<LocalDerivationGoal> LocalDerivationGoal::makeLocalDerivationGoa const BasicDerivation & drv, const OutputsSpec & wantedOutputs, Worker & worker, + bool isDependency, BuildMode buildMode ) { #if __linux__ return std::make_shared<LinuxLocalDerivationGoal>( - drvPath, drv, wantedOutputs, worker, buildMode + drvPath, drv, wantedOutputs, worker, isDependency, buildMode ); #elif __APPLE__ return std::make_shared<DarwinLocalDerivationGoal>( - drvPath, drv, wantedOutputs, worker, buildMode + drvPath, drv, wantedOutputs, worker, isDependency, buildMode ); #elif __FreeBSD__ return std::make_shared<FreeBSDLocalDerivationGoal>( - drvPath, drv, wantedOutputs, worker, buildMode + drvPath, drv, wantedOutputs, worker, isDependency, buildMode ); #else return std::make_shared<FallbackLocalDerivationGoal>( - drvPath, drv, wantedOutputs, worker, buildMode + drvPath, drv, wantedOutputs, worker, isDependency, buildMode ); #endif } diff --git a/src/libstore/platform/darwin.cc b/src/libstore/platform/darwin.cc index 1f7e9be23..956fb1e9b 100644 --- a/src/libstore/platform/darwin.cc +++ b/src/libstore/platform/darwin.cc @@ -2,6 +2,7 @@ #include "signals.hh" #include "platform/darwin.hh" #include "regex.hh" +#include "strings.hh" #include <sys/proc_info.h> #include <sys/sysctl.h> @@ -261,4 +262,9 @@ void DarwinLocalDerivationGoal::execBuilder(std::string builder, Strings args, S posix_spawn(nullptr, builder.c_str(), nullptr, &attrp, stringsToCharPtrs(args).data(), stringsToCharPtrs(envStrs).data()); } + +void registerLocalStore() { + StoreImplementations::add<DarwinLocalStore, LocalStoreConfig>(); +} + } diff --git a/src/libstore/platform/fallback.cc b/src/libstore/platform/fallback.cc index 5a01d64c8..0593ec204 100644 --- a/src/libstore/platform/fallback.cc +++ b/src/libstore/platform/fallback.cc @@ -1,5 +1,7 @@ #include "platform/fallback.hh" namespace nix { -static RegisterStoreImplementation<FallbackLocalStore, LocalStoreConfig> regLocalStore; +void registerLocalStore() { + Implementations::add<FallbackLocalStore, LocalStoreConfig>(); +} } diff --git a/src/libstore/platform/linux.cc b/src/libstore/platform/linux.cc index 03b8bc0be..486d71885 100644 --- a/src/libstore/platform/linux.cc +++ b/src/libstore/platform/linux.cc @@ -5,6 +5,7 @@ #include "signals.hh" #include "platform/linux.hh" #include "regex.hh" +#include "strings.hh" #include <grp.h> #include <regex> @@ -25,7 +26,9 @@ namespace { constexpr const std::string_view nativeSystem = SYSTEM; } -static RegisterStoreImplementation<LinuxLocalStore, LocalStoreConfig> regLocalStore; +void registerLocalStore() { + StoreImplementations::add<LinuxLocalStore, LocalStoreConfig>(); +} static void readProcLink(const std::string & file, UncheckedRoots & roots) { diff --git a/src/libstore/profiles.cc b/src/libstore/profiles.cc index e8b88693d..d88a3b9fe 100644 --- a/src/libstore/profiles.cc +++ b/src/libstore/profiles.cc @@ -1,12 +1,11 @@ #include "profiles.hh" -#include "store-api.hh" #include "local-fs-store.hh" #include "users.hh" +#include "strings.hh" #include <sys/types.h> #include <sys/stat.h> #include <unistd.h> -#include <errno.h> #include <stdio.h> diff --git a/src/libstore/remote-store.cc b/src/libstore/remote-store.cc index 36223051b..1f94ca03f 100644 --- a/src/libstore/remote-store.cc +++ b/src/libstore/remote-store.cc @@ -15,6 +15,8 @@ #include "finally.hh" #include "logging.hh" #include "filetransfer.hh" +#include "strings.hh" + #include <nlohmann/json.hpp> namespace nix { @@ -64,7 +66,7 @@ void RemoteStore::initConnection(Connection & conn) { /* Send the magic greeting, check for the reply. */ try { - conn.from.endOfFileError = "Nix daemon disconnected unexpectedly (maybe it crashed?)"; + conn.from.specialEndOfFileError = "Nix daemon disconnected unexpectedly (maybe it crashed?)"; conn.to << WORKER_MAGIC_1; conn.to.flush(); diff --git a/src/libstore/s3-binary-cache-store.cc b/src/libstore/s3-binary-cache-store.cc index ffebfda8d..b50084860 100644 --- a/src/libstore/s3-binary-cache-store.cc +++ b/src/libstore/s3-binary-cache-store.cc @@ -7,6 +7,7 @@ #include "globals.hh" #include "compression.hh" #include "filetransfer.hh" +#include "strings.hh" #include <aws/core/Aws.h> #include <aws/core/VersionConfig.h> @@ -526,8 +527,14 @@ struct S3BinaryCacheStoreImpl : virtual S3BinaryCacheStoreConfig, public virtual }; -static RegisterStoreImplementation<S3BinaryCacheStoreImpl, S3BinaryCacheStoreConfig> regS3BinaryCacheStore; +void registerS3BinaryCacheStore() { + StoreImplementations::add<S3BinaryCacheStoreImpl, S3BinaryCacheStoreConfig>(); +} } +#else +namespace nix { +void registerS3BinaryCacheStore() {} +} #endif diff --git a/src/libstore/s3-binary-cache-store.hh b/src/libstore/s3-binary-cache-store.hh index c62ea5147..baf21e2f4 100644 --- a/src/libstore/s3-binary-cache-store.hh +++ b/src/libstore/s3-binary-cache-store.hh @@ -29,4 +29,6 @@ public: virtual const Stats & getS3Stats() = 0; }; +void registerS3BinaryCacheStore(); + } diff --git a/src/libstore/serve-protocol-impl.hh b/src/libstore/serve-protocol-impl.hh index cfb1dd574..a1d1c797f 100644 --- a/src/libstore/serve-protocol-impl.hh +++ b/src/libstore/serve-protocol-impl.hh @@ -20,6 +20,7 @@ namespace nix { { \ return LengthPrefixedProtoHelper<ServeProto, T >::read(store, conn); \ } \ + /* NOLINTNEXTLINE(bugprone-macro-parentheses) */ \ TEMPLATE [[nodiscard]] WireFormatGenerator ServeProto::Serialise< T >::write(const Store & store, ServeProto::WriteConn conn, const T & t) \ { \ return LengthPrefixedProtoHelper<ServeProto, T >::write(store, conn, t); \ diff --git a/src/libstore/sqlite.hh b/src/libstore/sqlite.hh index ca021087f..5740c4e45 100644 --- a/src/libstore/sqlite.hh +++ b/src/libstore/sqlite.hh @@ -1,9 +1,9 @@ #pragma once ///@file -#include <functional> #include <string> +#include "types.hh" #include "error.hh" struct sqlite3; diff --git a/src/libstore/ssh-store.cc b/src/libstore/ssh-store.cc index 80d10eb0f..fb60326c1 100644 --- a/src/libstore/ssh-store.cc +++ b/src/libstore/ssh-store.cc @@ -1,13 +1,11 @@ -#include "ssh-store-config.hh" +#include "ssh-store.hh" #include "store-api.hh" -#include "local-fs-store.hh" #include "remote-store.hh" #include "remote-store-connection.hh" -#include "remote-fs-accessor.hh" -#include "archive.hh" #include "worker-protocol.hh" #include "pool.hh" #include "ssh.hh" +#include "strings.hh" namespace nix { @@ -110,6 +108,8 @@ ref<RemoteStore::Connection> SSHStore::openConnection() return conn; } -static RegisterStoreImplementation<SSHStore, SSHStoreConfig> regSSHStore; +void registerSSHStore() { + StoreImplementations::add<SSHStore, SSHStoreConfig>(); +} } diff --git a/src/libstore/ssh-store-config.hh b/src/libstore/ssh-store.hh index bf55d20cf..51951f80b 100644 --- a/src/libstore/ssh-store-config.hh +++ b/src/libstore/ssh-store.hh @@ -26,4 +26,6 @@ struct CommonSSHStoreConfig : virtual StoreConfig )"}; }; +void registerSSHStore(); + } diff --git a/src/libstore/store-api.cc b/src/libstore/store-api.cc index 6d9fec41b..cb0604233 100644 --- a/src/libstore/store-api.cc +++ b/src/libstore/store-api.cc @@ -5,10 +5,10 @@ #include "nar-info-disk-cache.hh" #include "thread-pool.hh" #include "url.hh" -#include "references.hh" #include "archive.hh" -#include "remote-store.hh" +#include "uds-remote-store.hh" #include "signals.hh" +#include "strings.hh" // FIXME this should not be here, see TODO below on // `addMultipleToStore`. #include "worker-protocol.hh" @@ -1472,7 +1472,7 @@ ref<Store> openStore(const std::string & uri_, parsedUri.authority.value_or("") + parsedUri.path ); - for (auto implem : *Implementations::registered) { + for (auto implem : *StoreImplementations::registered) { if (implem.uriSchemes.count(parsedUri.scheme)) { auto store = implem.create(parsedUri.scheme, baseURI, params); if (store) { @@ -1526,6 +1526,6 @@ std::list<ref<Store>> getDefaultSubstituters() return stores; } -std::vector<StoreFactory> * Implementations::registered = 0; +std::vector<StoreFactory> * StoreImplementations::registered = 0; } diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index efd0e4d9b..2da5cac39 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -50,11 +50,9 @@ namespace nix { * that calls `StoreConfig(params)` (otherwise you're gonna encounter an * `assertion failure` when trying to instantiate it). * - * You can then register the new store using: - * - * ``` - * cpp static RegisterStoreImplementation<Foo, FooConfig> regStore; - * ``` + * You can then register the new store by defining a registration function + * (using `StoreImplementations::add`) and calling it in + * `registerStoreImplementations` in `globals.cc`. */ MakeError(SubstError, Error); @@ -1004,7 +1002,7 @@ struct StoreFactory std::function<std::shared_ptr<StoreConfig> ()> getConfig; }; -struct Implementations +struct StoreImplementations { static std::vector<StoreFactory> * registered; @@ -1027,15 +1025,6 @@ struct Implementations } }; -template<typename T, typename TConfig> -struct RegisterStoreImplementation -{ - RegisterStoreImplementation() - { - Implementations::add<T, TConfig>(); - } -}; - /** * Display a set of paths in human-readable form (i.e., between quotes diff --git a/src/libstore/uds-remote-store.cc b/src/libstore/uds-remote-store.cc index 226cdf717..44dd45e88 100644 --- a/src/libstore/uds-remote-store.cc +++ b/src/libstore/uds-remote-store.cc @@ -88,6 +88,8 @@ void UDSRemoteStore::addIndirectRoot(const Path & path) } -static RegisterStoreImplementation<UDSRemoteStore, UDSRemoteStoreConfig> regUDSRemoteStore; +void registerUDSRemoteStore() { + StoreImplementations::add<UDSRemoteStore, UDSRemoteStoreConfig>(); +} } diff --git a/src/libstore/uds-remote-store.hh b/src/libstore/uds-remote-store.hh index ff7e9ae3f..8b56e0af0 100644 --- a/src/libstore/uds-remote-store.hh +++ b/src/libstore/uds-remote-store.hh @@ -63,4 +63,6 @@ private: std::optional<std::string> path; }; +void registerUDSRemoteStore(); + } diff --git a/src/libstore/worker-protocol-impl.hh b/src/libstore/worker-protocol-impl.hh index b2603b954..d99c59f84 100644 --- a/src/libstore/worker-protocol-impl.hh +++ b/src/libstore/worker-protocol-impl.hh @@ -20,6 +20,7 @@ namespace nix { { \ return LengthPrefixedProtoHelper<WorkerProto, T >::read(store, conn); \ } \ + /* NOLINTNEXTLINE(bugprone-macro-parentheses) */ \ TEMPLATE [[nodiscard]] WireFormatGenerator WorkerProto::Serialise< T >::write(const Store & store, WorkerProto::WriteConn conn, const T & t) \ { \ return LengthPrefixedProtoHelper<WorkerProto, T >::write(store, conn, t); \ diff --git a/src/libutil/args.cc b/src/libutil/args.cc index 1342e7c6a..edcab23ac 100644 --- a/src/libutil/args.cc +++ b/src/libutil/args.cc @@ -1,10 +1,11 @@ #include "args.hh" #include "args/root.hh" #include "hash.hh" -#include "json-utils.hh" +#include "strings.hh" +#include "json-utils.hh" // IWYU pragma: keep (instances) #include "environment-variables.hh" -#include "experimental-features-json.hh" +#include "experimental-features-json.hh" // IWYU pragma: keep (instances) #include "logging.hh" #include <glob.h> diff --git a/src/libutil/args.hh b/src/libutil/args.hh index 5fdbaba7e..e2bac6415 100644 --- a/src/libutil/args.hh +++ b/src/libutil/args.hh @@ -3,6 +3,8 @@ #include "experimental-features.hh" #include "types.hh" +#include "ref.hh" + #include <functional> #include <map> #include <memory> diff --git a/src/libutil/backed-string-view.hh b/src/libutil/backed-string-view.hh new file mode 100644 index 000000000..96136331c --- /dev/null +++ b/src/libutil/backed-string-view.hh @@ -0,0 +1,69 @@ +#pragma once +/// @file String view that can be either owned or borrowed. +#include <variant> +#include <string> +#include <string_view> + +/** + * This wants to be a little bit like rust's Cow type. + * Some parts of the evaluator benefit greatly from being able to reuse + * existing allocations for strings, but have to be able to also use + * newly allocated storage for values. + * + * We do not define implicit conversions, even with ref qualifiers, + * since those can easily become ambiguous to the reader and can degrade + * into copying behaviour we want to avoid. + */ +class BackedStringView { +private: + std::variant<std::string, std::string_view> data; + + /** + * Needed to introduce a temporary since operator-> must return + * a pointer. Without this we'd need to store the view object + * even when we already own a string. + */ + class Ptr { + private: + std::string_view view; + public: + Ptr(std::string_view view): view(view) {} + const std::string_view * operator->() const { return &view; } + }; + +public: + BackedStringView(std::string && s): data(std::move(s)) {} + BackedStringView(std::string_view sv): data(sv) {} + template<size_t N> + BackedStringView(const char (& lit)[N]): data(std::string_view(lit)) {} + + BackedStringView(const BackedStringView &) = delete; + BackedStringView & operator=(const BackedStringView &) = delete; + + /** + * We only want move operations defined since the sole purpose of + * this type is to avoid copies. + */ + BackedStringView(BackedStringView && other) = default; + BackedStringView & operator=(BackedStringView && other) = default; + + bool isOwned() const + { + return std::holds_alternative<std::string>(data); + } + + std::string toOwned() && + { + return isOwned() + ? std::move(std::get<std::string>(data)) + : std::string(std::get<std::string_view>(data)); + } + + std::string_view operator*() const + { + return isOwned() + ? std::get<std::string>(data) + : std::get<std::string_view>(data); + } + Ptr operator->() const { return Ptr(**this); } +}; diff --git a/src/libutil/error.cc b/src/libutil/error.cc index e5d6a9fa8..a7cbfbfd0 100644 --- a/src/libutil/error.cc +++ b/src/libutil/error.cc @@ -3,6 +3,7 @@ #include "logging.hh" #include "position.hh" #include "terminal.hh" +#include "strings.hh" #include <iostream> #include <optional> diff --git a/src/libutil/error.hh b/src/libutil/error.hh index 06dfba0df..73c1ccadd 100644 --- a/src/libutil/error.hh +++ b/src/libutil/error.hh @@ -16,16 +16,12 @@ */ #include "suggestions.hh" -#include "ref.hh" -#include "types.hh" #include "fmt.hh" #include <cstring> #include <list> #include <memory> -#include <map> #include <optional> -#include <compare> #include <sys/types.h> #include <sys/stat.h> @@ -173,6 +169,7 @@ public: }; #define MakeError(newClass, superClass) \ + /* NOLINTNEXTLINE(bugprone-macro-parentheses) */ \ class newClass : public superClass \ { \ public: \ diff --git a/src/libutil/file-system.cc b/src/libutil/file-system.cc index 234c73163..8c69c9864 100644 --- a/src/libutil/file-system.cc +++ b/src/libutil/file-system.cc @@ -10,6 +10,7 @@ #include "logging.hh" #include "serialise.hh" #include "signals.hh" +#include "strings.hh" #include "types.hh" #include "users.hh" diff --git a/src/libutil/finally.hh b/src/libutil/finally.hh index dc51d7b1e..5e62dfa3b 100644 --- a/src/libutil/finally.hh +++ b/src/libutil/finally.hh @@ -22,6 +22,7 @@ public: Finally(Finally &&other) : fun(std::move(other.fun)) { other.movedFrom = true; } + // NOLINTNEXTLINE(bugprone-exception-escape): the noexcept is declared properly here, the analysis seems broken ~Finally() noexcept(noexcept(fun())) { try { diff --git a/src/libutil/fmt.cc b/src/libutil/fmt.cc new file mode 100644 index 000000000..bff5af020 --- /dev/null +++ b/src/libutil/fmt.cc @@ -0,0 +1,24 @@ +#include "fmt.hh" // IWYU pragma: keep +// Darwin and FreeBSD stdenv do not define _GNU_SOURCE but do have _Unwind_Backtrace. +#if __APPLE__ || __FreeBSD__ +#define BOOST_STACKTRACE_GNU_SOURCE_NOT_REQUIRED +#endif +#include <boost/stacktrace/stacktrace.hpp> + +template class boost::basic_format<char>; + +namespace nix { + +// Explicit instantiation saves about 30 cpu-seconds of compile time +template HintFmt::HintFmt(const std::string &, const Uncolored<std::string> &s); +template HintFmt::HintFmt(const std::string &, const std::string &s); +template HintFmt::HintFmt(const std::string &, const uint64_t &, const char * const &); + +HintFmt::HintFmt(const std::string & literal) : HintFmt("%s", Uncolored(literal)) {} + +void printStackTrace() +{ + std::cerr << boost::stacktrace::stacktrace() << std::endl; +} + +} diff --git a/src/libutil/fmt.hh b/src/libutil/fmt.hh index d015f7e5f..ee3e1e2e7 100644 --- a/src/libutil/fmt.hh +++ b/src/libutil/fmt.hh @@ -3,17 +3,17 @@ #include <iostream> #include <string> -#include <optional> #include <boost/format.hpp> -// Darwin and FreeBSD stdenv do not define _GNU_SOURCE but do have _Unwind_Backtrace. -#if __APPLE__ || __FreeBSD__ -#define BOOST_STACKTRACE_GNU_SOURCE_NOT_REQUIRED -#endif -#include <boost/stacktrace.hpp> #include "ansicolor.hh" +// Explicit instantiation in fmt.cc +extern template class boost::basic_format<char>; + namespace nix { +/** Prints a C++ stack trace to stderr using boost stacktrace */ +void printStackTrace(); + /** * Values wrapped in this struct are printed in magenta. * @@ -157,7 +157,9 @@ public: * Format the given string literally, without interpolating format * placeholders. */ - HintFmt(const std::string & literal) : HintFmt("%s", Uncolored(literal)) {} + // Moved out of line because it was instantiating the template below in + // every file in the project. + HintFmt(const std::string & literal); /** * Interpolate the given arguments into the format string. @@ -172,14 +174,14 @@ public: std::cerr << "HintFmt received incorrect number of format args. Original format string: '"; std::cerr << format << "'; number of arguments: " << sizeof...(args) << "\n"; // And regardless of the coredump give me a damn stacktrace. - std::cerr << boost::stacktrace::stacktrace() << std::endl; + printStackTrace(); abort(); } } catch (boost::io::format_error & ex) { // Same thing, but for anything that happens in the member initializers. std::cerr << "HintFmt received incorrect format string. Original format string: '"; std::cerr << format << "'; number of arguments: " << sizeof...(args) << "\n"; - std::cerr << boost::stacktrace::stacktrace() << std::endl; + printStackTrace(); abort(); } @@ -193,6 +195,11 @@ public: } }; +// Explicit instantiations in fmt.cc +extern template HintFmt::HintFmt(const std::string &, const Uncolored<std::string> &s); +extern template HintFmt::HintFmt(const std::string &, const std::string &s); +extern template HintFmt::HintFmt(const std::string &, const uint64_t &, const char * const &); + std::ostream & operator<<(std::ostream & os, const HintFmt & hf); } diff --git a/src/libutil/hash.cc b/src/libutil/hash.cc index 925f71f80..d383e9802 100644 --- a/src/libutil/hash.cc +++ b/src/libutil/hash.cc @@ -7,8 +7,10 @@ #include "args.hh" #include "hash.hh" #include "archive.hh" +#include "charptr-cast.hh" #include "logging.hh" #include "split.hh" +#include "strings.hh" #include <sys/types.h> #include <sys/stat.h> @@ -129,7 +131,7 @@ std::string Hash::to_string(Base base, bool includeType) const break; case Base::Base64: case Base::SRI: - s += base64Encode(std::string_view(reinterpret_cast<const char *>(hash), hashSize)); + s += base64Encode(std::string_view(charptr_cast<const char *>(hash), hashSize)); break; } return s; diff --git a/src/libutil/meson.build b/src/libutil/meson.build index 7c0e45e4b..1ac31c7eb 100644 --- a/src/libutil/meson.build +++ b/src/libutil/meson.build @@ -17,6 +17,7 @@ libutil_sources = files( 'experimental-features.cc', 'file-descriptor.cc', 'file-system.cc', + 'fmt.cc', 'git.cc', 'hash.cc', 'hilite.cc', @@ -52,6 +53,7 @@ libutil_headers = files( 'archive.hh', 'args/root.hh', 'args.hh', + 'backed-string-view.hh', 'box_ptr.hh', 'canon-path.hh', 'cgroup.hh', @@ -93,6 +95,7 @@ libutil_headers = files( 'monitor-fd.hh', 'mount.hh', 'namespaces.hh', + 'notifying-counter.hh', 'pool.hh', 'position.hh', 'print-elided.hh', diff --git a/src/libutil/notifying-counter.hh b/src/libutil/notifying-counter.hh new file mode 100644 index 000000000..dc58aac91 --- /dev/null +++ b/src/libutil/notifying-counter.hh @@ -0,0 +1,99 @@ +#pragma once +/// @file + +#include <cassert> +#include <functional> +#include <memory> + +namespace nix { + +template<std::integral T> +class NotifyingCounter +{ +private: + T counter; + std::function<void()> notify; + +public: + class Bump + { + friend class NotifyingCounter; + + struct SubOnFree + { + T delta; + + void operator()(NotifyingCounter * c) const + { + c->add(-delta); + } + }; + + // lightly misuse unique_ptr to get RAII types with destructor callbacks + std::unique_ptr<NotifyingCounter<T>, SubOnFree> at; + + Bump(NotifyingCounter<T> & at, T delta) : at(&at, {delta}) {} + + public: + Bump() = default; + Bump(decltype(nullptr)) {} + + T delta() const + { + return at ? at.get_deleter().delta : 0; + } + + void reset() + { + at.reset(); + } + }; + + explicit NotifyingCounter(std::function<void()> notify, T initial = 0) + : counter(initial) + , notify(std::move(notify)) + { + assert(this->notify); + } + + // bumps hold pointers to this, so we should neither copy nor move. + NotifyingCounter(const NotifyingCounter &) = delete; + NotifyingCounter & operator=(const NotifyingCounter &) = delete; + NotifyingCounter(NotifyingCounter &&) = delete; + NotifyingCounter & operator=(NotifyingCounter &&) = delete; + + T get() const + { + return counter; + } + + operator T() const + { + return counter; + } + + void add(T delta) + { + counter += delta; + notify(); + } + + NotifyingCounter & operator+=(T delta) + { + add(delta); + return *this; + } + + NotifyingCounter & operator++(int) + { + return *this += 1; + } + + Bump addTemporarily(T delta) + { + add(delta); + return Bump{*this, delta}; + } +}; + +} diff --git a/src/libutil/processes.cc b/src/libutil/processes.cc index 61e1ad556..eec592221 100644 --- a/src/libutil/processes.cc +++ b/src/libutil/processes.cc @@ -3,6 +3,7 @@ #include "finally.hh" #include "logging.hh" #include "processes.hh" +#include "strings.hh" #include "serialise.hh" #include "signals.hh" diff --git a/src/libutil/regex.cc b/src/libutil/regex.cc index a9e6c6bee..a12d13550 100644 --- a/src/libutil/regex.cc +++ b/src/libutil/regex.cc @@ -1,6 +1,9 @@ #include <string> #include <regex> +// Declared as extern in precompiled-headers.hh +template class std::basic_regex<char>; + namespace nix::regex { std::string quoteRegexChars(const std::string & raw) { diff --git a/src/libutil/serialise.cc b/src/libutil/serialise.cc index f1db05b0b..f509fedff 100644 --- a/src/libutil/serialise.cc +++ b/src/libutil/serialise.cc @@ -1,4 +1,5 @@ #include "serialise.hh" +#include "charptr-cast.hh" #include "signals.hh" #include <cstring> @@ -8,6 +9,46 @@ namespace nix { +namespace { +/** + * Convert a little-endian integer to host order. + */ +template<typename T> +T readLittleEndian(unsigned char * p) +{ + T x = 0; + for (size_t i = 0; i < sizeof(x); ++i, ++p) { + x |= ((T) *p) << (i * 8); + } + return x; +} +} + +template<typename T> +T readNum(Source & source) +{ + unsigned char buf[8]; + source(charptr_cast<char *>(buf), sizeof(buf)); + + auto n = readLittleEndian<uint64_t>(buf); + + if (n > (uint64_t) std::numeric_limits<T>::max()) + throw SerialisationError("serialised integer %d is too large for type '%s'", n, typeid(T).name()); + + return (T) n; +} + +template bool readNum<bool>(Source & source); + +template unsigned char readNum<unsigned char>(Source & source); + +template unsigned int readNum<unsigned int>(Source & source); + +template unsigned long readNum<unsigned long>(Source & source); +template long readNum<long>(Source & source); + +template unsigned long long readNum<unsigned long long>(Source & source); +template long long readNum<long long>(Source & source); void BufferedSink::operator () (std::string_view data) { @@ -126,7 +167,7 @@ size_t FdSource::readUnbuffered(char * data, size_t len) n = ::read(fd, data, len); } while (n == -1 && errno == EINTR); if (n == -1) { _good = false; throw SysError("reading from file"); } - if (n == 0) { _good = false; throw EndOfFile(std::string(*endOfFileError)); } + if (n == 0) { _good = false; throw EndOfFile(endOfFileError()); } read += n; return n; } @@ -137,6 +178,11 @@ bool FdSource::good() return _good; } +std::string FdSource::endOfFileError() const +{ + return specialEndOfFileError.has_value() ? *specialEndOfFileError : "unexpected end-of-file"; +} + size_t StringSource::read(char * data, size_t len) { diff --git a/src/libutil/serialise.hh b/src/libutil/serialise.hh index 8218db440..612658b2d 100644 --- a/src/libutil/serialise.hh +++ b/src/libutil/serialise.hh @@ -1,11 +1,10 @@ #pragma once ///@file -#include <concepts> #include <memory> +#include "charptr-cast.hh" #include "generator.hh" -#include "strings.hh" #include "types.hh" #include "file-descriptor.hh" @@ -152,7 +151,10 @@ struct FdSource : BufferedSource { int fd; size_t read = 0; - BackedStringView endOfFileError{"unexpected end-of-file"}; + /** Defaults to "unexpected end-of-file" */ + std::optional<std::string> specialEndOfFileError; + + std::string endOfFileError() const; FdSource() : fd(-1) { } FdSource(int fd) : fd(fd) { } @@ -385,7 +387,7 @@ struct SerializingTransform buf[5] = (n >> 40) & 0xff; buf[6] = (n >> 48) & 0xff; buf[7] = (unsigned char) (n >> 56) & 0xff; - return {reinterpret_cast<const char *>(buf.begin()), 8}; + return {charptr_cast<const char *>(buf.begin()), 8}; } static Bytes padding(size_t unpadded) @@ -417,6 +419,9 @@ struct SerializingTransform void writePadding(size_t len, Sink & sink); +// NOLINTBEGIN(cppcoreguidelines-avoid-capturing-lambda-coroutines): +// These coroutines do their entire job before the semicolon and are not +// retained, so they live long enough. inline Sink & operator<<(Sink & sink, uint64_t u) { return sink << [&]() -> WireFormatGenerator { co_yield u; }(); @@ -441,23 +446,12 @@ inline Sink & operator<<(Sink & sink, const Error & ex) { return sink << [&]() -> WireFormatGenerator { co_yield ex; }(); } +// NOLINTEND(cppcoreguidelines-avoid-capturing-lambda-coroutines) MakeError(SerialisationError, Error); template<typename T> -T readNum(Source & source) -{ - unsigned char buf[8]; - source((char *) buf, sizeof(buf)); - - auto n = readLittleEndian<uint64_t>(buf); - - if (n > (uint64_t) std::numeric_limits<T>::max()) - throw SerialisationError("serialised integer %d is too large for type '%s'", n, typeid(T).name()); - - return (T) n; -} - +T readNum(Source & source); inline unsigned int readInt(Source & source) { @@ -540,13 +534,17 @@ struct FramedSource : Source ~FramedSource() { - if (!eof) { - while (true) { - auto n = readInt(from); - if (!n) break; - std::vector<char> data(n); - from(data.data(), n); + try { + if (!eof) { + while (true) { + auto n = readInt(from); + if (!n) break; + std::vector<char> data(n); + from(data.data(), n); + } } + } catch (...) { + ignoreException(); } } diff --git a/src/libutil/signals.cc b/src/libutil/signals.cc index a94c2802a..04a697d01 100644 --- a/src/libutil/signals.cc +++ b/src/libutil/signals.cc @@ -3,6 +3,7 @@ #include "sync.hh" #include "terminal.hh" +#include <map> #include <thread> namespace nix { diff --git a/src/libutil/source-path.cc b/src/libutil/source-path.cc index cfaac20c0..782005ef1 100644 --- a/src/libutil/source-path.cc +++ b/src/libutil/source-path.cc @@ -1,4 +1,5 @@ #include "source-path.hh" +#include "strings.hh" namespace nix { diff --git a/src/libutil/strings.hh b/src/libutil/strings.hh index 7330e2063..ebafab9ad 100644 --- a/src/libutil/strings.hh +++ b/src/libutil/strings.hh @@ -165,19 +165,6 @@ std::optional<N> string2Float(const std::string_view s); /** - * Convert a little-endian integer to host order. - */ -template<typename T> -T readLittleEndian(unsigned char * p) -{ - T x = 0; - for (size_t i = 0; i < sizeof(x); ++i, ++p) { - x |= ((T) *p) << (i * 8); - } - return x; -} - -/** * Convert a string to lower case. */ std::string toLower(const std::string & s); diff --git a/src/libutil/thread-pool.hh b/src/libutil/thread-pool.hh index 3db7ce88f..380e1a2d2 100644 --- a/src/libutil/thread-pool.hh +++ b/src/libutil/thread-pool.hh @@ -4,6 +4,7 @@ #include "error.hh" #include "sync.hh" +#include <map> #include <queue> #include <functional> #include <thread> diff --git a/src/libutil/types.hh b/src/libutil/types.hh index 13cb062fb..66c41fe59 100644 --- a/src/libutil/types.hh +++ b/src/libutil/types.hh @@ -1,17 +1,15 @@ #pragma once ///@file -#include "ref.hh" - #include <list> #include <optional> #include <set> #include <string> -#include <limits> +#include <string_view> #include <map> -#include <variant> #include <vector> #include <span> +#include <stdint.h> // IWYU pragma: keep (this is used literally everywhere) namespace nix { @@ -166,70 +164,4 @@ constexpr auto enumerate(T && iterable) template<class... Ts> struct overloaded : Ts... { using Ts::operator()...; }; template<class... Ts> overloaded(Ts...) -> overloaded<Ts...>; - - -/** - * This wants to be a little bit like rust's Cow type. - * Some parts of the evaluator benefit greatly from being able to reuse - * existing allocations for strings, but have to be able to also use - * newly allocated storage for values. - * - * We do not define implicit conversions, even with ref qualifiers, - * since those can easily become ambiguous to the reader and can degrade - * into copying behaviour we want to avoid. - */ -class BackedStringView { -private: - std::variant<std::string, std::string_view> data; - - /** - * Needed to introduce a temporary since operator-> must return - * a pointer. Without this we'd need to store the view object - * even when we already own a string. - */ - class Ptr { - private: - std::string_view view; - public: - Ptr(std::string_view view): view(view) {} - const std::string_view * operator->() const { return &view; } - }; - -public: - BackedStringView(std::string && s): data(std::move(s)) {} - BackedStringView(std::string_view sv): data(sv) {} - template<size_t N> - BackedStringView(const char (& lit)[N]): data(std::string_view(lit)) {} - - BackedStringView(const BackedStringView &) = delete; - BackedStringView & operator=(const BackedStringView &) = delete; - - /** - * We only want move operations defined since the sole purpose of - * this type is to avoid copies. - */ - BackedStringView(BackedStringView && other) = default; - BackedStringView & operator=(BackedStringView && other) = default; - - bool isOwned() const - { - return std::holds_alternative<std::string>(data); - } - - std::string toOwned() && - { - return isOwned() - ? std::move(std::get<std::string>(data)) - : std::string(std::get<std::string_view>(data)); - } - - std::string_view operator*() const - { - return isOwned() - ? std::get<std::string>(data) - : std::get<std::string_view>(data); - } - Ptr operator->() const { return Ptr(**this); } -}; - } diff --git a/src/libutil/url.hh b/src/libutil/url.hh index d2413ec0e..a821301ba 100644 --- a/src/libutil/url.hh +++ b/src/libutil/url.hh @@ -2,6 +2,7 @@ ///@file #include "error.hh" +#include <map> namespace nix { diff --git a/src/libutil/users.cc b/src/libutil/users.cc index a9a8a7353..ce36bad9b 100644 --- a/src/libutil/users.cc +++ b/src/libutil/users.cc @@ -36,7 +36,7 @@ Path getHome() std::optional<std::string> unownedUserHomeDir = {}; auto homeDir = getEnv("HOME"); if (homeDir) { - // Only use $HOME if doesn't exist or is owned by the current user. + // Only use `$HOME` if it exists and is owned by the current user. struct stat st; int result = stat(homeDir->c_str(), &st); if (result != 0) { diff --git a/src/libutil/variant-wrapper.hh b/src/libutil/variant-wrapper.hh index cedcb999c..a809cd2a4 100644 --- a/src/libutil/variant-wrapper.hh +++ b/src/libutil/variant-wrapper.hh @@ -8,6 +8,7 @@ * Force the default versions of all constructors (copy, move, copy * assignment). */ +// NOLINTBEGIN(bugprone-macro-parentheses) #define FORCE_DEFAULT_CONSTRUCTORS(CLASS_NAME) \ CLASS_NAME(const CLASS_NAME &) = default; \ CLASS_NAME(CLASS_NAME &) = default; \ @@ -15,6 +16,7 @@ \ CLASS_NAME & operator =(const CLASS_NAME &) = default; \ CLASS_NAME & operator =(CLASS_NAME &) = default; +// NOLINTEND(bugprone-macro-parentheses) /** * Make a wrapper constructor. All args are forwarded to the diff --git a/src/lix-doc/Cargo.toml b/src/lix-doc/Cargo.toml index 02494862f..b52e6bb3b 100644 --- a/src/lix-doc/Cargo.toml +++ b/src/lix-doc/Cargo.toml @@ -11,7 +11,7 @@ repository = "https://github.com/lf-/nix-doc" [dependencies] rnix = "0.11.0" # Necessary because rnix fails to export a critical trait (Rowan's AstNode). -rowan = "0.15.0" +rowan = "0.15.16" [dev-dependencies] expect-test = "1.1.0" diff --git a/src/lix-doc/meson.build b/src/lix-doc/meson.build index 9838984a5..132feebf3 100644 --- a/src/lix-doc/meson.build +++ b/src/lix-doc/meson.build @@ -1,5 +1,3 @@ -# The external crate rowan has an ambiguous pointer comparison warning, which -# we don't want to fail our whole build if werror is on. # FIXME: remove hack once we get rid of meson 1.4 rnix_name = 'rnix-0.11-rs' rowan_name = 'rowan-0.15-rs' @@ -8,8 +6,6 @@ if meson.version().version_compare('< 1.5') rowan_name = 'rowan-rs' endif -subproject(rowan_name, default_options : ['werror=false']) - rnix = dependency(rnix_name) rowan = dependency(rowan_name) diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc index 5762b0644..bc43aa7b1 100644 --- a/src/nix-store/nix-store.cc +++ b/src/nix-store/nix-store.cc @@ -1,6 +1,7 @@ #include "archive.hh" #include "derivations.hh" #include "dotgraph.hh" +#include "exit.hh" #include "globals.hh" #include "build-result.hh" #include "store-cast.hh" @@ -17,7 +18,6 @@ #include <iostream> #include <algorithm> -#include <cstdio> #include <sys/types.h> #include <sys/stat.h> diff --git a/src/nix/doctor.cc b/src/nix/doctor.cc index 4e1cfe8c0..17db3f764 100644 --- a/src/nix/doctor.cc +++ b/src/nix/doctor.cc @@ -7,6 +7,7 @@ #include "store-api.hh" #include "local-fs-store.hh" #include "worker-protocol.hh" +#include "exit.hh" using namespace nix; diff --git a/src/nix/main.cc b/src/nix/main.cc index e84e4f310..05c40db03 100644 --- a/src/nix/main.cc +++ b/src/nix/main.cc @@ -201,7 +201,7 @@ struct NixArgs : virtual MultiCommand, virtual MixCommonArgs, virtual RootArgs res["args"] = toJSON(); auto stores = nlohmann::json::object(); - for (auto & implem : *Implementations::registered) { + for (auto & implem : *StoreImplementations::registered) { auto storeConfig = implem.getConfig(); auto storeName = storeConfig->name(); auto & j = stores[storeName]; diff --git a/src/nix/verify.cc b/src/nix/verify.cc index 8783d4e04..d4ab352f4 100644 --- a/src/nix/verify.cc +++ b/src/nix/verify.cc @@ -1,10 +1,9 @@ #include "command.hh" #include "shared.hh" #include "store-api.hh" -#include "sync.hh" #include "thread-pool.hh" #include "signals.hh" -#include "references.hh" +#include "exit.hh" #include <atomic> diff --git a/src/pch/precompiled-headers.hh b/src/pch/precompiled-headers.hh index f52f1cab8..c417d27db 100644 --- a/src/pch/precompiled-headers.hh +++ b/src/pch/precompiled-headers.hh @@ -58,3 +58,19 @@ #include <unistd.h> #include <nlohmann/json.hpp> + +// This stuff is here to force the compiler to actually apply the extern +// template directives in all compilation units. To borrow a term, under +// complex microarchitectural conditions, clang ignores the extern template +// declaration, as revealed in the profile. +// +// In most cases, extern template works fine in the header itself. We don't +// have any idea why this happens. + +// Here because of all the regexes everywhere (it is infeasible to block instantiation everywhere) +// For some reason this does not actually prevent the instantiation of +// regex::_M_compile, and the regex compiler (my interpretation of what this is +// supposed to do is make the template bits out-of-line), but it *does* prevent +// a bunch of codegen of regex stuff, which seems to save about 30s on-cpu. +// Instantiated in libutil/regex.cc. +extern template class std::basic_regex<char>; |