gc: delay throwing error until cleanup is complete

Previously, paths not being deleted by gcDeleteSpecific would result in
(a) hardlinks not being cleaned up, and
(b) statistics not being reported correctly.

By throwing the error later, we fix both of these problems.

Change-Id: I8019f3e10d9f22e81ea87bb26b77f04ebc888a19

doc/manual/rl-next/fix-gc-links.md

@@ -0,0 +1,13 @@
+---
+synopsis: "nix-store --delete: always remove obsolete hardlinks"
+issues: []
+cls: [3188]
+category: Fixes
+credits: [lheckemann]
+---
+
+Deleting specific paths using `nix-store --delete` or `nix store
+delete` previously did not delete hard links created by `nix-store
+--optimise` even if they became obsolete, unless _all_ of the given
+paths were deleted successfully. Now, hard links are always cleaned
+up, even if some of the given paths could not be deleted.

doc/manual/rl-next/fix-gc-stats.md

@@ -0,0 +1,24 @@
+---
+synopsis: "Report GC statistics correctly"
+issues: []
+cls: [3188]
+category: Fixes
+credits: [lheckemann]
+---
+
+Deleting specific paths using `nix-store --delete` or `nix store delete` previously did
+not report statistics correctly when some of the paths could not be deleted, even if
+others were deleted:
+
+```
+$ nix store delete /nix/store/9bwryidal9q3g91cjm6xschfn4ikd82q-hello-2.12.1 --delete-closure -v
+finding garbage collector roots...
+deleting '/nix/store/9bwryidal9q3g91cjm6xschfn4ikd82q-hello-2.12.1'
+0 store paths deleted, 0.00 MiB freed
+error: Cannot delete some of the given paths because they are still alive. Paths not deleted:
+         k9bxzr1l92r5y6mihrkbpbr3fmc8qszx-libidn2-2.3.8
+         mbx9ii53lzjlrsnlrfmzpwm33ynljwdn-libunistring-1.3
+         rf8hcy6bldxdqc0g6q1dcka1vh47x69s-xgcc-14.2.1.20250322-libgcc
+         vbrdc5wgzn0w1zdp10xd2favkjn5fk7y-glibc-2.40-66
+       To find out why, use nix-store --query --roots and nix-store --query --referrers.
+```

lix/libstore/gc.cc

@@ -813,10 +813,10 @@ try {
         }
     };
 
+    PathSet kept;
     /* Either delete all garbage paths, or just the specified
        paths (for gcDeleteSpecific and gcTryDeleteSpecific). */
     if (deleteSpecific) {
-        PathSet kept;
         for (auto & i : options.pathsToDelete) {
             TRY_AWAIT(deleteReferrersClosure(i));
             if (!dead.count(i)) {
@@ -831,25 +831,6 @@ try {
                 printTalkative(path);
             }
         }
-        if (options.action == GCOptions::gcDeleteSpecific && !kept.empty()) {
-            std::ostringstream pathSummary;
-            for (auto const [n, path]: enumerate(kept)) {
-                pathSummary << "\n  " << path;
-                const int summaryThreshold = 10;
-                if (n >= summaryThreshold) {
-                    pathSummary << "\nand " << kept.size() - summaryThreshold << " others.";
-                    break;
-                }
-            }
-            throw Error(
-                "Cannot delete some of the given paths because they are still alive. "
-                "Paths not deleted:"
-                "%1%"
-                "\nTo find out why, use nix-store --query --roots and nix-store --query --referrers."
-                ,
-                pathSummary.str()
-            );
-        }
 
     } else if (options.maxFreed > 0) {
 
@@ -940,6 +921,25 @@ try {
         printInfo("note: currently hard linking saves %.2f MiB",
             ((unsharedSize - actualSize - overhead) / (1024.0 * 1024.0)));
     }
+    if (options.action == GCOptions::gcDeleteSpecific && !kept.empty()) {
+        std::ostringstream pathSummary;
+        for (auto const [n, path]: enumerate(kept)) {
+            pathSummary << "\n  " << path;
+            const int summaryThreshold = 10;
+            if (n >= summaryThreshold) {
+                pathSummary << "\nand " << kept.size() - summaryThreshold << " others.";
+                break;
+            }
+        }
+        throw Error(
+            "Cannot delete some of the given paths because they are still alive. "
+            "Paths not deleted:"
+            "%1%"
+            "\nTo find out why, use nix-store --query --roots and nix-store --query --referrers."
+            ,
+            pathSummary.str()
+        );
+    }
     co_return result::success();
 } catch (...) {
     co_return result::current_exception();

tests/functional/dependencies.nix

@@ -16,6 +16,8 @@ let
     name = "dependencies-input-2";
     buildCommand = ''
       mkdir $out
+      # Space-filler to test GC stats reporting
+      head -c 100k /dev/zero > $out/filler
       echo BAR > $out/bar
       echo ${input0} > $out/input0
     '';

tests/functional/gc.sh

@@ -69,11 +69,14 @@ 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
+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
 test -e $fodOut
+# and stats should be reported correctly
+grep "0.10 MiB freed$" delete-output
+test -z "$(grep "0 paths deleted" delete-output)"
 
 
 nix-collect-garbage