Generations API cleanup

src/libstore/profiles.cc

@@ -12,30 +12,24 @@
 namespace nix {
 
 
-static bool cmpGensByNumber(const Generation & a, const Generation & b)
-{
-    return a.number < b.number;
-}
-
-
 /* Parse a generation name of the format
    `<profilename>-<number>-link'. */
-static int parseName(const string & profileName, const string & name)
+static std::optional<GenerationNumber> parseName(const string & profileName, const string & name)
 {
-    if (string(name, 0, profileName.size() + 1) != profileName + "-") return -1;
+    if (string(name, 0, profileName.size() + 1) != profileName + "-") return {};
     string s = string(name, profileName.size() + 1);
     string::size_type p = s.find("-link");
-    if (p == string::npos) return -1;
+    if (p == string::npos) return {};
-    int n;
+    unsigned int n;
     if (string2Int(string(s, 0, p), n) && n >= 0)
         return n;
     else
-        return -1;
+        return {};
 }
 
 
 
-Generations findGenerations(Path profile, int & curGen)
+std::pair<Generations, std::optional<GenerationNumber>> findGenerations(Path profile)
 {
     Generations gens;
 
@@ -43,30 +37,34 @@ Generations findGenerations(Path profile, int & curGen)
     auto profileName = std::string(baseNameOf(profile));
 
     for (auto & i : readDirectory(profileDir)) {
-        int n;
+        if (auto n = parseName(profileName, i.name)) {
-        if ((n = parseName(profileName, i.name)) != -1) {
+            auto path = profileDir + "/" + i.name;
-            Generation gen;
-            gen.path = profileDir + "/" + i.name;
-            gen.number = n;
             struct stat st;
-            if (lstat(gen.path.c_str(), &st) != 0)
+            if (lstat(path.c_str(), &st) != 0)
-                throw SysError("statting '%1%'", gen.path);
+                throw SysError("statting '%1%'", path);
-            gen.creationTime = st.st_mtime;
+            gens.push_back({
-            gens.push_back(gen);
+                .number = *n,
+                .path = path,
+                .creationTime = st.st_mtime
+            });
         }
     }
 
-    gens.sort(cmpGensByNumber);
+    gens.sort([](const Generation & a, const Generation & b)
+    {
+        return a.number < b.number;
+    });
 
-    curGen = pathExists(profile)
+    return {
+        gens,
+        pathExists(profile)
         ? parseName(profileName, readLink(profile))
-        : -1;
+        : std::nullopt
-
+    };
-    return gens;
 }
 
 
-static void makeName(const Path & profile, unsigned int num,
+static void makeName(const Path & profile, GenerationNumber num,
     Path & outLink)
 {
     Path prefix = (format("%1%-%2%") % profile % num).str();
@@ -78,10 +76,9 @@ Path createGeneration(ref<LocalFSStore> store, Path profile, Path outPath)
 {
     /* The new generation number should be higher than old the
        previous ones. */
-    int dummy;
+    auto [gens, dummy] = findGenerations(profile);
-    Generations gens = findGenerations(profile, dummy);
 
-    unsigned int num;
+    GenerationNumber num;
     if (gens.size() > 0) {
         Generation last = gens.back();
 
@@ -121,7 +118,7 @@ static void removeFile(const Path & path)
 }
 
 
-void deleteGeneration(const Path & profile, unsigned int gen)
+void deleteGeneration(const Path & profile, GenerationNumber gen)
 {
     Path generation;
     makeName(profile, gen, generation);
@@ -129,7 +126,7 @@ void deleteGeneration(const Path & profile, unsigned int gen)
 }
 
 
-static void deleteGeneration2(const Path & profile, unsigned int gen, bool dryRun)
+static void deleteGeneration2(const Path & profile, GenerationNumber gen, bool dryRun)
 {
     if (dryRun)
         printInfo(format("would remove generation %1%") % gen);
@@ -140,31 +137,29 @@ static void deleteGeneration2(const Path & profile, unsigned int gen, bool dryRu
 }
 
 
-void deleteGenerations(const Path & profile, const std::set<unsigned int> & gensToDelete, bool dryRun)
+void deleteGenerations(const Path & profile, const std::set<GenerationNumber> & gensToDelete, bool dryRun)
 {
     PathLocks lock;
     lockProfile(lock, profile);
 
-    int curGen;
+    auto [gens, curGen] = findGenerations(profile);
-    Generations gens = findGenerations(profile, curGen);
 
-    if (gensToDelete.find(curGen) != gensToDelete.end())
+    if (gensToDelete.count(*curGen))
         throw Error("cannot delete current generation of profile %1%'", profile);
 
     for (auto & i : gens) {
-        if (gensToDelete.find(i.number) == gensToDelete.end()) continue;
+        if (!gensToDelete.count(i.number)) continue;
         deleteGeneration2(profile, i.number, dryRun);
     }
 }
 
-void deleteGenerationsGreaterThan(const Path & profile, int max, bool dryRun)
+void deleteGenerationsGreaterThan(const Path & profile, GenerationNumber max, bool dryRun)
 {
     PathLocks lock;
     lockProfile(lock, profile);
 
-    int curGen;
     bool fromCurGen = false;
-    Generations gens = findGenerations(profile, curGen);
+    auto [gens, curGen] = findGenerations(profile);
     for (auto i = gens.rbegin(); i != gens.rend(); ++i) {
         if (i->number == curGen) {
             fromCurGen = true;
@@ -186,8 +181,7 @@ void deleteOldGenerations(const Path & profile, bool dryRun)
     PathLocks lock;
     lockProfile(lock, profile);
 
-    int curGen;
+    auto [gens, curGen] = findGenerations(profile);
-    Generations gens = findGenerations(profile, curGen);
 
     for (auto & i : gens)
         if (i.number != curGen)
@@ -200,8 +194,7 @@ void deleteGenerationsOlderThan(const Path & profile, time_t t, bool dryRun)
     PathLocks lock;
     lockProfile(lock, profile);
 
-    int curGen;
+    auto [gens, curGen] = findGenerations(profile);
-    Generations gens = findGenerations(profile, curGen);
 
     bool canDelete = false;
     for (auto i = gens.rbegin(); i != gens.rend(); ++i)

src/libstore/profiles.hh

@@ -9,37 +9,32 @@
 namespace nix {
 
 
+typedef unsigned int GenerationNumber;
+
 struct Generation
 {
-    int number;
+    GenerationNumber number;
     Path path;
     time_t creationTime;
-    Generation()
-    {
-        number = -1;
-    }
-    operator bool() const
-    {
-        return number != -1;
-    }
 };
 
-typedef list<Generation> Generations;
+typedef std::list<Generation> Generations;
 
 
 /* Returns the list of currently present generations for the specified
-   profile, sorted by generation number. */
+   profile, sorted by generation number. Also returns the number of
-Generations findGenerations(Path profile, int & curGen);
+   the current generation. */
+std::pair<Generations, std::optional<GenerationNumber>> findGenerations(Path profile);
 
 class LocalFSStore;
 
 Path createGeneration(ref<LocalFSStore> store, Path profile, Path outPath);
 
-void deleteGeneration(const Path & profile, unsigned int gen);
+void deleteGeneration(const Path & profile, GenerationNumber gen);
 
-void deleteGenerations(const Path & profile, const std::set<unsigned int> & gensToDelete, bool dryRun);
+void deleteGenerations(const Path & profile, const std::set<GenerationNumber> & gensToDelete, bool dryRun);
 
-void deleteGenerationsGreaterThan(const Path & profile, const int max, bool dryRun);
+void deleteGenerationsGreaterThan(const Path & profile, GenerationNumber max, bool dryRun);
 
 void deleteOldGenerations(const Path & profile, bool dryRun);
 

src/nix-env/nix-env.cc

@@ -1208,18 +1208,17 @@ static void opSwitchProfile(Globals & globals, Strings opFlags, Strings opArgs)
 }
 
 
-static const int prevGen = -2;
+static constexpr GenerationNumber prevGen = std::numeric_limits<GenerationNumber>::max();
 
 
-static void switchGeneration(Globals & globals, int dstGen)
+static void switchGeneration(Globals & globals, GenerationNumber dstGen)
 {
     PathLocks lock;
     lockProfile(lock, globals.profile);
 
-    int curGen;
+    auto [gens, curGen] = findGenerations(globals.profile);
-    Generations gens = findGenerations(globals.profile, curGen);
 
-    Generation dst;
+    std::optional<Generation> dst;
     for (auto & i : gens)
         if ((dstGen == prevGen && i.number < curGen) ||
             (dstGen >= 0 && i.number == dstGen))
@@ -1227,18 +1226,16 @@ static void switchGeneration(Globals & globals, int dstGen)
 
     if (!dst) {
         if (dstGen == prevGen)
-            throw Error("no generation older than the current (%1%) exists",
+            throw Error("no generation older than the current (%1%) exists", curGen.value_or(0));
-                curGen);
         else
             throw Error("generation %1% does not exist", dstGen);
     }
 
-    printInfo(format("switching from generation %1% to %2%")
+    printInfo("switching from generation %1% to %2%", curGen.value_or(0), dst->number);
-        % curGen % dst.number);
 
     if (globals.dryRun) return;
 
-    switchLink(globals.profile, dst.path);
+    switchLink(globals.profile, dst->path);
 }
 
 
@@ -1249,7 +1246,7 @@ static void opSwitchGeneration(Globals & globals, Strings opFlags, Strings opArg
     if (opArgs.size() != 1)
         throw UsageError("exactly one argument expected");
 
-    int dstGen;
+    GenerationNumber dstGen;
     if (!string2Int(opArgs.front(), dstGen))
         throw UsageError("expected a generation number");
 
@@ -1278,8 +1275,7 @@ static void opListGenerations(Globals & globals, Strings opFlags, Strings opArgs
     PathLocks lock;
     lockProfile(lock, globals.profile);
 
-    int curGen;
+    auto [gens, curGen] = findGenerations(globals.profile);
-    Generations gens = findGenerations(globals.profile, curGen);
 
     RunPager pager;
 
@@ -1308,14 +1304,14 @@ static void opDeleteGenerations(Globals & globals, Strings opFlags, Strings opAr
         if(opArgs.front().size() < 2)
             throw Error("invalid number of generations ‘%1%’", opArgs.front());
         string str_max = string(opArgs.front(), 1, opArgs.front().size());
-        int max;
+        GenerationNumber max;
         if (!string2Int(str_max, max) || max == 0)
             throw Error("invalid number of generations to keep ‘%1%’", opArgs.front());
         deleteGenerationsGreaterThan(globals.profile, max, globals.dryRun);
     } else {
-        std::set<unsigned int> gens;
+        std::set<GenerationNumber> gens;
         for (auto & i : opArgs) {
-            unsigned int n;
+            GenerationNumber n;
             if (!string2Int(i, n))
                 throw UsageError("invalid generation number '%1%'", i);
             gens.insert(n);