From 170f092ce2429753ebbe54404c885b007cc60e50 Mon Sep 17 00:00:00 2001 From: Tom Hubrecht Date: Tue, 18 Nov 2025 23:37:28 +0100 Subject: [PATCH] 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 | 25 +++++++---------------- lix/legacy/nix-store.cc | 8 ++------ lix/libmain/shared.cc | 34 +++++++++++++++++++++++++------ lix/libmain/shared.hh | 11 +++++++--- lix/nix/store-delete.cc | 2 +- lix/nix/store-gc.cc | 2 +- tests/functional/gc.sh | 2 +- 7 files changed, 48 insertions(+), 36 deletions(-) diff --git a/lix/legacy/nix-collect-garbage.cc b/lix/legacy/nix-collect-garbage.cc index 5dc45df09..638fe3093 100644 --- a/lix/legacy/nix-collect-garbage.cc +++ b/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(*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(*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; } } diff --git a/lix/legacy/nix-store.cc b/lix/legacy/nix-store.cc index 57545c3ae..56f68ccb1 100644 --- a/lix/legacy/nix-store.cc +++ b/lix/legacy/nix-store.cc @@ -711,12 +711,8 @@ static void opGC(std::shared_ptr 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, AsyncIoRoot & aio, Strings opFlags, Strin auto & gcStore = require(*store); GCResults results; - PrintFreed freed(true, results); + PrintFreed freed(options.action, results); aio.blockOn(gcStore.collectGarbage(options, results)); } diff --git a/lix/libmain/shared.cc b/lix/libmain/shared.cc index a6918a5f2..28629bc3a 100644 --- a/lix/libmain/shared.cc +++ b/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; + } + } +} } diff --git a/lix/libmain/shared.hh b/lix/libmain/shared.hh index bb6a485ea..55a94b0df 100644 --- a/lix/libmain/shared.hh +++ b/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(); }; diff --git a/lix/nix/store-delete.cc b/lix/nix/store-delete.cc index d2020f071..23151218c 100644 --- a/lix/nix/store-delete.cc +++ b/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)); } }; diff --git a/lix/nix/store-gc.cc b/lix/nix/store-gc.cc index b66bc1628..cd6fade4f 100644 --- a/lix/nix/store-gc.cc +++ b/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)); } }; diff --git a/tests/functional/gc.sh b/tests/functional/gc.sh index 6df237eaa..062bd48be 100644 --- a/tests/functional/gc.sh +++ b/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