From a97a08411c4e34323b48e95ed593bb53d67bde23 Mon Sep 17 00:00:00 2001 From: John Ericson Date: Mon, 26 Aug 2024 12:24:37 -0400 Subject: [PATCH] More support for `std::filepath` in libnixutil We're not replacing `Path` in exposed definitions in many cases, but just adding alternatives. This will allow us to "top down" change `Path` to `std::fileysystem::path`, and then we can remove the `Path`-using utilities which will become unused. Also add some test files which we forgot to include in the libutil unit tests `meson.build`. Co-Authored-By: siddhantCodes --- src/libcmd/common-eval-args.cc | 6 +- src/libcmd/installables.cc | 4 +- src/libcmd/repl.cc | 4 +- src/libexpr/parser.y | 2 +- src/libstore/gc.cc | 2 +- src/libstore/local-overlay-store.cc | 2 +- src/libutil/args.hh | 22 ++++++ src/libutil/exec.hh | 5 +- src/libutil/executable-path.cc | 6 +- src/libutil/executable-path.hh | 6 +- src/libutil/file-system.cc | 32 ++++++--- src/libutil/file-system.hh | 69 ++++++++++++++++++- src/libutil/linux/namespaces.cc | 2 +- src/libutil/os-string.hh | 17 +++-- src/libutil/posix-source-accessor.cc | 2 +- src/libutil/serialise.cc | 6 +- src/libutil/strings.cc | 4 +- src/libutil/unix/users.cc | 2 + src/nix/bundle.cc | 4 +- src/nix/config-check.cc | 4 +- src/nix/develop.cc | 6 +- src/nix/flake.cc | 10 +-- src/nix/run.cc | 18 ++--- src/nix/self-exe.cc | 4 +- tests/unit/libfetchers/public-key.cc | 6 +- .../libstore-support/tests/nix_api_store.hh | 2 +- tests/unit/libstore-support/tests/protocol.hh | 6 +- .../libstore/derivation-advanced-attrs.cc | 6 +- tests/unit/libstore/derivation.cc | 6 +- tests/unit/libstore/machines.cc | 20 +++--- tests/unit/libstore/nar-info.cc | 6 +- tests/unit/libstore/path-info.cc | 6 +- tests/unit/libstore/store-reference.cc | 6 +- .../libutil-support/tests/characterization.hh | 10 +-- tests/unit/libutil/file-system.cc | 56 ++++++++------- tests/unit/libutil/git.cc | 6 +- tests/unit/libutil/meson.build | 3 + 37 files changed, 258 insertions(+), 120 deletions(-) diff --git a/src/libcmd/common-eval-args.cc b/src/libcmd/common-eval-args.cc index ae9994a05..ccbf957d9 100644 --- a/src/libcmd/common-eval-args.cc +++ b/src/libcmd/common-eval-args.cc @@ -18,6 +18,8 @@ namespace nix { +namespace fs { using namespace std::filesystem; } + fetchers::Settings fetchSettings; static GlobalConfig::Register rFetchSettings(&fetchSettings); @@ -119,8 +121,8 @@ MixEvalArgs::MixEvalArgs() .category = category, .labels = {"original-ref", "resolved-ref"}, .handler = {[&](std::string _from, std::string _to) { - auto from = parseFlakeRef(fetchSettings, _from, absPath(".")); - auto to = parseFlakeRef(fetchSettings, _to, absPath(".")); + auto from = parseFlakeRef(fetchSettings, _from, fs::current_path().string()); + auto to = parseFlakeRef(fetchSettings, _to, fs::current_path().string()); fetchers::Attrs extraAttrs; if (to.subdir != "") extraAttrs["dir"] = to.subdir; fetchers::overrideRegistry(from.input, to.input, extraAttrs); diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc index 0fe956ec0..22e7eb546 100644 --- a/src/libcmd/installables.cc +++ b/src/libcmd/installables.cc @@ -31,6 +31,8 @@ namespace nix { +namespace fs { using namespace std::filesystem; } + void completeFlakeInputPath( AddCompletions & completions, ref evalState, @@ -341,7 +343,7 @@ void completeFlakeRefWithFragment( auto flakeRefS = std::string(prefix.substr(0, hash)); // TODO: ideally this would use the command base directory instead of assuming ".". - auto flakeRef = parseFlakeRef(fetchSettings, expandTilde(flakeRefS), absPath(".")); + auto flakeRef = parseFlakeRef(fetchSettings, expandTilde(flakeRefS), fs::current_path().string()); auto evalCache = openEvalCache(*evalState, std::make_shared(lockFlake( diff --git a/src/libcmd/repl.cc b/src/libcmd/repl.cc index e7c43367c..63f6c1bdd 100644 --- a/src/libcmd/repl.cc +++ b/src/libcmd/repl.cc @@ -622,7 +622,7 @@ ProcessLineResult NixRepl::processLine(std::string line) // When missing, trigger the normal exception // e.g. :doc builtins.foo // behaves like - // nix-repl> builtins.foo + // nix-repl> builtins.foo // error: attribute 'foo' missing evalString(arg, v); assert(false); @@ -720,7 +720,7 @@ void NixRepl::loadFlake(const std::string & flakeRefS) if (flakeRefS.empty()) throw Error("cannot use ':load-flake' without a path specified. (Use '.' for the current working directory.)"); - auto flakeRef = parseFlakeRef(fetchSettings, flakeRefS, absPath("."), true); + auto flakeRef = parseFlakeRef(fetchSettings, flakeRefS, std::filesystem::current_path().string(), true); if (evalSettings.pureEval && !flakeRef.input.isLocked()) throw Error("cannot use ':load-flake' on locked flake reference '%s' (use --impure to override)", flakeRefS); diff --git a/src/libexpr/parser.y b/src/libexpr/parser.y index f2ccca7fc..a79abbf16 100644 --- a/src/libexpr/parser.y +++ b/src/libexpr/parser.y @@ -350,7 +350,7 @@ string_parts_interpolated path_start : PATH { - Path path(absPath({$1.p, $1.l}, state->basePath.path.abs())); + Path path(absPath(std::string_view{$1.p, $1.l}, state->basePath.path.abs())); /* add back in the trailing '/' to the first segment */ if ($1.p[$1.l-1] == '/' && $1.l > 1) path += "/"; diff --git a/src/libstore/gc.cc b/src/libstore/gc.cc index 1494712da..91cf76366 100644 --- a/src/libstore/gc.cc +++ b/src/libstore/gc.cc @@ -333,7 +333,7 @@ static std::string quoteRegexChars(const std::string & raw) } #if __linux__ -static void readFileRoots(const char * path, UncheckedRoots & roots) +static void readFileRoots(const std::filesystem::path & path, UncheckedRoots & roots) { try { roots[readFile(path)].emplace(path); diff --git a/src/libstore/local-overlay-store.cc b/src/libstore/local-overlay-store.cc index ec2c5f4e9..b86beba2c 100644 --- a/src/libstore/local-overlay-store.cc +++ b/src/libstore/local-overlay-store.cc @@ -31,7 +31,7 @@ LocalOverlayStore::LocalOverlayStore(std::string_view scheme, PathView path, con if (checkMount.get()) { std::smatch match; std::string mountInfo; - auto mounts = readFile("/proc/self/mounts"); + auto mounts = readFile(std::filesystem::path{"/proc/self/mounts"}); auto regex = std::regex(R"((^|\n)overlay )" + realStoreDir.get() + R"( .*(\n|$))"); // Mount points can be stacked, so there might be multiple matching entries. diff --git a/src/libutil/args.hh b/src/libutil/args.hh index c0236ee3d..513b8d811 100644 --- a/src/libutil/args.hh +++ b/src/libutil/args.hh @@ -113,6 +113,16 @@ protected: , arity(1) { } + Handler(std::filesystem::path * dest) + : fun([dest](std::vector ss) { *dest = ss[0]; }) + , arity(1) + { } + + Handler(std::optional * dest) + : fun([dest](std::vector ss) { *dest = ss[0]; }) + , arity(1) + { } + template Handler(T * dest, const T & val) : fun([dest, val](std::vector ss) { *dest = val; }) @@ -283,6 +293,18 @@ public: }); } + /** + * Expect a path argument. + */ + void expectArg(const std::string & label, std::filesystem::path * dest, bool optional = false) + { + expectArgs({ + .label = label, + .optional = optional, + .handler = {dest} + }); + } + /** * Expect 0 or more arguments. */ diff --git a/src/libutil/exec.hh b/src/libutil/exec.hh index 405e19268..cbbe80c4e 100644 --- a/src/libutil/exec.hh +++ b/src/libutil/exec.hh @@ -10,9 +10,6 @@ namespace nix { * * We use our own implementation unconditionally for consistency. */ -int execvpe( - const OsString::value_type * file0, - const OsString::value_type * const argv[], - const OsString::value_type * const envp[]); +int execvpe(const OsChar * file0, const OsChar * const argv[], const OsChar * const envp[]); } diff --git a/src/libutil/executable-path.cc b/src/libutil/executable-path.cc index da71088e7..9fb5214b2 100644 --- a/src/libutil/executable-path.cc +++ b/src/libutil/executable-path.cc @@ -6,7 +6,9 @@ namespace nix { -namespace fs = std::filesystem; +namespace fs { +using namespace std::filesystem; +} constexpr static const OsStringView path_var_separator{ &ExecutablePath::separator, @@ -24,7 +26,7 @@ ExecutablePath ExecutablePath::load() ExecutablePath ExecutablePath::parse(const OsString & path) { auto strings = path.empty() ? (std::list{}) - : basicSplitString, OsString::value_type>(path, path_var_separator); + : basicSplitString, OsChar>(path, path_var_separator); std::vector ret; ret.reserve(strings.size()); diff --git a/src/libutil/executable-path.hh b/src/libutil/executable-path.hh index f46d5e212..c5cfa1c39 100644 --- a/src/libutil/executable-path.hh +++ b/src/libutil/executable-path.hh @@ -7,11 +7,15 @@ namespace nix { MakeError(ExecutableLookupError, Error); +/** + * @todo rename, it is not just good for execuatable paths, but also + * other lists of paths. + */ struct ExecutablePath { std::vector directories; - constexpr static const OsString::value_type separator = + constexpr static const OsChar separator = #ifdef WIN32 L';' #else diff --git a/src/libutil/file-system.cc b/src/libutil/file-system.cc index aa5f3670c..edcacb50a 100644 --- a/src/libutil/file-system.cc +++ b/src/libutil/file-system.cc @@ -26,10 +26,10 @@ #include "strings-inline.hh" -namespace fs = std::filesystem; - namespace nix { +namespace fs { using namespace std::filesystem; } + /** * Treat the string as possibly an absolute path, by inspecting the * start of it. Return whether it was probably intended to be @@ -73,6 +73,10 @@ Path absPath(PathView path, std::optional dir, bool resolveSymlinks) return canonPath(path, resolveSymlinks); } +std::filesystem::path absPath(const std::filesystem::path & path, bool resolveSymlinks) +{ + return absPath(path.string(), std::nullopt, resolveSymlinks); +} Path canonPath(PathView path, bool resolveSymlinks) { @@ -206,10 +210,10 @@ bool pathExists(const Path & path) return maybeLstat(path).has_value(); } -bool pathAccessible(const Path & path) +bool pathAccessible(const std::filesystem::path & path) { try { - return pathExists(path); + return pathExists(path.string()); } catch (SysError & e) { // swallow EPERM if (e.errNo == EPERM) return false; @@ -238,6 +242,11 @@ std::string readFile(const Path & path) return readFile(fd.get()); } +std::string readFile(const std::filesystem::path & path) +{ + return readFile(os_string_to_string(PathViewNG { path })); +} + void readFile(const Path & path, Sink & sink) { @@ -324,7 +333,7 @@ void recursiveSync(const Path & path) /* If it's a file, just fsync and return. */ auto st = lstat(path); if (S_ISREG(st.st_mode)) { - AutoCloseFD fd = open(path.c_str(), O_RDONLY, 0); + AutoCloseFD fd = toDescriptor(open(path.c_str(), O_RDONLY, 0)); if (!fd) throw SysError("opening file '%1%'", path); fd.fsync(); @@ -344,7 +353,7 @@ void recursiveSync(const Path & path) if (fs::is_directory(st)) { dirsToEnumerate.emplace_back(entry.path()); } else if (fs::is_regular_file(st)) { - AutoCloseFD fd = open(entry.path().c_str(), O_RDONLY, 0); + AutoCloseFD fd = toDescriptor(open(entry.path().string().c_str(), O_RDONLY, 0)); if (!fd) throw SysError("opening file '%1%'", entry.path()); fd.fsync(); @@ -355,7 +364,7 @@ void recursiveSync(const Path & path) /* Fsync all the directories. */ for (auto dir = dirsToFsync.rbegin(); dir != dirsToFsync.rend(); ++dir) { - AutoCloseFD fd = open(dir->c_str(), O_RDONLY, 0); + AutoCloseFD fd = toDescriptor(open(dir->string().c_str(), O_RDONLY, 0)); if (!fd) throw SysError("opening directory '%1%'", *dir); fd.fsync(); @@ -595,19 +604,20 @@ void createSymlink(const Path & target, const Path & link) fs::create_symlink(target, link); } -void replaceSymlink(const Path & target, const Path & link) +void replaceSymlink(const fs::path & target, const fs::path & link) { for (unsigned int n = 0; true; n++) { - Path tmp = canonPath(fmt("%s/.%d_%s", dirOf(link), n, baseNameOf(link))); + auto tmp = link.parent_path() / fs::path{fmt(".%d_%s", n, link.filename().string())}; + tmp = tmp.lexically_normal(); try { - createSymlink(target, tmp); + fs::create_symlink(target, tmp); } catch (fs::filesystem_error & e) { if (e.code() == std::errc::file_exists) continue; throw; } - std::filesystem::rename(tmp, link); + fs::rename(tmp, link); break; } diff --git a/src/libutil/file-system.hh b/src/libutil/file-system.hh index 0f406a2de..eb3e4ec66 100644 --- a/src/libutil/file-system.hh +++ b/src/libutil/file-system.hh @@ -46,16 +46,33 @@ struct Source; * @return An absolutized path, resolving paths relative to the * specified directory, or the current directory otherwise. The path * is also canonicalised. + * + * In the process of being deprecated for `std::filesystem::absolute`. */ Path absPath(PathView path, std::optional dir = {}, bool resolveSymlinks = false); +inline Path absPath(const Path & path, + std::optional dir = {}, + bool resolveSymlinks = false) +{ + return absPath(PathView{path}, dir, resolveSymlinks); +} + +std::filesystem::path absPath(const std::filesystem::path & path, + bool resolveSymlinks = false); + /** * Canonicalise a path by removing all `.` or `..` components and * double or trailing slashes. Optionally resolves all symlink * components such that each component of the resulting path is *not* * a symbolic link. + * + * In the process of being deprecated for + * `std::filesystem::path::lexically_normal` (for the `resolveSymlinks = + * false` case), and `std::filesystem::weakly_canonical` (for the + * `resolveSymlinks = true` case). */ Path canonPath(PathView path, bool resolveSymlinks = false); @@ -64,12 +81,18 @@ Path canonPath(PathView path, bool resolveSymlinks = false); * everything before the final `/`. If the path is the root or an * immediate child thereof (e.g., `/foo`), this means `/` * is returned. + * + * In the process of being deprecated for + * `std::filesystem::path::parent_path`. */ Path dirOf(const PathView path); /** * @return the base name of the given canonical path, i.e., everything * following the final `/` (trailing slashes are removed). + * + * In the process of being deprecated for + * `std::filesystem::path::filename`. */ std::string_view baseNameOf(std::string_view path); @@ -98,20 +121,42 @@ std::optional maybeLstat(const Path & path); /** * @return true iff the given path exists. + * + * In the process of being deprecated for `fs::symlink_exists`. */ bool pathExists(const Path & path); +namespace fs { + +/** + * ``` + * symlink_exists(p) = std::filesystem::exists(std::filesystem::symlink_status(p)) + * ``` + * Missing convenience analogous to + * ``` + * std::filesystem::exists(p) = std::filesystem::exists(std::filesystem::status(p)) + * ``` + */ +inline bool symlink_exists(const std::filesystem::path & path) { + return std::filesystem::exists(std::filesystem::symlink_status(path)); +} + +} // namespace fs + /** * A version of pathExists that returns false on a permission error. * Useful for inferring default paths across directories that might not * be readable. * @return true iff the given path can be accessed and exists */ -bool pathAccessible(const Path & path); +bool pathAccessible(const std::filesystem::path & path); /** * Read the contents (target) of a symbolic link. The result is not * in any way canonicalised. + * + * In the process of being deprecated for + * `std::filesystem::read_symlink`. */ Path readLink(const Path & path); @@ -124,14 +169,23 @@ Descriptor openDirectory(const std::filesystem::path & path); * Read the contents of a file into a string. */ std::string readFile(const Path & path); +std::string readFile(const std::filesystem::path & path); void readFile(const Path & path, Sink & sink); /** * Write a string to a file. */ void writeFile(const Path & path, std::string_view s, mode_t mode = 0666, bool sync = false); +static inline void writeFile(const std::filesystem::path & path, std::string_view s, mode_t mode = 0666, bool sync = false) +{ + return writeFile(path.string(), s, mode, sync); +} void writeFile(const Path & path, Source & source, mode_t mode = 0666, bool sync = false); +static inline void writeFile(const std::filesystem::path & path, Source & source, mode_t mode = 0666, bool sync = false) +{ + return writeFile(path.string(), source, mode, sync); +} /** * Flush a path's parent directory to disk. @@ -154,6 +208,9 @@ void deletePath(const std::filesystem::path & path, uint64_t & bytesFreed); /** * Create a directory and all its parents, if necessary. + * + * In the process of being deprecated for + * `std::filesystem::create_directories`. */ void createDirs(const Path & path); inline void createDirs(PathView path) @@ -192,13 +249,21 @@ void setWriteTime(const std::filesystem::path & path, const struct stat & st); /** * Create a symlink. + * + * In the process of being deprecated for + * `std::filesystem::create_symlink`. */ void createSymlink(const Path & target, const Path & link); /** * Atomically create or replace a symlink. */ -void replaceSymlink(const Path & target, const Path & link); +void replaceSymlink(const std::filesystem::path & target, const std::filesystem::path & link); + +inline void replaceSymlink(const Path & target, const Path & link) +{ + return replaceSymlink(std::filesystem::path{target}, std::filesystem::path{link}); +} /** * Similar to 'renameFile', but fallback to a copy+remove if `src` and `dst` diff --git a/src/libutil/linux/namespaces.cc b/src/libutil/linux/namespaces.cc index d4766cbba..c5e21dffc 100644 --- a/src/libutil/linux/namespaces.cc +++ b/src/libutil/linux/namespaces.cc @@ -118,7 +118,7 @@ void saveMountNamespace() void restoreMountNamespace() { try { - auto savedCwd = absPath("."); + auto savedCwd = std::filesystem::current_path(); if (fdSavedMountNamespace && setns(fdSavedMountNamespace.get(), CLONE_NEWNS) == -1) throw SysError("restoring parent mount namespace"); diff --git a/src/libutil/os-string.hh b/src/libutil/os-string.hh index 0d75173e5..3e24763fb 100644 --- a/src/libutil/os-string.hh +++ b/src/libutil/os-string.hh @@ -11,21 +11,30 @@ namespace nix { * Named because it is similar to the Rust type, except it is in the * native encoding not WTF-8. * - * Same as `std::filesystem::path::string_type`, but manually defined to + * Same as `std::filesystem::path::value_type`, but manually defined to * avoid including a much more complex header. */ -using OsString = std::basic_string< +using OsChar = #if defined(_WIN32) && !defined(__CYGWIN__) wchar_t #else char #endif - >; + ; + +/** + * Named because it is similar to the Rust type, except it is in the + * native encoding not WTF-8. + * + * Same as `std::filesystem::path::string_type`, but manually defined + * for the same reason as `OsChar`. + */ +using OsString = std::basic_string; /** * `std::string_view` counterpart for `OsString`. */ -using OsStringView = std::basic_string_view; +using OsStringView = std::basic_string_view; std::string os_string_to_string(OsStringView path); diff --git a/src/libutil/posix-source-accessor.cc b/src/libutil/posix-source-accessor.cc index 2b1a485d5..8cec3388d 100644 --- a/src/libutil/posix-source-accessor.cc +++ b/src/libutil/posix-source-accessor.cc @@ -20,7 +20,7 @@ PosixSourceAccessor::PosixSourceAccessor() SourcePath PosixSourceAccessor::createAtRoot(const std::filesystem::path & path) { - std::filesystem::path path2 = absPath(path.string()); + std::filesystem::path path2 = absPath(path); return { make_ref(path2.root_path()), CanonPath { path2.relative_path().string() }, diff --git a/src/libutil/serialise.cc b/src/libutil/serialise.cc index 8a57858f5..4aa5ae385 100644 --- a/src/libutil/serialise.cc +++ b/src/libutil/serialise.cc @@ -9,6 +9,7 @@ #ifdef _WIN32 # include +# include # include "windows-error.hh" #else # include @@ -167,13 +168,14 @@ bool FdSource::hasData() while (true) { fd_set fds; FD_ZERO(&fds); - FD_SET(fd, &fds); + int fd_ = fromDescriptorReadOnly(fd); + FD_SET(fd_, &fds); struct timeval timeout; timeout.tv_sec = 0; timeout.tv_usec = 0; - auto n = select(fd + 1, &fds, nullptr, nullptr, &timeout); + auto n = select(fd_ + 1, &fds, nullptr, nullptr, &timeout); if (n < 0) { if (errno == EINTR) continue; throw SysError("polling file descriptor"); diff --git a/src/libutil/strings.cc b/src/libutil/strings.cc index 60297228e..5cad95758 100644 --- a/src/libutil/strings.cc +++ b/src/libutil/strings.cc @@ -14,8 +14,8 @@ template std::list splitString(std::string_view s, std::string_view template std::set splitString(std::string_view s, std::string_view separators); template std::vector splitString(std::string_view s, std::string_view separators); -template std::list basicSplitString( - std::basic_string_view s, std::basic_string_view separators); +template std::list +basicSplitString(std::basic_string_view s, std::basic_string_view separators); template std::string concatStringsSep(std::string_view, const std::list &); template std::string concatStringsSep(std::string_view, const std::set &); diff --git a/src/libutil/unix/users.cc b/src/libutil/unix/users.cc index 58063a953..107a6e04f 100644 --- a/src/libutil/unix/users.cc +++ b/src/libutil/unix/users.cc @@ -9,6 +9,8 @@ namespace nix { +namespace fs { using namespace std::filesystem; } + std::string getUserName() { auto pw = getpwuid(geteuid()); diff --git a/src/nix/bundle.cc b/src/nix/bundle.cc index e152c26f2..5b7862c4e 100644 --- a/src/nix/bundle.cc +++ b/src/nix/bundle.cc @@ -6,6 +6,8 @@ #include "local-fs-store.hh" #include "eval-inline.hh" +namespace nix::fs { using namespace std::filesystem; } + using namespace nix; struct CmdBundle : InstallableValueCommand @@ -78,7 +80,7 @@ struct CmdBundle : InstallableValueCommand auto [bundlerFlakeRef, bundlerName, extendedOutputsSpec] = parseFlakeRefWithFragmentAndExtendedOutputsSpec( - fetchSettings, bundler, absPath(".")); + fetchSettings, bundler, fs::current_path().string()); const flake::LockFlags lockFlags{ .writeLockFile = false }; InstallableFlake bundler{this, evalState, std::move(bundlerFlakeRef), bundlerName, std::move(extendedOutputsSpec), diff --git a/src/nix/config-check.cc b/src/nix/config-check.cc index 1a6574de2..6cf73785e 100644 --- a/src/nix/config-check.cc +++ b/src/nix/config-check.cc @@ -10,6 +10,8 @@ #include "worker-protocol.hh" #include "executable-path.hh" +namespace nix::fs { using namespace std::filesystem; } + using namespace nix; namespace { @@ -40,8 +42,6 @@ void checkInfo(const std::string & msg) { } -namespace fs = std::filesystem; - struct CmdConfigCheck : StoreCommand { bool success = true; diff --git a/src/nix/develop.cc b/src/nix/develop.cc index effc86a0a..04672e2ad 100644 --- a/src/nix/develop.cc +++ b/src/nix/develop.cc @@ -21,6 +21,8 @@ #include "strings.hh" +namespace nix::fs { using namespace std::filesystem; } + using namespace nix; struct DevelopSettings : Config @@ -341,7 +343,7 @@ struct Common : InstallableCommand, MixProfile ref store, const BuildEnvironment & buildEnvironment, const std::filesystem::path & tmpDir, - const std::filesystem::path & outputsDir = std::filesystem::path { absPath(".") } / "outputs") + const std::filesystem::path & outputsDir = fs::path { fs::current_path() } / "outputs") { // A list of colon-separated environment variables that should be // prepended to, rather than overwritten, in order to keep the shell usable. @@ -450,7 +452,7 @@ struct Common : InstallableCommand, MixProfile auto targetFilePath = tmpDir / OS_STR(".attrs."); targetFilePath += ext; - writeFile(targetFilePath.string(), content); + writeFile(targetFilePath, content); auto fileInBuilderEnv = buildEnvironment.vars.find(envVar); assert(fileInBuilderEnv != buildEnvironment.vars.end()); diff --git a/src/nix/flake.cc b/src/nix/flake.cc index b7bbb767b..2db1e039e 100644 --- a/src/nix/flake.cc +++ b/src/nix/flake.cc @@ -25,7 +25,7 @@ #include "strings-inline.hh" -namespace fs = std::filesystem; +namespace nix::fs { using namespace std::filesystem; } using namespace nix; using namespace nix::flake; @@ -53,7 +53,7 @@ public: FlakeRef getFlakeRef() { - return parseFlakeRef(fetchSettings, flakeUrl, absPath(".")); //FIXME + return parseFlakeRef(fetchSettings, flakeUrl, fs::current_path().string()); //FIXME } LockedFlake lockFlake() @@ -65,7 +65,7 @@ public: { return { // Like getFlakeRef but with expandTilde calld first - parseFlakeRef(fetchSettings, expandTilde(flakeUrl), absPath(".")) + parseFlakeRef(fetchSettings, expandTilde(flakeUrl), fs::current_path().string()) }; } }; @@ -880,7 +880,7 @@ struct CmdFlakeInitCommon : virtual Args, EvalCommand auto evalState = getEvalState(); auto [templateFlakeRef, templateName] = parseFlakeRefWithFragment( - fetchSettings, templateUrl, absPath(".")); + fetchSettings, templateUrl, fs::current_path().string()); auto installable = InstallableFlake(nullptr, evalState, std::move(templateFlakeRef), templateName, ExtendedOutputsSpec::Default(), @@ -927,7 +927,7 @@ struct CmdFlakeInitCommon : virtual Args, EvalCommand } continue; } else - writeFile(to2.string(), contents); + writeFile(to2, contents); } else if (fs::is_symlink(st)) { auto target = fs::read_symlink(from2); diff --git a/src/nix/run.cc b/src/nix/run.cc index dfe7f374f..63ae8a195 100644 --- a/src/nix/run.cc +++ b/src/nix/run.cc @@ -20,6 +20,8 @@ #include +namespace nix::fs { using namespace std::filesystem; } + using namespace nix; std::string chrootHelperName = "__run_in_chroot"; @@ -170,25 +172,25 @@ void chrootHelper(int argc, char * * argv) if (!pathExists(storeDir)) { // FIXME: Use overlayfs? - std::filesystem::path tmpDir = createTempDir(); + fs::path tmpDir = createTempDir(); createDirs(tmpDir + storeDir); if (mount(realStoreDir.c_str(), (tmpDir + storeDir).c_str(), "", MS_BIND, 0) == -1) throw SysError("mounting '%s' on '%s'", realStoreDir, storeDir); - for (auto entry : std::filesystem::directory_iterator{"/"}) { + for (auto entry : fs::directory_iterator{"/"}) { checkInterrupt(); auto src = entry.path(); - Path dst = tmpDir / entry.path().filename(); + fs::path dst = tmpDir / entry.path().filename(); if (pathExists(dst)) continue; auto st = entry.symlink_status(); - if (std::filesystem::is_directory(st)) { + if (fs::is_directory(st)) { if (mkdir(dst.c_str(), 0700) == -1) throw SysError("creating directory '%s'", dst); if (mount(src.c_str(), dst.c_str(), "", MS_BIND | MS_REC, 0) == -1) throw SysError("mounting '%s' on '%s'", src, dst); - } else if (std::filesystem::is_symlink(st)) + } else if (fs::is_symlink(st)) createSymlink(readLink(src), dst); } @@ -205,9 +207,9 @@ void chrootHelper(int argc, char * * argv) if (mount(realStoreDir.c_str(), storeDir.c_str(), "", MS_BIND, 0) == -1) throw SysError("mounting '%s' on '%s'", realStoreDir, storeDir); - writeFile("/proc/self/setgroups", "deny"); - writeFile("/proc/self/uid_map", fmt("%d %d %d", uid, uid, 1)); - writeFile("/proc/self/gid_map", fmt("%d %d %d", gid, gid, 1)); + writeFile(fs::path{"/proc/self/setgroups"}, "deny"); + writeFile(fs::path{"/proc/self/uid_map"}, fmt("%d %d %d", uid, uid, 1)); + writeFile(fs::path{"/proc/self/gid_map"}, fmt("%d %d %d", gid, gid, 1)); #if __linux__ if (system != "") diff --git a/src/nix/self-exe.cc b/src/nix/self-exe.cc index a260bafd5..81a117e60 100644 --- a/src/nix/self-exe.cc +++ b/src/nix/self-exe.cc @@ -5,7 +5,9 @@ namespace nix { -namespace fs = std::filesystem; +namespace fs { +using namespace std::filesystem; +} fs::path getNixBin(std::optional binaryNameOpt) { diff --git a/tests/unit/libfetchers/public-key.cc b/tests/unit/libfetchers/public-key.cc index 8a639da9f..80796bd0f 100644 --- a/tests/unit/libfetchers/public-key.cc +++ b/tests/unit/libfetchers/public-key.cc @@ -10,11 +10,11 @@ using nlohmann::json; class PublicKeyTest : public CharacterizationTest { - Path unitTestData = getUnitTestData() + "/public-key"; + std::filesystem::path unitTestData = getUnitTestData() / "public-key"; public: - Path goldenMaster(std::string_view testStem) const override { - return unitTestData + "/" + testStem; + std::filesystem::path goldenMaster(std::string_view testStem) const override { + return unitTestData / testStem; } }; diff --git a/tests/unit/libstore-support/tests/nix_api_store.hh b/tests/unit/libstore-support/tests/nix_api_store.hh index a2d35d083..193b44970 100644 --- a/tests/unit/libstore-support/tests/nix_api_store.hh +++ b/tests/unit/libstore-support/tests/nix_api_store.hh @@ -10,7 +10,7 @@ #include #include -namespace fs = std::filesystem; +namespace fs { using namespace std::filesystem; } namespace nixC { class nix_api_store_test : public nix_api_util_context diff --git a/tests/unit/libstore-support/tests/protocol.hh b/tests/unit/libstore-support/tests/protocol.hh index 3c9e52c11..3f6799d1c 100644 --- a/tests/unit/libstore-support/tests/protocol.hh +++ b/tests/unit/libstore-support/tests/protocol.hh @@ -12,10 +12,10 @@ namespace nix { template class ProtoTest : public CharacterizationTest, public LibStoreTest { - Path unitTestData = getUnitTestData() + "/" + protocolDir; + std::filesystem::path unitTestData = getUnitTestData() / protocolDir; - Path goldenMaster(std::string_view testStem) const override { - return unitTestData + "/" + testStem + ".bin"; + std::filesystem::path goldenMaster(std::string_view testStem) const override { + return unitTestData / (std::string { testStem + ".bin" }); } }; diff --git a/tests/unit/libstore/derivation-advanced-attrs.cc b/tests/unit/libstore/derivation-advanced-attrs.cc index 26cf947a8..4d839ddab 100644 --- a/tests/unit/libstore/derivation-advanced-attrs.cc +++ b/tests/unit/libstore/derivation-advanced-attrs.cc @@ -16,12 +16,12 @@ using nlohmann::json; class DerivationAdvancedAttrsTest : public CharacterizationTest, public LibStoreTest { - Path unitTestData = getUnitTestData() + "/derivation"; + std::filesystem::path unitTestData = getUnitTestData() / "derivation"; public: - Path goldenMaster(std::string_view testStem) const override + std::filesystem::path goldenMaster(std::string_view testStem) const override { - return unitTestData + "/" + testStem; + return unitTestData / testStem; } }; diff --git a/tests/unit/libstore/derivation.cc b/tests/unit/libstore/derivation.cc index 71979f885..14652921a 100644 --- a/tests/unit/libstore/derivation.cc +++ b/tests/unit/libstore/derivation.cc @@ -13,11 +13,11 @@ using nlohmann::json; class DerivationTest : public CharacterizationTest, public LibStoreTest { - Path unitTestData = getUnitTestData() + "/derivation"; + std::filesystem::path unitTestData = getUnitTestData() / "derivation"; public: - Path goldenMaster(std::string_view testStem) const override { - return unitTestData + "/" + testStem; + std::filesystem::path goldenMaster(std::string_view testStem) const override { + return unitTestData / testStem; } /** diff --git a/tests/unit/libstore/machines.cc b/tests/unit/libstore/machines.cc index 2307f4d62..2d66e9534 100644 --- a/tests/unit/libstore/machines.cc +++ b/tests/unit/libstore/machines.cc @@ -13,6 +13,8 @@ using testing::Eq; using testing::Field; using testing::SizeIs; +namespace nix::fs { using namespace std::filesystem; } + using namespace nix; TEST(machines, getMachinesWithEmptyBuilders) { @@ -135,10 +137,10 @@ TEST(machines, getMachinesWithIncorrectFormat) { } TEST(machines, getMachinesWithCorrectFileReference) { - auto path = absPath(getUnitTestData() + "/machines/valid"); - ASSERT_TRUE(pathExists(path)); + auto path = fs::weakly_canonical(getUnitTestData() / "machines/valid"); + ASSERT_TRUE(fs::exists(path)); - auto actual = Machine::parseConfig({}, "@" + path); + auto actual = Machine::parseConfig({}, "@" + path.string()); ASSERT_THAT(actual, SizeIs(3)); EXPECT_THAT(actual, Contains(Field(&Machine::storeUri, AuthorityMatches("nix@scratchy.labs.cs.uu.nl")))); EXPECT_THAT(actual, Contains(Field(&Machine::storeUri, AuthorityMatches("nix@itchy.labs.cs.uu.nl")))); @@ -146,20 +148,22 @@ TEST(machines, getMachinesWithCorrectFileReference) { } TEST(machines, getMachinesWithCorrectFileReferenceToEmptyFile) { - auto path = "/dev/null"; - ASSERT_TRUE(pathExists(path)); + fs::path path = "/dev/null"; + ASSERT_TRUE(fs::exists(path)); - auto actual = Machine::parseConfig({}, std::string{"@"} + path); + auto actual = Machine::parseConfig({}, "@" + path.string()); ASSERT_THAT(actual, SizeIs(0)); } TEST(machines, getMachinesWithIncorrectFileReference) { - auto actual = Machine::parseConfig({}, "@" + absPath("/not/a/file")); + auto path = fs::weakly_canonical("/not/a/file"); + ASSERT_TRUE(!fs::exists(path)); + auto actual = Machine::parseConfig({}, "@" + path.string()); ASSERT_THAT(actual, SizeIs(0)); } TEST(machines, getMachinesWithCorrectFileReferenceToIncorrectFile) { EXPECT_THROW( - Machine::parseConfig({}, "@" + absPath(getUnitTestData() + "/machines/bad_format")), + Machine::parseConfig({}, "@" + fs::weakly_canonical(getUnitTestData() / "machines" / "bad_format").string()), FormatError); } diff --git a/tests/unit/libstore/nar-info.cc b/tests/unit/libstore/nar-info.cc index a6cb62de4..0d155743d 100644 --- a/tests/unit/libstore/nar-info.cc +++ b/tests/unit/libstore/nar-info.cc @@ -13,10 +13,10 @@ using nlohmann::json; class NarInfoTest : public CharacterizationTest, public LibStoreTest { - Path unitTestData = getUnitTestData() + "/nar-info"; + std::filesystem::path unitTestData = getUnitTestData() / "nar-info"; - Path goldenMaster(PathView testStem) const override { - return unitTestData + "/" + testStem + ".json"; + std::filesystem::path goldenMaster(PathView testStem) const override { + return unitTestData / (testStem + ".json"); } }; diff --git a/tests/unit/libstore/path-info.cc b/tests/unit/libstore/path-info.cc index 9e9c6303d..d6c4c2a7f 100644 --- a/tests/unit/libstore/path-info.cc +++ b/tests/unit/libstore/path-info.cc @@ -12,10 +12,10 @@ using nlohmann::json; class PathInfoTest : public CharacterizationTest, public LibStoreTest { - Path unitTestData = getUnitTestData() + "/path-info"; + std::filesystem::path unitTestData = getUnitTestData() / "path-info"; - Path goldenMaster(PathView testStem) const override { - return unitTestData + "/" + testStem + ".json"; + std::filesystem::path goldenMaster(PathView testStem) const override { + return unitTestData / (testStem + ".json"); } }; diff --git a/tests/unit/libstore/store-reference.cc b/tests/unit/libstore/store-reference.cc index 052cd7bed..d4c42f0fd 100644 --- a/tests/unit/libstore/store-reference.cc +++ b/tests/unit/libstore/store-reference.cc @@ -13,11 +13,11 @@ using nlohmann::json; class StoreReferenceTest : public CharacterizationTest, public LibStoreTest { - Path unitTestData = getUnitTestData() + "/store-reference"; + std::filesystem::path unitTestData = getUnitTestData() / "store-reference"; - Path goldenMaster(PathView testStem) const override + std::filesystem::path goldenMaster(PathView testStem) const override { - return unitTestData + "/" + testStem + ".txt"; + return unitTestData / (testStem + ".txt"); } }; diff --git a/tests/unit/libutil-support/tests/characterization.hh b/tests/unit/libutil-support/tests/characterization.hh index 19ba824ac..5e790e75b 100644 --- a/tests/unit/libutil-support/tests/characterization.hh +++ b/tests/unit/libutil-support/tests/characterization.hh @@ -13,7 +13,7 @@ namespace nix { * The path to the unit test data directory. See the contributing guide * in the manual for further details. */ -static inline Path getUnitTestData() { +static inline std::filesystem::path getUnitTestData() { return getEnv("_NIX_TEST_UNIT_DATA").value(); } @@ -36,7 +36,7 @@ protected: * While the "golden master" for this characterization test is * located. It should not be shared with any other test. */ - virtual Path goldenMaster(PathView testStem) const = 0; + virtual std::filesystem::path goldenMaster(PathView testStem) const = 0; public: /** @@ -77,7 +77,7 @@ public: if (testAccept()) { - createDirs(dirOf(file)); + std::filesystem::create_directories(file.parent_path()); writeFile2(file, got); GTEST_SKIP() << "Updating golden master " @@ -97,10 +97,10 @@ public: { writeTest( testStem, test, - [](const Path & f) -> std::string { + [](const std::filesystem::path & f) -> std::string { return readFile(f); }, - [](const Path & f, const std::string & c) { + [](const std::filesystem::path & f, const std::string & c) { return writeFile(f, c); }); } diff --git a/tests/unit/libutil/file-system.cc b/tests/unit/libutil/file-system.cc index cfddaae1c..7ef804f34 100644 --- a/tests/unit/libutil/file-system.cc +++ b/tests/unit/libutil/file-system.cc @@ -12,8 +12,8 @@ #include #ifdef _WIN32 -# define FS_SEP "\\" -# define FS_ROOT "C:" FS_SEP // Need a mounted one, C drive is likely +# define FS_SEP L"\\" +# define FS_ROOT L"C:" FS_SEP // Need a mounted one, C drive is likely #else # define FS_SEP "/" # define FS_ROOT FS_SEP @@ -23,6 +23,12 @@ # define PATH_MAX 4096 #endif +#ifdef _WIN32 +# define GET_CWD _wgetcwd +#else +# define GET_CWD getcwd +#endif + namespace nix { /* ----------- tests for file-system.hh -------------------------------------*/ @@ -33,34 +39,34 @@ namespace nix { TEST(absPath, doesntChangeRoot) { - auto p = absPath(FS_ROOT); + auto p = absPath(std::filesystem::path{FS_ROOT}); ASSERT_EQ(p, FS_ROOT); } TEST(absPath, turnsEmptyPathIntoCWD) { - char cwd[PATH_MAX + 1]; - auto p = absPath(""); + OsChar cwd[PATH_MAX + 1]; + auto p = absPath(std::filesystem::path{""}); - ASSERT_EQ(p, getcwd((char *) &cwd, PATH_MAX)); + ASSERT_EQ(p, GET_CWD((OsChar *) &cwd, PATH_MAX)); } TEST(absPath, usesOptionalBasePathWhenGiven) { - char _cwd[PATH_MAX + 1]; - char * cwd = getcwd((char *) &_cwd, PATH_MAX); + OsChar _cwd[PATH_MAX + 1]; + OsChar * cwd = GET_CWD((OsChar *) &_cwd, PATH_MAX); - auto p = absPath("", cwd); + auto p = absPath(std::filesystem::path{""}.string(), std::filesystem::path{cwd}.string()); - ASSERT_EQ(p, cwd); + ASSERT_EQ(p, std::filesystem::path{cwd}.string()); } TEST(absPath, isIdempotent) { - char _cwd[PATH_MAX + 1]; - char * cwd = getcwd((char *) &_cwd, PATH_MAX); - auto p1 = absPath(cwd); + OsChar _cwd[PATH_MAX + 1]; + OsChar * cwd = GET_CWD((OsChar *) &_cwd, PATH_MAX); + auto p1 = absPath(std::filesystem::path{cwd}); auto p2 = absPath(p1); ASSERT_EQ(p1, p2); @@ -68,8 +74,8 @@ TEST(absPath, isIdempotent) TEST(absPath, pathIsCanonicalised) { - auto path = FS_ROOT "some/path/with/trailing/dot/."; - auto p1 = absPath(path); + auto path = FS_ROOT OS_STR("some/path/with/trailing/dot/."); + auto p1 = absPath(std::filesystem::path{path}); auto p2 = absPath(p1); ASSERT_EQ(p1, FS_ROOT "some" FS_SEP "path" FS_SEP "with" FS_SEP "trailing" FS_SEP "dot"); @@ -82,26 +88,26 @@ TEST(absPath, pathIsCanonicalised) TEST(canonPath, removesTrailingSlashes) { - auto path = FS_ROOT "this/is/a/path//"; - auto p = canonPath(path); + std::filesystem::path path = FS_ROOT "this/is/a/path//"; + auto p = canonPath(path.string()); - ASSERT_EQ(p, FS_ROOT "this" FS_SEP "is" FS_SEP "a" FS_SEP "path"); + ASSERT_EQ(p, std::filesystem::path{FS_ROOT "this" FS_SEP "is" FS_SEP "a" FS_SEP "path"}.string()); } TEST(canonPath, removesDots) { - auto path = FS_ROOT "this/./is/a/path/./"; - auto p = canonPath(path); + std::filesystem::path path = FS_ROOT "this/./is/a/path/./"; + auto p = canonPath(path.string()); - ASSERT_EQ(p, FS_ROOT "this" FS_SEP "is" FS_SEP "a" FS_SEP "path"); + ASSERT_EQ(p, std::filesystem::path{FS_ROOT "this" FS_SEP "is" FS_SEP "a" FS_SEP "path"}.string()); } TEST(canonPath, removesDots2) { - auto path = FS_ROOT "this/a/../is/a////path/foo/.."; - auto p = canonPath(path); + std::filesystem::path path = FS_ROOT "this/a/../is/a////path/foo/.."; + auto p = canonPath(path.string()); - ASSERT_EQ(p, FS_ROOT "this" FS_SEP "is" FS_SEP "a" FS_SEP "path"); + ASSERT_EQ(p, std::filesystem::path{FS_ROOT "this" FS_SEP "is" FS_SEP "a" FS_SEP "path"}.string()); } TEST(canonPath, requiresAbsolutePath) @@ -243,7 +249,7 @@ TEST(isDirOrInDir, DISABLED_shouldWork) TEST(pathExists, rootExists) { - ASSERT_TRUE(pathExists(FS_ROOT)); + ASSERT_TRUE(pathExists(std::filesystem::path{FS_ROOT}.string())); } TEST(pathExists, cwdExists) diff --git a/tests/unit/libutil/git.cc b/tests/unit/libutil/git.cc index 3d01d9806..9232de5b9 100644 --- a/tests/unit/libutil/git.cc +++ b/tests/unit/libutil/git.cc @@ -11,12 +11,12 @@ using namespace git; class GitTest : public CharacterizationTest { - Path unitTestData = getUnitTestData() + "/git"; + std::filesystem::path unitTestData = getUnitTestData() / "git"; public: - Path goldenMaster(std::string_view testStem) const override { - return unitTestData + "/" + testStem; + std::filesystem::path goldenMaster(std::string_view testStem) const override { + return unitTestData / std::string(testStem); } /** diff --git a/tests/unit/libutil/meson.build b/tests/unit/libutil/meson.build index 83cec13ec..c39db8cda 100644 --- a/tests/unit/libutil/meson.build +++ b/tests/unit/libutil/meson.build @@ -48,12 +48,14 @@ subdir('build-utils-meson/diagnostics') sources = files( 'args.cc', 'canon-path.cc', + 'checked-arithmetic.cc', 'chunked-vector.cc', 'closure.cc', 'compression.cc', 'config.cc', 'executable-path.cc', 'file-content-address.cc', + 'file-system.cc', 'git.cc', 'hash.cc', 'hilite.cc', @@ -62,6 +64,7 @@ sources = files( 'lru-cache.cc', 'nix_api_util.cc', 'pool.cc', + 'position.cc', 'processes.cc', 'references.cc', 'spawn.cc',