gc: Unify UX across the 3 cli ways to collect garbage

There are currently 4 different ways to run garbage collection using
lix:
- `nix-collect-garbage`
- `nix-store --gc`
- `nix store gc`
- (using the daemon directly)

As they were written all at different times, their output varies (and is
broken in some case). This unifies the display of informations in the
following ways:

- The list of paths in the results is always printed (in the nix3 cli it
is hidden unless `-v` is passed)
- The number of paths in the result set is always displayed
- The size of deleted paths is only shown when actually deleting things
(as it would have been 0B in any case)

Fixes #905

Change-Id: I40d9ec7c6d76795f6c6dd30df196d1e855bdb9db

lix/legacy/nix-collect-garbage.cc

@@ -64,7 +64,7 @@ static int main_nix_collect_garbage(AsyncIoRoot & aio, std::string programName,
     {
         bool removeOld = false;
 
-        GCOptions options;
+        GCOptions options = {.action = GCOptions::gcDeleteDead};
 
         LegacyArgs(aio, programName, [&](Strings::iterator & arg, const Strings::iterator & end) {
             if (*arg == "--help")
@@ -75,12 +75,13 @@ static int main_nix_collect_garbage(AsyncIoRoot & aio, std::string programName,
             else if (*arg == "--delete-older-than") {
                 removeOld = true;
                 deleteOlderThan = getArg(*arg, arg, end);
-            }
-            else if (*arg == "--dry-run") dryRun = true;
-            else if (*arg == "--max-freed")
+            } else if (*arg == "--dry-run") {
+                options.action = GCOptions::gcReturnDead;
+            } else if (*arg == "--max-freed") {
                 options.maxFreed = std::max(getIntArg<int64_t>(*arg, arg, end, true), (int64_t) 0);
-            else
+            } else {
                 return false;
+            }
             return true;
         }).parseCmdline(argv);
 
@@ -92,24 +93,12 @@ static int main_nix_collect_garbage(AsyncIoRoot & aio, std::string programName,
         }
 
         // Run the actual garbage collector.
-        if (!dryRun) {
-            options.action = GCOptions::gcDeleteDead;
-        } else {
-            options.action = GCOptions::gcReturnDead;
-        }
         auto store = aio.blockOn(openStore());
         auto & gcStore = require<GcStore>(*store);
         GCResults results;
-        PrintFreed freed(true, results);
+        PrintFreed freed(options.action, results);
         aio.blockOn(gcStore.collectGarbage(options, results));
 
-        if (dryRun) {
-            // Only print results for dry run; when !dryRun, paths will be printed as they're deleted.
-            for (auto & i : results.paths) {
-                printInfo("%s", Uncolored(i));
-            }
-        }
-
         return 0;
     }
 }

lix/legacy/nix-store.cc

@@ -711,12 +711,8 @@ static void opGC(std::shared_ptr<Store> store, AsyncIoRoot & aio, Strings opFlag
     }
 
     else {
-        PrintFreed freed(options.action == GCOptions::gcDeleteDead, results);
+        PrintFreed freed(options.action, results);
         aio.blockOn(gcStore.collectGarbage(options, results));
-
-        if (options.action != GCOptions::gcDeleteDead)
-            for (auto & i : results.paths)
-                cout << i << std::endl;
     }
 }
 
@@ -749,7 +745,7 @@ opDelete(std::shared_ptr<Store> store, AsyncIoRoot & aio, Strings opFlags, Strin
     auto & gcStore = require<GcStore>(*store);
 
     GCResults results;
-    PrintFreed freed(true, results);
+    PrintFreed freed(options.action, results);
     aio.blockOn(gcStore.collectGarbage(options, results));
 }
 

lix/libmain/shared.cc

@@ -4,6 +4,7 @@
 #include "lix/libstore/store-api.hh"
 #include "lix/libstore/gc-store.hh"
 #include "lix/libutil/c-calls.hh"
+#include "lix/libutil/logging.hh"
 #include "lix/libutil/result.hh"
 #include "lix/libutil/signals.hh"
 #include "lix/libmain/loggers.hh"
@@ -403,13 +404,34 @@ RunPager::~RunPager()
     }
 }
 
-
 PrintFreed::~PrintFreed()
 {
-    if (show)
-        std::cout << fmt("%d store paths deleted, %s freed\n",
-            results.paths.size(),
-            showBytes(results.bytesFreed));
-}
+    // When in dry-run mode, print the paths on stdout
+    if (action == GCOptions::gcReturnLive || action == GCOptions::gcReturnDead) {
+        for (auto & i : results.paths) {
+            logger->cout("%s", i);
+        };
+    }
 
+    switch (action) {
+    case GCOptions::gcReturnLive: {
+        notice("%1% store paths would be kept\n", results.paths.size());
+        break;
+    }
+    case GCOptions::gcReturnDead: {
+        notice("%1% store paths would be deleted\n", results.paths.size());
+        break;
+    }
+    case GCOptions::gcDeleteDead:
+    case GCOptions::gcDeleteSpecific:
+    case GCOptions::gcTryDeleteSpecific: {
+        notice(
+            "%1% store paths deleted, %2% freed\n",
+            results.paths.size(),
+            showBytes(results.bytesFreed)
+        );
+        break;
+    }
+    }
+}
 }

lix/libmain/shared.hh

@@ -1,6 +1,7 @@
 #pragma once
 ///@file
 
+#include "lix/libstore/gc-store.hh"
 #include "lix/libutil/args.hh"
 #include "lix/libutil/args/root.hh"
 #include "lix/libmain/common-args.hh"
@@ -99,10 +100,14 @@ struct GCResults;
 
 struct PrintFreed
 {
-    bool show;
+    GCOptions::GCAction action;
+
     const GCResults & results;
-    PrintFreed(bool show, const GCResults & results)
-        : show(show), results(results) { }
+    PrintFreed(GCOptions::GCAction action, const GCResults & results)
+        : action(action)
+        , results(results)
+    {
+    }
     ~PrintFreed();
 };
 

lix/nix/store-delete.cc

@@ -58,7 +58,7 @@ struct CmdStoreDelete : StorePathsCommand
         }
 
         GCResults results;
-        PrintFreed freed(true, results);
+        PrintFreed freed(options.action, results);
         aio().blockOn(gcStore.collectGarbage(options, results));
     }
 };

lix/nix/store-gc.cc

@@ -40,7 +40,7 @@ struct CmdStoreGC : StoreCommand, MixDryRun
 
         options.action = dryRun ? GCOptions::gcReturnDead : GCOptions::gcDeleteDead;
         GCResults results;
-        PrintFreed freed(options.action == GCOptions::gcDeleteDead, results);
+        PrintFreed freed(options.action, results);
         aio().blockOn(gcStore.collectGarbage(options, results));
     }
 };

tests/functional/gc.sh

@@ -69,7 +69,7 @@ nix-store --delete --skip-live $(readLink $outPath/reference-to-input-2)
 
 rm "$NIX_STATE_DIR"/gcroots/foo
 # with the dependent unrooted, we should be able to remove input2...
-nix-store --delete --delete-closure $input2 > delete-output
+nix-store --delete --delete-closure $input2 &> delete-output
 # which should remove input0, since only input2 and top depended on it and we passed --delete-closure
 ! test -e $input0
 # but fod should be unaffected, since it's not part of input-2's closure