diff options
Diffstat (limited to 'src')
-rw-r--r-- | src/libexpr/flake/flake.cc | 10 | ||||
-rw-r--r-- | src/libexpr/primops.cc | 4 | ||||
-rw-r--r-- | src/libfetchers/fetchers.hh | 2 | ||||
-rw-r--r-- | src/libmain/progress-bar.cc | 2 | ||||
-rw-r--r-- | src/libstore/build.cc | 88 | ||||
-rw-r--r-- | src/libstore/daemon.cc | 14 | ||||
-rw-r--r-- | src/libstore/store-api.hh | 36 | ||||
-rw-r--r-- | src/libutil/args.hh | 2 | ||||
-rw-r--r-- | src/nix/hash.cc | 1 |
9 files changed, 122 insertions, 37 deletions
diff --git a/src/libexpr/flake/flake.cc b/src/libexpr/flake/flake.cc index 460eea5ea..b4ede542c 100644 --- a/src/libexpr/flake/flake.cc +++ b/src/libexpr/flake/flake.cc @@ -48,17 +48,17 @@ static std::tuple<fetchers::Tree, FlakeRef, FlakeRef> fetchOrSubstituteTree( resolvedRef = originalRef.resolve(state.store); auto fetchedResolved = lookupInFlakeCache(flakeCache, originalRef); if (!fetchedResolved) fetchedResolved.emplace(resolvedRef.fetchTree(state.store)); - flakeCache.push_back({resolvedRef, fetchedResolved.value()}); - fetched.emplace(fetchedResolved.value()); + flakeCache.push_back({resolvedRef, *fetchedResolved}); + fetched.emplace(*fetchedResolved); } else { throw Error("'%s' is an indirect flake reference, but registry lookups are not allowed", originalRef); } } - flakeCache.push_back({originalRef, fetched.value()}); + flakeCache.push_back({originalRef, *fetched}); } - auto [tree, lockedRef] = fetched.value(); + auto [tree, lockedRef] = *fetched; debug("got tree '%s' from '%s'", state.store->printStorePath(tree.storePath), lockedRef); @@ -247,7 +247,7 @@ Flake getFlake(EvalState & state, const FlakeRef & originalRef, bool allowLookup } /* Compute an in-memory lock file for the specified top-level flake, - and optionally write it to file, it the flake is writable. */ + and optionally write it to file, if the flake is writable. */ LockedFlake lockFlake( EvalState & state, const FlakeRef & topRef, diff --git a/src/libexpr/primops.cc b/src/libexpr/primops.cc index 9cfe3f402..2b304aab0 100644 --- a/src/libexpr/primops.cc +++ b/src/libexpr/primops.cc @@ -2236,6 +2236,10 @@ static RegisterPrimOp primop_catAttrs({ static void prim_functionArgs(EvalState & state, const Pos & pos, Value * * args, Value & v) { state.forceValue(*args[0], pos); + if (args[0]->type == tPrimOpApp || args[0]->type == tPrimOp) { + state.mkAttrs(v, 0); + return; + } if (args[0]->type != tLambda) throw TypeError({ .hint = hintfmt("'functionArgs' requires a function"), diff --git a/src/libfetchers/fetchers.hh b/src/libfetchers/fetchers.hh index 89b1e6e7d..191e91978 100644 --- a/src/libfetchers/fetchers.hh +++ b/src/libfetchers/fetchers.hh @@ -73,7 +73,7 @@ public: StorePath computeStorePath(Store & store) const; - // Convience functions for common attributes. + // Convenience functions for common attributes. std::string getType() const; std::optional<Hash> getNarHash() const; std::optional<std::string> getRef() const; diff --git a/src/libmain/progress-bar.cc b/src/libmain/progress-bar.cc index be3c06a38..07b45b3b5 100644 --- a/src/libmain/progress-bar.cc +++ b/src/libmain/progress-bar.cc @@ -256,7 +256,7 @@ public: } else if (type == resBuildLogLine || type == resPostBuildLogLine) { - auto lastLine = trim(getS(fields, 0)); + auto lastLine = chomp(getS(fields, 0)); if (!lastLine.empty()) { auto i = state->its.find(act); assert(i != state->its.end()); diff --git a/src/libstore/build.cc b/src/libstore/build.cc index 3fc24b221..629e05f3a 100644 --- a/src/libstore/build.cc +++ b/src/libstore/build.cc @@ -296,9 +296,21 @@ public: ~Worker(); /* Make a goal (with caching). */ - GoalPtr makeDerivationGoal(const StorePath & drvPath, const StringSet & wantedOutputs, BuildMode buildMode = bmNormal); - std::shared_ptr<DerivationGoal> makeBasicDerivationGoal(const StorePath & drvPath, - const BasicDerivation & drv, BuildMode buildMode = bmNormal); + + /* derivation goal */ +private: + std::shared_ptr<DerivationGoal> makeDerivationGoalCommon( + const StorePath & drvPath, const StringSet & wantedOutputs, + std::function<std::shared_ptr<DerivationGoal>()> mkDrvGoal); +public: + std::shared_ptr<DerivationGoal> makeDerivationGoal( + const StorePath & drvPath, + const StringSet & wantedOutputs, BuildMode buildMode = bmNormal); + std::shared_ptr<DerivationGoal> makeBasicDerivationGoal( + const StorePath & drvPath, const BasicDerivation & drv, + const StringSet & wantedOutputs, BuildMode buildMode = bmNormal); + + /* substitution goal */ GoalPtr makeSubstitutionGoal(const StorePath & storePath, RepairFlag repair = NoRepair, std::optional<ContentAddress> ca = std::nullopt); /* Remove a dead goal. */ @@ -949,10 +961,12 @@ private: friend struct RestrictedStore; public: - DerivationGoal(const StorePath & drvPath, const StringSet & wantedOutputs, - Worker & worker, BuildMode buildMode = bmNormal); + DerivationGoal(const StorePath & drvPath, + const StringSet & wantedOutputs, Worker & worker, + BuildMode buildMode = bmNormal); DerivationGoal(const StorePath & drvPath, const BasicDerivation & drv, - Worker & worker, BuildMode buildMode = bmNormal); + const StringSet & wantedOutputs, Worker & worker, + BuildMode buildMode = bmNormal); ~DerivationGoal(); /* Whether we need to perform hash rewriting if there are valid output paths. */ @@ -1085,8 +1099,8 @@ private: const Path DerivationGoal::homeDir = "/homeless-shelter"; -DerivationGoal::DerivationGoal(const StorePath & drvPath, const StringSet & wantedOutputs, - Worker & worker, BuildMode buildMode) +DerivationGoal::DerivationGoal(const StorePath & drvPath, + const StringSet & wantedOutputs, Worker & worker, BuildMode buildMode) : Goal(worker) , useDerivation(true) , drvPath(drvPath) @@ -1094,7 +1108,9 @@ DerivationGoal::DerivationGoal(const StorePath & drvPath, const StringSet & want , buildMode(buildMode) { state = &DerivationGoal::getDerivation; - name = fmt("building of '%s'", worker.store.printStorePath(this->drvPath)); + name = fmt( + "building of '%s' from .drv file", + StorePathWithOutputs { drvPath, wantedOutputs }.to_string(worker.store)); trace("created"); mcExpectedBuilds = std::make_unique<MaintainCount<uint64_t>>(worker.expectedBuilds); @@ -1103,15 +1119,18 @@ DerivationGoal::DerivationGoal(const StorePath & drvPath, const StringSet & want DerivationGoal::DerivationGoal(const StorePath & drvPath, const BasicDerivation & drv, - Worker & worker, BuildMode buildMode) + const StringSet & wantedOutputs, Worker & worker, BuildMode buildMode) : Goal(worker) , useDerivation(false) , drvPath(drvPath) + , wantedOutputs(wantedOutputs) , buildMode(buildMode) { this->drv = std::make_unique<BasicDerivation>(BasicDerivation(drv)); state = &DerivationGoal::haveDerivation; - name = fmt("building of %s", StorePathWithOutputs { drvPath, drv.outputNames() }.to_string(worker.store)); + name = fmt( + "building of '%s' from in-memory derivation", + StorePathWithOutputs { drvPath, drv.outputNames() }.to_string(worker.store)); trace("created"); mcExpectedBuilds = std::make_unique<MaintainCount<uint64_t>>(worker.expectedBuilds); @@ -5040,35 +5059,52 @@ Worker::~Worker() } -GoalPtr Worker::makeDerivationGoal(const StorePath & path, - const StringSet & wantedOutputs, BuildMode buildMode) +std::shared_ptr<DerivationGoal> Worker::makeDerivationGoalCommon( + const StorePath & drvPath, + const StringSet & wantedOutputs, + std::function<std::shared_ptr<DerivationGoal>()> mkDrvGoal) { - GoalPtr goal = derivationGoals[path].lock(); // FIXME - if (!goal) { - goal = std::make_shared<DerivationGoal>(path, wantedOutputs, *this, buildMode); - derivationGoals.insert_or_assign(path, goal); + WeakGoalPtr & abstract_goal_weak = derivationGoals[drvPath]; + GoalPtr abstract_goal = abstract_goal_weak.lock(); // FIXME + std::shared_ptr<DerivationGoal> goal; + if (!abstract_goal) { + goal = mkDrvGoal(); + abstract_goal_weak = goal; wakeUp(goal); - } else - (dynamic_cast<DerivationGoal *>(goal.get()))->addWantedOutputs(wantedOutputs); + } else { + goal = std::dynamic_pointer_cast<DerivationGoal>(abstract_goal); + assert(goal); + goal->addWantedOutputs(wantedOutputs); + } return goal; } +std::shared_ptr<DerivationGoal> Worker::makeDerivationGoal(const StorePath & drvPath, + const StringSet & wantedOutputs, BuildMode buildMode) +{ + return makeDerivationGoalCommon(drvPath, wantedOutputs, [&]() { + return std::make_shared<DerivationGoal>(drvPath, wantedOutputs, *this, buildMode); + }); +} + + std::shared_ptr<DerivationGoal> Worker::makeBasicDerivationGoal(const StorePath & drvPath, - const BasicDerivation & drv, BuildMode buildMode) + const BasicDerivation & drv, const StringSet & wantedOutputs, BuildMode buildMode) { - auto goal = std::make_shared<DerivationGoal>(drvPath, drv, *this, buildMode); - wakeUp(goal); - return goal; + return makeDerivationGoalCommon(drvPath, wantedOutputs, [&]() { + return std::make_shared<DerivationGoal>(drvPath, drv, wantedOutputs, *this, buildMode); + }); } GoalPtr Worker::makeSubstitutionGoal(const StorePath & path, RepairFlag repair, std::optional<ContentAddress> ca) { - GoalPtr goal = substitutionGoals[path].lock(); // FIXME + WeakGoalPtr & goal_weak = substitutionGoals[path]; + GoalPtr goal = goal_weak.lock(); // FIXME if (!goal) { goal = std::make_shared<SubstitutionGoal>(path, *this, repair, ca); - substitutionGoals.insert_or_assign(path, goal); + goal_weak = goal; wakeUp(goal); } return goal; @@ -5499,7 +5535,7 @@ BuildResult LocalStore::buildDerivation(const StorePath & drvPath, const BasicDe BuildMode buildMode) { Worker worker(*this); - auto goal = worker.makeBasicDerivationGoal(drvPath, drv, buildMode); + auto goal = worker.makeBasicDerivationGoal(drvPath, drv, {}, buildMode); BuildResult result; diff --git a/src/libstore/daemon.cc b/src/libstore/daemon.cc index 83f8968b0..ec3391a6d 100644 --- a/src/libstore/daemon.cc +++ b/src/libstore/daemon.cc @@ -546,6 +546,20 @@ static void performOp(TunnelLogger * logger, ref<Store> store, are in fact content-addressed if we don't trust them. */ assert(derivationIsCA(drv.type()) || trusted); + /* Recompute the derivation path when we cannot trust the original. */ + if (!trusted) { + /* Recomputing the derivation path for input-address derivations + makes it harder to audit them after the fact, since we need the + original not-necessarily-resolved derivation to verify the drv + derivation as adequate claim to the input-addressed output + paths. */ + assert(derivationIsCA(drv.type())); + + Derivation drv2; + static_cast<BasicDerivation &>(drv2) = drv; + drvPath = writeDerivation(*store, Derivation { drv2 }); + } + auto res = store->buildDerivation(drvPath, drv, buildMode); logger->stopWork(); to << res.status << res.errorMsg; diff --git a/src/libstore/store-api.hh b/src/libstore/store-api.hh index 591140874..3ccee4f75 100644 --- a/src/libstore/store-api.hh +++ b/src/libstore/store-api.hh @@ -479,8 +479,38 @@ public: BuildMode buildMode = bmNormal); /* Build a single non-materialized derivation (i.e. not from an - on-disk .drv file). Note that ‘drvPath’ is only used for - informational purposes. */ + on-disk .drv file). + + ‘drvPath’ is used to deduplicate worker goals so it is imperative that + is correct. That said, it doesn't literally need to be store path that + would be calculated from writing this derivation to the store: it is OK + if it instead is that of a Derivation which would resolve to this (by + taking the outputs of it's input derivations and adding them as input + sources) such that the build time referenceable-paths are the same. + + In the input-addressed case, we usually *do* use an "original" + unresolved derivations's path, as that is what will be used in the + `buildPaths` case. Also, the input-addressed output paths are verified + only by that contents of that specific unresolved derivation, so it is + nice to keep that information around so if the original derivation is + ever obtained later, it can be verified whether the trusted user in fact + used the proper output path. + + In the content-addressed case, we want to always use the + resolved drv path calculated from the provided derivation. This serves + two purposes: + + - It keeps the operation trustless, by ruling out a maliciously + invalid drv path corresponding to a non-resolution-equivalent + derivation. + + - For the floating case in particular, it ensures that the derivation + to output mapping respects the resolution equivalence relation, so + one cannot choose different resolution-equivalent derivations to + subvert dependency coherence (i.e. the property that one doesn't end + up with multiple different versions of dependencies without + explicitly choosing to allow it). + */ virtual BuildResult buildDerivation(const StorePath & drvPath, const BasicDerivation & drv, BuildMode buildMode = bmNormal) = 0; @@ -517,7 +547,7 @@ public: - The collector isn't running, or it's just started but hasn't acquired the GC lock yet. In that case we get and release the lock right away, then exit. The collector scans the - permanent root and sees our's. + permanent root and sees ours. In either case the permanent root is seen by the collector. */ virtual void syncWithGC() { }; diff --git a/src/libutil/args.hh b/src/libutil/args.hh index 3c1f87f7e..f41242e17 100644 --- a/src/libutil/args.hh +++ b/src/libutil/args.hh @@ -192,7 +192,7 @@ public: { expectArgs({ .label = label, - .optional = true, + .optional = optional, .handler = {dest} }); } diff --git a/src/nix/hash.cc b/src/nix/hash.cc index 0eca4f8ea..494f00a20 100644 --- a/src/nix/hash.cc +++ b/src/nix/hash.cc @@ -44,6 +44,7 @@ struct CmdHash : Command switch (mode) { case FileIngestionMethod::Flat: d = "print cryptographic hash of a regular file"; + break; case FileIngestionMethod::Recursive: d = "print cryptographic hash of the NAR serialisation of a path"; }; |