Merge remote-tracking branch 'upstream/master' into ca-floating-upstream

17 files changed, 203 insertions, 162 deletions

diff --git a/src/libexpr/flake/flake.cc b/src/libexpr/flake/flake.cc
index 01f464859..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);
@@ -215,10 +215,9 @@ static Flake getFlake(
 
     if (auto outputs = vInfo.attrs->get(sOutputs)) {
         expectType(state, tLambda, *outputs->value, *outputs->pos);
-        flake.vOutputs = allocRootValue(outputs->value);
 
-        if ((*flake.vOutputs)->lambda.fun->matchAttrs) {
-            for (auto & formal : (*flake.vOutputs)->lambda.fun->formals->formals) {
+        if (outputs->value->lambda.fun->matchAttrs) {
+            for (auto & formal : outputs->value->lambda.fun->formals->formals) {
                 if (formal.name != state.sSelf)
                     flake.inputs.emplace(formal.name, FlakeInput {
                         .ref = parseFlakeRef(formal.name)
@@ -248,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,
@@ -367,7 +366,7 @@ LockedFlake lockFlake(
 
                 /* If we have an --update-input flag for an input
                    of this input, then we must fetch the flake to
-                   to update it. */
+                   update it. */
                 auto lb = lockFlags.inputUpdates.lower_bound(inputPath);
 
                 auto hasChildUpdate =
diff --git a/src/libexpr/flake/flake.hh b/src/libexpr/flake/flake.hh
index c2bb2888b..69c779af8 100644
--- a/src/libexpr/flake/flake.hh
+++ b/src/libexpr/flake/flake.hh
@@ -34,7 +34,6 @@ struct Flake
     std::optional<std::string> description;
     std::shared_ptr<const fetchers::Tree> sourceInfo;
     FlakeInputs inputs;
-    RootValue vOutputs;
     ~Flake();
 };
 
diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y
index 24b21f7da..a4c84c526 100644
--- a/src/libexpr/parser.y
+++ b/src/libexpr/parser.y
@@ -614,8 +614,7 @@ Path resolveExprPath(Path path)
         // Basic cycle/depth limit to avoid infinite loops.
         if (++followCount >= maxFollow)
             throw Error("too many symbolic links encountered while traversing the path '%s'", path);
-        if (lstat(path.c_str(), &st))
-            throw SysError("getting status of '%s'", path);
+        st = lstat(path);
         if (!S_ISLNK(st.st_mode)) break;
         path = absPath(readLink(path), dirOf(path));
     }
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 7761121ba..0a6f2ccb5 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. */
@@ -1087,8 +1101,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)
@@ -1096,7 +1110,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);
@@ -1105,15 +1121,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);
@@ -1647,6 +1666,13 @@ void DerivationGoal::tryToBuild()
 
     actLock.reset();
 
+    state = &DerivationGoal::tryLocalBuild;
+    worker.wakeUp(shared_from_this());
+}
+
+void DerivationGoal::tryLocalBuild() {
+    bool buildLocally = buildMode != bmNormal || parsedDrv->willBuildLocally(worker.store);
+
     /* Make sure that we are allowed to start a build.  If this
        derivation prefers to be done locally, do it even if
        maxBuildJobs is 0. */
@@ -1657,12 +1683,6 @@ void DerivationGoal::tryToBuild()
         return;
     }
 
-    state = &DerivationGoal::tryLocalBuild;
-    worker.wakeUp(shared_from_this());
-}
-
-void DerivationGoal::tryLocalBuild() {
-
     /* If `build-users-group' is not empty, then we have to build as
        one of the members of that group. */
     if (settings.buildUsersGroup != "" && getuid() == 0) {
@@ -1710,7 +1730,34 @@ void DerivationGoal::tryLocalBuild() {
 }
 
 
-void replaceValidPath(const Path & storePath, const Path tmpPath)
+static void chmod_(const Path & path, mode_t mode)
+{
+    if (chmod(path.c_str(), mode) == -1)
+        throw SysError("setting permissions on '%s'", path);
+}
+
+
+/* Move/rename path 'src' to 'dst'. Temporarily make 'src' writable if
+   it's a directory and we're not root (to be able to update the
+   directory's parent link ".."). */
+static void movePath(const Path & src, const Path & dst)
+{
+    auto st = lstat(src);
+
+    bool changePerm = (geteuid() && S_ISDIR(st.st_mode) && !(st.st_mode & S_IWUSR));
+
+    if (changePerm)
+        chmod_(src, st.st_mode | S_IWUSR);
+
+    if (rename(src.c_str(), dst.c_str()))
+        throw SysError("renaming '%1%' to '%2%'", src, dst);
+
+    if (changePerm)
+        chmod_(dst, st.st_mode);
+}
+
+
+void replaceValidPath(const Path & storePath, const Path & tmpPath)
 {
     /* We can't atomically replace storePath (the original) with
        tmpPath (the replacement), so we have to move it out of the
@@ -1718,11 +1765,20 @@ void replaceValidPath(const Path & storePath, const Path tmpPath)
        we're repairing (say) Glibc, we end up with a broken system. */
     Path oldPath = (format("%1%.old-%2%-%3%") % storePath % getpid() % random()).str();
     if (pathExists(storePath))
-        rename(storePath.c_str(), oldPath.c_str());
-    if (rename(tmpPath.c_str(), storePath.c_str()) == -1) {
-        rename(oldPath.c_str(), storePath.c_str()); // attempt to recover
-        throw SysError("moving '%s' to '%s'", tmpPath, storePath);
+        movePath(storePath, oldPath);
+
+    try {
+        movePath(tmpPath, storePath);
+    } catch (...) {
+        try {
+            // attempt to recover
+            movePath(oldPath, storePath);
+        } catch (...) {
+            ignoreException();
+        }
+        throw;
     }
+
     deletePath(oldPath);
 }
 
@@ -2043,13 +2099,6 @@ HookReply DerivationGoal::tryBuildHook()
 }
 
 
-static void chmod_(const Path & path, mode_t mode)
-{
-    if (chmod(path.c_str(), mode) == -1)
-        throw SysError("setting permissions on '%s'", path);
-}
-
-
 int childEntry(void * arg)
 {
     ((DerivationGoal *) arg)->runChild();
@@ -2405,10 +2454,7 @@ void DerivationGoal::startBuilder()
         for (auto & i : inputPaths) {
             auto p = worker.store.printStorePath(i);
             Path r = worker.store.toRealPath(p);
-            struct stat st;
-            if (lstat(r.c_str(), &st))
-                throw SysError("getting attributes of path '%s'", p);
-            if (S_ISDIR(st.st_mode))
+            if (S_ISDIR(lstat(r).st_mode))
                 dirsInChroot.insert_or_assign(p, r);
             else
                 linkOrCopy(r, chrootRootDir + p);
@@ -3182,9 +3228,7 @@ void DerivationGoal::addDependency(const StorePath & path)
             if (pathExists(target))
                 throw Error("store path '%s' already exists in the sandbox", worker.store.printStorePath(path));
 
-            struct stat st;
-            if (lstat(source.c_str(), &st))
-                throw SysError("getting attributes of path '%s'", source);
+            auto st = lstat(source);
 
             if (S_ISDIR(st.st_mode)) {
 
@@ -3773,29 +3817,6 @@ void DerivationGoal::runChild()
 }
 
 
-static void moveCheckToStore(const Path & src, const Path & dst)
-{
-    /* For the rename of directory to succeed, we must be running as root or
-       the directory must be made temporarily writable (to update the
-       directory's parent link ".."). */
-    struct stat st;
-    if (lstat(src.c_str(), &st) == -1) {
-        throw SysError("getting attributes of path '%1%'", src);
-    }
-
-    bool changePerm = (geteuid() && S_ISDIR(st.st_mode) && !(st.st_mode & S_IWUSR));
-
-    if (changePerm)
-        chmod_(src, st.st_mode | S_IWUSR);
-
-    if (rename(src.c_str(), dst.c_str()))
-        throw SysError("renaming '%1%' to '%2%'", src, dst);
-
-    if (changePerm)
-        chmod_(dst, st.st_mode);
-}
-
-
 void DerivationGoal::registerOutputs()
 {
     /* When using a build hook, the build hook can register the output
@@ -3951,7 +3972,6 @@ void DerivationGoal::registerOutputs()
                 outputRewrites[std::string { scratchPath.hashPart() }] = std::string { finalStorePath.hashPart() };
         };
 
-        bool rewritten = false;
         std::optional<StorePathSet> referencesOpt = std::visit(overloaded {
             [&](AlreadyRegistered skippedFinalPath) -> std::optional<StorePathSet> {
                 finish(skippedFinalPath.path);
@@ -3982,7 +4002,9 @@ void DerivationGoal::registerOutputs()
                 StringSource source(*sink.s);
                 restorePath(actualPath, source);
 
-                rewritten = true;
+                /* FIXME: set proper permissions in restorePath() so
+                   we don't have to do another traversal. */
+                canonicalisePathMetaData(actualPath, -1, inodesSeen);
             }
         };
 
@@ -4065,7 +4087,7 @@ void DerivationGoal::registerOutputs()
             [&](DerivationOutputInputAddressed output) {
                 /* input-addressed case */
                 auto requiredFinalPath = output.path;
-                /* Preemtively add rewrite rule for final hash, as that is
+                /* Preemptively add rewrite rule for final hash, as that is
                    what the NAR hash will use rather than normalized-self references */
                 if (scratchPath != requiredFinalPath)
                     outputRewrites.insert_or_assign(
@@ -4139,44 +4161,21 @@ void DerivationGoal::registerOutputs()
                    else. No moving needed. */
                 assert(newInfo.ca);
             } else {
-                /* Temporarily add write perm so we can move, will be fixed
-                   later. */
-                {
-                    struct stat st;
-                    auto & mode = st.st_mode;
-                    if (lstat(actualPath.c_str(), &st))
-                        throw SysError("getting attributes of path '%1%'", actualPath);
-                    mode |= 0200;
-                    /* Try to change the perms, but only if the file isn't a
-                       symlink as symlinks permissions are mostly ignored and
-                       calling `chmod` on it will just forward the call to the
-                       target of the link. */
-                    if (!S_ISLNK(st.st_mode))
-                        if (chmod(actualPath.c_str(), mode) == -1)
-                            throw SysError("changing mode of '%1%' to %2$o", actualPath, mode);
-                }
-                if (rename(
-                        actualPath.c_str(),
-                        worker.store.toRealPath(finalDestPath).c_str()) == -1)
-                    throw SysError("moving build output '%1%' from it's temporary location to the Nix store", finalDestPath);
-                actualPath = worker.store.toRealPath(finalDestPath);
+                auto destPath = worker.store.toRealPath(finalDestPath);
+                movePath(actualPath, destPath);
+                actualPath = destPath;
             }
         }
 
-        /* Get rid of all weird permissions.  This also checks that
-           all files are owned by the build user, if applicable. */
-        canonicalisePathMetaData(actualPath,
-            buildUser && !rewritten ? buildUser->getUID() : -1, inodesSeen);
-
         if (buildMode == bmCheck) {
             if (!worker.store.isValidPath(newInfo.path)) continue;
             ValidPathInfo oldInfo(*worker.store.queryPathInfo(newInfo.path));
             if (newInfo.narHash != oldInfo.narHash) {
                 worker.checkMismatch = true;
                 if (settings.runDiffHook || settings.keepFailed) {
-                    Path dst = worker.store.toRealPath(finalDestPath + checkSuffix);
+                    auto dst = worker.store.toRealPath(finalDestPath + checkSuffix);
                     deletePath(dst);
-                    moveCheckToStore(actualPath, dst);
+                    movePath(actualPath, dst);
 
                     handleDiffHook(
                         buildUser ? buildUser->getUID() : getuid(),
@@ -5109,35 +5108,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;
@@ -5568,7 +5584,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/gc.cc b/src/libstore/gc.cc
index 08b53c702..518a357ef 100644
--- a/src/libstore/gc.cc
+++ b/src/libstore/gc.cc
@@ -663,9 +663,7 @@ void LocalStore::removeUnusedLinks(const GCState & state)
         if (name == "." || name == "..") continue;
         Path path = linksDir + "/" + name;
 
-        struct stat st;
-        if (lstat(path.c_str(), &st) == -1)
-            throw SysError("statting '%1%'", path);
+        auto st = lstat(path);
 
         if (st.st_nlink != 1) {
             actualSize += st.st_size;
diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc
index 96f1eaacc..d29236a9c 100644
--- a/src/libstore/local-store.cc
+++ b/src/libstore/local-store.cc
@@ -114,8 +114,7 @@ LocalStore::LocalStore(const Params & params)
         Path path = realStoreDir;
         struct stat st;
         while (path != "/") {
-            if (lstat(path.c_str(), &st))
-                throw SysError("getting status of '%1%'", path);
+            st = lstat(path);
             if (S_ISLNK(st.st_mode))
                 throw Error(
                         "the path '%1%' is a symlink; "
@@ -419,10 +418,7 @@ static void canonicaliseTimestampAndPermissions(const Path & path, const struct
 
 void canonicaliseTimestampAndPermissions(const Path & path)
 {
-    struct stat st;
-    if (lstat(path.c_str(), &st))
-        throw SysError("getting attributes of path '%1%'", path);
-    canonicaliseTimestampAndPermissions(path, st);
+    canonicaliseTimestampAndPermissions(path, lstat(path));
 }
 
 
@@ -440,9 +436,7 @@ static void canonicalisePathMetaData_(const Path & path, uid_t fromUid, InodesSe
     }
 #endif
 
-    struct stat st;
-    if (lstat(path.c_str(), &st))
-        throw SysError("getting attributes of path '%1%'", path);
+    auto st = lstat(path);
 
     /* Really make sure that the path is of a supported type. */
     if (!(S_ISREG(st.st_mode) || S_ISDIR(st.st_mode) || S_ISLNK(st.st_mode)))
@@ -478,8 +472,7 @@ static void canonicalisePathMetaData_(const Path & path, uid_t fromUid, InodesSe
        ensure that we don't fail on hard links within the same build
        (i.e. "touch $out/foo; ln $out/foo $out/bar"). */
     if (fromUid != (uid_t) -1 && st.st_uid != fromUid) {
-        assert(!S_ISDIR(st.st_mode));
-        if (inodesSeen.find(Inode(st.st_dev, st.st_ino)) == inodesSeen.end())
+        if (S_ISDIR(st.st_mode) || !inodesSeen.count(Inode(st.st_dev, st.st_ino)))
             throw BuildError("invalid ownership on file '%1%'", path);
         mode_t mode = st.st_mode & ~S_IFMT;
         assert(S_ISLNK(st.st_mode) || (st.st_uid == geteuid() && (mode == 0444 || mode == 0555) && st.st_mtime == mtimeStore));
@@ -522,9 +515,7 @@ void canonicalisePathMetaData(const Path & path, uid_t fromUid, InodesSeen & ino
 
     /* On platforms that don't have lchown(), the top-level path can't
        be a symlink, since we can't change its ownership. */
-    struct stat st;
-    if (lstat(path.c_str(), &st))
-        throw SysError("getting attributes of path '%1%'", path);
+    auto st = lstat(path);
 
     if (st.st_uid != geteuid()) {
         assert(S_ISLNK(st.st_mode));
@@ -1495,7 +1486,7 @@ static void makeMutable(const Path & path)
 {
     checkInterrupt();
 
-    struct stat st = lstat(path);
+    auto st = lstat(path);
 
     if (!S_ISDIR(st.st_mode) && !S_ISREG(st.st_mode)) return;
 
diff --git a/src/libstore/optimise-store.cc b/src/libstore/optimise-store.cc
index e4b4b6213..c032a5e22 100644
--- a/src/libstore/optimise-store.cc
+++ b/src/libstore/optimise-store.cc
@@ -17,9 +17,7 @@ namespace nix {
 
 static void makeWritable(const Path & path)
 {
-    struct stat st;
-    if (lstat(path.c_str(), &st))
-        throw SysError("getting attributes of path '%1%'", path);
+    auto st = lstat(path);
     if (chmod(path.c_str(), st.st_mode | S_IWUSR) == -1)
         throw SysError("changing writability of '%1%'", path);
 }
@@ -94,9 +92,7 @@ void LocalStore::optimisePath_(Activity * act, OptimiseStats & stats,
 {
     checkInterrupt();
 
-    struct stat st;
-    if (lstat(path.c_str(), &st))
-        throw SysError("getting attributes of path '%1%'", path);
+    auto st = lstat(path);
 
 #if __APPLE__
     /* HFS/macOS has some undocumented security feature disabling hardlinking for
@@ -187,9 +183,7 @@ void LocalStore::optimisePath_(Activity * act, OptimiseStats & stats,
 
     /* Yes!  We've seen a file with the same contents.  Replace the
        current file with a hard link to that file. */
-    struct stat stLink;
-    if (lstat(linkPath.c_str(), &stLink))
-        throw SysError("getting attributes of path '%1%'", linkPath);
+    auto stLink = lstat(linkPath);
 
     if (st.st_ino == stLink.st_ino) {
         debug(format("'%1%' is already linked to '%2%'") % path % linkPath);
diff --git a/src/libstore/profiles.cc b/src/libstore/profiles.cc
index c20386e2b..c3809bad7 100644
--- a/src/libstore/profiles.cc
+++ b/src/libstore/profiles.cc
@@ -39,13 +39,10 @@ std::pair<Generations, std::optional<GenerationNumber>> findGenerations(Path pro
     for (auto & i : readDirectory(profileDir)) {
         if (auto n = parseName(profileName, i.name)) {
             auto path = profileDir + "/" + i.name;
-            struct stat st;
-            if (lstat(path.c_str(), &st) != 0)
-                throw SysError("statting '%1%'", path);
             gens.push_back({
                 .number = *n,
                 .path = path,
-                .creationTime = st.st_mtime
+                .creationTime = lstat(path).st_mtime
             });
         }
     }
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/archive.cc b/src/libutil/archive.cc
index 14399dea3..6ad9fa238 100644
--- a/src/libutil/archive.cc
+++ b/src/libutil/archive.cc
@@ -27,6 +27,8 @@ struct ArchiveSettings : Config
         #endif
         "use-case-hack",
         "Whether to enable a Darwin-specific hack for dealing with file name collisions."};
+    Setting<bool> preallocateContents{this, true, "preallocate-contents",
+        "Whether to preallocate files when writing objects with known size."};
 };
 
 static ArchiveSettings archiveSettings;
@@ -66,9 +68,7 @@ static void dump(const Path & path, Sink & sink, PathFilter & filter)
 {
     checkInterrupt();
 
-    struct stat st;
-    if (lstat(path.c_str(), &st))
-        throw SysError("getting attributes of path '%1%'", path);
+    auto st = lstat(path);
 
     sink << "(";
 
@@ -325,6 +325,9 @@ struct RestoreSink : ParseSink
 
     void preallocateContents(uint64_t len)
     {
+        if (!archiveSettings.preallocateContents)
+            return;
+
 #if HAVE_POSIX_FALLOCATE
         if (len) {
             errno = posix_fallocate(fd.get(), 0, len);
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";
         };
diff --git a/src/resolve-system-dependencies/resolve-system-dependencies.cc b/src/resolve-system-dependencies/resolve-system-dependencies.cc
index 434ad80a6..d30227e4e 100644
--- a/src/resolve-system-dependencies/resolve-system-dependencies.cc
+++ b/src/resolve-system-dependencies/resolve-system-dependencies.cc
@@ -111,11 +111,7 @@ std::set<std::string> runResolver(const Path & filename)
 
 bool isSymlink(const Path & path)
 {
-    struct stat st;
-    if (lstat(path.c_str(), &st) == -1)
-        throw SysError("getting attributes of path '%1%'", path);
-
-    return S_ISLNK(st.st_mode);
+    return S_ISLNK(lstat(path).st_mode);
 }
 
 Path resolveSymlink(const Path & path)