From 6e0ca0242579dc6114ccfc2417406372acc3004c Mon Sep 17 00:00:00 2001
From: Quantum Jump <quantumjump451@gmail.com>
Date: Fri, 5 Jul 2024 20:22:53 +0100
Subject: [PATCH] Fix dry-run flag for nix-collect-garbage

`nix-collect-garbage --dry-run` previously elided the entire garbage
collection check, meaning that it would just exit the script without
printing anything.

This change makes the dry run flag instead set the GC action to
`gcReturnDead` rather than `gcDeleteDead`, and then continue with the
script. So if you set `--dry-run`, it will print the paths it *would*
have garbage collected, but not actually delete them.

I filed a bug for this: https://git.lix.systems/lix-project/lix/issues/432 but then realised I could give fixing it a go myself.

Change-Id: I062dbf1a80bbab192b5fd0b3a453a0b555ad16f2
---
 doc/manual/change-authors.yml                 |  4 ++
 doc/manual/rl-next/fix-gc-dry-run.md          | 29 ++++++++++++++
 .../nix-collect-garbage.cc                    | 19 ++++++---
 tests/functional/meson.build                  |  1 +
 .../functional/nix-collect-garbage-dry-run.sh | 40 +++++++++++++++++++
 5 files changed, 88 insertions(+), 5 deletions(-)
 create mode 100644 doc/manual/rl-next/fix-gc-dry-run.md
 create mode 100644 tests/functional/nix-collect-garbage-dry-run.sh

diff --git a/doc/manual/change-authors.yml b/doc/manual/change-authors.yml
index 28a20b25f..f56a1e6fb 100644
--- a/doc/manual/change-authors.yml
+++ b/doc/manual/change-authors.yml
@@ -96,6 +96,10 @@ puck:
   forgejo: puck
   github: puckipedia
 
+quantumjump:
+  display_name: Quantum Jump
+  github: QuantumBJump
+
 r-vdp:
   github: r-vdp
 
diff --git a/doc/manual/rl-next/fix-gc-dry-run.md b/doc/manual/rl-next/fix-gc-dry-run.md
new file mode 100644
index 000000000..6095d7727
--- /dev/null
+++ b/doc/manual/rl-next/fix-gc-dry-run.md
@@ -0,0 +1,29 @@
+---
+synopsis: Fix nix-collect-garbage --dry-run
+issues: [fj#432]
+cls: [1566]
+category: Fixes
+credits: [quantumjump]
+---
+
+`nix-collect-garbage --dry-run` did not previously give any output - it simply
+exited without even checking to see what paths would be deleted.
+
+```
+$ nix-collect-garbage --dry-run
+$
+```
+
+We updated the behaviour of the flag such that instead it prints out how many
+paths it *would* delete, but doesn't actually delete them.
+
+```
+$ nix-collect-garbage --dry-run
+finding garbage collector roots...
+determining live/dead paths...
+...
+<nix store paths>
+...
+2670 store paths deleted, 0.00MiB freed
+$
+```
diff --git a/src/nix-collect-garbage/nix-collect-garbage.cc b/src/nix-collect-garbage/nix-collect-garbage.cc
index 54de68232..c831f132f 100644
--- a/src/nix-collect-garbage/nix-collect-garbage.cc
+++ b/src/nix-collect-garbage/nix-collect-garbage.cc
@@ -89,12 +89,21 @@ static int main_nix_collect_garbage(int argc, char * * argv)
 
         // Run the actual garbage collector.
         if (!dryRun) {
-            auto store = openStore();
-            auto & gcStore = require<GcStore>(*store);
             options.action = GCOptions::gcDeleteDead;
-            GCResults results;
-            PrintFreed freed(true, results);
-            gcStore.collectGarbage(options, results);
+        } else {
+            options.action = GCOptions::gcReturnDead;
+        }
+        auto store = openStore();
+        auto & gcStore = require<GcStore>(*store);
+        GCResults results;
+        PrintFreed freed(true, results);
+        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", i);
+            }
         }
 
         return 0;
diff --git a/tests/functional/meson.build b/tests/functional/meson.build
index eede1834c..cbf6a1563 100644
--- a/tests/functional/meson.build
+++ b/tests/functional/meson.build
@@ -73,6 +73,7 @@ functional_tests_scripts = [
   'flakes/flake-in-submodule.sh',
   'gc.sh',
   'nix-collect-garbage-d.sh',
+  'nix-collect-garbage-dry-run.sh',
   'remote-store.sh',
   'legacy-ssh-store.sh',
   'lang.sh',
diff --git a/tests/functional/nix-collect-garbage-dry-run.sh b/tests/functional/nix-collect-garbage-dry-run.sh
new file mode 100644
index 000000000..00e6f4885
--- /dev/null
+++ b/tests/functional/nix-collect-garbage-dry-run.sh
@@ -0,0 +1,40 @@
+source common.sh
+
+clearStore
+
+## Test `nix-collect-garbage --dry-run`
+
+
+testCollectGarbageDryRun () {
+    clearProfiles
+    # Install then uninstall a package
+    # This should leave packages ready to be garbage collected.
+    nix-env -f ./user-envs.nix -i foo-1.0
+    nix-env -f ./user-envs.nix -e foo-1.0
+
+
+    nix-env --delete-generations old
+    [[ $(nix-store --gc --print-dead | wc -l) -eq 7 ]]
+
+    nix-collect-garbage --dry-run
+    [[ $(nix-store --gc --print-dead | wc -l) -eq 7 ]]
+
+}
+
+testCollectGarbageDryRun
+
+# Run the same test, but forcing the profiles an arbitrary location.
+rm ~/.nix-profile
+ln -s $TEST_ROOT/blah ~/.nix-profile
+testCollectGarbageDryRun
+
+# Run the same test, but forcing the profiles at their legacy location under
+# /nix/var/nix.
+#
+# Note that we *don't* use the default profile; `nix-collect-garbage` will
+# need to check the legacy conditional unconditionally not just follow
+# `~/.nix-profile` to pass this test.
+#
+# Regression test for #8294
+rm ~/.nix-profile
+testCollectGarbageDryRun --profile "$NIX_STATE_DIR/profiles/per-user/me"