From a87589a0359ac01fe03086cbd46d396e12256f65 Mon Sep 17 00:00:00 2001
From: Las <me@las.rs>
Date: Mon, 10 Mar 2025 22:31:13 +0000
Subject: [PATCH] Simplify local drv goal a bit more

- `chrootParentDir` can be a local variable instead of a class variable.

- `getChildStatus` can be inlined. Again, we have the `assert(!hook);`
  in the local building case, which makes for a simpler thing inlined.
---
 src/libstore/build/derivation-goal.cc           | 17 ++++++-----------
 src/libstore/build/derivation-goal.hh           |  2 --
 .../unix/build/local-derivation-goal.cc         | 10 ++--------
 .../unix/build/local-derivation-goal.hh         |  9 ---------
 4 files changed, 8 insertions(+), 30 deletions(-)

diff --git a/src/libstore/build/derivation-goal.cc b/src/libstore/build/derivation-goal.cc
index c5931f00e..476f29238 100644
--- a/src/libstore/build/derivation-goal.cc
+++ b/src/libstore/build/derivation-goal.cc
@@ -801,16 +801,6 @@ void replaceValidPath(const Path & storePath, const Path & tmpPath)
 }
 
 
-int DerivationGoal::getChildStatus()
-{
-#ifndef _WIN32 // TODO enable build hook on Windows
-    return hook->pid.kill();
-#else
-    return 0;
-#endif
-}
-
-
 void runPostBuildHook(
     Store & store,
     Logger & logger,
@@ -908,7 +898,12 @@ Goal::Co DerivationGoal::hookDone()
        to have terminated.  In fact, the builder could also have
        simply have closed its end of the pipe, so just to be sure,
        kill it. */
-    int status = getChildStatus();
+    int status =
+#ifndef _WIN32 // TODO enable build hook on Windows
+        hook->pid.kill();
+#else
+        0;
+#endif
 
     debug("builder process for '%s' finished", worker.store.printStorePath(drvPath));
 
diff --git a/src/libstore/build/derivation-goal.hh b/src/libstore/build/derivation-goal.hh
index 0fd9afa71..177892996 100644
--- a/src/libstore/build/derivation-goal.hh
+++ b/src/libstore/build/derivation-goal.hh
@@ -262,8 +262,6 @@ struct DerivationGoal : public Goal
      */
     HookReply tryBuildHook();
 
-    virtual int getChildStatus();
-
     /**
      * Check that the derivation outputs all exist and register them
      * as valid.
diff --git a/src/libstore/unix/build/local-derivation-goal.cc b/src/libstore/unix/build/local-derivation-goal.cc
index bfc5e4c34..91dc85fdc 100644
--- a/src/libstore/unix/build/local-derivation-goal.cc
+++ b/src/libstore/unix/build/local-derivation-goal.cc
@@ -282,7 +282,7 @@ Goal::Co LocalDerivationGoal::tryLocalBuild()
        to have terminated.  In fact, the builder could also have
        simply have closed its end of the pipe, so just to be sure,
        kill it. */
-    int status = getChildStatus();
+    int status = pid.kill();
 
     debug("builder process for '%s' finished", worker.store.printStorePath(drvPath));
 
@@ -412,12 +412,6 @@ static void movePath(const Path & src, const Path & dst)
 extern void replaceValidPath(const Path & storePath, const Path & tmpPath);
 
 
-int LocalDerivationGoal::getChildStatus()
-{
-    return hook ? DerivationGoal::getChildStatus() : pid.kill();
-}
-
-
 bool LocalDerivationGoal::cleanupDecideWhetherDiskFull()
 {
     bool diskFull = false;
@@ -787,7 +781,7 @@ void LocalDerivationGoal::startBuilder()
            environment using bind-mounts.  We put it in the Nix store
            so that the build outputs can be moved efficiently from the
            chroot to their final location. */
-        chrootParentDir = worker.store.Store::toRealPath(drvPath) + ".chroot";
+        auto chrootParentDir = worker.store.Store::toRealPath(drvPath) + ".chroot";
         deletePath(chrootParentDir);
 
         /* Clean up the chroot directory automatically. */
diff --git a/src/libstore/unix/build/local-derivation-goal.hh b/src/libstore/unix/build/local-derivation-goal.hh
index d52008eef..67d544b65 100644
--- a/src/libstore/unix/build/local-derivation-goal.hh
+++ b/src/libstore/unix/build/local-derivation-goal.hh
@@ -71,13 +71,6 @@ struct LocalDerivationGoal : public DerivationGoal
      */
     bool useChroot = false;
 
-    /**
-     * The parent directory of `chrootRootDir`. It has permission 700
-     * and is owned by root to ensure other users cannot mess with
-     * `chrootRootDir`.
-     */
-    Path chrootParentDir;
-
     /**
      * The root of the chroot environment.
      */
@@ -237,8 +230,6 @@ struct LocalDerivationGoal : public DerivationGoal
      */
     void chownToBuilder(const Path & path);
 
-    int getChildStatus() override;
-
     /**
      * Run the builder's process.
      */