diff --git a/src/libstore/build/derivation-building-goal.cc b/src/libstore/build/derivation-building-goal.cc index 1065d2f15d5..a86a01fd8bb 100644 --- a/src/libstore/build/derivation-building-goal.cc +++ b/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); diff --git a/src/libstore/globals.cc b/src/libstore/globals.cc index 27d17e1a9b9..87a68b2e3ea 100644 --- a/src/libstore/globals.cc +++ b/src/libstore/globals.cc @@ -335,7 +335,16 @@ void BaseSetting::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(); + cp.optional = j.at("optional").get(); +} template<> PathsInChroot BaseSetting::parse(const std::string & str) const @@ -368,7 +377,9 @@ std::string BaseSetting::to_string() const { std::vector 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)); diff --git a/src/libstore/include/nix/store/build/derivation-builder.hh b/src/libstore/include/nix/store/build/derivation-builder.hh index f28fd8e5630..29011ee5b55 100644 --- a/src/libstore/include/nix/store/build/derivation-builder.hh +++ b/src/libstore/include/nix/store/build/derivation-builder.hh @@ -1,6 +1,7 @@ #pragma once ///@file +#include #include #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 PathsInChroot; // maps target path to source path +typedef std::map 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 args; }; diff --git a/src/libstore/unix/build/chroot-derivation-builder.cc b/src/libstore/unix/build/chroot-derivation-builder.cc index 354a604f535..b0f6e3bf81b 100644 --- a/src/libstore/unix/build/chroot-derivation-builder.cc +++ b/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,41 +58,41 @@ 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(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); @@ -100,7 +100,7 @@ struct ChrootDerivationBuilder : virtual DerivationBuilderImpl /* 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", @@ -108,7 +108,7 @@ struct ChrootDerivationBuilder : virtual DerivationBuilderImpl /* 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 addDependencyPrep(const StorePath & path) + std::pair 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. diff --git a/src/libstore/unix/build/darwin-derivation-builder.cc b/src/libstore/unix/build/darwin-derivation-builder.cc index 24329bffce1..eba96225abe 100644 --- a/src/libstore/unix/build/darwin-derivation-builder.cc +++ b/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"); diff --git a/src/libstore/unix/build/derivation-builder.cc b/src/libstore/unix/build/derivation-builder.cc index eb0bf575735..697d4048647 100644 --- a/src/libstore/unix/build/derivation-builder.cc +++ b/src/libstore/unix/build/derivation-builder.cc @@ -129,13 +129,13 @@ class DerivationBuilderImpl : public DerivationBuilder, public DerivationBuilder /** * The temporary directory used for the build. */ - Path tmpDir; + std::filesystem::path tmpDir; /** * The top-level temporary directory. `tmpDir` is either equal to * or a child of this directory. */ - Path topTmpDir; + std::filesystem::path topTmpDir; /** * The file descriptor of the temporary directory. @@ -175,7 +175,7 @@ class DerivationBuilderImpl : public DerivationBuilder, public DerivationBuilder */ OutputPathMap scratchOutputs; - const static Path homeDir; + const static std::filesystem::path homeDir; /** * The recursive Nix daemon socket. @@ -254,7 +254,7 @@ class DerivationBuilderImpl : public DerivationBuilder, public DerivationBuilder /** * Return the path of the temporary directory in the sandbox. */ - virtual Path tmpDirInSandbox() + virtual std::filesystem::path tmpDirInSandbox() { assert(!topTmpDir.empty()); return topTmpDir; @@ -280,9 +280,9 @@ class DerivationBuilderImpl : public DerivationBuilder, public DerivationBuilder return Strings({store.printStorePath(drvPath)}); } - virtual Path realPathInSandbox(const Path & p) + virtual std::filesystem::path realPathInSandbox(const std::filesystem::path & p) { - return store.toRealPath(p); + return store.toRealPath(p.native()); } /** @@ -342,12 +342,12 @@ class DerivationBuilderImpl : public DerivationBuilder, public DerivationBuilder * SAFETY: this function is prone to TOCTOU as it receives a path and not a descriptor. * It's only safe to call in a child of a directory only visible to the owner. */ - void chownToBuilder(const Path & path); + void chownToBuilder(const std::filesystem::path & path); /** * Make a file owned by the builder addressed by its file descriptor. */ - void chownToBuilder(int fd, const Path & path); + void chownToBuilder(int fd, const std::filesystem::path & path); /** * Create a file in `tmpDir` owned by the builder. @@ -437,7 +437,12 @@ class DerivationBuilderImpl : public DerivationBuilder, public DerivationBuilder }; void handleDiffHook( - uid_t uid, uid_t gid, const Path & tryA, const Path & tryB, const Path & drvPath, const Path & tmpDir) + uid_t uid, + uid_t gid, + const std::filesystem::path & tryA, + const std::filesystem::path & tryB, + const std::filesystem::path & drvPath, + const std::filesystem::path & tmpDir) { auto & diffHookOpt = settings.diffHook.get(); if (diffHookOpt && settings.runDiffHook) { @@ -465,7 +470,7 @@ void handleDiffHook( } } -const Path DerivationBuilderImpl::homeDir = "/homeless-shelter"; +const std::filesystem::path DerivationBuilderImpl::homeDir = "/homeless-shelter"; void DerivationBuilderImpl::killSandbox(bool getStats) { @@ -560,16 +565,16 @@ SingleDrvOutputs DerivationBuilderImpl::unprepareBuild() return builtOutputs; } -static void chmod_(const Path & path, mode_t mode) +static void chmod_(const std::filesystem::path & path, mode_t mode) { if (chmod(path.c_str(), mode) == -1) - throw SysError("setting permissions on '%s'", path); + throw SysError("setting permissions on %s", path); } /* Move/rename path 'src' to 'dst'. Temporarily make 'src' writable if it's a directory and we're not root (to be able to update the directory's parent link ".."). */ -static void movePath(const Path & src, const Path & dst) +static void movePath(const std::filesystem::path & src, const std::filesystem::path & dst) { auto st = lstat(src); @@ -584,13 +589,13 @@ static void movePath(const Path & src, const Path & dst) chmod_(dst, st.st_mode); } -static void replaceValidPath(const Path & storePath, const Path & tmpPath) +static void replaceValidPath(const std::filesystem::path & storePath, const std::filesystem::path & tmpPath) { /* We can't atomically replace storePath (the original) with tmpPath (the replacement), so we have to move it out of the way first. We'd better not be interrupted here, because if we're repairing (say) Glibc, we end up with a broken system. */ - Path oldPath; + std::filesystem::path oldPath; if (pathExists(storePath)) { // why do we loop here? @@ -723,7 +728,7 @@ std::optional DerivationBuilderImpl::startBuild() POSIX semantics.*/ tmpDirFd = AutoCloseFD{open(tmpDir.c_str(), O_RDONLY | O_NOFOLLOW | O_DIRECTORY)}; if (!tmpDirFd) - throw SysError("failed to open the build temporary directory descriptor '%1%'", tmpDir); + throw SysError("failed to open the build temporary directory descriptor %1%", tmpDir); chownToBuilder(tmpDirFd.get(), tmpDir); @@ -789,7 +794,7 @@ std::optional DerivationBuilderImpl::startBuild() if (needsHashRewrite() && pathExists(homeDir)) throw Error( - "home directory '%1%' exists; please remove it to assure purity of builds without sandboxing", homeDir); + "home directory %1% exists; please remove it to assure purity of builds without sandboxing", homeDir); /* Fire up a Nix daemon to process recursive Nix calls from the builder. */ @@ -847,7 +852,7 @@ PathsInChroot DerivationBuilderImpl::getPathsInSandbox() host file system. */ PathsInChroot pathsInChroot = defaultPathsInChroot; - if (hasPrefix(store.storeDir, tmpDirInSandbox())) { + if (hasPrefix(store.storeDir, tmpDirInSandbox().native())) { throw Error("`sandbox-build-dir` must not contain the storeDir"); } pathsInChroot[tmpDirInSandbox()] = {.source = tmpDir}; @@ -862,10 +867,10 @@ PathsInChroot DerivationBuilderImpl::getPathsInSandbox() /* Note: we're not resolving symlinks here to prevent giving a non-root user info about inaccessible files. */ - Path canonI = canonPath(i); + std::filesystem::path canonI = canonPath(i); /* If only we had a trie to do this more efficiently :) luckily, these are generally going to be pretty small */ for (auto & a : allowedPaths) { - Path canonA = canonPath(a); + std::filesystem::path canonA = canonPath(a); if (isDirOrInDir(canonI, canonA)) { found = true; break; @@ -1045,7 +1050,7 @@ void DerivationBuilderImpl::initEnv() *not* `drv.env`, because we've desugared things like like "passAFile", "expandReferencesGraph", structured attrs, etc. */ for (const auto & [name, info] : desugaredEnv.variables) { - env[name] = info.prependBuildDirectory ? tmpDirInSandbox() + "/" + info.value : info.value; + env[name] = info.prependBuildDirectory ? (tmpDirInSandbox() / info.value).string() : info.value; } /* Add extra files, similar to `finalEnv` */ @@ -1124,8 +1129,8 @@ void DerivationBuilderImpl::startDaemon() addedPaths.clear(); auto socketName = ".nix-socket"; - Path socketPath = tmpDir + "/" + socketName; - env["NIX_REMOTE"] = "unix://" + tmpDirInSandbox() + "/" + socketName; + std::filesystem::path socketPath = tmpDir / socketName; + env["NIX_REMOTE"] = "unix://" + (tmpDirInSandbox() / socketName).native(); daemonSocket = createUnixDomainSocket(socketPath, 0600); @@ -1207,20 +1212,20 @@ void DerivationBuilderImpl::addDependencyImpl(const StorePath & path) addedPaths.insert(path); } -void DerivationBuilderImpl::chownToBuilder(const Path & path) +void DerivationBuilderImpl::chownToBuilder(const std::filesystem::path & path) { if (!buildUser) return; if (chown(path.c_str(), buildUser->getUID(), buildUser->getGID()) == -1) - throw SysError("cannot change ownership of '%1%'", path); + throw SysError("cannot change ownership of %1%", path); } -void DerivationBuilderImpl::chownToBuilder(int fd, const Path & path) +void DerivationBuilderImpl::chownToBuilder(int fd, const std::filesystem::path & path) { if (!buildUser) return; if (fchown(fd, buildUser->getUID(), buildUser->getGID()) == -1) - throw SysError("cannot change ownership of file '%1%'", path); + throw SysError("cannot change ownership of file %1%", path); } void DerivationBuilderImpl::writeBuilderFile(const std::string & name, std::string_view contents) @@ -1271,7 +1276,7 @@ void DerivationBuilderImpl::runChild(RunChildArgs args) enterChroot(); if (chdir(tmpDirInSandbox().c_str()) == -1) - throw SysError("changing into '%1%'", tmpDir); + throw SysError("changing into %1%", tmpDir); /* Close all other file descriptors. */ unix::closeExtraFDs(); @@ -1468,7 +1473,7 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs() if (discardReferences) debug("discarding references of output '%s'", outputName); else { - debug("scanning for references for output '%s' in temp location '%s'", outputName, actualPath); + debug("scanning for references for output '%s' in temp location %s", outputName, actualPath); /* Pass blank Sink as we are not ready to hash data at this stage. */ NullSink blank; @@ -1564,7 +1569,7 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs() auto rewriteOutput = [&](const StringMap & rewrites) { /* Apply hash rewriting if necessary. */ if (!rewrites.empty()) { - debug("rewriting hashes in '%1%'; cross fingers", actualPath); + debug("rewriting hashes in %1%; cross fingers", actualPath); /* FIXME: Is this actually streaming? */ auto source = sinkToSource([&](Sink & nextSink) { @@ -1572,7 +1577,7 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs() dumpPath(actualPath, rsink); rsink.flush(); }); - Path tmpPath = actualPath + ".tmp"; + std::filesystem::path tmpPath = actualPath.native() + ".tmp"; restorePath(tmpPath, *source); deletePath(actualPath); movePath(tmpPath, actualPath); @@ -1632,11 +1637,13 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs() case FileIngestionMethod::NixArchive: { HashModuloSink caSink{outputHash.hashAlgo, oldHashPart}; auto fim = outputHash.method.getFileIngestionMethod(); - dumpPath({getFSSourceAccessor(), CanonPath(actualPath)}, caSink, (FileSerialisationMethod) fim); + dumpPath( + {getFSSourceAccessor(), CanonPath(actualPath.native())}, caSink, (FileSerialisationMethod) fim); return caSink.finish().hash; } case FileIngestionMethod::Git: { - return git::dumpHash(outputHash.hashAlgo, {getFSSourceAccessor(), CanonPath(actualPath)}).hash; + return git::dumpHash(outputHash.hashAlgo, {getFSSourceAccessor(), CanonPath(actualPath.native())}) + .hash; } } assert(false); @@ -1658,7 +1665,7 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs() { HashResult narHashAndSize = hashPath( - {getFSSourceAccessor(), CanonPath(actualPath)}, + {getFSSourceAccessor(), CanonPath(actualPath.native())}, FileSerialisationMethod::NixArchive, HashAlgorithm::SHA256); newInfo0.narHash = narHashAndSize.hash; @@ -1682,7 +1689,7 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs() std::string{scratchPath->hashPart()}, std::string{requiredFinalPath.hashPart()}); rewriteOutput(outputRewrites); HashResult narHashAndSize = hashPath( - {getFSSourceAccessor(), CanonPath(actualPath)}, + {getFSSourceAccessor(), CanonPath(actualPath.native())}, FileSerialisationMethod::NixArchive, HashAlgorithm::SHA256); ValidPathInfo newInfo0{requiredFinalPath, {store, narHashAndSize.hash}}; @@ -1699,8 +1706,8 @@ SingleDrvOutputs DerivationBuilderImpl::registerOutputs() // Replace the output by a fresh copy of itself to make sure // that there's no stale file descriptor pointing to it - Path tmpOutput = actualPath + ".tmp"; - copyFile(std::filesystem::path(actualPath), std::filesystem::path(tmpOutput), true); + std::filesystem::path tmpOutput = actualPath.native() + ".tmp"; + copyFile(actualPath, tmpOutput, true); std::filesystem::rename(tmpOutput, actualPath); @@ -1916,7 +1923,7 @@ void DerivationBuilderImpl::cleanupBuild(bool force) /* Don't keep temporary directories for builtins because they might have privileged stuff (like a copy of netrc). */ if (settings.keepFailed && !force && !drv.isBuiltin()) { - printError("note: keeping build directory '%s'", tmpDir); + printError("note: keeping build directory %s", tmpDir); chmod(topTmpDir.c_str(), 0755); chmod(tmpDir.c_str(), 0755); } else diff --git a/src/libstore/unix/build/external-derivation-builder.cc b/src/libstore/unix/build/external-derivation-builder.cc index 7ddb6e093b1..35421c9977d 100644 --- a/src/libstore/unix/build/external-derivation-builder.cc +++ b/src/libstore/unix/build/external-derivation-builder.cc @@ -15,7 +15,7 @@ struct ExternalDerivationBuilder : DerivationBuilderImpl experimentalFeatureSettings.require(Xp::ExternalBuilders); } - Path tmpDirInSandbox() override + std::filesystem::path tmpDirInSandbox() override { /* In a sandbox, for determinism, always use the same temporary directory. */ @@ -24,7 +24,7 @@ struct ExternalDerivationBuilder : DerivationBuilderImpl void setBuildTmpDir() override { - tmpDir = topTmpDir + "/build"; + tmpDir = topTmpDir / "build"; createDir(tmpDir, 0700); } @@ -49,9 +49,9 @@ struct ExternalDerivationBuilder : DerivationBuilderImpl j.emplace(name, rewriteStrings(value, inputRewrites)); json.emplace("env", std::move(j)); } - json.emplace("topTmpDir", topTmpDir); - json.emplace("tmpDir", tmpDir); - json.emplace("tmpDirInSandbox", tmpDirInSandbox()); + json.emplace("topTmpDir", topTmpDir.native()); + json.emplace("tmpDir", tmpDir.native()); + json.emplace("tmpDirInSandbox", tmpDirInSandbox().native()); json.emplace("storeDir", store.storeDir); json.emplace("realStoreDir", store.config->realStoreDir.get()); json.emplace("system", drv.platform); @@ -88,7 +88,7 @@ struct ExternalDerivationBuilder : DerivationBuilderImpl args.insert(args.end(), jsonFile); if (chdir(tmpDir.c_str()) == -1) - throw SysError("changing into '%1%'", tmpDir); + throw SysError("changing into %1%", tmpDir); chownToBuilder(topTmpDir); @@ -97,7 +97,7 @@ struct ExternalDerivationBuilder : DerivationBuilderImpl debug("executing external builder: %s", concatStringsSep(" ", args)); execv(externalBuilder.program.c_str(), stringsToCharPtrs(args).data()); - throw SysError("executing '%s'", externalBuilder.program); + throw SysError("executing %s", externalBuilder.program); } catch (...) { handleChildException(true); _exit(1); diff --git a/src/libstore/unix/build/linux-derivation-builder.cc b/src/libstore/unix/build/linux-derivation-builder.cc index 4c32acdbde0..c81390fb709 100644 --- a/src/libstore/unix/build/linux-derivation-builder.cc +++ b/src/libstore/unix/build/linux-derivation-builder.cc @@ -123,13 +123,13 @@ static void setupSeccomp() # endif } -static void doBind(const Path & source, const Path & target, bool optional = false) +static void doBind(const std::filesystem::path & source, const std::filesystem::path & target, bool optional = false) { - debug("bind mounting '%1%' to '%2%'", source, target); + debug("bind mounting %1% to %2%", source, target); auto bindMount = [&]() { if (mount(source.c_str(), target.c_str(), "", MS_BIND | MS_REC, 0) == -1) - throw SysError("bind mount from '%1%' to '%2%' failed", source, target); + throw SysError("bind mount from %1% to %2% failed", source, target); }; auto maybeSt = maybeLstat(source); @@ -137,7 +137,7 @@ static void doBind(const Path & source, const Path & target, bool optional = fal if (optional) return; else - throw SysError("getting attributes of path '%1%'", source); + throw SysError("getting attributes of path %1%", source); } auto st = *maybeSt; @@ -146,10 +146,10 @@ static void doBind(const Path & source, const Path & target, bool optional = fal bindMount(); } else if (S_ISLNK(st.st_mode)) { // Symlinks can (apparently) not be bind-mounted, so just copy it - createDirs(dirOf(target)); - copyFile(std::filesystem::path(source), std::filesystem::path(target), false); + createDirs(target.parent_path()); + copyFile(source, target, false); } else { - createDirs(dirOf(target)); + createDirs(target.parent_path()); writeFile(target, ""); bindMount(); } @@ -167,6 +167,8 @@ struct LinuxDerivationBuilder : virtual DerivationBuilderImpl } }; +static const std::filesystem::path procPath = "/proc"; + struct ChrootLinuxDerivationBuilder : ChrootDerivationBuilder, LinuxDerivationBuilder { /** @@ -190,7 +192,7 @@ struct ChrootLinuxDerivationBuilder : ChrootDerivationBuilder, LinuxDerivationBu /** * The cgroup of the builder, if any. */ - std::optional cgroup; + std::optional cgroup; ChrootLinuxDerivationBuilder( LocalStore & store, std::unique_ptr miscMethods, DerivationBuilderParams params) @@ -228,7 +230,7 @@ struct ChrootLinuxDerivationBuilder : ChrootDerivationBuilder, LinuxDerivationBu auto cgroupFS = getCgroupFS(); if (!cgroupFS) throw Error("cannot determine the cgroups file system"); - auto rootCgroupPath = canonPath(*cgroupFS + "/" + rootCgroup); + auto rootCgroupPath = canonPath((*cgroupFS / rootCgroup).native()); if (!pathExists(rootCgroupPath)) throw Error("expected cgroup directory '%s'", rootCgroupPath); @@ -237,13 +239,13 @@ struct ChrootLinuxDerivationBuilder : ChrootDerivationBuilder, LinuxDerivationBu cgroup = buildUser ? fmt("%s/nix-build-uid-%d", rootCgroupPath, buildUser->getUID()) : fmt("%s/nix-build-pid-%d-%d", rootCgroupPath, getpid(), counter++); - debug("using cgroup '%s'", *cgroup); + debug("using cgroup %s", *cgroup); /* When using a build user, record the cgroup we used for that user so that if we got interrupted previously, we can kill any left-over cgroup first. */ if (buildUser) { - auto cgroupsDir = settings.nixStateDir + "/cgroups"; + auto cgroupsDir = std::filesystem::path{settings.nixStateDir} / "cgroups"; createDirs(cgroupsDir); auto cgroupFile = fmt("%s/%d", cgroupsDir, buildUser->getUID()); @@ -253,7 +255,7 @@ struct ChrootLinuxDerivationBuilder : ChrootDerivationBuilder, LinuxDerivationBu destroyCgroup(prevCgroup); } - writeFile(cgroupFile, *cgroup); + writeFile(cgroupFile, cgroup->native()); } } @@ -267,11 +269,11 @@ struct ChrootLinuxDerivationBuilder : ChrootDerivationBuilder, LinuxDerivationBu if (cgroup) { if (mkdir(cgroup->c_str(), 0755) != 0) - throw SysError("creating cgroup '%s'", *cgroup); + throw SysError("creating cgroup %s", *cgroup); chownToBuilder(*cgroup); - chownToBuilder(*cgroup + "/cgroup.procs"); - chownToBuilder(*cgroup + "/cgroup.threads"); - // chownToBuilder(*cgroup + "/cgroup.subtree_control"); + chownToBuilder(*cgroup / "cgroup.procs"); + chownToBuilder(*cgroup / "cgroup.threads"); + // chownToBuilder(*cgroup / "cgroup.subtree_control"); } } @@ -390,6 +392,7 @@ struct ChrootLinuxDerivationBuilder : ChrootDerivationBuilder, LinuxDerivationBu auto ss = tokenizeString>(sendPidSource.readLine()); assert(ss.size() == 1); pid = string2Int(ss[0]).value(); + auto thisProcPath = procPath / std::to_string(static_cast(pid)); if (usingUserNamespace) { /* Set the UID/GID mapping of the builder's user namespace @@ -399,12 +402,12 @@ struct ChrootLinuxDerivationBuilder : ChrootDerivationBuilder, LinuxDerivationBu uid_t hostGid = buildUser ? buildUser->getGID() : getgid(); uid_t nrIds = buildUser ? buildUser->getUIDCount() : 1; - writeFile("/proc/" + std::to_string(pid) + "/uid_map", fmt("%d %d %d", sandboxUid(), hostUid, nrIds)); + writeFile(thisProcPath / "uid_map", fmt("%d %d %d", sandboxUid(), hostUid, nrIds)); if (!buildUser || buildUser->getUIDCount() == 1) - writeFile("/proc/" + std::to_string(pid) + "/setgroups", "deny"); + writeFile(thisProcPath / "setgroups", "deny"); - writeFile("/proc/" + std::to_string(pid) + "/gid_map", fmt("%d %d %d", sandboxGid(), hostGid, nrIds)); + writeFile(thisProcPath / "gid_map", fmt("%d %d %d", sandboxGid(), hostGid, nrIds)); } else { debug("note: not using a user namespace"); if (!buildUser) @@ -415,7 +418,7 @@ struct ChrootLinuxDerivationBuilder : ChrootDerivationBuilder, LinuxDerivationBu /* Now that we now the sandbox uid, we can write /etc/passwd. */ writeFile( - chrootRootDir + "/etc/passwd", + chrootRootDir / "etc" / "passwd", fmt("root:x:0:0:Nix build user:%3%:/noshell\n" "nixbld:x:%1%:%2%:Nix build user:%3%:/noshell\n" "nobody:x:65534:65534:Nobody:/:/noshell\n", @@ -425,19 +428,20 @@ struct ChrootLinuxDerivationBuilder : ChrootDerivationBuilder, LinuxDerivationBu /* Save the mount- and user namespace of the child. We have to do this *before* the child does a chroot. */ - sandboxMountNamespace = open(fmt("/proc/%d/ns/mnt", (pid_t) pid).c_str(), O_RDONLY); + auto sandboxPath = thisProcPath / "ns"; + sandboxMountNamespace = open((sandboxPath / "mnt").c_str(), O_RDONLY); if (sandboxMountNamespace.get() == -1) throw SysError("getting sandbox mount namespace"); if (usingUserNamespace) { - sandboxUserNamespace = open(fmt("/proc/%d/ns/user", (pid_t) pid).c_str(), O_RDONLY); + sandboxUserNamespace = open((sandboxPath / "user").c_str(), O_RDONLY); if (sandboxUserNamespace.get() == -1) throw SysError("getting sandbox user namespace"); } /* Move the child into its own cgroup. */ if (cgroup) - writeFile(*cgroup + "/cgroup.procs", fmt("%d", (pid_t) pid)); + writeFile(*cgroup / "cgroup.procs", fmt("%d", (pid_t) pid)); /* Signal the builder that we've updated its user namespace. */ writeFull(userNamespaceSync.writeSide.get(), "1\n"); @@ -489,7 +493,7 @@ struct ChrootLinuxDerivationBuilder : ChrootDerivationBuilder, LinuxDerivationBu /* Bind-mount chroot directory to itself, to treat it as a different filesystem from /, as needed for pivot_root. */ if (mount(chrootRootDir.c_str(), chrootRootDir.c_str(), 0, MS_BIND, 0) == -1) - throw SysError("unable to bind mount '%1%'", chrootRootDir); + throw SysError("unable to bind mount %1%", chrootRootDir); /* Bind-mount the sandbox's Nix store onto itself so that we can mark it as a "shared" subtree, allowing bind @@ -499,20 +503,20 @@ struct ChrootLinuxDerivationBuilder : ChrootDerivationBuilder, LinuxDerivationBu Marking chrootRootDir as MS_SHARED causes pivot_root() to fail with EINVAL. Don't know why. */ - Path chrootStoreDir = chrootRootDir + store.storeDir; + std::filesystem::path chrootStoreDir = chrootRootDir / std::filesystem::path(store.storeDir).relative_path(); if (mount(chrootStoreDir.c_str(), chrootStoreDir.c_str(), 0, MS_BIND, 0) == -1) throw SysError("unable to bind mount the Nix store", chrootStoreDir); if (mount(0, chrootStoreDir.c_str(), 0, MS_SHARED, 0) == -1) - throw SysError("unable to make '%s' shared", chrootStoreDir); + throw SysError("unable to make %s shared", chrootStoreDir); /* Set up a nearly empty /dev, unless the user asked to bind-mount the host /dev. */ Strings ss; if (pathsInChroot.find("/dev") == pathsInChroot.end()) { - createDirs(chrootRootDir + "/dev/shm"); - createDirs(chrootRootDir + "/dev/pts"); + createDirs(chrootRootDir / "dev" / "shm"); + createDirs(chrootRootDir / "dev" / "pts"); ss.push_back("/dev/full"); if (systemFeatures.count("kvm")) { if (pathExists("/dev/kvm")) { @@ -529,10 +533,10 @@ struct ChrootLinuxDerivationBuilder : ChrootDerivationBuilder, LinuxDerivationBu ss.push_back("/dev/tty"); ss.push_back("/dev/urandom"); ss.push_back("/dev/zero"); - createSymlink("/proc/self/fd", chrootRootDir + "/dev/fd"); - createSymlink("/proc/self/fd/0", chrootRootDir + "/dev/stdin"); - createSymlink("/proc/self/fd/1", chrootRootDir + "/dev/stdout"); - createSymlink("/proc/self/fd/2", chrootRootDir + "/dev/stderr"); + createSymlink("/proc/self/fd", chrootRootDir / "dev" / "fd"); + createSymlink("/proc/self/fd/0", chrootRootDir / "dev" / "stdin"); + createSymlink("/proc/self/fd/1", chrootRootDir / "dev" / "stdout"); + createSymlink("/proc/self/fd/2", chrootRootDir / "dev" / "stderr"); } /* Fixed-output derivations typically need to access the @@ -543,7 +547,7 @@ struct ChrootLinuxDerivationBuilder : ChrootDerivationBuilder, LinuxDerivationBu // services. Don’t use it for anything else that may // be configured for this system. This limits the // potential impurities introduced in fixed-outputs. - writeFile(chrootRootDir + "/etc/nsswitch.conf", "hosts: files dns\nservices: files\n"); + writeFile(chrootRootDir / "etc" / "nsswitch.conf", "hosts: files dns\nservices: files\n"); /* N.B. it is realistic that these paths might not exist. It happens when testing Nix building fixed-output derivations @@ -553,9 +557,10 @@ struct ChrootLinuxDerivationBuilder : ChrootDerivationBuilder, LinuxDerivationBu ss.push_back(path); if (settings.caFile != "") { - Path caFile = settings.caFile; + std::filesystem::path caFile = settings.caFile.get(); if (pathExists(caFile)) - pathsInChroot.try_emplace("/etc/ssl/certs/ca-certificates.crt", canonPath(caFile, true), true); + pathsInChroot.try_emplace( + "/etc/ssl/certs/ca-certificates.crt", canonPath(caFile.native(), true), true); } } @@ -578,26 +583,26 @@ struct ChrootLinuxDerivationBuilder : ChrootDerivationBuilder, LinuxDerivationBu static unsigned char sh[] = { # include "embedded-sandbox-shell.gen.hh" }; - auto dst = chrootRootDir + i.first; - createDirs(dirOf(dst)); + auto dst = chrootRootDir / i.first.relative_path(); + createDirs(dst.parent_path()); writeFile(dst, std::string_view((const char *) sh, sizeof(sh))); chmod_(dst, 0555); } else # endif { - doBind(i.second.source, chrootRootDir + i.first, i.second.optional); + doBind(i.second.source, chrootRootDir / i.first.relative_path(), i.second.optional); } } /* Bind a new instance of procfs on /proc. */ - createDirs(chrootRootDir + "/proc"); - if (mount("none", (chrootRootDir + "/proc").c_str(), "proc", 0, 0) == -1) + createDirs(chrootRootDir / "proc"); + if (mount("none", (chrootRootDir / "proc").c_str(), "proc", 0, 0) == -1) throw SysError("mounting /proc"); /* Mount sysfs on /sys. */ if (buildUser && buildUser->getUIDCount() != 1) { - createDirs(chrootRootDir + "/sys"); - if (mount("none", (chrootRootDir + "/sys").c_str(), "sysfs", 0, 0) == -1) + createDirs(chrootRootDir / "sys"); + if (mount("none", (chrootRootDir / "sys").c_str(), "sysfs", 0, 0) == -1) throw SysError("mounting /sys"); } @@ -606,7 +611,7 @@ struct ChrootLinuxDerivationBuilder : ChrootDerivationBuilder, LinuxDerivationBu if (pathExists("/dev/shm") && mount( "none", - (chrootRootDir + "/dev/shm").c_str(), + (chrootRootDir / "dev" / "shm").c_str(), "tmpfs", 0, fmt("size=%s", settings.sandboxShmSize).c_str()) @@ -617,25 +622,25 @@ struct ChrootLinuxDerivationBuilder : ChrootDerivationBuilder, LinuxDerivationBu requires the kernel to be compiled with CONFIG_DEVPTS_MULTIPLE_INSTANCES=y (which is the case if /dev/ptx/ptmx exists). */ - if (pathExists("/dev/pts/ptmx") && !pathExists(chrootRootDir + "/dev/ptmx") + if (pathExists("/dev/pts/ptmx") && !pathExists(chrootRootDir / "dev" / "ptmx") && !pathsInChroot.count("/dev/pts")) { - if (mount("none", (chrootRootDir + "/dev/pts").c_str(), "devpts", 0, "newinstance,mode=0620") == 0) { - createSymlink("/dev/pts/ptmx", chrootRootDir + "/dev/ptmx"); + if (mount("none", (chrootRootDir / "dev" / "pts").c_str(), "devpts", 0, "newinstance,mode=0620") == 0) { + createSymlink("/dev/pts/ptmx", chrootRootDir / "dev" / "ptmx"); /* Make sure /dev/pts/ptmx is world-writable. With some Linux versions, it is created with permissions 0. */ - chmod_(chrootRootDir + "/dev/pts/ptmx", 0666); + chmod_(chrootRootDir / "dev" / "pts" / "ptmx", 0666); } else { if (errno != EINVAL) throw SysError("mounting /dev/pts"); - doBind("/dev/pts", chrootRootDir + "/dev/pts"); - doBind("/dev/ptmx", chrootRootDir + "/dev/ptmx"); + doBind("/dev/pts", chrootRootDir / "dev" / "pts"); + doBind("/dev/ptmx", chrootRootDir / "dev" / "ptmx"); } } /* Make /etc unwritable */ if (!drvOptions.useUidRange(drv)) - chmod_(chrootRootDir + "/etc", 0555); + chmod_(chrootRootDir / "etc", 0555); /* Unshare this mount namespace. This is necessary because pivot_root() below changes the root of the mount @@ -658,16 +663,16 @@ struct ChrootLinuxDerivationBuilder : ChrootDerivationBuilder, LinuxDerivationBu /* Do the chroot(). */ if (chdir(chrootRootDir.c_str()) == -1) - throw SysError("cannot change directory to '%1%'", chrootRootDir); + throw SysError("cannot change directory to %1%", chrootRootDir); if (mkdir("real-root", 0500) == -1) throw SysError("cannot create real-root directory"); if (pivot_root(".", "real-root") == -1) - throw SysError("cannot pivot old root directory onto '%1%'", (chrootRootDir + "/real-root")); + throw SysError("cannot pivot old root directory onto %1%", chrootRootDir / "real-root"); if (chroot(".") == -1) - throw SysError("cannot change root directory to '%1%'", chrootRootDir); + throw SysError("cannot change root directory to %1%", chrootRootDir); if (umount2("real-root", MNT_DETACH) == -1) throw SysError("cannot unmount real root filesystem"); diff --git a/src/libutil/linux/cgroup.cc b/src/libutil/linux/cgroup.cc index 928b44d6c50..b0d5fa9c614 100644 --- a/src/libutil/linux/cgroup.cc +++ b/src/libutil/linux/cgroup.cc @@ -15,9 +15,9 @@ namespace nix { -std::optional getCgroupFS() +std::optional getCgroupFS() { - static auto res = [&]() -> std::optional { + static auto res = [&]() -> std::optional { auto fp = fopen("/proc/mounts", "r"); if (!fp) return std::nullopt; @@ -32,7 +32,7 @@ std::optional getCgroupFS() } // FIXME: obsolete, check for cgroup2 -StringMap getCgroups(const Path & cgroupFile) +StringMap getCgroups(const std::filesystem::path & cgroupFile) { StringMap cgroups; @@ -150,7 +150,7 @@ static CgroupStats destroyCgroup(const std::filesystem::path & cgroup, bool retu return stats; } -CgroupStats destroyCgroup(const Path & cgroup) +CgroupStats destroyCgroup(const std::filesystem::path & cgroup) { return destroyCgroup(cgroup, true); } diff --git a/src/libutil/linux/include/nix/util/cgroup.hh b/src/libutil/linux/include/nix/util/cgroup.hh index a759bdd0852..1872f745044 100644 --- a/src/libutil/linux/include/nix/util/cgroup.hh +++ b/src/libutil/linux/include/nix/util/cgroup.hh @@ -9,9 +9,9 @@ namespace nix { -std::optional getCgroupFS(); +std::optional getCgroupFS(); -StringMap getCgroups(const Path & cgroupFile); +StringMap getCgroups(const std::filesystem::path & cgroupFile); struct CgroupStats { @@ -29,7 +29,7 @@ CgroupStats getCgroupStats(const std::filesystem::path & cgroup); * been killed. Also return statistics from the cgroup just before * destruction. */ -CgroupStats destroyCgroup(const Path & cgroup); +CgroupStats destroyCgroup(const std::filesystem::path & cgroup); std::string getCurrentCgroup(); diff --git a/tests/functional/build-remote.sh b/tests/functional/build-remote.sh index f396bc72e8f..6776d912af2 100644 --- a/tests/functional/build-remote.sh +++ b/tests/functional/build-remote.sh @@ -86,5 +86,5 @@ out="$(nix-build 2>&1 failing.nix \ [[ "$out" =~ .*"note: keeping build directory".* ]] [[ "$out" =~ .*"The failed build directory was kept on the remote builder due to".* ]] -build_dir="$(grep "note: keeping build" <<< "$out" | sed -E "s/^(.*)note: keeping build directory '(.*)'(.*)$/\2/")" +build_dir="$(grep "note: keeping build" <<< "$out" | sed -E "s/^(.*)note: keeping build directory \"(.*)\"(.*)$/\2/")" [[ "foo" = $(<"$build_dir"/bar) ]]