Merge remote-tracking branch 'upstream/master' into ca-drv-exotic

doc/manual/src/command-ref/opt-common.md

@@ -203,10 +203,9 @@ Most Nix commands accept the following command-line options:
     instead.
 
   - <span id="opt-I">[`-I`](#opt-I)</span> *path*\
-    Add a path to the Nix expression search path. This option may be
-    given multiple times. See the `NIX_PATH` environment variable for
-    information on the semantics of the Nix search path. Paths added
-    through `-I` take precedence over `NIX_PATH`.
+    Add an entry to the [Nix expression search path](@docroot@/command-ref/conf-file.md#conf-nix-path).
+    This option may be given multiple times.
+    Paths added through `-I` take precedence over [`NIX_PATH`](@docroot@/command-ref/env-common.md#env-NIX_PATH).
 
   - <span id="opt-option">[`--option`](#opt-option)</span> *name* *value*\
     Set the Nix configuration option *name* to *value*. This overrides

src/build-remote/build-remote.cc

@@ -311,8 +311,9 @@ connected:
                 auto thisOutputId = DrvOutput{ thisOutputHash, outputName };
                 if (!store->queryRealisation(thisOutputId)) {
                     debug("missing output %s", outputName);
-                    assert(result.builtOutputs.count(thisOutputId));
-                    auto newRealisation = result.builtOutputs.at(thisOutputId);
+                    auto i = result.builtOutputs.find(outputName);
+                    assert(i != result.builtOutputs.end());
+                    auto & newRealisation = i->second;
                     missingRealisations.insert(newRealisation);
                     missingPaths.insert(newRealisation.outPath);
                 }

src/libcmd/installables.cc

@@ -593,8 +593,8 @@ std::vector<std::pair<ref<Installable>, BuiltPathWithResult>> Installable::build
                 std::visit(overloaded {
                     [&](const DerivedPath::Built & bfd) {
                         std::map<std::string, StorePath> outputs;
-                        for (auto & path : buildResult.builtOutputs)
-                            outputs.emplace(path.first.outputName, path.second.outPath);
+                        for (auto & [outputName, realisation] : buildResult.builtOutputs)
+                            outputs.emplace(outputName, realisation.outPath);
                         res.push_back({aux.installable, {
                             .path = BuiltPath::Built { bfd.drvPath, outputs },
                             .info = aux.info,

src/libstore/build-result.hh

@@ -83,16 +83,11 @@ struct BuildResult
      */
     bool isNonDeterministic = false;
 
-    /**
-     * The derivation we built or the store path we substituted.
-     */
-    DerivedPath path;
-
     /**
      * For derivations, a mapping from the names of the wanted outputs
      * to actual paths.
      */
-    DrvOutputs builtOutputs;
+    SingleDrvOutputs builtOutputs;
 
     /**
      * The start/stop times of the build (or one of the rounds, if it
@@ -116,4 +111,15 @@ struct BuildResult
     }
 };
 
+/**
+ * A `BuildResult` together with its "primary key".
+ */
+struct KeyedBuildResult : BuildResult
+{
+    /**
+     * The derivation we built or the store path we substituted.
+     */
+    DerivedPath path;
+};
+
 }

src/libstore/build/derivation-goal.cc

@@ -145,8 +145,20 @@ void DerivationGoal::work()
 void DerivationGoal::addWantedOutputs(const OutputsSpec & outputs)
 {
     auto newWanted = wantedOutputs.union_(outputs);
-    if (!newWanted.isSubsetOf(wantedOutputs))
-        needRestart = true;
+    switch (needRestart) {
+    case NeedRestartForMoreOutputs::OutputsUnmodifedDontNeed:
+        if (!newWanted.isSubsetOf(wantedOutputs))
+            needRestart = NeedRestartForMoreOutputs::OutputsAddedDoNeed;
+        break;
+    case NeedRestartForMoreOutputs::OutputsAddedDoNeed:
+        /* No need to check whether we added more outputs, because a
+           restart is already queued up. */
+        break;
+    case NeedRestartForMoreOutputs::BuildInProgressWillNotNeed:
+        /* We are already building all outputs, so it doesn't matter if
+           we now want more. */
+        break;
+    };
     wantedOutputs = newWanted;
 }
 
@@ -297,12 +309,29 @@ void DerivationGoal::outputsSubstitutionTried()
         In particular, it may be the case that the hole in the closure is
         an output of the current derivation, which causes a loop if retried.
      */
-    if (nrIncompleteClosure > 0 && nrIncompleteClosure == nrFailed) retrySubstitution = true;
+    {
+        bool substitutionFailed =
+            nrIncompleteClosure > 0 &&
+            nrIncompleteClosure == nrFailed;
+        switch (retrySubstitution) {
+        case RetrySubstitution::NoNeed:
+            if (substitutionFailed)
+                retrySubstitution = RetrySubstitution::YesNeed;
+            break;
+        case RetrySubstitution::YesNeed:
+            // Should not be able to reach this state from here.
+            assert(false);
+            break;
+        case RetrySubstitution::AlreadyRetried:
+            debug("substitution failed again, but we already retried once. Not retrying again.");
+            break;
+        }
+    }
 
     nrFailed = nrNoSubstituters = nrIncompleteClosure = 0;
 
-    if (needRestart) {
-        needRestart = false;
+    if (needRestart == NeedRestartForMoreOutputs::OutputsAddedDoNeed) {
+        needRestart = NeedRestartForMoreOutputs::OutputsUnmodifedDontNeed;
         haveDerivation();
         return;
     }
@@ -330,6 +359,10 @@ void DerivationGoal::outputsSubstitutionTried()
    produced using a substitute.  So we have to build instead. */
 void DerivationGoal::gaveUpOnSubstitution()
 {
+    /* At this point we are building all outputs, so if more are wanted there
+       is no need to restart. */
+    needRestart = NeedRestartForMoreOutputs::BuildInProgressWillNotNeed;
+
     /* The inputs must be built before we can build this goal. */
     inputDrvOutputs.clear();
     if (useDerivation)
@@ -451,8 +484,8 @@ void DerivationGoal::inputsRealised()
         return;
     }
 
-    if (retrySubstitution && !retriedSubstitution) {
-        retriedSubstitution = true;
+    if (retrySubstitution == RetrySubstitution::YesNeed) {
+        retrySubstitution = RetrySubstitution::AlreadyRetried;
         haveDerivation();
         return;
     }
@@ -570,8 +603,6 @@ void DerivationGoal::inputsRealised()
        build hook. */
     state = &DerivationGoal::tryToBuild;
     worker.wakeUp(shared_from_this());
-
-    buildResult = BuildResult { .path = buildResult.path };
 }
 
 void DerivationGoal::started()
@@ -982,7 +1013,7 @@ void DerivationGoal::resolvedFinished()
     auto resolvedDrv = *resolvedDrvGoal->drv;
     auto & resolvedResult = resolvedDrvGoal->buildResult;
 
-    DrvOutputs builtOutputs;
+    SingleDrvOutputs builtOutputs;
 
     if (resolvedResult.success()) {
         auto resolvedHashes = staticOutputHashes(worker.store, resolvedDrv);
@@ -1008,7 +1039,7 @@ void DerivationGoal::resolvedFinished()
                     worker.store.printStorePath(drvPath), wantedOutput);
 
             auto realisation = [&]{
-              auto take1 = get(resolvedResult.builtOutputs, DrvOutput { *resolvedHash, wantedOutput });
+              auto take1 = get(resolvedResult.builtOutputs, wantedOutput);
               if (take1) return *take1;
 
               /* The above `get` should work. But sateful tracking of
@@ -1033,7 +1064,7 @@ void DerivationGoal::resolvedFinished()
                 worker.store.registerDrvOutput(newRealisation);
             }
             outputPaths.insert(realisation.outPath);
-            builtOutputs.emplace(realisation.id, realisation);
+            builtOutputs.emplace(wantedOutput, realisation);
         }
 
         runPostBuildHook(
@@ -1158,7 +1189,7 @@ HookReply DerivationGoal::tryBuildHook()
 }
 
 
-DrvOutputs DerivationGoal::registerOutputs()
+SingleDrvOutputs DerivationGoal::registerOutputs()
 {
     /* When using a build hook, the build hook can register the output
        as valid (by doing `nix-store --import').  If so we don't have
@@ -1320,7 +1351,7 @@ OutputPathMap DerivationGoal::queryDerivationOutputMap()
 }
 
 
-std::pair<bool, DrvOutputs> DerivationGoal::checkPathValidity()
+std::pair<bool, SingleDrvOutputs> DerivationGoal::checkPathValidity()
 {
     if (!drv->type().isPure()) return { false, {} };
 
@@ -1333,7 +1364,7 @@ std::pair<bool, DrvOutputs> DerivationGoal::checkPathValidity()
             return static_cast<StringSet>(names);
         },
     }, wantedOutputs.raw());
-    DrvOutputs validOutputs;
+    SingleDrvOutputs validOutputs;
 
     for (auto & i : queryPartialDerivationOutputMap()) {
         auto initialOutput = get(initialOutputs, i.first);
@@ -1376,7 +1407,7 @@ std::pair<bool, DrvOutputs> DerivationGoal::checkPathValidity()
             }
         }
         if (info.wanted && info.known && info.known->isValid())
-            validOutputs.emplace(drvOutput, Realisation { drvOutput, info.known->path });
+            validOutputs.emplace(i.first, Realisation { drvOutput, info.known->path });
     }
 
     // If we requested all the outputs, we are always fine.
@@ -1400,7 +1431,7 @@ std::pair<bool, DrvOutputs> DerivationGoal::checkPathValidity()
 }
 
 
-DrvOutputs DerivationGoal::assertPathValidity()
+SingleDrvOutputs DerivationGoal::assertPathValidity()
 {
     auto [allValid, validOutputs] = checkPathValidity();
     if (!allValid)
@@ -1411,7 +1442,7 @@ DrvOutputs DerivationGoal::assertPathValidity()
 
 void DerivationGoal::done(
     BuildResult::Status status,
-    DrvOutputs builtOutputs,
+    SingleDrvOutputs builtOutputs,
     std::optional<Error> ex)
 {
     buildResult.status = status;
@@ -1452,12 +1483,28 @@ void DerivationGoal::waiteeDone(GoalPtr waitee, ExitCode result)
 {
     Goal::waiteeDone(waitee, result);
 
-    if (waitee->buildResult.success())
-        if (auto bfd = std::get_if<DerivedPath::Built>(&waitee->buildResult.path))
-            for (auto & [output, realisation] : waitee->buildResult.builtOutputs)
+    if (!useDerivation) return;
+    auto & fullDrv = *dynamic_cast<Derivation *>(drv.get());
+
+    auto * dg = dynamic_cast<DerivationGoal *>(&*waitee);
+    if (!dg) return;
+
+    auto outputs = fullDrv.inputDrvs.find(dg->drvPath);
+    if (outputs == fullDrv.inputDrvs.end()) return;
+
+    for (auto & outputName : outputs->second) {
+        auto buildResult = dg->getBuildResult(DerivedPath::Built {
+            .drvPath = dg->drvPath,
+            .outputs = OutputsSpec::Names { outputName },
+        });
+        if (buildResult.success()) {
+            auto i = buildResult.builtOutputs.find(outputName);
+            if (i != buildResult.builtOutputs.end())
                 inputDrvOutputs.insert_or_assign(
-                    { bfd->drvPath, output.outputName },
-                    realisation.outPath);
+                    { dg->drvPath, outputName },
+                    i->second.outPath);
+        }
+    }
 }
 
 }

src/libstore/build/derivation-goal.hh

@@ -78,22 +78,58 @@ struct DerivationGoal : public Goal
      */
     std::map<std::pair<StorePath, std::string>, StorePath> inputDrvOutputs;
 
+    /**
+     * See `needRestart`; just for that field.
+     */
+    enum struct NeedRestartForMoreOutputs {
+        /**
+         * The goal state machine is progressing based on the current value of
+         * `wantedOutputs. No actions are needed.
+         */
+        OutputsUnmodifedDontNeed,
+        /**
+         * `wantedOutputs` has been extended, but the state machine is
+         * proceeding according to its old value, so we need to restart.
+         */
+        OutputsAddedDoNeed,
+        /**
+         * The goal state machine has progressed to the point of doing a build,
+         * in which case all outputs will be produced, so extensions to
+         * `wantedOutputs` no longer require a restart.
+         */
+        BuildInProgressWillNotNeed,
+    };
+
     /**
      * Whether additional wanted outputs have been added.
      */
-    bool needRestart = false;
+    NeedRestartForMoreOutputs needRestart = NeedRestartForMoreOutputs::OutputsUnmodifedDontNeed;
+
+    /**
+     * See `retrySubstitution`; just for that field.
+     */
+    enum RetrySubstitution {
+        /**
+         * No issues have yet arose, no need to restart.
+         */
+        NoNeed,
+        /**
+         * Something failed and there is an incomplete closure. Let's retry
+         * substituting.
+         */
+        YesNeed,
+        /**
+         * We are current or have already retried substitution, and whether or
+         * not something goes wrong we will not retry again.
+         */
+        AlreadyRetried,
+    };
 
     /**
      * Whether to retry substituting the outputs after building the
      * inputs. This is done in case of an incomplete closure.
      */
-    bool retrySubstitution = false;
-
-    /**
-     * Whether we've retried substitution, in which case we won't try
-     * again.
-     */
-    bool retriedSubstitution = false;
+    RetrySubstitution retrySubstitution = RetrySubstitution::NoNeed;
 
     /**
      * The derivation stored at drvPath.
@@ -217,7 +253,7 @@ struct DerivationGoal : public Goal
      * Check that the derivation outputs all exist and register them
      * as valid.
      */
-    virtual DrvOutputs registerOutputs();
+    virtual SingleDrvOutputs registerOutputs();
 
     /**
      * Open a log file and a pipe to it.
@@ -270,17 +306,17 @@ struct DerivationGoal : public Goal
      * Update 'initialOutputs' to determine the current status of the
      * outputs of the derivation. Also returns a Boolean denoting
      * whether all outputs are valid and non-corrupt, and a
-     * 'DrvOutputs' structure containing the valid and wanted
+     * 'SingleDrvOutputs' structure containing the valid and wanted
      * outputs.
      */
-    std::pair<bool, DrvOutputs> checkPathValidity();
+    std::pair<bool, SingleDrvOutputs> checkPathValidity();
 
     /**
      * Aborts if any output is not valid or corrupt, and otherwise
-     * returns a 'DrvOutputs' structure containing the wanted
+     * returns a 'SingleDrvOutputs' structure containing the wanted
      * outputs.
      */
-    DrvOutputs assertPathValidity();
+    SingleDrvOutputs assertPathValidity();
 
     /**
      * Forcibly kill the child process, if any.
@@ -293,7 +329,7 @@ struct DerivationGoal : public Goal
 
     void done(
         BuildResult::Status status,
-        DrvOutputs builtOutputs = {},
+        SingleDrvOutputs builtOutputs = {},
         std::optional<Error> ex = {});
 
     void waiteeDone(GoalPtr waitee, ExitCode result) override;

src/libstore/build/entry-points.cc

@@ -10,16 +10,8 @@ void Store::buildPaths(const std::vector<DerivedPath> & reqs, BuildMode buildMod
     Worker worker(*this, evalStore ? *evalStore : *this);
 
     Goals goals;
-    for (const auto & br : reqs) {
-        std::visit(overloaded {
-            [&](const DerivedPath::Built & bfd) {
-                goals.insert(worker.makeDerivationGoal(bfd.drvPath, bfd.outputs, buildMode));
-            },
-            [&](const DerivedPath::Opaque & bo) {
-                goals.insert(worker.makePathSubstitutionGoal(bo.path, buildMode == bmRepair ? Repair : NoRepair));
-            },
-        }, br.raw());
-    }
+    for (auto & br : reqs)
+        goals.insert(worker.makeGoal(br, buildMode));
 
     worker.run(goals);
 
@@ -47,7 +39,7 @@ void Store::buildPaths(const std::vector<DerivedPath> & reqs, BuildMode buildMod
     }
 }
 
-std::vector<BuildResult> Store::buildPathsWithResults(
+std::vector<KeyedBuildResult> Store::buildPathsWithResults(
     const std::vector<DerivedPath> & reqs,
     BuildMode buildMode,
     std::shared_ptr<Store> evalStore)
@@ -55,23 +47,23 @@ std::vector<BuildResult> Store::buildPathsWithResults(
     Worker worker(*this, evalStore ? *evalStore : *this);
 
     Goals goals;
-    for (const auto & br : reqs) {
-        std::visit(overloaded {
-            [&](const DerivedPath::Built & bfd) {
-                goals.insert(worker.makeDerivationGoal(bfd.drvPath, bfd.outputs, buildMode));
-            },
-            [&](const DerivedPath::Opaque & bo) {
-                goals.insert(worker.makePathSubstitutionGoal(bo.path, buildMode == bmRepair ? Repair : NoRepair));
-            },
-        }, br.raw());
+    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);
 
-    std::vector<BuildResult> results;
+    std::vector<KeyedBuildResult> results;
 
-    for (auto & i : goals)
-        results.push_back(i->buildResult);
+    for (auto & [req, goalPtr] : state)
+        results.emplace_back(KeyedBuildResult {
+            goalPtr->getBuildResult(req),
+            /* .path = */ req,
+        });
 
     return results;
 }
@@ -84,15 +76,14 @@ BuildResult Store::buildDerivation(const StorePath & drvPath, const BasicDerivat
 
     try {
         worker.run(Goals{goal});
-        return goal->buildResult;
+        return goal->getBuildResult(DerivedPath::Built {
+            .drvPath = drvPath,
+            .outputs = OutputsSpec::All {},
+        });
     } catch (Error & e) {
         return BuildResult {
             .status = BuildResult::MiscFailure,
             .errorMsg = e.msg(),
-            .path = DerivedPath::Built {
-                .drvPath = drvPath,
-                .outputs = OutputsSpec::All { },
-            },
         };
     };
 }

src/libstore/build/goal.cc

@@ -11,6 +11,29 @@ bool CompareGoalPtrs::operator() (const GoalPtr & a, const GoalPtr & b) const {
 }
 
 
+BuildResult Goal::getBuildResult(const DerivedPath & req) {
+    BuildResult res { buildResult };
+
+    if (auto pbp = std::get_if<DerivedPath::Built>(&req)) {
+        auto & bp = *pbp;
+
+        /* Because goals are in general shared between derived paths
+           that share the same derivation, we need to filter their
+           results to get back just the results we care about.
+         */
+
+        for (auto it = res.builtOutputs.begin(); it != res.builtOutputs.end();) {
+            if (bp.outputs.contains(it->first))
+                ++it;
+            else
+                it = res.builtOutputs.erase(it);
+        }
+    }
+
+    return res;
+}
+
+
 void addToWeakGoals(WeakGoals & goals, GoalPtr p)
 {
     if (goals.find(p) != goals.end())

src/libstore/build/goal.hh

@@ -81,11 +81,26 @@ struct Goal : public std::enable_shared_from_this<Goal>
      */
     ExitCode exitCode = ecBusy;
 
+protected:
     /**
      * Build result.
      */
     BuildResult buildResult;
 
+public:
+
+    /**
+     * Project a `BuildResult` with just the information that pertains
+     * to the given request.
+     *
+     * In general, goals may be aliased between multiple requests, and
+     * the stored `BuildResult` has information for the union of all
+     * requests. We don't want to leak what the other request are for
+     * sake of both privacy and determinism, and this "safe accessor"
+     * ensures we don't.
+     */
+    BuildResult getBuildResult(const DerivedPath &);
+
     /**
      * Exception containing an error message, if any.
      */
@@ -93,7 +108,6 @@ struct Goal : public std::enable_shared_from_this<Goal>
 
     Goal(Worker & worker, DerivedPath path)
         : worker(worker)
-        , buildResult { .path = std::move(path) }
     { }
 
     virtual ~Goal()

src/libstore/build/local-derivation-goal.cc

@@ -1335,7 +1335,7 @@ struct RestrictedStore : public virtual RestrictedStoreConfig, public virtual Lo
                 result.rethrow();
     }
 
-    std::vector<BuildResult> buildPathsWithResults(
+    std::vector<KeyedBuildResult> buildPathsWithResults(
         const std::vector<DerivedPath> & paths,
         BuildMode buildMode = bmNormal,
         std::shared_ptr<Store> evalStore = nullptr) override
@@ -2174,7 +2174,7 @@ void LocalDerivationGoal::runChild()
 }
 
 
-DrvOutputs LocalDerivationGoal::registerOutputs()
+SingleDrvOutputs LocalDerivationGoal::registerOutputs()
 {
     /* When using a build hook, the build hook can register the output
        as valid (by doing `nix-store --import').  If so we don't have
@@ -2695,7 +2695,7 @@ DrvOutputs LocalDerivationGoal::registerOutputs()
        means it's safe to link the derivation to the output hash. We must do
        that for floating CA derivations, which otherwise couldn't be cached,
        but it's fine to do in all cases. */
-    DrvOutputs builtOutputs;
+    SingleDrvOutputs builtOutputs;
 
     for (auto & [outputName, newInfo] : infos) {
         auto oldinfo = get(initialOutputs, outputName);
@@ -2714,7 +2714,7 @@ DrvOutputs LocalDerivationGoal::registerOutputs()
             worker.store.registerDrvOutput(thisRealisation);
         }
         if (wantedOutputs.contains(outputName))
-            builtOutputs.emplace(thisRealisation.id, thisRealisation);
+            builtOutputs.emplace(outputName, thisRealisation);
     }
 
     return builtOutputs;

src/libstore/build/local-derivation-goal.hh

@@ -237,7 +237,7 @@ struct LocalDerivationGoal : public DerivationGoal
      * Check that the derivation outputs all exist and register them
      * as valid.
      */
-    DrvOutputs registerOutputs() override;
+    SingleDrvOutputs registerOutputs() override;
 
     void signRealisation(Realisation &) override;
 

src/libstore/build/worker.cc

@@ -92,6 +92,7 @@ std::shared_ptr<PathSubstitutionGoal> Worker::makePathSubstitutionGoal(const Sto
     return goal;
 }
 
+
 std::shared_ptr<DrvOutputSubstitutionGoal> Worker::makeDrvOutputSubstitutionGoal(const DrvOutput& id, RepairFlag repair, std::optional<ContentAddress> ca)
 {
     std::weak_ptr<DrvOutputSubstitutionGoal> & goal_weak = drvOutputSubstitutionGoals[id];
@@ -104,6 +105,20 @@ std::shared_ptr<DrvOutputSubstitutionGoal> Worker::makeDrvOutputSubstitutionGoal
     return goal;
 }
 
+
+GoalPtr Worker::makeGoal(const DerivedPath & req, BuildMode buildMode)
+{
+    return std::visit(overloaded {
+        [&](const DerivedPath::Built & bfd) -> GoalPtr {
+            return makeDerivationGoal(bfd.drvPath, bfd.outputs, buildMode);
+        },
+        [&](const DerivedPath::Opaque & bo) -> GoalPtr {
+            return makePathSubstitutionGoal(bo.path, buildMode == bmRepair ? Repair : NoRepair);
+        },
+    }, req.raw());
+}
+
+
 template<typename K, typename G>
 static void removeGoal(std::shared_ptr<G> goal, std::map<K, std::weak_ptr<G>> & goalMap)
 {

src/libstore/build/worker.hh

@@ -181,7 +181,7 @@ public:
      */
 
     /**
-     * derivation goal
+     * @ref DerivationGoal "derivation goal"
      */
 private:
     std::shared_ptr<DerivationGoal> makeDerivationGoalCommon(
@@ -196,11 +196,19 @@ public:
         const OutputsSpec & wantedOutputs, BuildMode buildMode = bmNormal);
 
     /**
-     * substitution goal
+     * @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);
 
+    /**
+     * Make a goal corresponding to the `DerivedPath`.
+     *
+     * It will be a `DerivationGoal` for a `DerivedPath::Built` or
+     * a `SubstitutionGoal` for a `DerivedPath::Opaque`.
+     */
+    GoalPtr makeGoal(const DerivedPath & req, BuildMode buildMode = bmNormal);
+
     /**
      * Remove a dead goal.
      */

src/libstore/daemon.cc

@@ -641,7 +641,10 @@ static void performOp(TunnelLogger * logger, ref<Store> store,
             to << res.timesBuilt << res.isNonDeterministic << res.startTime << res.stopTime;
         }
         if (GET_PROTOCOL_MINOR(clientVersion) >= 28) {
-            worker_proto::write(*store, to, res.builtOutputs);
+            DrvOutputs builtOutputs;
+            for (auto & [output, realisation] : res.builtOutputs)
+                builtOutputs.insert_or_assign(realisation.id, realisation);
+            worker_proto::write(*store, to, builtOutputs);
         }
         break;
     }
@@ -1068,6 +1071,8 @@ void processConnection(
 
             opCount++;
 
+            debug("performing daemon worker op: %d", op);
+
             try {
                 performOp(tunnelLogger, store, trusted, recursive, clientVersion, from, to, op);
             } catch (Error & e) {

src/libstore/globals.cc

@@ -22,6 +22,9 @@
 #include <dlfcn.h>
 #endif
 
+#include "config-impl.hh"
+
+
 namespace nix {
 
 
@@ -192,18 +195,18 @@ NLOHMANN_JSON_SERIALIZE_ENUM(SandboxMode, {
     {SandboxMode::smDisabled, false},
 });
 
-template<> void BaseSetting<SandboxMode>::set(const std::string & str, bool append)
+template<> SandboxMode BaseSetting<SandboxMode>::parse(const std::string & str) const
 {
-    if (str == "true") value = smEnabled;
-    else if (str == "relaxed") value = smRelaxed;
-    else if (str == "false") value = smDisabled;
+    if (str == "true") return smEnabled;
+    else if (str == "relaxed") return smRelaxed;
+    else if (str == "false") return smDisabled;
     else throw UsageError("option '%s' has invalid value '%s'", name, str);
 }
 
-template<> bool BaseSetting<SandboxMode>::isAppendable()
+template<> struct BaseSetting<SandboxMode>::trait
 {
-    return false;
-}
+    static constexpr bool appendable = false;
+};
 
 template<> std::string BaseSetting<SandboxMode>::to_string() const
 {
@@ -235,23 +238,23 @@ template<> void BaseSetting<SandboxMode>::convertToArg(Args & args, const std::s
     });
 }
 
-void MaxBuildJobsSetting::set(const std::string & str, bool append)
+unsigned int MaxBuildJobsSetting::parse(const std::string & str) const
 {
-    if (str == "auto") value = std::max(1U, std::thread::hardware_concurrency());
+    if (str == "auto") return std::max(1U, std::thread::hardware_concurrency());
     else {
         if (auto n = string2Int<decltype(value)>(str))
-            value = *n;
+            return *n;
         else
             throw UsageError("configuration setting '%s' should be 'auto' or an integer", name);
     }
 }
 
 
-void PluginFilesSetting::set(const std::string & str, bool append)
+Paths PluginFilesSetting::parse(const std::string & str) const
 {
     if (pluginsLoaded)
         throw UsageError("plugin-files set after plugins were loaded, you may need to move the flag before the subcommand");
-    BaseSetting<Paths>::set(str, append);
+    return BaseSetting<Paths>::parse(str);
 }
 
 

src/libstore/globals.hh

@@ -26,7 +26,7 @@ struct MaxBuildJobsSetting : public BaseSetting<unsigned int>
         options->addSetting(this);
     }
 
-    void set(const std::string & str, bool append = false) override;
+    unsigned int parse(const std::string & str) const override;
 };
 
 struct PluginFilesSetting : public BaseSetting<Paths>
@@ -43,7 +43,7 @@ struct PluginFilesSetting : public BaseSetting<Paths>
         options->addSetting(this);
     }
 
-    void set(const std::string & str, bool append = false) override;
+    Paths parse(const std::string & str) const override;
 };
 
 const uint32_t maxIdsPerBuild =

src/libstore/legacy-ssh-store.cc

@@ -287,19 +287,18 @@ public:
 
         conn->to.flush();
 
-        BuildResult status {
-            .path = DerivedPath::Built {
-                .drvPath = drvPath,
-                .outputs = OutputsSpec::All { },
-            },
-        };
+        BuildResult status;
         status.status = (BuildResult::Status) readInt(conn->from);
         conn->from >> status.errorMsg;
 
         if (GET_PROTOCOL_MINOR(conn->remoteVersion) >= 3)
             conn->from >> status.timesBuilt >> status.isNonDeterministic >> status.startTime >> status.stopTime;
         if (GET_PROTOCOL_MINOR(conn->remoteVersion) >= 6) {
-            status.builtOutputs = worker_proto::read(*this, conn->from, Phantom<DrvOutputs> {});
+            auto builtOutputs = worker_proto::read(*this, conn->from, Phantom<DrvOutputs> {});
+            for (auto && [output, realisation] : builtOutputs)
+                status.builtOutputs.insert_or_assign(
+                    std::move(output.outputName),
+                    std::move(realisation));
         }
         return status;
     }
@@ -330,7 +329,7 @@ public:
 
         conn->to.flush();
 
-        BuildResult result { .path = DerivedPath::Opaque { StorePath::dummy } };
+        BuildResult result;
         result.status = (BuildResult::Status) readInt(conn->from);
 
         if (!result.success()) {

src/libstore/realisation.hh

@@ -13,9 +13,25 @@ namespace nix {
 
 class Store;
 
+/**
+ * A general `Realisation` key.
+ *
+ * This is similar to a `DerivedPath::Opaque`, but the derivation is
+ * identified by its "hash modulo" instead of by its store path.
+ */
 struct DrvOutput {
-    // The hash modulo of the derivation
+    /**
+     * The hash modulo of the derivation.
+     *
+     * Computed from the derivation itself for most types of
+     * derivations, but computed from the (fixed) content address of the
+     * output for fixed-output derivations.
+     */
     Hash drvHash;
+
+    /**
+     * The name of the output.
+     */
     std::string outputName;
 
     std::string to_string() const;
@@ -60,6 +76,21 @@ struct Realisation {
     GENERATE_CMP(Realisation, me->id, me->outPath);
 };
 
+/**
+ * Collection type for a single derivation's outputs' `Realisation`s.
+ *
+ * Since these are the outputs of a single derivation, we know the
+ * output names are unique so we can use them as the map key.
+ */
+typedef std::map<std::string, Realisation> SingleDrvOutputs;
+
+/**
+ * Collection type for multiple derivations' outputs' `Realisation`s.
+ *
+ * `DrvOutput` is used because in general the derivations are not all
+ * the same, so we need to identify firstly which derivation, and
+ * secondly which output of that derivation.
+ */
 typedef std::map<DrvOutput, Realisation> DrvOutputs;
 
 struct OpaquePath {

src/libstore/remote-store.cc

@@ -125,10 +125,26 @@ void write(const Store & store, Sink & out, const DrvOutput & drvOutput)
 }
 
 
-BuildResult read(const Store & store, Source & from, Phantom<BuildResult> _)
+KeyedBuildResult read(const Store & store, Source & from, Phantom<KeyedBuildResult> _)
 {
     auto path = worker_proto::read(store, from, Phantom<DerivedPath> {});
-    BuildResult res { .path = path };
+    auto br = worker_proto::read(store, from, Phantom<BuildResult> {});
+    return KeyedBuildResult {
+        std::move(br),
+        /* .path = */ std::move(path),
+    };
+}
+
+void write(const Store & store, Sink & to, const KeyedBuildResult & res)
+{
+    worker_proto::write(store, to, res.path);
+    worker_proto::write(store, to, static_cast<const BuildResult &>(res));
+}
+
+
+BuildResult read(const Store & store, Source & from, Phantom<BuildResult> _)
+{
+    BuildResult res;
     res.status = (BuildResult::Status) readInt(from);
     from
         >> res.errorMsg
@@ -136,13 +152,16 @@ BuildResult read(const Store & store, Source & from, Phantom<BuildResult> _)
         >> res.isNonDeterministic
         >> res.startTime
         >> res.stopTime;
-    res.builtOutputs = worker_proto::read(store, from, Phantom<DrvOutputs> {});
+    auto builtOutputs = worker_proto::read(store, from, Phantom<DrvOutputs> {});
+    for (auto && [output, realisation] : builtOutputs)
+        res.builtOutputs.insert_or_assign(
+            std::move(output.outputName),
+            std::move(realisation));
     return res;
 }
 
 void write(const Store & store, Sink & to, const BuildResult & res)
 {
-    worker_proto::write(store, to, res.path);
     to
         << res.status
         << res.errorMsg
@@ -150,7 +169,10 @@ void write(const Store & store, Sink & to, const BuildResult & res)
         << res.isNonDeterministic
         << res.startTime
         << res.stopTime;
-    worker_proto::write(store, to, res.builtOutputs);
+    DrvOutputs builtOutputs;
+    for (auto & [output, realisation] : res.builtOutputs)
+        builtOutputs.insert_or_assign(realisation.id, realisation);
+    worker_proto::write(store, to, builtOutputs);
 }
 
 
@@ -869,7 +891,7 @@ void RemoteStore::buildPaths(const std::vector<DerivedPath> & drvPaths, BuildMod
     readInt(conn->from);
 }
 
-std::vector<BuildResult> RemoteStore::buildPathsWithResults(
+std::vector<KeyedBuildResult> RemoteStore::buildPathsWithResults(
     const std::vector<DerivedPath> & paths,
     BuildMode buildMode,
     std::shared_ptr<Store> evalStore)
@@ -884,7 +906,7 @@ std::vector<BuildResult> RemoteStore::buildPathsWithResults(
         writeDerivedPaths(*this, conn, paths);
         conn->to << buildMode;
         conn.processStderr();
-        return worker_proto::read(*this, conn->from, Phantom<std::vector<BuildResult>> {});
+        return worker_proto::read(*this, conn->from, Phantom<std::vector<KeyedBuildResult>> {});
     } else {
         // Avoid deadlock.
         conn_.reset();
@@ -893,21 +915,25 @@ std::vector<BuildResult> RemoteStore::buildPathsWithResults(
         // fails, but meh.
         buildPaths(paths, buildMode, evalStore);
 
-        std::vector<BuildResult> results;
+        std::vector<KeyedBuildResult> results;
 
         for (auto & path : paths) {
             std::visit(
                 overloaded {
                     [&](const DerivedPath::Opaque & bo) {
-                        results.push_back(BuildResult {
-                            .status = BuildResult::Substituted,
-                            .path = bo,
+                        results.push_back(KeyedBuildResult {
+                            {
+                                .status = BuildResult::Substituted,
+                            },
+                            /* .path = */ bo,
                         });
                     },
                     [&](const DerivedPath::Built & bfd) {
-                        BuildResult res {
-                            .status = BuildResult::Built,
-                            .path = bfd,
+                        KeyedBuildResult res {
+                            {
+                                .status = BuildResult::Built
+                            },
+                            /* .path = */ bfd,
                         };
 
                         OutputPathMap outputs;
@@ -926,10 +952,10 @@ std::vector<BuildResult> RemoteStore::buildPathsWithResults(
                                     queryRealisation(outputId);
                                 if (!realisation)
                                     throw MissingRealisation(outputId);
-                                res.builtOutputs.emplace(realisation->id, *realisation);
+                                res.builtOutputs.emplace(output, *realisation);
                             } else {
                                 res.builtOutputs.emplace(
-                                    outputId,
+                                    output,
                                     Realisation {
                                         .id = outputId,
                                         .outPath = outputPath,
@@ -956,12 +982,7 @@ BuildResult RemoteStore::buildDerivation(const StorePath & drvPath, const BasicD
     writeDerivation(conn->to, *this, drv);
     conn->to << buildMode;
     conn.processStderr();
-    BuildResult res {
-        .path = DerivedPath::Built {
-            .drvPath = drvPath,
-            .outputs = OutputsSpec::All { },
-        },
-    };
+    BuildResult res;
     res.status = (BuildResult::Status) readInt(conn->from);
     conn->from >> res.errorMsg;
     if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 29) {
@@ -969,7 +990,10 @@ BuildResult RemoteStore::buildDerivation(const StorePath & drvPath, const BasicD
     }
     if (GET_PROTOCOL_MINOR(conn->daemonVersion) >= 28) {
         auto builtOutputs = worker_proto::read(*this, conn->from, Phantom<DrvOutputs> {});
-        res.builtOutputs = builtOutputs;
+        for (auto && [output, realisation] : builtOutputs)
+            res.builtOutputs.insert_or_assign(
+                std::move(output.outputName),
+                std::move(realisation));
     }
     return res;
 }

src/libstore/remote-store.hh

@@ -115,7 +115,7 @@ public:
 
     void buildPaths(const std::vector<DerivedPath> & paths, BuildMode buildMode, std::shared_ptr<Store> evalStore) override;
 
-    std::vector<BuildResult> buildPathsWithResults(
+    std::vector<KeyedBuildResult> buildPathsWithResults(
         const std::vector<DerivedPath> & paths,
         BuildMode buildMode,
         std::shared_ptr<Store> evalStore) override;

src/libstore/store-api.hh

@@ -92,6 +92,7 @@ enum BuildMode { bmNormal, bmRepair, bmCheck };
 enum TrustedFlag : bool { NotTrusted = false, Trusted = true };
 
 struct BuildResult;
+struct KeyedBuildResult;
 
 
 typedef std::map<StorePath, std::optional<ContentAddress>> StorePathCAMap;
@@ -569,7 +570,7 @@ public:
      * case of a build/substitution error, this function won't throw an
      * exception, but return a BuildResult containing an error message.
      */
-    virtual std::vector<BuildResult> buildPathsWithResults(
+    virtual std::vector<KeyedBuildResult> buildPathsWithResults(
         const std::vector<DerivedPath> & paths,
         BuildMode buildMode = bmNormal,
         std::shared_ptr<Store> evalStore = nullptr);

src/libstore/worker-protocol.hh

@@ -103,6 +103,7 @@ MAKE_WORKER_PROTO(, DerivedPath);
 MAKE_WORKER_PROTO(, Realisation);
 MAKE_WORKER_PROTO(, DrvOutput);
 MAKE_WORKER_PROTO(, BuildResult);
+MAKE_WORKER_PROTO(, KeyedBuildResult);
 MAKE_WORKER_PROTO(, std::optional<TrustedFlag>);
 
 MAKE_WORKER_PROTO(template<typename T>, std::vector<T>);

src/libutil/config-impl.hh

@@ -0,0 +1,71 @@
+#pragma once
+/**
+ * @file
+ *
+ * Template implementations (as opposed to mere declarations).
+ *
+ * One only needs to include this when one is declaring a
+ * `BaseClass<CustomType>` setting, or as derived class of such an
+ * instantiation.
+ */
+
+#include "config.hh"
+
+namespace nix {
+
+template<> struct BaseSetting<Strings>::trait
+{
+    static constexpr bool appendable = true;
+};
+template<> struct BaseSetting<StringSet>::trait
+{
+    static constexpr bool appendable = true;
+};
+template<> struct BaseSetting<StringMap>::trait
+{
+    static constexpr bool appendable = true;
+};
+template<> struct BaseSetting<std::set<ExperimentalFeature>>::trait
+{
+    static constexpr bool appendable = true;
+};
+
+template<typename T>
+struct BaseSetting<T>::trait
+{
+    static constexpr bool appendable = false;
+};
+
+template<typename T>
+bool BaseSetting<T>::isAppendable()
+{
+    return trait::appendable;
+}
+
+template<> void BaseSetting<Strings>::appendOrSet(Strings && newValue, bool append);
+template<> void BaseSetting<StringSet>::appendOrSet(StringSet && newValue, bool append);
+template<> void BaseSetting<StringMap>::appendOrSet(StringMap && newValue, bool append);
+template<> void BaseSetting<std::set<ExperimentalFeature>>::appendOrSet(std::set<ExperimentalFeature> && newValue, bool append);
+
+template<typename T>
+void BaseSetting<T>::appendOrSet(T && newValue, bool append)
+{
+    static_assert(!trait::appendable, "using default `appendOrSet` implementation with an appendable type");
+    assert(!append);
+    value = std::move(newValue);
+}
+
+template<typename T>
+void BaseSetting<T>::set(const std::string & str, bool append)
+{
+    if (experimentalFeatureSettings.isEnabled(experimentalFeature))
+        appendOrSet(parse(str), append);
+    else {
+        assert(experimentalFeature);
+        warn("Ignoring setting '%s' because experimental feature '%s' is not enabled",
+            name,
+            showExperimentalFeature(*experimentalFeature));
+    }
+}
+
+}

src/libutil/config.cc

@@ -3,6 +3,8 @@
 #include "abstract-setting-to-json.hh"
 #include "experimental-features.hh"
 
+#include "config-impl.hh"
+
 #include <nlohmann/json.hpp>
 
 namespace nix {
@@ -80,6 +82,8 @@ void Config::getSettings(std::map<std::string, SettingInfo> & res, bool overridd
 void AbstractConfig::applyConfig(const std::string & contents, const std::string & path) {
     unsigned int pos = 0;
 
+    std::vector<std::pair<std::string, std::string>> parsedContents;
+
     while (pos < contents.size()) {
         std::string line;
         while (pos < contents.size() && contents[pos] != '\n')
@@ -125,8 +129,21 @@ void AbstractConfig::applyConfig(const std::string & contents, const std::string
         auto i = tokens.begin();
         advance(i, 2);
 
-        set(name, concatStringsSep(" ", Strings(i, tokens.end()))); // FIXME: slow
+        parsedContents.push_back({
+            name,
+            concatStringsSep(" ", Strings(i, tokens.end())),
+        });
     };
+
+    // First apply experimental-feature related settings
+    for (auto & [name, value] : parsedContents)
+        if (name == "experimental-features" || name == "extra-experimental-features")
+            set(name, value);
+
+    // Then apply other settings
+    for (auto & [name, value] : parsedContents)
+        if (name != "experimental-features" && name != "extra-experimental-features")
+            set(name, value);
 }
 
 void AbstractConfig::applyConfigFile(const Path & path)
@@ -202,12 +219,6 @@ void AbstractSetting::convertToArg(Args & args, const std::string & category)
 {
 }
 
-template<typename T>
-bool BaseSetting<T>::isAppendable()
-{
-    return false;
-}
-
 template<typename T>
 void BaseSetting<T>::convertToArg(Args & args, const std::string & category)
 {
@@ -231,9 +242,9 @@ void BaseSetting<T>::convertToArg(Args & args, const std::string & category)
         });
 }
 
-template<> void BaseSetting<std::string>::set(const std::string & str, bool append)
+template<> std::string BaseSetting<std::string>::parse(const std::string & str) const
 {
-    value = str;
+    return str;
 }
 
 template<> std::string BaseSetting<std::string>::to_string() const
@@ -242,11 +253,11 @@ template<> std::string BaseSetting<std::string>::to_string() const
 }
 
 template<typename T>
-void BaseSetting<T>::set(const std::string & str, bool append)
+T BaseSetting<T>::parse(const std::string & str) const
 {
     static_assert(std::is_integral<T>::value, "Integer required.");
     if (auto n = string2Int<T>(str))
-        value = *n;
+        return *n;
     else
         throw UsageError("setting '%s' has invalid value '%s'", name, str);
 }
@@ -258,12 +269,12 @@ std::string BaseSetting<T>::to_string() const
     return std::to_string(value);
 }
 
-template<> void BaseSetting<bool>::set(const std::string & str, bool append)
+template<> bool BaseSetting<bool>::parse(const std::string & str) const
 {
     if (str == "true" || str == "yes" || str == "1")
-        value = true;
+        return true;
     else if (str == "false" || str == "no" || str == "0")
-        value = false;
+        return false;
     else
         throw UsageError("Boolean setting '%s' has invalid value '%s'", name, str);
 }
@@ -291,16 +302,15 @@ template<> void BaseSetting<bool>::convertToArg(Args & args, const std::string &
     });
 }
 
-template<> void BaseSetting<Strings>::set(const std::string & str, bool append)
+template<> Strings BaseSetting<Strings>::parse(const std::string & str) const
 {
-    auto ss = tokenizeString<Strings>(str);
-    if (!append) value.clear();
-    for (auto & s : ss) value.push_back(std::move(s));
+    return tokenizeString<Strings>(str);
 }
 
-template<> bool BaseSetting<Strings>::isAppendable()
+template<> void BaseSetting<Strings>::appendOrSet(Strings && newValue, bool append)
 {
-    return true;
+    if (!append) value.clear();
+    for (auto && s : std::move(newValue)) value.push_back(std::move(s));
 }
 
 template<> std::string BaseSetting<Strings>::to_string() const
@@ -308,16 +318,16 @@ template<> std::string BaseSetting<Strings>::to_string() const
     return concatStringsSep(" ", value);
 }
 
-template<> void BaseSetting<StringSet>::set(const std::string & str, bool append)
+template<> StringSet BaseSetting<StringSet>::parse(const std::string & str) const
 {
-    if (!append) value.clear();
-    for (auto & s : tokenizeString<StringSet>(str))
-        value.insert(s);
+    return tokenizeString<StringSet>(str);
 }
 
-template<> bool BaseSetting<StringSet>::isAppendable()
+template<> void BaseSetting<StringSet>::appendOrSet(StringSet && newValue, bool append)
 {
-    return true;
+    if (!append) value.clear();
+    for (auto && s : std::move(newValue))
+        value.insert(s);
 }
 
 template<> std::string BaseSetting<StringSet>::to_string() const
@@ -325,21 +335,24 @@ template<> std::string BaseSetting<StringSet>::to_string() const
     return concatStringsSep(" ", value);
 }
 
-template<> void BaseSetting<std::set<ExperimentalFeature>>::set(const std::string & str, bool append)
+template<> std::set<ExperimentalFeature> BaseSetting<std::set<ExperimentalFeature>>::parse(const std::string & str) const
 {
-    if (!append) value.clear();
+    std::set<ExperimentalFeature> res;
     for (auto & s : tokenizeString<StringSet>(str)) {
         auto thisXpFeature = parseExperimentalFeature(s);
         if (thisXpFeature)
-            value.insert(thisXpFeature.value());
+            res.insert(thisXpFeature.value());
         else
             warn("unknown experimental feature '%s'", s);
     }
+    return res;
 }
 
-template<> bool BaseSetting<std::set<ExperimentalFeature>>::isAppendable()
+template<> void BaseSetting<std::set<ExperimentalFeature>>::appendOrSet(std::set<ExperimentalFeature> && newValue, bool append)
 {
-    return true;
+    if (!append) value.clear();
+    for (auto && s : std::move(newValue))
+        value.insert(s);
 }
 
 template<> std::string BaseSetting<std::set<ExperimentalFeature>>::to_string() const
@@ -350,20 +363,23 @@ template<> std::string BaseSetting<std::set<ExperimentalFeature>>::to_string() c
     return concatStringsSep(" ", stringifiedXpFeatures);
 }
 
-template<> void BaseSetting<StringMap>::set(const std::string & str, bool append)
+template<> StringMap BaseSetting<StringMap>::parse(const std::string & str) const
 {
-    if (!append) value.clear();
+    StringMap res;
     for (auto & s : tokenizeString<Strings>(str)) {
         auto eq = s.find_first_of('=');
         if (std::string::npos != eq)
-            value.emplace(std::string(s, 0, eq), std::string(s, eq + 1));
+            res.emplace(std::string(s, 0, eq), std::string(s, eq + 1));
         // else ignored
     }
+    return res;
 }
 
-template<> bool BaseSetting<StringMap>::isAppendable()
+template<> void BaseSetting<StringMap>::appendOrSet(StringMap && newValue, bool append)
 {
-    return true;
+    if (!append) value.clear();
+    for (auto && [k, v] : std::move(newValue))
+        value.emplace(std::move(k), std::move(v));
 }
 
 template<> std::string BaseSetting<StringMap>::to_string() const
@@ -387,15 +403,15 @@ template class BaseSetting<StringSet>;
 template class BaseSetting<StringMap>;
 template class BaseSetting<std::set<ExperimentalFeature>>;
 
-void PathSetting::set(const std::string & str, bool append)
+Path PathSetting::parse(const std::string & str) const
 {
     if (str == "") {
         if (allowEmpty)
-            value = "";
+            return "";
         else
             throw UsageError("setting '%s' cannot be empty", name);
     } else
-        value = canonPath(str);
+        return canonPath(str);
 }
 
 bool GlobalConfig::set(const std::string & name, const std::string & value)

src/libutil/config.hh

@@ -215,8 +215,11 @@ protected:
 
     virtual void set(const std::string & value, bool append = false) = 0;
 
-    virtual bool isAppendable()
-    { return false; }
+    /**
+     * Whether the type is appendable; i.e. whether the `append`
+     * parameter to `set()` is allowed to be `true`.
+     */
+    virtual bool isAppendable() = 0;
 
     virtual std::string to_string() const = 0;
 
@@ -241,6 +244,23 @@ protected:
     const T defaultValue;
     const bool documentDefault;
 
+    /**
+     * Parse the string into a `T`.
+     *
+     * Used by `set()`.
+     */
+    virtual T parse(const std::string & str) const;
+
+    /**
+     * Append or overwrite `value` with `newValue`.
+     *
+     * Some types to do not support appending in which case `append`
+     * should never be passed. The default handles this case.
+     *
+     * @param append Whether to append or overwrite.
+     */
+    virtual void appendOrSet(T && newValue, bool append);
+
 public:
 
     BaseSetting(const T & def,
@@ -268,9 +288,25 @@ public:
     template<typename U>
     void setDefault(const U & v) { if (!overridden) value = v; }
 
-    void set(const std::string & str, bool append = false) override;
+    /**
+     * Require any experimental feature the setting depends on
+     *
+     * Uses `parse()` to get the value from `str`, and `appendOrSet()`
+     * to set it.
+     */
+    void set(const std::string & str, bool append = false) override final;
 
-    bool isAppendable() override;
+    /**
+     * C++ trick; This is template-specialized to compile-time indicate whether
+     * the type is appendable.
+     */
+    struct trait;
+
+    /**
+     * Always defined based on the C++ magic
+     * with `trait` above.
+     */
+    bool isAppendable() override final;
 
     virtual void override(const T & v)
     {
@@ -336,7 +372,7 @@ public:
         options->addSetting(this);
     }
 
-    void set(const std::string & str, bool append = false) override;
+    Path parse(const std::string & str) const override;
 
     Path operator +(const char * p) const { return value + p; }
 

src/libutil/experimental-features.cc

@@ -12,7 +12,7 @@ struct ExperimentalFeatureDetails
     std::string_view description;
 };
 
-constexpr std::array<ExperimentalFeatureDetails, 11> xpFeatureDetails = {{
+constexpr std::array<ExperimentalFeatureDetails, 12> xpFeatureDetails = {{
     {
         .tag = Xp::CaDerivations,
         .name = "ca-derivations",
@@ -189,6 +189,16 @@ constexpr std::array<ExperimentalFeatureDetails, 11> xpFeatureDetails = {{
             runtime dependencies.
         )",
     },
+    {
+        .tag = Xp::DaemonTrustOverride,
+        .name = "daemon-trust-override",
+        .description = R"(
+            Allow forcing trusting or not trusting clients with
+            `nix-daemon`. This is useful for testing, but possibly also
+            useful for various experiments with `nix-daemon --stdio`
+            networking.
+        )",
+    },
 }};
 
 static_assert(

src/libutil/experimental-features.hh

@@ -28,6 +28,7 @@ enum struct ExperimentalFeature
     AutoAllocateUids,
     Cgroups,
     DiscardReferences,
+    DaemonTrustOverride,
 };
 
 /**

src/libutil/tests/config.cc

@@ -82,6 +82,7 @@ namespace nix {
             TestSetting() : AbstractSetting("test", "test", {}) {}
             void set(const std::string & value, bool append) override {}
             std::string to_string() const override { return {}; }
+            bool isAppendable() override { return false; }
         };
 
         Config config;
@@ -90,6 +91,7 @@ namespace nix {
         ASSERT_FALSE(config.set("test", "value"));
         config.addSetting(&setting);
         ASSERT_TRUE(config.set("test", "value"));
+        ASSERT_FALSE(config.set("extra-test", "value"));
     }
 
     TEST(Config, withInitialValue) {

src/nix-store/nix-store.cc

@@ -941,7 +941,10 @@ static void opServe(Strings opFlags, Strings opArgs)
                 if (GET_PROTOCOL_MINOR(clientVersion) >= 3)
                     out << status.timesBuilt << status.isNonDeterministic << status.startTime << status.stopTime;
                 if (GET_PROTOCOL_MINOR(clientVersion) >= 6) {
-                    worker_proto::write(*store, out, status.builtOutputs);
+                    DrvOutputs builtOutputs;
+                    for (auto & [output, realisation] : status.builtOutputs)
+                        builtOutputs.insert_or_assign(realisation.id, realisation);
+                    worker_proto::write(*store, out, builtOutputs);
                 }
 
                 break;

src/nix/daemon.cc

@@ -273,8 +273,12 @@ static std::pair<TrustedFlag, std::string> authPeer(const PeerInfo & peer)
 /**
  * Run a server. The loop opens a socket and accepts new connections from that
  * socket.
+ *
+ * @param forceTrustClientOpt If present, force trusting or not trusted
+ * the client. Otherwise, decide based on the authentication settings
+ * and user credentials (from the unix domain socket).
  */
-static void daemonLoop()
+static void daemonLoop(std::optional<TrustedFlag> forceTrustClientOpt)
 {
     if (chdir("/") == -1)
         throw SysError("cannot change current directory");
@@ -317,9 +321,18 @@ static void daemonLoop()
 
             closeOnExec(remote.get());
 
-            PeerInfo peer = getPeerInfo(remote.get());
-            auto [_trusted, user] = authPeer(peer);
-            auto trusted = _trusted;
+            PeerInfo peer { .pidKnown = false };
+            TrustedFlag trusted;
+            std::string user;
+
+            if (forceTrustClientOpt)
+                trusted = *forceTrustClientOpt;
+            else {
+                peer = getPeerInfo(remote.get());
+                auto [_trusted, _user] = authPeer(peer);
+                trusted = _trusted;
+                user = _user;
+            };
 
             printInfo((std::string) "accepted connection from pid %1%, user %2%" + (trusted ? " (trusted)" : ""),
                 peer.pidKnown ? std::to_string(peer.pid) : "<unknown>",
@@ -410,38 +423,47 @@ static void forwardStdioConnection(RemoteStore & store) {
  * Unlike `forwardStdioConnection()` we do process commands ourselves in
  * this case, not delegating to another daemon.
  *
- * @note `Trusted` is unconditionally passed because in this mode we
- * blindly trust the standard streams. Limiting access to those is
- * explicitly not `nix-daemon`'s responsibility.
+ * @param trustClient Whether to trust the client. Forwarded directly to
+ * `processConnection()`.
  */
-static void processStdioConnection(ref<Store> store)
+static void processStdioConnection(ref<Store> store, TrustedFlag trustClient)
 {
     FdSource from(STDIN_FILENO);
     FdSink to(STDOUT_FILENO);
-    processConnection(store, from, to, Trusted, NotRecursive);
+    processConnection(store, from, to, trustClient, NotRecursive);
 }
 
 /**
  * Entry point shared between the new CLI `nix daemon` and old CLI
  * `nix-daemon`.
+ *
+ * @param forceTrustClientOpt See `daemonLoop()` and the parameter with
+ * the same name over there for details.
  */
-static void runDaemon(bool stdio)
+static void runDaemon(bool stdio, std::optional<TrustedFlag> forceTrustClientOpt)
 {
     if (stdio) {
         auto store = openUncachedStore();
 
-        if (auto remoteStore = store.dynamic_pointer_cast<RemoteStore>())
+        // If --force-untrusted is passed, we cannot forward the connection and
+        // must process it ourselves (before delegating to the next store) to
+        // force untrusting the client.
+        if (auto remoteStore = store.dynamic_pointer_cast<RemoteStore>(); remoteStore && (!forceTrustClientOpt || *forceTrustClientOpt != NotTrusted))
             forwardStdioConnection(*remoteStore);
         else
-            processStdioConnection(store);
+            // `Trusted` is passed in the auto (no override case) because we
+            // cannot see who is on the other side of a plain pipe. Limiting
+            // access to those is explicitly not `nix-daemon`'s responsibility.
+            processStdioConnection(store, forceTrustClientOpt.value_or(Trusted));
     } else
-        daemonLoop();
+        daemonLoop(forceTrustClientOpt);
 }
 
 static int main_nix_daemon(int argc, char * * argv)
 {
     {
         auto stdio = false;
+        std::optional<TrustedFlag> isTrustedOpt = std::nullopt;
 
         parseCmdLine(argc, argv, [&](Strings::iterator & arg, const Strings::iterator & end) {
             if (*arg == "--daemon")
@@ -452,11 +474,20 @@ static int main_nix_daemon(int argc, char * * argv)
                 printVersion("nix-daemon");
             else if (*arg == "--stdio")
                 stdio = true;
-            else return false;
+            else if (*arg == "--force-trusted") {
+                experimentalFeatureSettings.require(Xp::DaemonTrustOverride);
+                isTrustedOpt = Trusted;
+            } else if (*arg == "--force-untrusted") {
+                experimentalFeatureSettings.require(Xp::DaemonTrustOverride);
+                isTrustedOpt = NotTrusted;
+            } else if (*arg == "--default-trust") {
+                experimentalFeatureSettings.require(Xp::DaemonTrustOverride);
+                isTrustedOpt = std::nullopt;
+            } else return false;
             return true;
         });
 
-        runDaemon(stdio);
+        runDaemon(stdio, isTrustedOpt);
 
         return 0;
     }
@@ -482,7 +513,7 @@ struct CmdDaemon : StoreCommand
 
     void run(ref<Store> store) override
     {
-        runDaemon(false);
+        runDaemon(false, std::nullopt);
     }
 };
 

tests/build-remote-trustless-after.sh

@@ -0,0 +1,2 @@
+outPath=$(readlink -f $TEST_ROOT/result)
+grep 'FOO BAR BAZ' ${remoteDir}/${outPath}

tests/build-remote-trustless-should-fail-0.sh

@@ -0,0 +1,29 @@
+source common.sh
+
+enableFeatures "daemon-trust-override"
+
+restartDaemon
+
+[[ $busybox =~ busybox ]] || skipTest "no busybox"
+
+unset NIX_STORE_DIR
+unset NIX_STATE_DIR
+
+# We first build a dependency of the derivation we eventually want to
+# build.
+nix-build build-hook.nix -A passthru.input2 \
+  -o "$TEST_ROOT/input2" \
+  --arg busybox "$busybox" \
+  --store "$TEST_ROOT/local" \
+  --option system-features bar
+
+# Now when we go to build that downstream derivation, Nix will fail
+# because we cannot trustlessly build input-addressed derivations with
+# `inputDrv` dependencies.
+
+file=build-hook.nix
+prog=$(readlink -e ./nix-daemon-untrusting.sh)
+proto=ssh-ng
+
+expectStderr 1 source build-remote-trustless.sh \
+    | grepQuiet "you are not privileged to build input-addressed derivations"

tests/build-remote-trustless-should-pass-0.sh

@@ -0,0 +1,9 @@
+source common.sh
+
+# Remote trusts us
+file=build-hook.nix
+prog=nix-store
+proto=ssh
+
+source build-remote-trustless.sh
+source build-remote-trustless-after.sh

tests/build-remote-trustless-should-pass-1.sh

@@ -0,0 +1,9 @@
+source common.sh
+
+# Remote trusts us
+file=build-hook.nix
+prog=nix-daemon
+proto=ssh-ng
+
+source build-remote-trustless.sh
+source build-remote-trustless-after.sh

tests/build-remote-trustless-should-pass-3.sh

@@ -0,0 +1,14 @@
+source common.sh
+
+enableFeatures "daemon-trust-override"
+
+restartDaemon
+
+# Remote doesn't trusts us, but this is fine because we are only
+# building (fixed) CA derivations.
+file=build-hook-ca-fixed.nix
+prog=$(readlink -e ./nix-daemon-untrusting.sh)
+proto=ssh-ng
+
+source build-remote-trustless.sh
+source build-remote-trustless-after.sh

tests/build-remote-trustless.sh

@@ -0,0 +1,14 @@
+requireSandboxSupport
+[[ $busybox =~ busybox ]] || skipTest "no busybox"
+
+unset NIX_STORE_DIR
+unset NIX_STATE_DIR
+
+remoteDir=$TEST_ROOT/remote
+
+# Note: ssh{-ng}://localhost bypasses ssh. See tests/build-remote.sh for
+# more details.
+nix-build $file -o $TEST_ROOT/result --max-jobs 0 \
+  --arg busybox $busybox \
+  --store $TEST_ROOT/local \
+  --builders "$proto://localhost?remote-program=$prog&remote-store=${remoteDir}%3Fsystem-features=foo%20bar%20baz - - 1 1 foo,bar,baz"

tests/experimental-features.sh

@@ -23,20 +23,64 @@ source common.sh
 # # Medium case, the configuration effects --help
 # grep_both_ways store gc --help
 
-expect 1 nix --experimental-features 'nix-command' show-config --flake-registry 'https://no'
-nix --experimental-features 'nix-command flakes' show-config --flake-registry 'https://no'
+# Test settings that are gated on experimental features; the setting is ignored
+# with a warning if the experimental feature is not enabled. The order of the
+# `setting = value` lines in the configuration should not matter.
+
+# 'flakes' experimental-feature is disabled before, ignore and warn
+NIX_CONFIG='
+  experimental-features = nix-command
+  accept-flake-config = true
+' nix show-config accept-flake-config 1>$TEST_ROOT/stdout 2>$TEST_ROOT/stderr
+grepQuiet "false" $TEST_ROOT/stdout
+grepQuiet "Ignoring setting 'accept-flake-config' because experimental feature 'flakes' is not enabled" $TEST_ROOT/stderr
+
+# 'flakes' experimental-feature is disabled after, ignore and warn
+NIX_CONFIG='
+  accept-flake-config = true
+  experimental-features = nix-command
+' nix show-config accept-flake-config 1>$TEST_ROOT/stdout 2>$TEST_ROOT/stderr
+grepQuiet "false" $TEST_ROOT/stdout
+grepQuiet "Ignoring setting 'accept-flake-config' because experimental feature 'flakes' is not enabled" $TEST_ROOT/stderr
+
+# 'flakes' experimental-feature is enabled before, process
+NIX_CONFIG='
+  experimental-features = nix-command flakes
+  accept-flake-config = true
+' nix show-config accept-flake-config 1>$TEST_ROOT/stdout 2>$TEST_ROOT/stderr
+grepQuiet "true" $TEST_ROOT/stdout
+grepQuietInverse "Ignoring setting 'accept-flake-config'" $TEST_ROOT/stderr
+
+# 'flakes' experimental-feature is enabled after, process
+NIX_CONFIG='
+  accept-flake-config = true
+  experimental-features = nix-command flakes
+' nix show-config accept-flake-config 1>$TEST_ROOT/stdout 2>$TEST_ROOT/stderr
+grepQuiet "true" $TEST_ROOT/stdout
+grepQuietInverse "Ignoring setting 'accept-flake-config'" $TEST_ROOT/stderr
+
+function exit_code_both_ways {
+    expect 1 nix --experimental-features 'nix-command' "$@" 1>/dev/null
+    nix --experimental-features 'nix-command flakes' "$@" 1>/dev/null
+
+    # Also, the order should not matter
+    expect 1 nix "$@" --experimental-features 'nix-command' 1>/dev/null
+    nix "$@" --experimental-features 'nix-command flakes' 1>/dev/null
+}
+
+exit_code_both_ways show-config --flake-registry 'https://no'
 
 # Double check these are stable
-nix --experimental-features '' --help
-nix --experimental-features '' doctor --help
-nix --experimental-features '' repl --help
-nix --experimental-features '' upgrade-nix --help
+nix --experimental-features '' --help 1>/dev/null
+nix --experimental-features '' doctor --help 1>/dev/null
+nix --experimental-features '' repl --help 1>/dev/null
+nix --experimental-features '' upgrade-nix --help 1>/dev/null
 
 # These 3 arguments are currently given to all commands, which is wrong (as not
 # all care). To deal with fixing later, we simply make them require the
 # nix-command experimental features --- it so happens that the commands we wish
 # stabilizing to do not need them anyways.
 for arg in '--print-build-logs' '--offline' '--refresh'; do
-    nix --experimental-features 'nix-command' "$arg" --help
-    ! nix --experimental-features '' "$arg" --help
+    nix --experimental-features 'nix-command' "$arg" --help 1>/dev/null
+    expect 1 nix --experimental-features '' "$arg" --help 1>/dev/null
 done

tests/local.mk

@@ -70,6 +70,10 @@ nix_tests = \
   check-reqs.sh \
   build-remote-content-addressed-fixed.sh \
   build-remote-content-addressed-floating.sh \
+  build-remote-trustless-should-pass-0.sh \
+  build-remote-trustless-should-pass-1.sh \
+  build-remote-trustless-should-pass-3.sh \
+  build-remote-trustless-should-fail-0.sh \
   nar-access.sh \
   pure-eval.sh \
   eval.sh \

tests/nix-daemon-untrusting.sh

@@ -0,0 +1,3 @@
+#!/bin/sh
+
+exec nix-daemon --force-untrusted "$@"