Merge pull request #13039 from obsidiansystems/derivation-builder-in-separate-file

Move `DerivationBuilder` to its own files

maintainers/flake-module.nix

@@ -284,6 +284,8 @@
               ''^src/libstore/build/goal\.cc$''
               ''^src/libstore/include/nix/store/build/goal\.hh$''
               ''^src/libstore/unix/build/hook-instance\.cc$''
+              ''^src/libstore/unix/build/derivation-builder\.cc$''
+              ''^src/libstore/unix/include/nix/store/build/derivation-builder\.hh$''
               ''^src/libstore/unix/build/local-derivation-goal\.cc$''
               ''^src/libstore/unix/include/nix/store/build/local-derivation-goal\.hh$''
               ''^src/libstore/build/substitution-goal\.cc$''

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

@@ -0,0 +1,52 @@
+#pragma once
+/**
+ * @file Misc type defitions for both local building and remote (RPC building)
+ */
+
+#include "nix/util/hash.hh"
+#include "nix/store/path.hh"
+
+namespace nix {
+
+class Store;
+
+/**
+ * Unless we are repairing, we don't both to test validity and just assume it,
+ * so the choices are `Absent` or `Valid`.
+ */
+enum struct PathStatus {
+    Corrupt,
+    Absent,
+    Valid,
+};
+
+struct InitialOutputStatus
+{
+    StorePath path;
+    PathStatus status;
+    /**
+     * Valid in the store, and additionally non-corrupt if we are repairing
+     */
+    bool isValid() const
+    {
+        return status == PathStatus::Valid;
+    }
+    /**
+     * Merely present, allowed to be corrupt
+     */
+    bool isPresent() const
+    {
+        return status == PathStatus::Corrupt || status == PathStatus::Valid;
+    }
+};
+
+struct InitialOutput
+{
+    bool wanted;
+    Hash outputHash;
+    std::optional<InitialOutputStatus> known;
+};
+
+void runPostBuildHook(Store & store, Logger & logger, const StorePath & drvPath, const StorePathSet & outputPaths);
+
+}

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

@@ -4,9 +4,7 @@
 #include "nix/store/parsed-derivations.hh"
 #include "nix/store/derivations.hh"
 #include "nix/store/derivation-options.hh"
-#ifndef _WIN32
+#include "nix/store/build/derivation-building-misc.hh"
-#  include "nix/store/user-lock.hh"
-#endif
 #include "nix/store/outputs-spec.hh"
 #include "nix/store/store-api.hh"
 #include "nix/store/pathlocks.hh"
@@ -22,40 +20,6 @@ struct HookInstance;
 
 typedef enum {rpAccept, rpDecline, rpPostpone} HookReply;
 
-/**
- * Unless we are repairing, we don't both to test validity and just assume it,
- * so the choices are `Absent` or `Valid`.
- */
-enum struct PathStatus {
-    Corrupt,
-    Absent,
-    Valid,
-};
-
-struct InitialOutputStatus {
-    StorePath path;
-    PathStatus status;
-    /**
-     * Valid in the store, and additionally non-corrupt if we are repairing
-     */
-    bool isValid() const {
-        return status == PathStatus::Valid;
-    }
-    /**
-     * Merely present, allowed to be corrupt
-     */
-    bool isPresent() const {
-        return status == PathStatus::Corrupt
-            || status == PathStatus::Valid;
-    }
-};
-
-struct InitialOutput {
-    bool wanted;
-    Hash outputHash;
-    std::optional<InitialOutputStatus> known;
-};
-
 /** Used internally */
 void runPostBuildHook(
     Store & store,
@@ -308,6 +272,4 @@ struct DerivationGoal : public Goal
     };
 };
 
-MakeError(NotDeterministic, BuildError);
-
 }

src/libstore/include/nix/store/meson.build

@@ -13,6 +13,7 @@ headers = [config_pub_h] + files(
   'binary-cache-store.hh',
   'build-result.hh',
   'build/derivation-goal.hh',
+  'build/derivation-building-misc.hh',
   'build/drv-output-substitution-goal.hh',
   'build/goal.hh',
   'build/substitution-goal.hh',

src/libstore/unix/include/nix/store/build/derivation-builder.hh

@@ -0,0 +1,197 @@
+#pragma once
+///@file
+
+#include "nix/store/build-result.hh"
+#include "nix/store/derivation-options.hh"
+#include "nix/store/build/derivation-building-misc.hh"
+#include "nix/store/derivations.hh"
+#include "nix/store/parsed-derivations.hh"
+#include "nix/util/processes.hh"
+#include "nix/store/restricted-store.hh"
+#include "nix/store/user-lock.hh"
+
+namespace nix {
+
+/**
+ * Parameters by (mostly) `const` reference for `DerivationBuilder`.
+ */
+struct DerivationBuilderParams
+{
+    /** The path of the derivation. */
+    const StorePath & drvPath;
+
+    BuildResult & buildResult;
+
+    /**
+     * The derivation stored at drvPath.
+     *
+     * @todo Remove double indirection by delaying when this is
+     * initialized.
+     */
+    const std::unique_ptr<Derivation> & drv;
+
+    const std::unique_ptr<StructuredAttrs> & parsedDrv;
+    const std::unique_ptr<DerivationOptions> & drvOptions;
+
+    // The remainder is state held during the build.
+
+    /**
+     * All input paths (that is, the union of FS closures of the
+     * immediate input paths).
+     */
+    const StorePathSet & inputPaths;
+
+    /**
+     * @note we do in fact mutate this
+     */
+    std::map<std::string, InitialOutput> & initialOutputs;
+
+    const BuildMode & buildMode;
+
+    DerivationBuilderParams(
+        const StorePath & drvPath,
+        const BuildMode & buildMode,
+        BuildResult & buildResult,
+        const std::unique_ptr<Derivation> & drv,
+        const std::unique_ptr<StructuredAttrs> & parsedDrv,
+        const std::unique_ptr<DerivationOptions> & drvOptions,
+        const StorePathSet & inputPaths,
+        std::map<std::string, InitialOutput> & initialOutputs)
+        : drvPath{drvPath}
+        , buildResult{buildResult}
+        , drv{drv}
+        , parsedDrv{parsedDrv}
+        , drvOptions{drvOptions}
+        , inputPaths{inputPaths}
+        , initialOutputs{initialOutputs}
+        , buildMode{buildMode}
+    { }
+
+    DerivationBuilderParams(DerivationBuilderParams &&) = default;
+};
+
+/**
+ * Callbacks that `DerivationBuilder` needs.
+ */
+struct DerivationBuilderCallbacks
+{
+    /**
+     * Open a log file and a pipe to it.
+     */
+    virtual Path openLogFile() = 0;
+
+    /**
+     * Close the log file.
+     */
+    virtual void closeLogFile() = 0;
+
+    /**
+     * Aborts if any output is not valid or corrupt, and otherwise
+     * returns a 'SingleDrvOutputs' structure containing all outputs.
+     *
+     * @todo Probably should just be in `DerivationGoal`.
+     */
+    virtual SingleDrvOutputs assertPathValidity() = 0;
+
+    virtual void appendLogTailErrorMsg(std::string & msg) = 0;
+
+    /**
+     * Hook up `builderOut` to some mechanism to ingest the log
+     *
+     * @todo this should be reworked
+     */
+    virtual void childStarted() = 0;
+
+    /**
+     * @todo this should be reworked
+     */
+    virtual void childTerminated() = 0;
+
+    virtual void noteHashMismatch(void) = 0;
+    virtual void noteCheckMismatch(void) = 0;
+
+    virtual void markContentsGood(const StorePath & path) = 0;
+};
+
+/**
+ * This class represents the state for building locally.
+ *
+ * @todo Ideally, it would not be a class, but a single function.
+ * However, besides the main entry point, there are a few more methods
+ * which are externally called, and need to be gotten rid of. There are
+ * also some virtual methods (either directly here or inherited from
+ * `DerivationBuilderCallbacks`, a stop-gap) that represent outgoing
+ * rather than incoming call edges that either should be removed, or
+ * become (higher order) function parameters.
+ */
+struct DerivationBuilder : RestrictionContext
+{
+    /**
+     * User selected for running the builder.
+     */
+    std::unique_ptr<UserLock> buildUser;
+
+    /**
+     * The process ID of the builder.
+     */
+    Pid pid;
+
+    DerivationBuilder() = default;
+    virtual ~DerivationBuilder() = default;
+
+    /**
+     * Master side of the pseudoterminal used for the builder's
+     * standard output/error.
+     */
+    AutoCloseFD builderOut;
+
+    /**
+     * Set up build environment / sandbox, acquiring resources (e.g.
+     * locks as needed). After this is run, the builder should be
+     * started.
+     *
+     * @returns true if successful, false if we could not acquire a build
+     * user. In that case, the caller must wait and then try again.
+     */
+    virtual bool prepareBuild() = 0;
+
+    /**
+     * Start building a derivation.
+     */
+    virtual void startBuilder() = 0;
+
+    /**
+     * Tear down build environment after the builder exits (either on
+     * its own or if it is killed).
+     *
+     * @returns The first case indicates failure during output
+     * processing. A status code and exception are returned, providing
+     * more information. The second case indicates success, and
+     * realisations for each output of the derivation are returned.
+     */
+    virtual std::variant<std::pair<BuildResult::Status, Error>, SingleDrvOutputs> unprepareBuild() = 0;
+
+    /**
+     * Stop the in-process nix daemon thread.
+     * @see startDaemon
+     */
+    virtual void stopDaemon() = 0;
+
+    /**
+     * Delete the temporary directory, if we have one.
+     */
+    virtual void deleteTmpDir(bool force) = 0;
+
+    /**
+     * Kill any processes running under the build user UID or in the
+     * cgroup of the build.
+     */
+    virtual void killSandbox(bool getStats) = 0;
+};
+
+std::unique_ptr<DerivationBuilder> makeDerivationBuilder(
+    Store & store,
+    DerivationBuilderCallbacks & miscMethods,
+    DerivationBuilderParams params);
+
+}

src/libstore/unix/include/nix/store/meson.build

@@ -2,6 +2,7 @@ include_dirs += include_directories('../..')
 
 headers += files(
   'build/child.hh',
+  'build/derivation-builder.hh',
   'build/hook-instance.hh',
   'build/local-derivation-goal.hh',
   'user-lock.hh',

src/libstore/unix/meson.build

@@ -1,5 +1,6 @@
 sources += files(
   'build/child.cc',
+  'build/derivation-builder.cc',
   'build/hook-instance.cc',
   'build/local-derivation-goal.cc',
   'pathlocks.cc',