Merge pull request #4570 from obsidiansystems/split-building-planning

Split {,local-}derivation-goal.{cc,hh}

src/libstore/build/derivation-goal.hh

@@ -2,7 +2,8 @@
 
 #include "parsed-derivations.hh"
 #include "lock.hh"
-#include "local-store.hh"
+#include "store-api.hh"
+#include "pathlocks.hh"
 #include "goal.hh"
 
 namespace nix {
@@ -79,18 +80,6 @@ struct DerivationGoal : public Goal
 
     std::map<std::string, InitialOutput> initialOutputs;
 
-    /* User selected for running the builder. */
-    std::unique_ptr<UserLock> buildUser;
-
-    /* The process ID of the builder. */
-    Pid pid;
-
-    /* The temporary directory. */
-    Path tmpDir;
-
-    /* The path of the temporary directory in the sandbox. */
-    Path tmpDirInSandbox;
-
     /* File descriptor for the log file. */
     AutoCloseFD fdLogFile;
     std::shared_ptr<BufferedSink> logFileSink, logSink;
@@ -106,79 +95,15 @@ struct DerivationGoal : public Goal
 
     std::string currentHookLine;
 
-    /* Pipe for the builder's standard output/error. */
-    Pipe builderOut;
-
-    /* Pipe for synchronising updates to the builder namespaces. */
-    Pipe userNamespaceSync;
-
-    /* The mount namespace of the builder, used to add additional
-       paths to the sandbox as a result of recursive Nix calls. */
-    AutoCloseFD sandboxMountNamespace;
-
-    /* On Linux, whether we're doing the build in its own user
-       namespace. */
-    bool usingUserNamespace = true;
-
     /* The build hook. */
     std::unique_ptr<HookInstance> hook;
 
-    /* Whether we're currently doing a chroot build. */
-    bool useChroot = false;
-
-    Path chrootRootDir;
-
-    /* RAII object to delete the chroot directory. */
-    std::shared_ptr<AutoDelete> autoDelChroot;
-
     /* The sort of derivation we are building. */
     DerivationType derivationType;
 
-    /* Whether to run the build in a private network namespace. */
-    bool privateNetwork = false;
-
     typedef void (DerivationGoal::*GoalState)();
     GoalState state;
 
-    /* Stuff we need to pass to initChild(). */
-    struct ChrootPath {
-        Path source;
-        bool optional;
-        ChrootPath(Path source = "", bool optional = false)
-            : source(source), optional(optional)
-        { }
-    };
-    typedef map<Path, ChrootPath> DirsInChroot; // maps target path to source path
-    DirsInChroot dirsInChroot;
-
-    typedef map<string, string> Environment;
-    Environment env;
-
-#if __APPLE__
-    typedef string SandboxProfile;
-    SandboxProfile additionalSandboxProfile;
-#endif
-
-    /* Hash rewriting. */
-    StringMap inputRewrites, outputRewrites;
-    typedef map<StorePath, StorePath> RedirectedOutputs;
-    RedirectedOutputs redirectedOutputs;
-
-    /* The outputs paths used during the build.
-
-       - Input-addressed derivations or fixed content-addressed outputs are
-         sometimes built when some of their outputs already exist, and can not
-         be hidden via sandboxing. We use temporary locations instead and
-         rewrite after the build. Otherwise the regular predetermined paths are
-         put here.
-
-       - Floating content-addressed derivations do not know their final build
-         output paths until the outputs are hashed, so random locations are
-         used, and then renamed. The randomness helps guard against hidden
-         self-references.
-     */
-    OutputPathMap scratchOutputs;
-
     /* The final output paths of the build.
 
        - For input-addressed derivations, always the precomputed paths
@@ -191,11 +116,6 @@ struct DerivationGoal : public Goal
 
     BuildMode buildMode;
 
-    /* If we're repairing without a chroot, there may be outputs that
-       are valid but corrupt.  So we redirect these outputs to
-       temporary paths. */
-    StorePathSet redirectedBadOutputs;
-
     BuildResult result;
 
     /* The current round, if we're building multiple times. */
@@ -203,17 +123,6 @@ struct DerivationGoal : public Goal
 
     size_t nrRounds;
 
-    /* Path registration info from the previous round, if we're
-       building multiple times. Since this contains the hash, it
-       allows us to compare whether two rounds produced the same
-       result. */
-    std::map<Path, ValidPathInfo> prevInfos;
-
-    uid_t sandboxUid() { return usingUserNamespace ? 1000 : buildUser->getUID(); }
-    gid_t sandboxGid() { return usingUserNamespace ?  100 : buildUser->getGID(); }
-
-    const static Path homeDir;
-
     std::unique_ptr<MaintainCount<uint64_t>> mcExpectedBuilds, mcRunningBuilds;
 
     std::unique_ptr<Activity> act;
@@ -226,39 +135,13 @@ struct DerivationGoal : public Goal
     /* The remote machine on which we're building. */
     std::string machineName;
 
-    /* The recursive Nix daemon socket. */
-    AutoCloseFD daemonSocket;
-
-    /* The daemon main thread. */
-    std::thread daemonThread;
-
-    /* The daemon worker threads. */
-    std::vector<std::thread> daemonWorkerThreads;
-
-    /* Paths that were added via recursive Nix calls. */
-    StorePathSet addedPaths;
-
-    /* Recursive Nix calls are only allowed to build or realize paths
-       in the original input closure or added via a recursive Nix call
-       (so e.g. you can't do 'nix-store -r /nix/store/<bla>' where
-       /nix/store/<bla> is some arbitrary path in a binary cache). */
-    bool isAllowed(const StorePath & path)
-    {
-        return inputPaths.count(path) || addedPaths.count(path);
-    }
-
-    friend struct RestrictedStore;
-
     DerivationGoal(const StorePath & drvPath,
         const StringSet & wantedOutputs, Worker & worker,
         BuildMode buildMode = bmNormal);
     DerivationGoal(const StorePath & drvPath, const BasicDerivation & drv,
         const StringSet & wantedOutputs, Worker & worker,
         BuildMode buildMode = bmNormal);
-    ~DerivationGoal();
-
-    /* Whether we need to perform hash rewriting if there are valid output paths. */
-    bool needsHashRewrite();
+    virtual ~DerivationGoal();
 
     void timedOut(Error && ex) override;
 
@@ -280,7 +163,7 @@ struct DerivationGoal : public Goal
     void closureRepaired();
     void inputsRealised();
     void tryToBuild();
-    void tryLocalBuild();
+    virtual void tryLocalBuild();
     void buildDone();
 
     void resolvedFinished();
@@ -288,40 +171,11 @@ struct DerivationGoal : public Goal
     /* Is the build hook willing to perform the build? */
     HookReply tryBuildHook();
 
-    /* Start building a derivation. */
-    void startBuilder();
-
-    /* Fill in the environment for the builder. */
-    void initEnv();
-
-    /* Setup tmp dir location. */
-    void initTmpDir();
-
-    /* Write a JSON file containing the derivation attributes. */
-    void writeStructuredAttrs();
-
-    void startDaemon();
-
-    void stopDaemon();
-
-    /* Add 'path' to the set of paths that may be referenced by the
-       outputs, and make it appear in the sandbox. */
-    void addDependency(const StorePath & path);
-
-    /* Make a file owned by the builder. */
-    void chownToBuilder(const Path & path);
-
-    /* Run the builder's process. */
-    void runChild();
+    virtual int getChildStatus();
 
     /* Check that the derivation outputs all exist and register them
        as valid. */
-    void registerOutputs();
-
-    /* Check that an output meets the requirements specified by the
-       'outputChecks' attribute (or the legacy
-       '{allowed,disallowed}{References,Requisites}' attributes). */
-    void checkOutputs(const std::map<std::string, ValidPathInfo> & outputs);
+    virtual void registerOutputs();
 
     /* Open a log file and a pipe to it. */
     Path openLogFile();
@@ -329,8 +183,18 @@ struct DerivationGoal : public Goal
     /* Close the log file. */
     void closeLogFile();
 
-    /* Delete the temporary directory, if we have one. */
-    void deleteTmpDir(bool force);
+    /* Close the read side of the logger pipe. */
+    virtual void closeReadPipes();
+
+    /* Cleanup hooks for buildDone() */
+    virtual void cleanupHookFinally();
+    virtual void cleanupPreChildKill();
+    virtual void cleanupPostChildKill();
+    virtual bool cleanupDecideWhetherDiskFull();
+    virtual void cleanupPostOutputsRegisteredModeCheck();
+    virtual void cleanupPostOutputsRegisteredModeNonCheck();
+
+    virtual bool isReadDesc(int fd);
 
     /* Callback used by the worker to write to the log. */
     void handleChildOutput(int fd, const string & data) override;
@@ -347,17 +211,7 @@ struct DerivationGoal : public Goal
     void checkPathValidity();
 
     /* Forcibly kill the child process, if any. */
-    void killChild();
-
-    /* Create alternative path calculated from but distinct from the
-       input, so we can avoid overwriting outputs (or other store paths)
-       that already exist. */
-    StorePath makeFallbackPath(const StorePath & path);
-    /* Make a path to another based on the output name along with the
-       derivation hash. */
-    /* FIXME add option to randomize, so we can audit whether our
-       rewrites caught everything */
-    StorePath makeFallbackPath(std::string_view outputName);
+    virtual void killChild();
 
     void repairClosure();
 
@@ -370,4 +224,6 @@ struct DerivationGoal : public Goal
     StorePathSet exportReferences(const StorePathSet & storePaths);
 };
 
+MakeError(NotDeterministic, BuildError);
+
 }

src/libstore/build/entry-points.cc

@@ -2,6 +2,7 @@
 #include "worker.hh"
 #include "substitution-goal.hh"
 #include "derivation-goal.hh"
+#include "local-store.hh"
 
 namespace nix {
 

src/libstore/build/local-derivation-goal.hh

@@ -0,0 +1,199 @@
+#pragma once
+
+#include "derivation-goal.hh"
+#include "local-store.hh"
+
+namespace nix {
+
+struct LocalDerivationGoal : public DerivationGoal
+{
+    LocalStore & getLocalStore();
+
+    /* User selected for running the builder. */
+    std::unique_ptr<UserLock> buildUser;
+
+    /* The process ID of the builder. */
+    Pid pid;
+
+    /* The temporary directory. */
+    Path tmpDir;
+
+    /* The path of the temporary directory in the sandbox. */
+    Path tmpDirInSandbox;
+
+    /* Pipe for the builder's standard output/error. */
+    Pipe builderOut;
+
+    /* Pipe for synchronising updates to the builder namespaces. */
+    Pipe userNamespaceSync;
+
+    /* The mount namespace of the builder, used to add additional
+       paths to the sandbox as a result of recursive Nix calls. */
+    AutoCloseFD sandboxMountNamespace;
+
+    /* On Linux, whether we're doing the build in its own user
+       namespace. */
+    bool usingUserNamespace = true;
+
+    /* Whether we're currently doing a chroot build. */
+    bool useChroot = false;
+
+    Path chrootRootDir;
+
+    /* RAII object to delete the chroot directory. */
+    std::shared_ptr<AutoDelete> autoDelChroot;
+
+    /* Whether to run the build in a private network namespace. */
+    bool privateNetwork = false;
+
+    /* Stuff we need to pass to initChild(). */
+    struct ChrootPath {
+        Path source;
+        bool optional;
+        ChrootPath(Path source = "", bool optional = false)
+            : source(source), optional(optional)
+        { }
+    };
+    typedef map<Path, ChrootPath> DirsInChroot; // maps target path to source path
+    DirsInChroot dirsInChroot;
+
+    typedef map<string, string> Environment;
+    Environment env;
+
+#if __APPLE__
+    typedef string SandboxProfile;
+    SandboxProfile additionalSandboxProfile;
+#endif
+
+    /* Hash rewriting. */
+    StringMap inputRewrites, outputRewrites;
+    typedef map<StorePath, StorePath> RedirectedOutputs;
+    RedirectedOutputs redirectedOutputs;
+
+    /* The outputs paths used during the build.
+
+       - Input-addressed derivations or fixed content-addressed outputs are
+         sometimes built when some of their outputs already exist, and can not
+         be hidden via sandboxing. We use temporary locations instead and
+         rewrite after the build. Otherwise the regular predetermined paths are
+         put here.
+
+       - Floating content-addressed derivations do not know their final build
+         output paths until the outputs are hashed, so random locations are
+         used, and then renamed. The randomness helps guard against hidden
+         self-references.
+     */
+    OutputPathMap scratchOutputs;
+
+    /* Path registration info from the previous round, if we're
+       building multiple times. Since this contains the hash, it
+       allows us to compare whether two rounds produced the same
+       result. */
+    std::map<Path, ValidPathInfo> prevInfos;
+
+    uid_t sandboxUid() { return usingUserNamespace ? 1000 : buildUser->getUID(); }
+    gid_t sandboxGid() { return usingUserNamespace ?  100 : buildUser->getGID(); }
+
+    const static Path homeDir;
+
+    /* The recursive Nix daemon socket. */
+    AutoCloseFD daemonSocket;
+
+    /* The daemon main thread. */
+    std::thread daemonThread;
+
+    /* The daemon worker threads. */
+    std::vector<std::thread> daemonWorkerThreads;
+
+    /* Paths that were added via recursive Nix calls. */
+    StorePathSet addedPaths;
+
+    /* Recursive Nix calls are only allowed to build or realize paths
+       in the original input closure or added via a recursive Nix call
+       (so e.g. you can't do 'nix-store -r /nix/store/<bla>' where
+       /nix/store/<bla> is some arbitrary path in a binary cache). */
+    bool isAllowed(const StorePath & path)
+    {
+        return inputPaths.count(path) || addedPaths.count(path);
+    }
+
+    friend struct RestrictedStore;
+
+    using DerivationGoal::DerivationGoal;
+
+    virtual ~LocalDerivationGoal() override;
+
+    /* Whether we need to perform hash rewriting if there are valid output paths. */
+    bool needsHashRewrite();
+
+    /* The additional states. */
+    void tryLocalBuild() override;
+
+    /* Start building a derivation. */
+    void startBuilder();
+
+    /* Fill in the environment for the builder. */
+    void initEnv();
+
+    /* Setup tmp dir location. */
+    void initTmpDir();
+
+    /* Write a JSON file containing the derivation attributes. */
+    void writeStructuredAttrs();
+
+    void startDaemon();
+
+    void stopDaemon();
+
+    /* Add 'path' to the set of paths that may be referenced by the
+       outputs, and make it appear in the sandbox. */
+    void addDependency(const StorePath & path);
+
+    /* Make a file owned by the builder. */
+    void chownToBuilder(const Path & path);
+
+    int getChildStatus() override;
+
+    /* Run the builder's process. */
+    void runChild();
+
+    /* Check that the derivation outputs all exist and register them
+       as valid. */
+    void registerOutputs() override;
+
+    /* Check that an output meets the requirements specified by the
+       'outputChecks' attribute (or the legacy
+       '{allowed,disallowed}{References,Requisites}' attributes). */
+    void checkOutputs(const std::map<std::string, ValidPathInfo> & outputs);
+
+    /* Close the read side of the logger pipe. */
+    void closeReadPipes() override;
+
+    /* Cleanup hooks for buildDone() */
+    void cleanupHookFinally() override;
+    void cleanupPreChildKill() override;
+    void cleanupPostChildKill() override;
+    bool cleanupDecideWhetherDiskFull() override;
+    void cleanupPostOutputsRegisteredModeCheck() override;
+    void cleanupPostOutputsRegisteredModeNonCheck() override;
+
+    bool isReadDesc(int fd) override;
+
+    /* Delete the temporary directory, if we have one. */
+    void deleteTmpDir(bool force);
+
+    /* Forcibly kill the child process, if any. */
+    void killChild() override;
+
+    /* Create alternative path calculated from but distinct from the
+       input, so we can avoid overwriting outputs (or other store paths)
+       that already exist. */
+    StorePath makeFallbackPath(const StorePath & path);
+    /* Make a path to another based on the output name along with the
+       derivation hash. */
+    /* FIXME add option to randomize, so we can audit whether our
+       rewrites caught everything */
+    StorePath makeFallbackPath(std::string_view outputName);
+};
+
+}

src/libstore/build/worker.cc

@@ -1,7 +1,7 @@
 #include "machines.hh"
 #include "worker.hh"
 #include "substitution-goal.hh"
-#include "derivation-goal.hh"
+#include "local-derivation-goal.hh"
 #include "hook-instance.hh"
 
 #include <poll.h>
@@ -59,8 +59,10 @@ std::shared_ptr<DerivationGoal> Worker::makeDerivationGoalCommon(
 std::shared_ptr<DerivationGoal> Worker::makeDerivationGoal(const StorePath & drvPath,
     const StringSet & wantedOutputs, BuildMode buildMode)
 {
-    return makeDerivationGoalCommon(drvPath, wantedOutputs, [&]() {
-        return std::make_shared<DerivationGoal>(drvPath, wantedOutputs, *this, buildMode);
+    return makeDerivationGoalCommon(drvPath, wantedOutputs, [&]() -> std::shared_ptr<DerivationGoal> {
+        return !dynamic_cast<LocalStore *>(&store)
+            ? std::make_shared</* */DerivationGoal>(drvPath, wantedOutputs, *this, buildMode)
+            : std::make_shared<LocalDerivationGoal>(drvPath, wantedOutputs, *this, buildMode);
     });
 }
 
@@ -68,8 +70,10 @@ std::shared_ptr<DerivationGoal> Worker::makeDerivationGoal(const StorePath & drv
 std::shared_ptr<DerivationGoal> Worker::makeBasicDerivationGoal(const StorePath & drvPath,
     const BasicDerivation & drv, const StringSet & wantedOutputs, BuildMode buildMode)
 {
-    return makeDerivationGoalCommon(drvPath, wantedOutputs, [&]() {
-        return std::make_shared<DerivationGoal>(drvPath, drv, wantedOutputs, *this, buildMode);
+    return makeDerivationGoalCommon(drvPath, wantedOutputs, [&]() -> std::shared_ptr<DerivationGoal> {
+        return !dynamic_cast<LocalStore *>(&store)
+            ? std::make_shared</* */DerivationGoal>(drvPath, drv, wantedOutputs, *this, buildMode)
+            : std::make_shared<LocalDerivationGoal>(drvPath, drv, wantedOutputs, *this, buildMode);
     });
 }
 

src/libstore/local-store.hh

@@ -280,7 +280,7 @@ private:
 
     void createUser(const std::string & userName, uid_t userId) override;
 
-    friend struct DerivationGoal;
+    friend struct LocalDerivationGoal;
     friend struct SubstitutionGoal;
 };