Make DerivationBuilder not require a Store

src/libstore/build/derivation-builder-impl.hh

@@ -36,7 +36,13 @@ protected:
      */
     Pid pid;
 
-    LocalStore & store;
+    std::shared_ptr<BuildingStore> store;
+
+    /**
+     * Just the store layout, for FFI: a `StoreDirConfig` can be made
+     * over FFI where a whole `Store` cannot.
+     */
+    const StoreDirConfig & storeDirConfig;
 
     std::shared_ptr<DerivationBuilderCallbacks> miscMethods;
 
@@ -52,7 +58,7 @@ protected:
      */
     const derivation::Type derivationType;
 
-    const LocalSettings & localSettings = store.config->getLocalSettings();
+    const LocalSettings & localSettings = store->getLocalSettings();
 
 #ifndef _WIN32
     /**
@@ -92,41 +98,34 @@ protected:
      */
     virtual std::filesystem::path realPathInHost(const std::filesystem::path & p)
     {
-        return store.toRealPath(store.parseStorePath(p.string()));
+        return store->toRealPath(storeDirConfig.parseStorePath(p.string()));
     }
 
 
 public:
 
     DerivationBuilderImpl(
-        LocalStore & store, std::shared_ptr<DerivationBuilderCallbacks> miscMethods, DerivationBuilderParams params)
+        std::shared_ptr<BuildingStore> store,
+        std::shared_ptr<DerivationBuilderCallbacks> miscMethods,
+        DerivationBuilderParams params)
         : DerivationBuilderParams{std::move(params)}
-        , store{store}
+        , store{std::move(store)}
+        , storeDirConfig{*this->store}
         , miscMethods{std::move(miscMethods)}
         , derivationType{derivation::type(drv)}
     {
     }
 
-protected:
+public:
 
-    /**
-     * Check that the derivation outputs all exist and register them
-     * as valid.
-     *
-     * For subclasses to call at the end of `unprepareBuild`.
-     */
-    SingleDrvOutputs registerOutputs();
+    SingleDrvOutputs registerOutputs(LocalStore & localStore) override;
 
     /**
      * Output paths from the `SubmitOutput` store command
      */
     Sync<OutputPathMap> submittedOutputs;
 
-    /**
-     * Check that the derivation outputs submitted by recursive-nix exist
-     * and attach them to the derivation
-     */
-    SingleDrvOutputs checkSubmittedOutputs();
+    SingleDrvOutputs checkSubmittedOutputs(LocalStore & localStore) override;
 };
 
 } // namespace nix

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

@@ -967,13 +967,13 @@ Goal::Co DerivationBuildingGoal::buildLocally(
                           throw UnimplementedError("external builders are not yet supported on Windows")
 #else
                           makeExternalDerivationBuilder(
-                              localBuildCap.localStore,
+                              makeBuildingStoreFromLocalStore(localBuildCap.localStore),
                               std::make_shared<DerivationBuildingGoalCallbacks>(*this, openLogFile, closeLogFile),
                               std::move(params),
                               *localBuildCap.externalBuilder)
 #endif
                           : makeDerivationBuilder(
-                                localBuildCap.localStore,
+                                makeBuildingStoreFromLocalStore(localBuildCap.localStore),
                                 std::make_shared<DerivationBuildingGoalCallbacks>(*this, openLogFile, closeLogFile),
                                 std::move(params)
 #ifdef _WIN32
@@ -1030,9 +1030,34 @@ Goal::Co DerivationBuildingGoal::buildLocally(
 
     trace("build done");
 
+    auto [status, diskFull] = builder->unprepareBuild();
+
+    /* Check the exit status. */
+    if (!statusOk(status)) {
+        builder->cleanupBuild(false);
+        builder.reset();
+        outputLocks.unlock();
+        co_return doneFailure(fixupBuilderFailureErrorMessage(
+            {
+                !derivation::type(*drv).isSandboxed() || diskFull ? BuildResult::Failure::TransientFailure
+                                                                  : BuildResult::Failure::PermanentFailure,
+                status,
+                diskFull ? "\nnote: build failure may have been caused by lack of free disk space" : "",
+            },
+            *buildLog));
+    }
+
     SingleDrvOutputs builtOutputs;
     try {
-        builtOutputs = builder->unprepareBuild();
+        /* Compute the FS closure of the outputs and register them as
+           being valid. With builder-rpc-v0 the builder already submitted
+           the outputs, so check those instead. */
+        auto * localStoreP = dynamic_cast<LocalStore *>(&worker.store);
+        assert(localStoreP);
+        builtOutputs = drvOptions.getRequiredSystemFeatures(*drv).count(std::string{drvFeatureBuilderRpcV0})
+                           ? builder->checkSubmittedOutputs(*localStoreP)
+                           : builder->registerOutputs(*localStoreP);
+        builder->cleanupBuild(true);
     } catch (BuilderFailureError & e) {
         builder.reset();
         outputLocks.unlock();

src/libstore/build/derivation-check.cc

@@ -10,7 +10,7 @@
 namespace nix {
 
 void checkCAOutput(
-    StoreDirConfig & store,
+    const StoreDirConfig & store,
     const StorePath & drvPath,
     const DerivationOutput & outputSpec,
     const ValidPathInfo & info,

src/libstore/build/derivation-check.hh

@@ -13,7 +13,7 @@ namespace nix {
  * Do nothing if outputSpec is not a CAFixed or CAFloating output.
  */
 void checkCAOutput(
-    StoreDirConfig & store,
+    const StoreDirConfig & store,
     const StorePath & drvPath,
     const DerivationOutput & outputSpec,
     const ValidPathInfo & info,

src/libstore/darwin/build/darwin-derivation-builder.cc

@@ -63,21 +63,21 @@ void DarwinDerivationBuilder::setUser()
 
         /* And we want the store in there regardless of how empty pathsInChroot. We include the innermost
            path component this time, since it's typically /nix/store and we care about that. */
-        std::filesystem::path cur = store.storeDir;
+        std::filesystem::path cur = storeDirConfig.storeDir;
         while (cur != "/") {
             ancestry.insert(cur.native());
             cur = cur.parent_path();
         }
 
         /* Add all our input paths to the chroot */
         for (auto & i : inputPaths) {
-            auto p = store.printStorePath(i);
+            auto p = storeDirConfig.printStorePath(i);
             pathsInChroot.insert_or_assign(p, ChrootPath{.source = p});
         }
 
         /* Violations will go to the syslog if you set this. Unfortunately the destination does not appear to be
          * configurable */
-        if (store.config->getLocalSettings().darwinLogSandboxViolations) {
+        if (store->getLocalSettings().darwinLogSandboxViolations) {
             sandboxProfile += "(deny default)\n";
         } else {
             sandboxProfile += "(deny default (with no-log))\n";
@@ -95,7 +95,7 @@ void DarwinDerivationBuilder::setUser()
         /* Add the output paths we'll use at build-time to the chroot */
         sandboxProfile += "(allow file-read* file-write* process-exec\n";
         for (auto & [_, path] : scratchOutputs)
-            sandboxProfile += fmt("\t(subpath \"%s\")\n", store.printStorePath(path));
+            sandboxProfile += fmt("\t(subpath \"%s\")\n", storeDirConfig.printStorePath(path));
 
         sandboxProfile += ")\n";
 

src/libstore/darwin/build/darwin-derivation-builder.hh

@@ -15,11 +15,11 @@ struct DarwinDerivationBuilder : UnixDerivationBuilderImpl
     bool useSandbox;
 
     DarwinDerivationBuilder(
-        LocalStore & store,
+        std::shared_ptr<BuildingStore> store,
         std::shared_ptr<DerivationBuilderCallbacks> miscMethods,
         DerivationBuilderParams params,
         bool useSandbox)
-        : UnixDerivationBuilderImpl(store, miscMethods, std::move(params))
+        : UnixDerivationBuilderImpl(std::move(store), miscMethods, std::move(params))
         , useSandbox(useSandbox)
     {
     }

src/libstore/freebsd/build/chroot-freebsd-derivation-builder.hh

@@ -12,7 +12,9 @@ struct ChrootFreeBSDDerivationBuilder : ChrootDerivationBuilder, FreeBSDDerivati
     std::shared_ptr<AutoRemoveJail> autoDelJail = std::make_shared<AutoRemoveJail>();
 
     ChrootFreeBSDDerivationBuilder(
-        LocalStore & store, std::shared_ptr<DerivationBuilderCallbacks> miscMethods, DerivationBuilderParams params)
+        std::shared_ptr<BuildingStore> store,
+        std::shared_ptr<DerivationBuilderCallbacks> miscMethods,
+        DerivationBuilderParams params)
         : UnixDerivationBuilderImpl{store, miscMethods, params}
         , ChrootDerivationBuilder{store, miscMethods, params}
         , FreeBSDDerivationBuilder{store, miscMethods, params}

src/libstore/freebsd/build/freebsd-derivation-builder.cc

@@ -210,15 +210,15 @@ void ChrootFreeBSDDerivationBuilder::prepareSandbox()
             .uid = 0,
             .gid = 0,
             .description = "Nix build user",
-            .home = store.config->getLocalSettings().sandboxBuildDir,
+            .home = store->getLocalSettings().sandboxBuildDir,
             .shell = "/noshell",
         },
         {
             .name = "nixbld",
             .uid = buildUser->getUID(),
             .gid = sandboxGid(),
             .description = "Nix build user",
-            .home = store.config->getLocalSettings().sandboxBuildDir,
+            .home = store->getLocalSettings().sandboxBuildDir,
             .shell = "/noshell",
         },
         {
@@ -303,7 +303,7 @@ void ChrootFreeBSDDerivationBuilder::prepareSandbox()
         debug("setting up a nullfs mount from %1% to %2%", PathFmt(chrootPath.source), PathFmt(path));
 
         int flags = 0;
-        if (store.isInStore(target.native()))
+        if (storeDirConfig.isInStore(target.native()))
             /* While we are at it, enforce invariants about store paths. Anything located at the "logical" store
                location must be readonly (file permission canonicalisation enforces this on the host filesystem).
                Also the store must never contain setuid binaries for the same reason. This is just defense-in-depth. */
@@ -478,7 +478,8 @@ void ChrootFreeBSDDerivationBuilder::enterChroot()
 void ChrootFreeBSDDerivationBuilder::addDependencyImpl(const StorePath & path)
 {
     throw UnimplementedError(
-        "adding store path '%s' to the sandbox is not implemented (recursive-nix)", store.printStorePath(path));
+        "adding store path '%s' to the sandbox is not implemented (recursive-nix)",
+        storeDirConfig.printStorePath(path));
 }
 
 } // namespace nix

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

@@ -155,6 +155,23 @@ struct DerivationBuilderCallbacks
         daemon::RecursiveFlag recursiveFlag) = 0;
 };
 
+/**
+ * The outcome of tearing down the build environment, from
+ * `DerivationBuilder::unprepareBuild`.
+ */
+struct BuilderExit
+{
+    /**
+     * The builder's exit status.
+     */
+    int status;
+
+    /**
+     * Whether the disk seemed full when the builder exited.
+     */
+    bool diskFull = false;
+};
+
 /**
  * This class represents the state for building locally.
  *
@@ -234,14 +251,34 @@ public:
      * 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.
+     * @returns The builder's exit status and whether the disk seemed
+     * full at exit time.
+     */
+    virtual BuilderExit unprepareBuild() = 0;
+
+    /**
+     * Check that the derivation outputs all exist and register them
+     * as valid.
+     *
+     * Not used with `builder-rpc-v0`; see `checkSubmittedOutputs`.
+     */
+    virtual SingleDrvOutputs registerOutputs(LocalStore & store) = 0;
+
+    /**
+     * Check that the derivation outputs submitted by recursive-nix
+     * exist and attach them to the derivation.
+     *
+     * Only used with `builder-rpc-v0`.
+     */
+    virtual SingleDrvOutputs checkSubmittedOutputs(LocalStore & store) = 0;
+
+    /**
+     * Delete the temporary directory, if we have one.
      *
-     * @throws BuildError
+     * @param force We know the build succeeded, so don't attempt to
+     * preserve anything for debugging.
      */
-    virtual SingleDrvOutputs unprepareBuild() = 0;
+    virtual void cleanupBuild(bool force) = 0;
 
     /**
      * Forcibly kill the child process, if any.
@@ -272,6 +309,42 @@ struct ExternalBuilder
     std::vector<std::string> args;
 };
 
+struct LocalSettings;
+
+/**
+ * This type exists to aid with FFI: we cannot make a full `LocalStore`
+ * with everything (including building, which uses this!) from FFI, but
+ * we do have a chance of making something that just has the methods we
+ * actually need from `LocalStore`.
+ */
+struct BuildingStore : StoreDirConfig
+{
+    BuildingStore(const std::string & storeDir)
+        : StoreDirConfig{storeDir}
+    {
+    }
+
+    virtual ~BuildingStore();
+
+    virtual std::filesystem::path getRealStoreDir() const = 0;
+
+    virtual std::filesystem::path getBuildDir() const = 0;
+
+    virtual const LocalSettings & getLocalSettings() const = 0;
+
+    /**
+     * Make the store that recursive-Nix daemon connections talk to.
+     */
+    virtual ref<Store> makeRecursiveNixStore(RestrictionContext & ctx) = 0;
+
+    std::filesystem::path toRealPath(const StorePath & storePath) const
+    {
+        return getRealStoreDir() / std::string(storePath.to_string());
+    }
+};
+
+std::unique_ptr<BuildingStore> makeBuildingStoreFromLocalStore(LocalStore &);
+
 struct DerivationBuilderDeleter
 {
     void operator()(DerivationBuilder * builder) noexcept;
@@ -283,7 +356,7 @@ using DerivationBuilderUnique = std::unique_ptr<DerivationBuilder, DerivationBui
  * @param ioport The worker's I/O completion port, which the log pipe is tied to.
  */
 DerivationBuilderUnique makeDerivationBuilder(
-    LocalStore & store,
+    std::unique_ptr<BuildingStore> store,
     std::shared_ptr<DerivationBuilderCallbacks> miscMethods,
     DerivationBuilderParams params
 #ifdef _WIN32
@@ -298,7 +371,7 @@ DerivationBuilderUnique makeDerivationBuilder(
  * derivation.
  */
 DerivationBuilderUnique makeExternalDerivationBuilder(
-    LocalStore & store,
+    std::unique_ptr<BuildingStore> store,
     std::shared_ptr<DerivationBuilderCallbacks> miscMethods,
     DerivationBuilderParams params,
     const ExternalBuilder & handler);

src/libstore/linux/build/chroot-linux-derivation-builder.hh

@@ -31,7 +31,9 @@ struct ChrootLinuxDerivationBuilder : ChrootDerivationBuilder, LinuxDerivationBu
     std::optional<std::filesystem::path> cgroup;
 
     ChrootLinuxDerivationBuilder(
-        LocalStore & store, std::shared_ptr<DerivationBuilderCallbacks> miscMethods, DerivationBuilderParams params)
+        std::shared_ptr<BuildingStore> store,
+        std::shared_ptr<DerivationBuilderCallbacks> miscMethods,
+        DerivationBuilderParams params)
         : UnixDerivationBuilderImpl{store, miscMethods, params}
         , ChrootDerivationBuilder{store, miscMethods, params}
         , LinuxDerivationBuilder{store, miscMethods, params}
@@ -54,7 +56,7 @@ struct ChrootLinuxDerivationBuilder : ChrootDerivationBuilder, LinuxDerivationBu
 
     void setUser() override;
 
-    SingleDrvOutputs unprepareBuild() override;
+    BuilderExit unprepareBuild() override;
 
     void killSandbox(bool getStats) override;