Convert profiles to use std::filesystem::path

Co-authored-by: Vinayak Goyal <vinayakankugoyal@gmail.com>
Co-authored-by: Eelco Dolstra <edolstra@gmail.com>

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

@@ -307,7 +307,7 @@ Goal::Co DerivationBuildingGoal::tryToBuild()
            crashes.  If we can't acquire the lock, then continue; hopefully some
            other goal can start a build, and if not, the main loop will sleep a few
            seconds and then retry this goal. */
-        PathSet lockFiles;
+        std::set<std::filesystem::path> lockFiles;
         /* FIXME: Should lock something like the drv itself so we don't build same
            CA drv concurrently */
         if (auto * localStore = dynamic_cast<LocalStore *>(&worker.store)) {

src/libstore/include/nix/store/pathlocks.hh

@@ -1,6 +1,8 @@
 #pragma once
 ///@file
 
+#include <filesystem>
+
 #include "nix/util/file-descriptor.hh"
 
 namespace nix {
@@ -10,12 +12,12 @@ namespace nix {
  * -1 is returned if create is false and the lock could not be opened
  * because it doesn't exist.  Any other error throws an exception.
  */
-AutoCloseFD openLockFile(const Path & path, bool create);
+AutoCloseFD openLockFile(const std::filesystem::path & path, bool create);
 
 /**
  * Delete an open lock file.
  */
-void deleteLockFile(const Path & path, Descriptor desc);
+void deleteLockFile(const std::filesystem::path & path, Descriptor desc);
 
 enum LockType { ltRead, ltWrite, ltNone };
 
@@ -24,14 +26,14 @@ bool lockFile(Descriptor desc, LockType lockType, bool wait);
 class PathLocks
 {
 private:
-    typedef std::pair<Descriptor, Path> FDPair;
+    typedef std::pair<Descriptor, std::filesystem::path> FDPair;
     std::list<FDPair> fds;
     bool deletePaths;
 
 public:
     PathLocks();
-    PathLocks(const PathSet & paths, const std::string & waitMsg = "");
-    bool lockPaths(const PathSet & _paths, const std::string & waitMsg = "", bool wait = true);
+    PathLocks(const std::set<std::filesystem::path> & paths, const std::string & waitMsg = "");
+    bool lockPaths(const std::set<std::filesystem::path> & _paths, const std::string & waitMsg = "", bool wait = true);
     ~PathLocks();
     void unlock();
     void setDeletion(bool deletePaths);

src/libstore/include/nix/store/profiles.hh

@@ -7,12 +7,13 @@
  * See the manual for additional information.
  */
 
-#include "nix/util/types.hh"
-#include "nix/store/pathlocks.hh"
-
+#include <filesystem>
 #include <optional>
 #include <time.h>
 
+#include "nix/util/types.hh"
+#include "nix/store/pathlocks.hh"
+
 namespace nix {
 
 class StorePath;
@@ -47,9 +48,9 @@ struct Generation
      * distinct contents to avoid bloat, but nothing stops two
      * non-adjacent generations from having the same contents.
      *
-     * @todo Use `StorePath` instead of `Path`?
+     * @todo Use `StorePath` instead of `std::filesystem::path`?
      */
-    Path path;
+    std::filesystem::path path;
 
     /**
      * When the generation was created. This is extra metadata about the
@@ -81,7 +82,7 @@ typedef std::list<Generation> Generations;
  *
  * Note that the current/active generation need not be the latest one.
  */
-std::pair<Generations, std::optional<GenerationNumber>> findGenerations(Path profile);
+std::pair<Generations, std::optional<GenerationNumber>> findGenerations(std::filesystem::path profile);
 
 struct LocalFSStore;
 
@@ -96,7 +97,7 @@ struct LocalFSStore;
  * The behavior of reusing existing generations like this makes this
  * procedure idempotent. It also avoids clutter.
  */
-Path createGeneration(LocalFSStore & store, Path profile, StorePath outPath);
+std::filesystem::path createGeneration(LocalFSStore & store, std::filesystem::path profile, StorePath outPath);
 
 /**
  * Unconditionally delete a generation
@@ -111,7 +112,7 @@ Path createGeneration(LocalFSStore & store, Path profile, StorePath outPath);
  *
  * @todo Should we expose this at all?
  */
-void deleteGeneration(const Path & profile, GenerationNumber gen);
+void deleteGeneration(const std::filesystem::path & profile, GenerationNumber gen);
 
 /**
  * Delete the given set of generations.
@@ -128,7 +129,8 @@ void deleteGeneration(const Path & profile, GenerationNumber gen);
  * Trying to delete the currently active generation will fail, and cause
  * no generations to be deleted.
  */
-void deleteGenerations(const Path & profile, const std::set<GenerationNumber> & gensToDelete, bool dryRun);
+void deleteGenerations(
+    const std::filesystem::path & profile, const std::set<GenerationNumber> & gensToDelete, bool dryRun);
 
 /**
  * Delete generations older than `max` passed the current generation.
@@ -142,7 +144,7 @@ void deleteGenerations(const Path & profile, const std::set<GenerationNumber> &
  * @param dryRun Log what would be deleted instead of actually doing
  * so.
  */
-void deleteGenerationsGreaterThan(const Path & profile, GenerationNumber max, bool dryRun);
+void deleteGenerationsGreaterThan(const std::filesystem::path & profile, GenerationNumber max, bool dryRun);
 
 /**
  * Delete all generations other than the current one
@@ -153,7 +155,7 @@ void deleteGenerationsGreaterThan(const Path & profile, GenerationNumber max, bo
  * @param dryRun Log what would be deleted instead of actually doing
  * so.
  */
-void deleteOldGenerations(const Path & profile, bool dryRun);
+void deleteOldGenerations(const std::filesystem::path & profile, bool dryRun);
 
 /**
  * Delete generations older than `t`, except for the most recent one
@@ -165,7 +167,7 @@ void deleteOldGenerations(const Path & profile, bool dryRun);
  * @param dryRun Log what would be deleted instead of actually doing
  * so.
  */
-void deleteGenerationsOlderThan(const Path & profile, time_t t, bool dryRun);
+void deleteGenerationsOlderThan(const std::filesystem::path & profile, time_t t, bool dryRun);
 
 /**
  * Parse a temp spec intended for `deleteGenerationsOlderThan()`.
@@ -180,19 +182,19 @@ time_t parseOlderThanTimeSpec(std::string_view timeSpec);
  *
  * @todo Always use `switchGeneration()` instead, and delete this.
  */
-void switchLink(Path link, Path target);
+void switchLink(std::filesystem::path link, std::filesystem::path target);
 
 /**
  * Roll back a profile to the specified generation, or to the most
  * recent one older than the current.
  */
-void switchGeneration(const Path & profile, std::optional<GenerationNumber> dstGen, bool dryRun);
+void switchGeneration(const std::filesystem::path & profile, std::optional<GenerationNumber> dstGen, bool dryRun);
 
 /**
  * Ensure exclusive access to a profile.  Any command that modifies
  * the profile first acquires this lock.
  */
-void lockProfile(PathLocks & lock, const Path & profile);
+void lockProfile(PathLocks & lock, const std::filesystem::path & profile);
 
 /**
  * Optimistic locking is used by long-running operations like `nix-env
@@ -205,34 +207,34 @@ void lockProfile(PathLocks & lock, const Path & profile);
  * store.  Most of the time, only the user environment has to be
  * rebuilt.
  */
-std::string optimisticLockProfile(const Path & profile);
+std::string optimisticLockProfile(const std::filesystem::path & profile);
 
 /**
  * Create and return the path to a directory suitable for storing the user’s
  * profiles.
  */
-Path profilesDir();
+std::filesystem::path profilesDir();
 
 /**
  * Return the path to the profile directory for root (but don't try creating it)
  */
-Path rootProfilesDir();
+std::filesystem::path rootProfilesDir();
 
 /**
  * Create and return the path to the file used for storing the users's channels
  */
-Path defaultChannelsDir();
+std::filesystem::path defaultChannelsDir();
 
 /**
  * Return the path to the channel directory for root (but don't try creating it)
  */
-Path rootChannelsDir();
+std::filesystem::path rootChannelsDir();
 
 /**
  * Resolve the default profile (~/.nix-profile by default,
  * $XDG_STATE_HOME/nix/profile if XDG Base Directory Support is enabled),
  * and create if doesn't exist
  */
-Path getDefaultProfile();
+std::filesystem::path getDefaultProfile();
 
 } // namespace nix

src/libstore/include/nix/store/store-api.hh

@@ -996,6 +996,12 @@ OutputPathMap resolveDerivedPath(Store &, const DerivedPath::Built &, Store * ev
  */
 std::string showPaths(const PathSet & paths);
 
+/**
+ * Display a set of paths in human-readable form (i.e., between quotes
+ * and separated by commas).
+ */
+std::string showPaths(const std::set<std::filesystem::path> paths);
+
 std::optional<ValidPathInfo>
 decodeValidPathInfo(const Store & store, std::istream & str, std::optional<HashResult> hashGiven = std::nullopt);
 

src/libstore/pathlocks.cc

@@ -13,7 +13,7 @@ PathLocks::PathLocks()
 {
 }
 
-PathLocks::PathLocks(const PathSet & paths, const std::string & waitMsg)
+PathLocks::PathLocks(const std::set<std::filesystem::path> & paths, const std::string & waitMsg)
     : deletePaths(false)
 {
     lockPaths(paths, waitMsg);

src/libstore/profiles.cc

@@ -31,12 +31,12 @@ static std::optional<GenerationNumber> parseName(const std::string & profileName
         return {};
 }
 
-std::pair<Generations, std::optional<GenerationNumber>> findGenerations(Path profile)
+std::pair<Generations, std::optional<GenerationNumber>> findGenerations(std::filesystem::path profile)
 {
     Generations gens;
 
-    std::filesystem::path profileDir = dirOf(profile);
-    auto profileName = std::string(baseNameOf(profile));
+    std::filesystem::path profileDir = profile.parent_path();
+    auto profileName = profile.filename().string();
 
     for (auto & i : DirectoryIterator{profileDir}) {
         checkInterrupt();
@@ -48,18 +48,20 @@ std::pair<Generations, std::optional<GenerationNumber>> findGenerations(Path pro
 
     gens.sort([](const Generation & a, const Generation & b) { return a.number < b.number; });
 
-    return {gens, pathExists(profile) ? parseName(profileName, readLink(profile)) : std::nullopt};
+    return {gens, pathExists(profile) ? parseName(profileName, readLink(profile).string()) : std::nullopt};
 }
 
 /**
  * Create a generation name that can be parsed by `parseName()`.
  */
-static Path makeName(const Path & profile, GenerationNumber num)
+static std::filesystem::path makeName(const std::filesystem::path & profile, GenerationNumber num)
 {
-    return fmt("%s-%s-link", profile, num);
+    /* NB std::filesystem::path when put in format strings is
+       quoted automatically. */
+    return fmt("%s-%s-link", profile.string(), num);
 }
 
-Path createGeneration(LocalFSStore & store, Path profile, StorePath outPath)
+std::filesystem::path createGeneration(LocalFSStore & store, std::filesystem::path profile, StorePath outPath)
 {
     /* The new generation number should be higher than old the
        previous ones. */
@@ -90,21 +92,24 @@ Path createGeneration(LocalFSStore & store, Path profile, StorePath outPath)
        to the permanent roots (of which the GC would have a stale
        view).  If we didn't do it this way, the GC might remove the
        user environment etc. we've just built. */
-    Path generation = makeName(profile, num + 1);
-    store.addPermRoot(outPath, generation);
+    auto generation = makeName(profile, num + 1);
+    store.addPermRoot(outPath, generation.string());
 
     return generation;
 }
 
-static void removeFile(const Path & path)
+static void removeFile(const std::filesystem::path & path)
 {
-    if (remove(path.c_str()) == -1)
-        throw SysError("cannot unlink '%1%'", path);
+    try {
+        std::filesystem::remove(path);
+    } catch (std::filesystem::filesystem_error & e) {
+        throw SysError("removing file '%1%'", path);
+    }
 }
 
-void deleteGeneration(const Path & profile, GenerationNumber gen)
+void deleteGeneration(const std::filesystem::path & profile, GenerationNumber gen)
 {
-    Path generation = makeName(profile, gen);
+    std::filesystem::path generation = makeName(profile, gen);
     removeFile(generation);
 }
 
@@ -117,7 +122,7 @@ void deleteGeneration(const Path & profile, GenerationNumber gen)
  *
  *  - We only actually delete if `dryRun` is false.
  */
-static void deleteGeneration2(const Path & profile, GenerationNumber gen, bool dryRun)
+static void deleteGeneration2(const std::filesystem::path & profile, GenerationNumber gen, bool dryRun)
 {
     if (dryRun)
         notice("would remove profile version %1%", gen);
@@ -127,7 +132,8 @@ static void deleteGeneration2(const Path & profile, GenerationNumber gen, bool d
     }
 }
 
-void deleteGenerations(const Path & profile, const std::set<GenerationNumber> & gensToDelete, bool dryRun)
+void deleteGenerations(
+    const std::filesystem::path & profile, const std::set<GenerationNumber> & gensToDelete, bool dryRun)
 {
     PathLocks lock;
     lockProfile(lock, profile);
@@ -153,7 +159,7 @@ static inline void iterDropUntil(Generations & gens, auto && i, auto && cond)
         ;
 }
 
-void deleteGenerationsGreaterThan(const Path & profile, GenerationNumber max, bool dryRun)
+void deleteGenerationsGreaterThan(const std::filesystem::path & profile, GenerationNumber max, bool dryRun)
 {
     if (max == 0)
         throw Error("Must keep at least one generation, otherwise the current one would be deleted");
@@ -178,7 +184,7 @@ void deleteGenerationsGreaterThan(const Path & profile, GenerationNumber max, bo
         deleteGeneration2(profile, i->number, dryRun);
 }
 
-void deleteOldGenerations(const Path & profile, bool dryRun)
+void deleteOldGenerations(const std::filesystem::path & profile, bool dryRun)
 {
     PathLocks lock;
     lockProfile(lock, profile);
@@ -190,7 +196,7 @@ void deleteOldGenerations(const Path & profile, bool dryRun)
             deleteGeneration2(profile, i.number, dryRun);
 }
 
-void deleteGenerationsOlderThan(const Path & profile, time_t t, bool dryRun)
+void deleteGenerationsOlderThan(const std::filesystem::path & profile, time_t t, bool dryRun)
 {
     PathLocks lock;
     lockProfile(lock, profile);
@@ -238,16 +244,16 @@ time_t parseOlderThanTimeSpec(std::string_view timeSpec)
     return curTime - *days * 24 * 3600;
 }
 
-void switchLink(Path link, Path target)
+void switchLink(std::filesystem::path link, std::filesystem::path target)
 {
     /* Hacky. */
-    if (dirOf(target) == dirOf(link))
-        target = baseNameOf(target);
+    if (target.parent_path() == link.parent_path())
+        target = target.filename();
 
     replaceSymlink(target, link);
 }
 
-void switchGeneration(const Path & profile, std::optional<GenerationNumber> dstGen, bool dryRun)
+void switchGeneration(const std::filesystem::path & profile, std::optional<GenerationNumber> dstGen, bool dryRun)
 {
     PathLocks lock;
     lockProfile(lock, profile);
@@ -274,44 +280,47 @@ void switchGeneration(const Path & profile, std::optional<GenerationNumber> dstG
     switchLink(profile, dst->path);
 }
 
-void lockProfile(PathLocks & lock, const Path & profile)
+void lockProfile(PathLocks & lock, const std::filesystem::path & profile)
 {
     lock.lockPaths({profile}, fmt("waiting for lock on profile '%1%'", profile));
     lock.setDeletion(true);
 }
 
-std::string optimisticLockProfile(const Path & profile)
+std::string optimisticLockProfile(const std::filesystem::path & profile)
 {
-    return pathExists(profile) ? readLink(profile) : "";
+    return pathExists(profile) ? readLink(profile).string() : "";
 }
 
-Path profilesDir()
+std::filesystem::path profilesDir()
 {
-    auto profileRoot = isRootUser() ? rootProfilesDir() : createNixStateDir() + "/profiles";
+    auto profileRoot = isRootUser() ? rootProfilesDir() : std::filesystem::path{createNixStateDir()} / "profiles";
     createDirs(profileRoot);
     return profileRoot;
 }
 
-Path rootProfilesDir()
+std::filesystem::path rootProfilesDir()
 {
-    return settings.nixStateDir + "/profiles/per-user/root";
+    return std::filesystem::path{settings.nixStateDir} / "profiles/per-user/root";
 }
 
-Path getDefaultProfile()
+std::filesystem::path getDefaultProfile()
 {
-    Path profileLink = settings.useXDGBaseDirectories ? createNixStateDir() + "/profile" : getHome() + "/.nix-profile";
+    std::filesystem::path profileLink = settings.useXDGBaseDirectories
+                                            ? std::filesystem::path{createNixStateDir()} / "profile"
+                                            : std::filesystem::path{getHome()} / ".nix-profile";
     try {
-        auto profile = profilesDir() + "/profile";
+        auto profile = profilesDir() / "profile";
         if (!pathExists(profileLink)) {
             replaceSymlink(profile, profileLink);
         }
         // Backwards compatibility measure: Make root's profile available as
         // `.../default` as it's what NixOS and most of the init scripts expect
-        Path globalProfileLink = settings.nixStateDir + "/profiles/default";
+        auto globalProfileLink = std::filesystem::path{settings.nixStateDir} / "profiles" / "default";
         if (isRootUser() && !pathExists(globalProfileLink)) {
             replaceSymlink(profile, globalProfileLink);
         }
-        return absPath(readLink(profileLink), dirOf(profileLink));
+        auto linkDir = profileLink.parent_path();
+        return absPath(readLink(profileLink), &linkDir);
     } catch (Error &) {
         return profileLink;
     } catch (std::filesystem::filesystem_error &) {
@@ -319,14 +328,14 @@ Path getDefaultProfile()
     }
 }
 
-Path defaultChannelsDir()
+std::filesystem::path defaultChannelsDir()
 {
-    return profilesDir() + "/channels";
+    return profilesDir() / "channels";
 }
 
-Path rootChannelsDir()
+std::filesystem::path rootChannelsDir()
 {
-    return rootProfilesDir() + "/channels";
+    return rootProfilesDir() / "channels";
 }
 
 } // namespace nix

src/libstore/store-api.cc

@@ -1126,6 +1126,11 @@ std::string StoreDirConfig::showPaths(const StorePathSet & paths) const
     return s;
 }
 
+std::string showPaths(const std::set<std::filesystem::path> paths)
+{
+    return concatStringsSep(", ", quoteFSPaths(paths));
+}
+
 std::string showPaths(const PathSet & paths)
 {
     return concatStringsSep(", ", quoteStrings(paths));

src/libstore/unix/pathlocks.cc

@@ -13,7 +13,7 @@
 
 namespace nix {
 
-AutoCloseFD openLockFile(const Path & path, bool create)
+AutoCloseFD openLockFile(const std::filesystem::path & path, bool create)
 {
     AutoCloseFD fd;
 
@@ -24,7 +24,7 @@ AutoCloseFD openLockFile(const Path & path, bool create)
     return fd;
 }
 
-void deleteLockFile(const Path & path, Descriptor desc)
+void deleteLockFile(const std::filesystem::path & path, Descriptor desc)
 {
     /* Get rid of the lock file.  Have to be careful not to introduce
        races.  Write a (meaningless) token to the file to indicate to
@@ -69,7 +69,7 @@ bool lockFile(Descriptor desc, LockType lockType, bool wait)
     return true;
 }
 
-bool PathLocks::lockPaths(const PathSet & paths, const std::string & waitMsg, bool wait)
+bool PathLocks::lockPaths(const std::set<std::filesystem::path> & paths, const std::string & waitMsg, bool wait)
 {
     assert(fds.empty());
 
@@ -81,7 +81,7 @@ bool PathLocks::lockPaths(const PathSet & paths, const std::string & waitMsg, bo
        preventing deadlocks. */
     for (auto & path : paths) {
         checkInterrupt();
-        Path lockPath = path + ".lock";
+        std::filesystem::path lockPath = path + ".lock";
 
         debug("locking path '%1%'", path);
 

src/libstore/windows/pathlocks.cc

@@ -13,10 +13,10 @@ namespace nix {
 
 using namespace nix::windows;
 
-void deleteLockFile(const Path & path, Descriptor desc)
+void deleteLockFile(const std::filesystem::path & path, Descriptor desc)
 {
 
-    int exit = DeleteFileA(path.c_str());
+    int exit = DeleteFileW(path.c_str());
     if (exit == 0)
         warn("%s: &s", path, std::to_string(GetLastError()));
 }
@@ -36,9 +36,9 @@ void PathLocks::unlock()
     fds.clear();
 }
 
-AutoCloseFD openLockFile(const Path & path, bool create)
+AutoCloseFD openLockFile(const std::filesystem::path & path, bool create)
 {
-    AutoCloseFD desc = CreateFileA(
+    AutoCloseFD desc = CreateFileW(
         path.c_str(),
         GENERIC_READ | GENERIC_WRITE,
         FILE_SHARE_READ | FILE_SHARE_WRITE,
@@ -103,13 +103,14 @@ bool lockFile(Descriptor desc, LockType lockType, bool wait)
     }
 }
 
-bool PathLocks::lockPaths(const PathSet & paths, const std::string & waitMsg, bool wait)
+bool PathLocks::lockPaths(const std::set<std::filesystem::path> & paths, const std::string & waitMsg, bool wait)
 {
     assert(fds.empty());
 
     for (auto & path : paths) {
         checkInterrupt();
-        Path lockPath = path + ".lock";
+        std::filesystem::path lockPath = path;
+        lockPath += L".lock";
         debug("locking path '%1%'", path);
 
         AutoCloseFD fd;