Skip to content

nix store gc --dry-run: print paths#5705

Closed
dtzWill wants to merge 1 commit into
NixOS:masterfrom
dtzWill:fix/issue-5704
Closed

nix store gc --dry-run: print paths#5705
dtzWill wants to merge 1 commit into
NixOS:masterfrom
dtzWill:fix/issue-5704

Conversation

@dtzWill
Copy link
Copy Markdown
Member

@dtzWill dtzWill commented Dec 1, 2021

Fixes #5704 .

Functionality is basic, and indeed code was copied from how it's already done here for non-experimental version:

if (options.action != GCOptions::gcDeleteDead)


Would add a test, but there appears no tests for nix store gc in-tree so wasn't sure where to put it (and adding new test/setup seems beyond scope here). If there's a good place let me know 😄.

I'm seeing many ways to print things, so mostly just did what we already do for this but maybe that's not right for new code-- LMK and happy to fix 👍.

(seems sometimes with fmt("%s\n", x) and mix of newlines using << "\n" and << std::endl. Each with trade-offs re:allocations, race against other prints (possibly separating path from newline, maybe), and flushing behavior, I think?)

Anyway short fix for issue I filed earlier, apologies if this is in wrong direction haven't been following development very closely but wanted to chip in 😇.

Thanks!

@edolstra
Copy link
Copy Markdown
Member

edolstra commented Dec 1, 2021

IMHO writing (potentially) thousands of paths to stdout that the user didn't ask for is not really good UX. It's also not obvious whether this should print the dead or alive paths. I think it would be better to add a --json flag, and then the JSON output could include both dead and alive paths.

For the non-JSON case, what it should print is something like "X paths (YYY.ZZ MiB) would be deleted".

@edolstra edolstra added the new-cli Relating to the "nix" command label Dec 1, 2021
@dtzWill
Copy link
Copy Markdown
Member Author

dtzWill commented Dec 2, 2021

Okay. Unfortunately the similarly trivial implementation doesn't quite work as GCResults's bytesFreed doesn't update the count in this case (although comment about the field declaration suggests it does, FWIW). Haven't looked into how hard plumbing that through the relevant code would be, unsure at a glance 😄. I'm briefly concerned some stuff (like link cleanup?) might be more of a "do it and count as you go" sort of situation (delete until count is 1, or whatever) but I don't know.

I'm interested but unsure when I can commit, apologies.


Quick test/sample of this here: dtzWill@46f4002, not pushing because it doesn't work (as described above) and can't mark this as draft myself apparently (boo, PR authors should be able to surely? anyway). And maybe poking at PrintFreed to handle the "would be" language, but perhaps not. Presumably it's the way it is now so that exceptions during GC still print what was freed or something?

Anyway, this seems more than I can tackle immediately but maybe in a bit I can poke at the relevant code 👍.

Thanks for taking a look and no worries glad for such concerns as what is good UX. Especially for the new cli! 😁.

A --json option seems neat, and straightforward enough to add. Sounds good to me 👍.

@stale
Copy link
Copy Markdown

stale Bot commented Jun 12, 2022

I marked this as stale due to inactivity. → More info

@stale stale Bot added the stale label Jun 12, 2022
@Ericson2314 Ericson2314 marked this pull request as draft June 23, 2023 00:09
@Ericson2314
Copy link
Copy Markdown
Member

Draft because reviewer and author agreed on other approach.

@tomberek
Copy link
Copy Markdown
Contributor

stale: re-open and re-base as needed

@tomberek tomberek closed this May 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

new-cli Relating to the "nix" command

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"nix store gc --dry-run" doesn't print anything: fix or remove?

4 participants