Merge remote-tracking branch 'origin/2.29-maintenance' into detsys-main

src/libstore/build/drv-output-substitution-goal.cc

@@ -3,6 +3,7 @@
 #include "nix/store/build/worker.hh"
 #include "nix/store/build/substitution-goal.hh"
 #include "nix/util/callback.hh"
+#include "nix/store/store-open.hh"
 
 namespace nix {
 
@@ -11,7 +12,7 @@ DrvOutputSubstitutionGoal::DrvOutputSubstitutionGoal(
     Worker & worker,
     RepairFlag repair,
     std::optional<ContentAddress> ca)
-    : Goal(worker, DerivedPath::Opaque { StorePath::dummy })
+    : Goal(worker)
     , id(id)
 {
     name = fmt("substitution of '%s'", id.to_string());
@@ -87,6 +88,8 @@ Goal::Co DrvOutputSubstitutionGoal::init()
 
         bool failed = false;
 
+        Goals waitees;
+
         for (const auto & [depId, depPath] : outputInfo->dependentRealisations) {
             if (depId != id) {
                 if (auto localOutputInfo = worker.store.queryRealisation(depId);
@@ -103,13 +106,13 @@ Goal::Co DrvOutputSubstitutionGoal::init()
                     failed = true;
                     break;
                 }
-                addWaitee(worker.makeDrvOutputSubstitutionGoal(depId));
+                waitees.insert(worker.makeDrvOutputSubstitutionGoal(depId));
             }
         }
 
         if (failed) continue;
 
-        co_return realisationFetched(outputInfo, sub);
+        co_return realisationFetched(std::move(waitees), outputInfo, sub);
     }
 
     /* None left.  Terminate this goal and let someone else deal
@@ -127,10 +130,10 @@ Goal::Co DrvOutputSubstitutionGoal::init()
     co_return amDone(substituterFailed ? ecFailed : ecNoSubstituters);
 }
 
-Goal::Co DrvOutputSubstitutionGoal::realisationFetched(std::shared_ptr<const Realisation> outputInfo, nix::ref<nix::Store> sub) {
-    addWaitee(worker.makePathSubstitutionGoal(outputInfo->outPath));
+Goal::Co DrvOutputSubstitutionGoal::realisationFetched(Goals waitees, std::shared_ptr<const Realisation> outputInfo, nix::ref<nix::Store> sub) {
+    waitees.insert(worker.makePathSubstitutionGoal(outputInfo->outPath));
 
-    if (!waitees.empty()) co_await Suspend{};
+    co_await await(std::move(waitees));
 
     trace("output path substituted");
 

src/libstore/build/goal.cc

@@ -132,38 +132,18 @@ void addToWeakGoals(WeakGoals & goals, GoalPtr p)
     goals.insert(p);
 }
 
-
-void Goal::addWaitee(GoalPtr waitee)
+Co Goal::await(Goals new_waitees)
 {
-    waitees.insert(waitee);
-    addToWeakGoals(waitee->waiters, shared_from_this());
-}
-
-
-void Goal::waiteeDone(GoalPtr waitee, ExitCode result)
-{
-    assert(waitees.count(waitee));
-    waitees.erase(waitee);
-
-    trace(fmt("waitee '%s' done; %d left", waitee->name, waitees.size()));
-
-    if (result == ecFailed || result == ecNoSubstituters || result == ecIncompleteClosure) ++nrFailed;
-
-    if (result == ecNoSubstituters) ++nrNoSubstituters;
-
-    if (result == ecIncompleteClosure) ++nrIncompleteClosure;
-
-    if (waitees.empty() || (result == ecFailed && !settings.keepGoing)) {
-
-        /* If we failed and keepGoing is not set, we remove all
-           remaining waitees. */
-        for (auto & goal : waitees) {
-            goal->waiters.extract(shared_from_this());
+    assert(waitees.empty());
+    if (!new_waitees.empty()) {
+        waitees = std::move(new_waitees);
+        for (auto waitee : waitees) {
+            addToWeakGoals(waitee->waiters, shared_from_this());
         }
-        waitees.clear();
-
-        worker.wakeUp(shared_from_this());
+        co_await Suspend{};
+        assert(waitees.empty());
     }
+    co_return Return{};
 }
 
 Goal::Done Goal::amDone(ExitCode result, std::optional<Error> ex)
@@ -183,7 +163,32 @@ Goal::Done Goal::amDone(ExitCode result, std::optional<Error> ex)
 
     for (auto & i : waiters) {
         GoalPtr goal = i.lock();
-        if (goal) goal->waiteeDone(shared_from_this(), result);
+        if (goal) {
+            auto me = shared_from_this();
+            assert(goal->waitees.count(me));
+            goal->waitees.erase(me);
+
+            goal->trace(fmt("waitee '%s' done; %d left", name, goal->waitees.size()));
+
+            if (result == ecFailed || result == ecNoSubstituters || result == ecIncompleteClosure) ++goal->nrFailed;
+
+            if (result == ecNoSubstituters) ++goal->nrNoSubstituters;
+
+            if (result == ecIncompleteClosure) ++goal->nrIncompleteClosure;
+
+            if (goal->waitees.empty()) {
+                worker.wakeUp(goal);
+            } else if (result == ecFailed && !settings.keepGoing) {
+                /* If we failed and keepGoing is not set, we remove all
+                   remaining waitees. */
+                for (auto & g : goal->waitees) {
+                    g->waiters.extract(goal);
+                }
+                goal->waitees.clear();
+
+                worker.wakeUp(goal);
+            }
+        }
     }
     waiters.clear();
     worker.removeGoal(shared_from_this());
@@ -215,5 +220,22 @@ void Goal::work()
     assert(top_co || exitCode != ecBusy);
 }
 
+Goal::Co Goal::yield() {
+    worker.wakeUp(shared_from_this());
+    co_await Suspend{};
+    co_return Return{};
+}
+
+Goal::Co Goal::waitForAWhile() {
+    worker.waitForAWhile(shared_from_this());
+    co_await Suspend{};
+    co_return Return{};
+}
+
+Goal::Co Goal::waitForBuildSlot() {
+    worker.waitForBuildSlot(shared_from_this());
+    co_await Suspend{};
+    co_return Return{};
+}
 
 }

src/libstore/build/substitution-goal.cc

@@ -1,4 +1,5 @@
 #include "nix/store/build/worker.hh"
+#include "nix/store/store-open.hh"
 #include "nix/store/build/substitution-goal.hh"
 #include "nix/store/nar-info.hh"
 #include "nix/util/finally.hh"
@@ -11,7 +12,7 @@
 namespace nix {
 
 PathSubstitutionGoal::PathSubstitutionGoal(const StorePath & storePath, Worker & worker, RepairFlag repair, std::optional<ContentAddress> ca)
-    : Goal(worker, DerivedPath::Opaque { storePath })
+    : Goal(worker)
     , storePath(storePath)
     , repair(repair)
     , ca(ca)
@@ -133,20 +134,22 @@ Goal::Co PathSubstitutionGoal::init()
         /* Bail out early if this substituter lacks a valid
            signature. LocalStore::addToStore() also checks for this, but
            only after we've downloaded the path. */
-        if (!sub->isTrusted && worker.store.pathInfoIsUntrusted(*info))
+        if (!sub->config.isTrusted && worker.store.pathInfoIsUntrusted(*info))
         {
             warn("ignoring substitute for '%s' from '%s', as it's not signed by any of the keys in 'trusted-public-keys'",
                 worker.store.printStorePath(storePath), sub->getUri());
             continue;
         }
 
+        Goals waitees;
+
         /* To maintain the closure invariant, we first have to realise the
            paths referenced by this one. */
         for (auto & i : info->references)
             if (i != storePath) /* ignore self-references */
-                addWaitee(worker.makePathSubstitutionGoal(i));
+                waitees.insert(worker.makePathSubstitutionGoal(i));
 
-        if (!waitees.empty()) co_await Suspend{};
+        co_await await(std::move(waitees));
 
         // FIXME: consider returning boolean instead of passing in reference
         bool out = false; // is mutated by tryToRun
@@ -184,11 +187,15 @@ Goal::Co PathSubstitutionGoal::tryToRun(StorePath subPath, nix::ref<Store> sub,
     }
 
     for (auto & i : info->references)
-        if (i != storePath) /* ignore self-references */
-            assert(worker.store.isValidPath(i));
+         /* ignore self-references */
+        if (i != storePath) {
+            if (!worker.store.isValidPath(i)) {
+                throw Error("reference '%s' of path '%s' is not a valid path",
+                            worker.store.printStorePath(i), worker.store.printStorePath(storePath));
+            }
+        }
 
-    worker.wakeUp(shared_from_this());
-    co_await Suspend{};
+    co_await yield();
 
     trace("trying to run");
 
@@ -196,8 +203,7 @@ Goal::Co PathSubstitutionGoal::tryToRun(StorePath subPath, nix::ref<Store> sub,
        if maxSubstitutionJobs == 0, we still allow a substituter to run. This
        prevents infinite waiting. */
     while (worker.getNrSubstitutions() >= std::max(1U, (unsigned int) settings.maxSubstitutionJobs)) {
-        worker.waitForBuildSlot(shared_from_this());
-        co_await Suspend{};
+        co_await waitForBuildSlot();
     }
 
     auto maintainRunningSubstitutions = std::make_unique<MaintainCount<uint64_t>>(worker.runningSubstitutions);
@@ -222,7 +228,7 @@ Goal::Co PathSubstitutionGoal::tryToRun(StorePath subPath, nix::ref<Store> sub,
             PushActivity pact(act.id);
 
             copyStorePath(*sub, worker.store,
-                subPath, repair, sub->isTrusted ? NoCheckSigs : CheckSigs);
+                subPath, repair, sub->config.isTrusted ? NoCheckSigs : CheckSigs);
 
             promise.set_value();
         } catch (...) {

src/libstore/build/worker.cc

@@ -5,7 +5,6 @@
 #include "nix/store/build/drv-output-substitution-goal.hh"
 #include "nix/store/build/derivation-goal.hh"
 #ifndef _WIN32 // TODO Enable building on Windows
-#  include "nix/store/build/local-derivation-goal.hh"
 #  include "nix/store/build/hook-instance.hh"
 #endif
 #include "nix/util/signals.hh"
@@ -65,13 +64,7 @@ std::shared_ptr<DerivationGoal> Worker::makeDerivationGoal(const StorePath & drv
     const OutputsSpec & wantedOutputs, BuildMode buildMode)
 {
     return makeDerivationGoalCommon(drvPath, wantedOutputs, [&]() -> std::shared_ptr<DerivationGoal> {
-        return
-#ifndef _WIN32 // TODO Enable building on Windows
-            dynamic_cast<LocalStore *>(&store)
-            ? std::make_shared<LocalDerivationGoal>(drvPath, wantedOutputs, *this, buildMode)
-            :
-#endif
-            std::make_shared</* */DerivationGoal>(drvPath, wantedOutputs, *this, buildMode);
+        return std::make_shared<DerivationGoal>(drvPath, wantedOutputs, *this, buildMode);
     });
 }
 
@@ -79,13 +72,7 @@ std::shared_ptr<DerivationGoal> Worker::makeBasicDerivationGoal(const StorePath
     const BasicDerivation & drv, const OutputsSpec & wantedOutputs, BuildMode buildMode)
 {
     return makeDerivationGoalCommon(drvPath, wantedOutputs, [&]() -> std::shared_ptr<DerivationGoal> {
-        return
-#ifndef _WIN32 // TODO Enable building on Windows
-            dynamic_cast<LocalStore *>(&store)
-            ? std::make_shared<LocalDerivationGoal>(drvPath, drv, wantedOutputs, *this, buildMode)
-            :
-#endif
-            std::make_shared</* */DerivationGoal>(drvPath, drv, wantedOutputs, *this, buildMode);
+        return std::make_shared<DerivationGoal>(drvPath, drv, wantedOutputs, *this, buildMode);
     });
 }
 
@@ -524,7 +511,7 @@ bool Worker::pathContentsGood(const StorePath & path)
         res = false;
     else {
         auto current = hashPath(
-            {store.getFSAccessor(), CanonPath(store.printStorePath(path))},
+            {store.getFSAccessor(), CanonPath(path.to_string())},
             FileIngestionMethod::NixArchive, info->narHash.algo).first;
         Hash nullHash(HashAlgorithm::SHA256);
         res = info->narHash == nullHash || info->narHash == current;
@@ -552,4 +539,9 @@ GoalPtr upcast_goal(std::shared_ptr<DrvOutputSubstitutionGoal> subGoal)
     return subGoal;
 }
 
+GoalPtr upcast_goal(std::shared_ptr<DerivationGoal> subGoal)
+{
+    return subGoal;
+}
+
 }