libstore: hide Worker goal factory methods

this doesn't serve a great purpose yet except to confine construction of
goals to the stack frame of Worker::run() and its child frames. we don't
need this yet (and the goal constructors remain fully visible), but in a
future change that fully removes the current worker loop we'll need some
way of knowing which goals are top-level goals without passing the goals
themselves around. once that's possible we can remove visible goals as a
concept and rely on build result futures and a scheduler built upon them

Change-Id: Ia73cdeffcfb9ba1ce9d69b702dc0bc637a4c4ce6

src/libstore/build/derivation-goal.cc

@@ -170,7 +170,7 @@ Goal::WorkResult DerivationGoal::getDerivation(bool inBuildSlot)
 
 
     state = &DerivationGoal::loadDerivation;
-    return WaitForGoals{{worker.makePathSubstitutionGoal(drvPath)}};
+    return WaitForGoals{{worker.goalFactory().makePathSubstitutionGoal(drvPath)}};
 }
 
 
@@ -268,14 +268,14 @@ Goal::WorkResult DerivationGoal::haveDerivation(bool inBuildSlot)
                 if (!status.wanted) continue;
                 if (!status.known)
                     result.goals.insert(
-                        worker.makeDrvOutputSubstitutionGoal(
+                        worker.goalFactory().makeDrvOutputSubstitutionGoal(
                             DrvOutput{status.outputHash, outputName},
                             buildMode == bmRepair ? Repair : NoRepair
                         )
                     );
                 else {
                     auto * cap = getDerivationCA(*drv);
-                    result.goals.insert(worker.makePathSubstitutionGoal(
+                    result.goals.insert(worker.goalFactory().makePathSubstitutionGoal(
                         status.known->path,
                         buildMode == bmRepair ? Repair : NoRepair,
                         cap ? std::optional { *cap } : std::nullopt));
@@ -374,7 +374,7 @@ Goal::WorkResult DerivationGoal::gaveUpOnSubstitution(bool inBuildSlot)
 
         addWaiteeDerivedPath = [&](ref<SingleDerivedPath> inputDrv, const DerivedPathMap<StringSet>::ChildNode & inputNode) {
             if (!inputNode.value.empty())
-                result.goals.insert(worker.makeGoal(
+                result.goals.insert(worker.goalFactory().makeGoal(
                     DerivedPath::Built {
                         .drvPath = inputDrv,
                         .outputs = inputNode.value,
@@ -419,7 +419,7 @@ Goal::WorkResult DerivationGoal::gaveUpOnSubstitution(bool inBuildSlot)
         if (!settings.useSubstitutes)
             throw Error("dependency '%s' of '%s' does not exist, and substitution is disabled",
                 worker.store.printStorePath(i), worker.store.printStorePath(drvPath));
-        result.goals.insert(worker.makePathSubstitutionGoal(i));
+        result.goals.insert(worker.goalFactory().makePathSubstitutionGoal(i));
     }
 
     if (result.goals.empty()) {/* to prevent hang (no wake-up event) */
@@ -475,9 +475,9 @@ Goal::WorkResult DerivationGoal::repairClosure()
             worker.store.printStorePath(i), worker.store.printStorePath(drvPath));
         auto drvPath2 = outputsToDrv.find(i);
         if (drvPath2 == outputsToDrv.end())
-            result.goals.insert(worker.makePathSubstitutionGoal(i, Repair));
+            result.goals.insert(worker.goalFactory().makePathSubstitutionGoal(i, Repair));
         else
-            result.goals.insert(worker.makeGoal(
+            result.goals.insert(worker.goalFactory().makeGoal(
                 DerivedPath::Built {
                     .drvPath = makeConstantStorePathRef(drvPath2->second),
                     .outputs = OutputsSpec::All { },
@@ -580,7 +580,7 @@ Goal::WorkResult DerivationGoal::inputsRealised(bool inBuildSlot)
                        worker.store.printStorePath(pathResolved),
                    });
 
-            resolvedDrvGoal = worker.makeDerivationGoal(
+            resolvedDrvGoal = worker.goalFactory().makeDerivationGoal(
                 pathResolved, wantedOutputs, buildMode);
 
             state = &DerivationGoal::resolvedFinished;

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

@@ -112,11 +112,11 @@ Goal::WorkResult DrvOutputSubstitutionGoal::realisationFetched(bool inBuildSlot)
                 );
                 return tryNext(inBuildSlot);
             }
-            result.goals.insert(worker.makeDrvOutputSubstitutionGoal(depId));
+            result.goals.insert(worker.goalFactory().makeDrvOutputSubstitutionGoal(depId));
         }
     }
 
-    result.goals.insert(worker.makePathSubstitutionGoal(outputInfo->outPath));
+    result.goals.insert(worker.goalFactory().makePathSubstitutionGoal(outputInfo->outPath));
 
     if (result.goals.empty()) {
         return outPathValid(inBuildSlot);

src/libstore/build/entry-points.cc

@@ -10,11 +10,12 @@ void Store::buildPaths(const std::vector<DerivedPath> & reqs, BuildMode buildMod
 {
     Worker worker(*this, evalStore ? *evalStore : *this);
 
-    Goals goals;
+    auto goals = worker.run([&](GoalFactory & gf) {
-    for (auto & br : reqs)
+        Goals goals;
-        goals.insert(worker.makeGoal(br, buildMode));
+        for (auto & br : reqs)
-
+            goals.insert(gf.makeGoal(br, buildMode));
-    worker.run(goals);
+        return goals;
+    });
 
     StringSet failed;
     std::shared_ptr<Error> ex;
@@ -48,17 +49,17 @@ std::vector<KeyedBuildResult> Store::buildPathsWithResults(
     std::shared_ptr<Store> evalStore)
 {
     Worker worker(*this, evalStore ? *evalStore : *this);
-
-    Goals goals;
     std::vector<std::pair<const DerivedPath &, GoalPtr>> state;
 
-    for (const auto & req : reqs) {
+    auto goals = worker.run([&](GoalFactory & gf) {
-        auto goal = worker.makeGoal(req, buildMode);
+        Goals goals;
-        goals.insert(goal);
+        for (const auto & req : reqs) {
-        state.push_back({req, goal});
+            auto goal = gf.makeGoal(req, buildMode);
-    }
+            goals.insert(goal);
-
+            state.push_back({req, goal});
-    worker.run(goals);
+        }
+        return goals;
+    });
 
     std::vector<KeyedBuildResult> results;
 
@@ -72,10 +73,12 @@ BuildResult Store::buildDerivation(const StorePath & drvPath, const BasicDerivat
     BuildMode buildMode)
 {
     Worker worker(*this, *this);
-    auto goal = worker.makeBasicDerivationGoal(drvPath, drv, OutputsSpec::All {}, buildMode);
 
     try {
-        worker.run(Goals{goal});
+        auto goals = worker.run([&](GoalFactory & gf) -> Goals {
+            return Goals{gf.makeBasicDerivationGoal(drvPath, drv, OutputsSpec::All{}, buildMode)};
+        });
+        auto goal = *goals.begin();
         return goal->buildResult.restrictTo(DerivedPath::Built {
             .drvPath = makeConstantStorePathRef(drvPath),
             .outputs = OutputsSpec::All {},
@@ -95,10 +98,10 @@ void Store::ensurePath(const StorePath & path)
     if (isValidPath(path)) return;
 
     Worker worker(*this, *this);
-    GoalPtr goal = worker.makePathSubstitutionGoal(path);
-    Goals goals = {goal};
 
-    worker.run(goals);
+    auto goals =
+        worker.run([&](GoalFactory & gf) { return Goals{gf.makePathSubstitutionGoal(path)}; });
+    auto goal = *goals.begin();
 
     if (goal->exitCode != Goal::ecSuccess) {
         if (goal->ex) {
@@ -113,23 +116,27 @@ void Store::ensurePath(const StorePath & path)
 void Store::repairPath(const StorePath & path)
 {
     Worker worker(*this, *this);
-    GoalPtr goal = worker.makePathSubstitutionGoal(path, Repair);
-    Goals goals = {goal};
 
-    worker.run(goals);
+    auto goals = worker.run([&](GoalFactory & gf) {
+        return Goals{gf.makePathSubstitutionGoal(path, Repair)};
+    });
+    auto goal = *goals.begin();
 
     if (goal->exitCode != Goal::ecSuccess) {
         /* Since substituting the path didn't work, if we have a valid
            deriver, then rebuild the deriver. */
         auto info = queryPathInfo(path);
         if (info->deriver && isValidPath(*info->deriver)) {
-            goals.clear();
+            worker.run([&](GoalFactory & gf) {
-            goals.insert(worker.makeGoal(DerivedPath::Built {
+                return Goals{gf.makeGoal(
-                .drvPath = makeConstantStorePathRef(*info->deriver),
+                    DerivedPath::Built{
-                // FIXME: Should just build the specific output we need.
+                        .drvPath = makeConstantStorePathRef(*info->deriver),
-                .outputs = OutputsSpec::All { },
+                        // FIXME: Should just build the specific output we need.
-            }, bmRepair));
+                        .outputs = OutputsSpec::All{},
-            worker.run(goals);
+                    },
+                    bmRepair
+                )};
+            });
         } else
             throw Error(worker.failingExitStatus(), "cannot repair path '%s'", printStorePath(path));
     }

src/libstore/build/substitution-goal.cc

@@ -161,7 +161,7 @@ Goal::WorkResult PathSubstitutionGoal::tryNext(bool inBuildSlot)
     WaitForGoals result;
     for (auto & i : info->references)
         if (i != storePath) /* ignore self-references */
-            result.goals.insert(worker.makePathSubstitutionGoal(i));
+            result.goals.insert(worker.goalFactory().makePathSubstitutionGoal(i));
 
     if (result.goals.empty()) {/* to prevent hang (no wake-up event) */
         return referencesValid(inBuildSlot);

src/libstore/build/worker.cc

@@ -326,8 +326,9 @@ void Worker::waitForAWhile(GoalPtr goal)
 }
 
 
-void Worker::run(const Goals & _topGoals)
+Goals Worker::run(std::function<Goals (GoalFactory &)> req)
 {
+    auto _topGoals = req(goalFactory());
     std::vector<nix::DerivedPath> topPaths;
 
     assert(!running);
@@ -411,6 +412,8 @@ void Worker::run(const Goals & _topGoals)
     assert(!settings.keepGoing || awake.empty());
     assert(!settings.keepGoing || wantingToBuild.empty());
     assert(!settings.keepGoing || children.empty());
+
+    return _topGoals;
 }
 
 void Worker::waitForInput()

src/libstore/build/worker.hh

@@ -40,10 +40,57 @@ struct Child
 /* Forward definition. */
 struct HookInstance;
 
+class GoalFactory
+{
+public:
+    virtual std::shared_ptr<DerivationGoal> makeDerivationGoal(
+        const StorePath & drvPath, const OutputsSpec & wantedOutputs, BuildMode buildMode = bmNormal
+    ) = 0;
+    virtual std::shared_ptr<DerivationGoal> makeBasicDerivationGoal(
+        const StorePath & drvPath,
+        const BasicDerivation & drv,
+        const OutputsSpec & wantedOutputs,
+        BuildMode buildMode = bmNormal
+    ) = 0;
+
+    /**
+     * @ref SubstitutionGoal "substitution goal"
+     */
+    virtual std::shared_ptr<PathSubstitutionGoal> makePathSubstitutionGoal(
+        const StorePath & storePath,
+        RepairFlag repair = NoRepair,
+        std::optional<ContentAddress> ca = std::nullopt
+    ) = 0;
+    virtual std::shared_ptr<DrvOutputSubstitutionGoal> makeDrvOutputSubstitutionGoal(
+        const DrvOutput & id,
+        RepairFlag repair = NoRepair,
+        std::optional<ContentAddress> ca = std::nullopt
+    ) = 0;
+
+    /**
+     * Make a goal corresponding to the `DerivedPath`.
+     *
+     * It will be a `DerivationGoal` for a `DerivedPath::Built` or
+     * a `SubstitutionGoal` for a `DerivedPath::Opaque`.
+     */
+    virtual GoalPtr makeGoal(const DerivedPath & req, BuildMode buildMode = bmNormal) = 0;
+};
+
+// elaborate hoax to let goals access factory methods while hiding them from the public
+class WorkerBase : protected GoalFactory
+{
+    friend struct DerivationGoal;
+    friend struct PathSubstitutionGoal;
+    friend class DrvOutputSubstitutionGoal;
+
+protected:
+    GoalFactory & goalFactory() { return *this; }
+};
+
 /**
  * The worker class.
  */
-class Worker
+class Worker : public WorkerBase
 {
 private:
 
@@ -215,19 +262,18 @@ private:
     std::shared_ptr<DerivationGoal> makeDerivationGoalCommon(
         const StorePath & drvPath, const OutputsSpec & wantedOutputs,
         std::function<std::shared_ptr<DerivationGoal>()> mkDrvGoal);
-public:
     std::shared_ptr<DerivationGoal> makeDerivationGoal(
         const StorePath & drvPath,
-        const OutputsSpec & wantedOutputs, BuildMode buildMode = bmNormal);
+        const OutputsSpec & wantedOutputs, BuildMode buildMode = bmNormal) override;
     std::shared_ptr<DerivationGoal> makeBasicDerivationGoal(
         const StorePath & drvPath, const BasicDerivation & drv,
-        const OutputsSpec & wantedOutputs, BuildMode buildMode = bmNormal);
+        const OutputsSpec & wantedOutputs, BuildMode buildMode = bmNormal) override;
 
     /**
      * @ref SubstitutionGoal "substitution goal"
      */
-    std::shared_ptr<PathSubstitutionGoal> makePathSubstitutionGoal(const StorePath & storePath, RepairFlag repair = NoRepair, std::optional<ContentAddress> ca = std::nullopt);
+    std::shared_ptr<PathSubstitutionGoal> makePathSubstitutionGoal(const StorePath & storePath, RepairFlag repair = NoRepair, std::optional<ContentAddress> ca = std::nullopt) override;
-    std::shared_ptr<DrvOutputSubstitutionGoal> makeDrvOutputSubstitutionGoal(const DrvOutput & id, RepairFlag repair = NoRepair, std::optional<ContentAddress> ca = std::nullopt);
+    std::shared_ptr<DrvOutputSubstitutionGoal> makeDrvOutputSubstitutionGoal(const DrvOutput & id, RepairFlag repair = NoRepair, std::optional<ContentAddress> ca = std::nullopt) override;
 
     /**
      * Make a goal corresponding to the `DerivedPath`.
@@ -235,8 +281,9 @@ public:
      * It will be a `DerivationGoal` for a `DerivedPath::Built` or
      * a `SubstitutionGoal` for a `DerivedPath::Opaque`.
      */
-    GoalPtr makeGoal(const DerivedPath & req, BuildMode buildMode = bmNormal);
+    GoalPtr makeGoal(const DerivedPath & req, BuildMode buildMode = bmNormal) override;
 
+public:
     /**
      * Unregisters a running child process.
      */
@@ -245,7 +292,7 @@ public:
     /**
      * Loop until the specified top-level goals have finished.
      */
-    void run(const Goals & topGoals);
+    Goals run(std::function<Goals (GoalFactory &)> req);
 
     /***
      * The exit status in case of failure.