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
e437b08 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.

Author: Ericson2314

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

@@ -456,7 +456,7 @@ Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution()
         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,
@@ -466,7 +466,7 @@ Goal::Co DerivationBuildingGoal::gaveUpOnSubstitution()
             Logger::Fields{
                 worker.store.printStorePath(drvPath),
 #ifndef _WIN32 // TODO enable build hook on Windows
-                hook ? machineName :
+                hook ? hook->machineName :
 #endif
                      "",
                 1,
@@ -1042,7 +1042,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;

src/libstore/include/nix/store/build/derivation-building-goal.hh

@@ -87,11 +87,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;

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 efficency: 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;