Use std::filesystem::path in DerivationBuilder

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

@@ -581,7 +581,7 @@ Goal::Co DerivationBuildingGoal::tryToBuild()
                     goal.worker.childTerminated(&goal);
                 }
 
-                Path openLogFile() override
+                std::filesystem::path openLogFile() override
                 {
                     return goal.openLogFile();
                 }
@@ -602,8 +602,9 @@ Goal::Co DerivationBuildingGoal::tryToBuild()
             StorePathSet closure;
             for (auto & i : defaultPathsInChroot)
                 try {
-                    if (worker.store.isInStore(i.second.source))
-                        worker.store.computeFSClosure(worker.store.toStorePath(i.second.source).first, closure);
+                    if (worker.store.isInStore(i.second.source.string()))
+                        worker.store.computeFSClosure(
+                            worker.store.toStorePath(i.second.source.string()).first, closure);
                 } catch (InvalidPath & e) {
                 } catch (Error & e) {
                     e.addTrace({}, "while processing sandbox path '%s'", i.second.source);

src/libstore/globals.cc

@@ -335,7 +335,16 @@ void BaseSetting<SandboxMode>::convertToArg(Args & args, const std::string & cat
     });
 }
 
-NLOHMANN_DEFINE_TYPE_NON_INTRUSIVE(ChrootPath, source, optional)
+void to_json(nlohmann::json & j, const ChrootPath & cp)
+{
+    j = nlohmann::json{{"source", cp.source.string()}, {"optional", cp.optional}};
+}
+
+void from_json(const nlohmann::json & j, ChrootPath & cp)
+{
+    cp.source = j.at("source").get<std::string>();
+    cp.optional = j.at("optional").get<bool>();
+}
 
 template<>
 PathsInChroot BaseSetting<PathsInChroot>::parse(const std::string & str) const
@@ -368,7 +377,9 @@ std::string BaseSetting<PathsInChroot>::to_string() const
 {
     std::vector<std::string> accum;
     for (auto & [name, cp] : value) {
-        std::string s = name == cp.source ? name : name + "=" + cp.source;
+        auto nameStr = name.string();
+        auto sourceStr = cp.source.string();
+        std::string s = name == cp.source ? nameStr : nameStr + "=" + sourceStr;
         if (cp.optional)
             s += "?";
         accum.push_back(std::move(s));

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

@@ -1,6 +1,7 @@
 #pragma once
 ///@file
 
+#include <filesystem>
 #include <nlohmann/json_fwd.hpp>
 
 #include "nix/store/build-result.hh"
@@ -43,11 +44,11 @@ struct BuilderFailureError : BuildError
  */
 struct ChrootPath
 {
-    Path source;
+    std::filesystem::path source;
     bool optional = false;
 };
 
-typedef std::map<Path, ChrootPath> PathsInChroot; // maps target path to source path
+typedef std::map<std::filesystem::path, ChrootPath> PathsInChroot; // maps target path to source path
 
 /**
  * Parameters by (mostly) `const` reference for `DerivationBuilder`.
@@ -110,7 +111,7 @@ struct DerivationBuilderCallbacks
     /**
      * Open a log file and a pipe to it.
      */
-    virtual Path openLogFile() = 0;
+    virtual std::filesystem::path openLogFile() = 0;
 
     /**
      * Close the log file.
@@ -185,7 +186,7 @@ struct DerivationBuilder : RestrictionContext
 struct ExternalBuilder
 {
     StringSet systems;
-    Path program;
+    std::filesystem::path program;
     std::vector<std::string> args;
 };
 

src/libstore/unix/build/chroot-derivation-builder.cc

@@ -13,7 +13,7 @@ struct ChrootDerivationBuilder : virtual DerivationBuilderImpl
     /**
      * The root of the chroot environment.
      */
-    Path chrootRootDir;
+    std::filesystem::path chrootRootDir;
 
     /**
      * RAII object to delete the chroot directory.
@@ -36,15 +36,15 @@ struct ChrootDerivationBuilder : virtual DerivationBuilderImpl
            On macOS, we don't use an actual chroot, so this isn't
            possible. Any mitigation along these lines would have to be
            done directly in the sandbox profile. */
-        tmpDir = topTmpDir + "/build";
+        tmpDir = topTmpDir / "build";
         createDir(tmpDir, 0700);
     }
 
-    Path tmpDirInSandbox() override
+    std::filesystem::path tmpDirInSandbox() override
     {
         /* In a sandbox, for determinism, always use the same temporary
            directory. */
-        return settings.sandboxBuildDir;
+        return settings.sandboxBuildDir.get();
     }
 
     virtual gid_t sandboxGid()
@@ -58,57 +58,57 @@ struct ChrootDerivationBuilder : virtual DerivationBuilderImpl
            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. */
-        auto chrootParentDir = store.toRealPath(drvPath) + ".chroot";
+        std::filesystem::path chrootParentDir = store.toRealPath(drvPath) + ".chroot";
         deletePath(chrootParentDir);
 
         /* Clean up the chroot directory automatically. */
         autoDelChroot = std::make_shared<AutoDelete>(chrootParentDir);
 
-        printMsg(lvlChatty, "setting up chroot environment in '%1%'", chrootParentDir);
+        printMsg(lvlChatty, "setting up chroot environment in %1%", chrootParentDir);
 
         if (mkdir(chrootParentDir.c_str(), 0700) == -1)
-            throw SysError("cannot create '%s'", chrootRootDir);
+            throw SysError("cannot create %s", chrootRootDir);
 
-        chrootRootDir = chrootParentDir + "/root";
+        chrootRootDir = chrootParentDir / "root";
 
         if (mkdir(chrootRootDir.c_str(), buildUser && buildUser->getUIDCount() != 1 ? 0755 : 0750) == -1)
-            throw SysError("cannot create '%1%'", chrootRootDir);
+            throw SysError("cannot create %1%", chrootRootDir);
 
         if (buildUser
             && chown(
                    chrootRootDir.c_str(), buildUser->getUIDCount() != 1 ? buildUser->getUID() : 0, buildUser->getGID())
                    == -1)
-            throw SysError("cannot change ownership of '%1%'", chrootRootDir);
+            throw SysError("cannot change ownership of %1%", chrootRootDir);
 
         /* Create a writable /tmp in the chroot.  Many builders need
            this.  (Of course they should really respect $TMPDIR
            instead.) */
-        Path chrootTmpDir = chrootRootDir + "/tmp";
+        std::filesystem::path chrootTmpDir = chrootRootDir / "tmp";
         createDirs(chrootTmpDir);
         chmod_(chrootTmpDir, 01777);
 
         /* Create a /etc/passwd with entries for the build user and the
            nobody account.  The latter is kind of a hack to support
            Samba-in-QEMU. */
-        createDirs(chrootRootDir + "/etc");
+        createDirs(chrootRootDir / "etc");
         if (drvOptions.useUidRange(drv))
-            chownToBuilder(chrootRootDir + "/etc");
+            chownToBuilder(chrootRootDir / "etc");
 
         if (drvOptions.useUidRange(drv) && (!buildUser || buildUser->getUIDCount() < 65536))
             throw Error("feature 'uid-range' requires the setting '%s' to be enabled", settings.autoAllocateUids.name);
 
         /* Declare the build user's group so that programs get a consistent
            view of the system (e.g., "id -gn"). */
         writeFile(
-            chrootRootDir + "/etc/group",
+            chrootRootDir / "etc" / "group",
             fmt("root:x:0:\n"
                 "nixbld:!:%1%:\n"
                 "nogroup:x:65534:\n",
                 sandboxGid()));
 
         /* Create /etc/hosts with localhost entry. */
         if (derivationType.isSandboxed())
-            writeFile(chrootRootDir + "/etc/hosts", "127.0.0.1 localhost\n::1 localhost\n");
+            writeFile(chrootRootDir / "etc" / "hosts", "127.0.0.1 localhost\n::1 localhost\n");
 
         /* Make the closure of the inputs available in the chroot,
            rather than the whole Nix store.  This prevents any access
@@ -117,12 +117,12 @@ struct ChrootDerivationBuilder : virtual DerivationBuilderImpl
            can be bind-mounted).  !!! As an extra security
            precaution, make the fake Nix store only writable by the
            build user. */
-        Path chrootStoreDir = chrootRootDir + store.storeDir;
+        std::filesystem::path chrootStoreDir = chrootRootDir / std::filesystem::path(store.storeDir).relative_path();
         createDirs(chrootStoreDir);
         chmod_(chrootStoreDir, 01775);
 
         if (buildUser && chown(chrootStoreDir.c_str(), 0, buildUser->getGID()) == -1)
-            throw SysError("cannot change ownership of '%1%'", chrootStoreDir);
+            throw SysError("cannot change ownership of %1%", chrootStoreDir);
 
         pathsInChroot = getPathsInSandbox();
 
@@ -150,13 +150,14 @@ struct ChrootDerivationBuilder : virtual DerivationBuilderImpl
     Strings getPreBuildHookArgs() override
     {
         assert(!chrootRootDir.empty());
-        return Strings({store.printStorePath(drvPath), chrootRootDir});
+        return Strings({store.printStorePath(drvPath), chrootRootDir.native()});
     }
 
-    Path realPathInSandbox(const Path & p) override
+    std::filesystem::path realPathInSandbox(const std::filesystem::path & p) override
     {
         // FIXME: why the needsHashRewrite() conditional?
-        return !needsHashRewrite() ? chrootRootDir + p : store.toRealPath(p);
+        return !needsHashRewrite() ? chrootRootDir / p.relative_path()
+                                   : std::filesystem::path(store.toRealPath(p.native()));
     }
 
     void cleanupBuild(bool force) override
@@ -171,22 +172,24 @@ struct ChrootDerivationBuilder : virtual DerivationBuilderImpl
                     continue;
                 if (buildMode != bmCheck && status.known->isValid())
                     continue;
-                auto p = store.toRealPath(status.known->path);
-                if (pathExists(chrootRootDir + p))
-                    std::filesystem::rename((chrootRootDir + p), p);
+                std::filesystem::path p = store.toRealPath(status.known->path);
+                std::filesystem::path chrootPath = chrootRootDir / p.relative_path();
+                if (pathExists(chrootPath))
+                    std::filesystem::rename(chrootPath, p);
             }
 
         autoDelChroot.reset(); /* this runs the destructor */
     }
 
-    std::pair<Path, Path> addDependencyPrep(const StorePath & path)
+    std::pair<std::filesystem::path, std::filesystem::path> addDependencyPrep(const StorePath & path)
     {
         DerivationBuilderImpl::addDependencyImpl(path);
 
         debug("materialising '%s' in the sandbox", store.printStorePath(path));
 
-        Path source = store.toRealPath(path);
-        Path target = chrootRootDir + store.printStorePath(path);
+        std::filesystem::path source = store.toRealPath(path);
+        std::filesystem::path target =
+            chrootRootDir / std::filesystem::path(store.printStorePath(path)).relative_path();
 
         if (pathExists(target)) {
             // There is a similar debug message in doBind, so only run it in this block to not have double messages.

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

@@ -73,19 +73,19 @@ struct DarwinDerivationBuilder : DerivationBuilderImpl
                all have the same parents (the store), and there might be lots of inputs. This isn't
                particularly efficient... I doubt it'll be a bottleneck in practice */
             for (auto & i : pathsInChroot) {
-                Path cur = i.first;
-                while (cur.compare("/") != 0) {
-                    cur = dirOf(cur);
-                    ancestry.insert(cur);
+                std::filesystem::path cur = i.first;
+                while (cur != "/") {
+                    cur = cur.parent_path();
+                    ancestry.insert(cur.native());
                 }
             }
 
             /* 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. */
-            Path cur = store.storeDir;
-            while (cur.compare("/") != 0) {
-                ancestry.insert(cur);
-                cur = dirOf(cur);
+            std::filesystem::path cur = store.storeDir;
+            while (cur != "/") {
+                ancestry.insert(cur.native());
+                cur = cur.parent_path();
             }
 
             /* Add all our input paths to the chroot */
@@ -174,18 +174,19 @@ struct DarwinDerivationBuilder : DerivationBuilderImpl
         /* The tmpDir in scope points at the temporary build directory for our derivation. Some packages try different
            mechanisms to find temporary directories, so we want to open up a broader place for them to put their files,
            if needed. */
-        Path globalTmpDir = canonPath(defaultTempDir().string(), true);
+        std::filesystem::path globalTmpDir = canonPath(defaultTempDir().native(), true);
 
         /* They don't like trailing slashes on subpath directives */
-        while (!globalTmpDir.empty() && globalTmpDir.back() == '/')
-            globalTmpDir.pop_back();
+        std::string globalTmpDirStr = globalTmpDir.native();
+        while (!globalTmpDirStr.empty() && globalTmpDirStr.back() == '/')
+            globalTmpDirStr.pop_back();
 
         if (getEnv("_NIX_TEST_NO_SANDBOX") != "1") {
             Strings sandboxArgs;
             sandboxArgs.push_back("_NIX_BUILD_TOP");
-            sandboxArgs.push_back(tmpDir);
+            sandboxArgs.push_back(tmpDir.native());
             sandboxArgs.push_back("_GLOBAL_TMP_DIR");
-            sandboxArgs.push_back(globalTmpDir);
+            sandboxArgs.push_back(globalTmpDirStr);
             if (drvOptions.allowLocalNetworking) {
                 sandboxArgs.push_back("_ALLOW_LOCAL_NETWORKING");
                 sandboxArgs.push_back("1");