From 450633aa8cd0c0871164a24ac34eac2386218bc7 Mon Sep 17 00:00:00 2001
From: John Ericson <John.Ericson@Obsidian.Systems>
Date: Fri, 29 Aug 2025 15:49:58 -0400
Subject: [PATCH] Move `machineName` from `DerivationBuildingGoal` to
 `HookInstance`

Exactly why is is correct is a little subtle, because sometimes the
worker is owned by the worker. But the commit message in
e437b0825018b1935f9a849382c12b1df0aeae06 explained the situation well
enough: I made that commit message part of the ABI docs, and now it
should be understandable to the next person.
---
 src/libstore/build/derivation-building-goal.cc  |  6 +++---
 .../nix/store/build/derivation-building-goal.hh |  5 -----
 .../include/nix/store/build/hook-instance.hh    | 17 +++++++++++++++++
 3 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/src/libstore/build/derivation-building-goal.cc b/src/libstore/build/derivation-building-goal.cc
index 008549acb..75295eab7 100644
--- a/src/libstore/build/derivation-building-goal.cc
+++ b/src/libstore/build/derivation-building-goal.cc
@@ -450,7 +450,7 @@ Goal::Co DerivationBuildingGoal::tryToBuild()
         fmt("building '%s'", worker.store.printStorePath(drvPath));
 #ifndef _WIN32 // TODO enable build hook on Windows
         if (hook)
-            msg += fmt(" on '%s'", machineName);
+            msg += fmt(" on '%s'", hook->machineName);
 #endif
         act = std::make_unique<Activity>(
             *logger,
@@ -460,7 +460,7 @@ Goal::Co DerivationBuildingGoal::tryToBuild()
             Logger::Fields{
                 worker.store.printStorePath(drvPath),
 #ifndef _WIN32 // TODO enable build hook on Windows
-                hook ? machineName :
+                hook ? hook->machineName :
 #endif
                      "",
                 1,
@@ -1027,7 +1027,7 @@ HookReply DerivationBuildingGoal::tryBuildHook()
     hook = std::move(worker.hook);
 
     try {
-        machineName = readLine(hook->fromHook.readSide.get());
+        hook->machineName = readLine(hook->fromHook.readSide.get());
     } catch (Error & e) {
         e.addTrace({}, "while reading the machine name from the build hook");
         throw;
diff --git a/src/libstore/include/nix/store/build/derivation-building-goal.hh b/src/libstore/include/nix/store/build/derivation-building-goal.hh
index 2cb111760..07f9b21ae 100644
--- a/src/libstore/include/nix/store/build/derivation-building-goal.hh
+++ b/src/libstore/include/nix/store/build/derivation-building-goal.hh
@@ -95,11 +95,6 @@ private:
 
     std::map<ActivityId, Activity> builderActivities;
 
-    /**
-     * The remote machine on which we're building.
-     */
-    std::string machineName;
-
     void timedOut(Error && ex) override;
 
     std::string key() override;
diff --git a/src/libstore/unix/include/nix/store/build/hook-instance.hh b/src/libstore/unix/include/nix/store/build/hook-instance.hh
index 87e03665c..7657d5dbd 100644
--- a/src/libstore/unix/include/nix/store/build/hook-instance.hh
+++ b/src/libstore/unix/include/nix/store/build/hook-instance.hh
@@ -7,6 +7,14 @@
 
 namespace nix {
 
+/**
+ * @note Sometimes this is owned by the `Worker`, and sometimes it is
+ * owned by a `Goal`. This is for efficiency: rather than starting the
+ * hook every time we want to ask whether we can run a remote build
+ * (which can be very often), we reuse a hook process for answering
+ * those queries until it accepts a build.  So if there are N
+ * derivations to be built, at most N hooks will be started.
+ */
 struct HookInstance
 {
     /**
@@ -29,6 +37,15 @@ struct HookInstance
      */
     Pid pid;
 
+    /**
+     * The remote machine on which we're building.
+     *
+     * @Invariant When the hook instance is owned by the `Worker`, this
+     * is the empty string. When it is owned by a `Goal`, this should be
+     * set.
+     */
+    std::string machineName;
+
     FdSink sink;
 
     std::map<ActivityId, Activity> activities;