Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

nix store gc: Stabilize #7608

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

roberth
Copy link
Member

@roberth roberth commented Jan 16, 2023

TODO

  • Fix --dry-run (no output, but maybe my nix daemon is too old)
  • Remove --print-build-logs
  • Remove --offline and --refresh
  • Remove --accept-flake-config(?)

Depends on: #7851

@Ericson2314
Copy link
Member

Remove --accept-flake-config(?)

Builtins have an experimental features field. We should probably do the same for settings so we can skip them in --help etc.

@Ericson2314
Copy link
Member

Ericson2314 commented Jan 16, 2023

I am looking into that. edit #7609 solves it.

@Ericson2314 Ericson2314 mentioned this pull request Jan 17, 2023
7 tasks
@Ericson2314
Copy link
Member

#7610 makes the other 3 flags (--print-build-logs, --offline, --refresh) go away without the nix-comand experimental feature --- not a good permanent solution, but fine for now I think!

@Ericson2314
Copy link
Member

#7611 This shrinks down the (already stable, whoops!) nix --help to reveal only enabled features.

@edolstra
Copy link
Member

I'm not in favor of stabilizing subcommands one by one, as we discussed yesterday at length. Rather I'd prefer to stabilize the entire nix CLI except some subcommands. And actually nix store gc is one of the subcommands that probably shouldn't be stabilized yet because it isn't really finished yet.

@roberth
Copy link
Member Author

roberth commented Jan 17, 2023

According to the meeting notes:

consensus: we want to stabilise the new CLI incrementally, independently of developing flakes

It seems that we didn't know that we didn't agree on what incremental means then.

Rather I'd prefer to stabilize the entire nix CLI except some subcommands.

I think the benefit of doing them one by one is that we can pay attention to their behavior and make sure that we don't stabilize something that we later find out we'd want to break. Maybe the concern is that we're still learning while we do these final reviews? I'm open to other ideas.

@roberth
Copy link
Member Author

roberth commented Jan 17, 2023

it isn't really finished yet.

Apart from --dry-run producing no output, it seemed pretty complete to me.
I guess the nix profile commands aren't quite adequate yet? nix profile wipe-history seems to operate on a default rather than all. Maybe --all should be a flag there?

@Ericson2314
Copy link
Member

@edolstra what exactly are you trying to solve with that? I recall you being worried 1-by-1 would take too long, but I think we are making pretty quick work of it.

@fricklerhandwerk
Copy link
Contributor

Discussed in the Nix team meeting 2023-01-27:

  • @edolstra: Doesn't agree with the idea of incrementally de-xp the cli
    • We should do it in batch to avoid the inconsistent state in the middle
  • We need a way to track what we feel is ready
    • Could have a nix-command-v0 experimental feature
    • Or just a checkbox in an issue
      • Could also raise a community effort towards that
  • Opened CLI stabilization effort #7701 to track that

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2023-01-27-nix-team-meeting-minutes-27/25434/1

@fricklerhandwerk
Copy link
Contributor

Discussed in the Nix team meeting 2023-01-27:

  • observations:
    • design considerations around global options (such as --print-build-logs) are issues orthogonal to the individual commands
  • decisions:
    • we will draft a formal announcement of our plan for stabilisation, publish it, and ask for feedback and reviews
    • the currently envisioned stabilisation process is sketched as follows:
      1. formally stabilise the "hierarchical command line structure" first, without any commands in it
      2. stabilise the global options and settings relevant to currently experimental commands
        - those apply to multiple commands and effectively can't be changed after stabilisation
      3. stabilise separate commands once all requirements (to be determined in more detail) are met
    • as a working assumption, do not consider part of the stable interface of the command in question
      • collect issues on global options and settings as we inspect the new commands, and deal with all of them separately before the first command itself is declared stable
    • nix store gc should only operate on garbage collection roots
      • we will consider a nix gc command that would also clean up profile generations
    • the configuraton options min-free and max-free (subject to renaming) will be used to determine limits on the garbage collection run, instead of a dedicated flag
      • there will be a new, separate option to enable automatic garbage collection during builds
      • update command documentation accordingly
    • there will be no output on stdout
Complete discussion
  • @edolstra shares the screen, we look at nix store gc --help

    • is there anything in the interface that would cause problems in the future?
      • @Ericson2314: there are a few things mentioned in the issue:
        • Fix --dry-run (no output, but maybe my nix daemon is too old)
        • Remove --print-build-logs
          • it does not have anything to with this command
          • @edolstra: this is a global option
          • the purpose of stabilisation is to ask the purpose of each thing we stabilise
            • if we stabilise one command that way, we'd stabilise all the global options, too. we can't go through all of them right now
              • @fricklerhandwerk: agreed
              • @edolstra: really don't want to go through the super slow, incremental process
                • don't want to go through this again, the functionality has been around for years, and no one complained
                • @fricklerhandwerk: one part of that is, as I read it, that a significant portion of the audience of heavy Nix users gave up on the development process and stopped engaging with development of unstable features
                • @Ericson2314: in addition there is a risk of breakage. while things were unstable we haven't had the practice to keep them stable.
                • @edolstra: don't agree
        • Remove --offline and --refresh
        • Remove --accept-flake-config
  • @tomberek: there seems to be the orthogonal questions how we deal with global questions. can we focus on one specific thing to stabilise?

    • we could say that the command gets stabilised with the caveat that global options are not decidedly part of that interface
      • @edolstra: the --<setting> flags are already part of the old CLI
        • fear that the discussion will be picked apart and grind to a halt, again
        • @Ericson2314: propose to go incrementally to gain momentum on uncontroversial and conceptually related aspects
        • @edolstra: then we should rather first stabilise the global properties first, such as settings and options
          • same for the installables syntax, because it affects most other commands
  • agreement that we will formally stabilise the "hierarchical command line structure" first, without any commands in it, then follow up with individual commands

  • @fricklerhandwerk: no one wants to rehash the discussion around. do we have a repository of the current state of discussion, so we can see the open questions and other potential for controversy? ideally something in the style @infinisil recently did for his design documents and Nixpkgs Architecture Team RFC

  • @roberth: the goal was to sketch a checklist for stabilising the other commands. going through the archives sounds like a lot of work. (if we did, we should collect information grouped by command.) we could instead just take any aspect and handle it on its own to the best of our knowledge.

  • @tomberek: another approach: go through each command and derive global issues to handle separately. what are the blockers for nix store gc?

    • @edolstra: it's an open question if nix store gc without arguments should also GC old profiles, and it arguably should by default to make it a convenient command
      • @Ericson2314: to sidestep this debate, nix store commands should work exclusively at the store layer and only deal with GC roots. Nix profile generations are at a different architectural layer. they are just more GC roots from the perspective of the store layer.
        • @edolstra: yes. later ideally we'd have a nix gc command to clean up everything
        • agreement
    • any other blockers? let's compare to nix-store --gc
      • --max-freed vs --max
        • @Ericson2314: what's the behavior today if you speficy 1B and your smallest GC root has 1GB?
          • @tomberek: it will delete that object and stop
          • @Ericson2314: arguably it should be --min instead then!
          • @roberth: what most users will want to do is ensure that a certain amount of space is available on the store
            • @Ericson2314: such an idempotent behavior is highly desirable. should keep this flag unstable until we determine it possible to make it idempotent
              • no disagreement
              • @edolstra: maybe instead the command should run auto-GC and obey min-free and max-free
                • that would enforce the policy you specify instead of blindly deleting everything
                • @fricklerhandwerk: that would be nice because they're overridable by options
                • @roberth: should highlight these at the top of the man page
                  • also should rename the settings to be more self-explanatory
                • @fricklerhandwerk: there should be a separate flag to enable auto-GC then
      • old one has multiple dry-run options
        • @edolstra: should be separate verbs e.g. nix store show-roots
        • @tomberek: is their inclusion necessary for stability?
          • @edolstra: no, only a blocker for deprecating the old command
            • @fricklerhandwerk: is the goal to ultimately wrap the old commands around the old ones?
              • @edolstra: they already mostly use the same underlying implementation
      • @edolstra: should define what the output of the command is
        • there should be no output on stdout
          • agreement
        • @fricklerhandwerk: side issue: progress output third party tools can plug into
          • @edolstra: it's not part of the specified interface, it's purely informational
            • on the other hand, if one can specify JSON output, one can expect certain types messages to be in there
        • @roberth: --log-format is global and therefore an orthogonal concern
        • Example:
        • $ nix --extra-experimental-features nix-command store gc --dry-run --verbose --log-format internal-json
          @nix {"action":"msg","level":0,"msg":"finding garbage collector roots..."}
          @nix {"action":"msg","level":0,"msg":"determining live/dead paths..."}
          
    • @roberth: --print-build-logs should be removed
      • @edolstra: it should instead be a setting in nix.conf
      • @fricklerhandwerk: why is it global to begin with? it only concerns commands that run builders
        • @edolstra: many things do builds where you may not expect it, such as evaluation that does IFD
    • @fricklerhandwerk: we could mark the global options experimental in the

    ation, as we agreed to not to stabilise them yet
    - @edolstra: do not want code churn, switching on and off separate options all the time
    - also, if it's only in the manual, people won't consider it experimental

  • agreement that setting the GC bounds through configuraton options (i.e. removing the --max flag) and enabling automatic GC in a separate setting is sufficient to stabilise nix store gc

    • this does not touch global options, which will still be considered unstable
  • @fricklerhandwerk: what about stabilising global options? other options than the granular approach proposed by @Ericson2314

    • @edolstra: go through all commands and sort it out step by step, then stabilise the whole thing
      • @fricklerhandwerk: there seem to be design issues still open. if I were to document e.g. --print-build-logs I'd need a clear understanding how it works, and the way it's currently described to interact with different commands does not sound like its design is obviously right
    • @Ericson2314: we could just try the incremental approach and see how it works
      • @edolstra: fear we may spend too much time on this. should instead go through subcommands and other aspects of the user interface
      • @Ericson2314: fear that way we'd dump a deluge of decisions on the community that users would have to deal with wholesale. that would miss the opportunity of getting feedback as we go
      • @edolstra: the risk with stabilising something is that once it's out it's too late
        • @fricklerhandwerk: that's exactly @Ericon2314's argument. the larger the chunk, the more things for which it could be too late
  • (some discussion how to communicate the envisioned changes and the process around them)

  • @edolstra: for me it's important to get feedback in the review process

    • @fricklerhandwerk: we should take great care to stay in a dialogue with users and post updates on our work timely and regularly

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2023-02-10-nix-team-meeting-minutes-32/25442/1

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/2023-04-14-nix-team-meeting-minutes-48/27358/1

@thufschmitt thufschmitt added this to the CLI Stabilisation milestone Sep 4, 2023
@lucc
Copy link
Contributor

lucc commented Dec 7, 2023

As noted in #4429 (comment) I would vote for some option or subcomand of nix store gc to provide the information of nix-store --gc --print-roots. To my knowledge this old style nix command can not be reproduced with the new nix cli.

@fricklerhandwerk
Copy link
Contributor

@lucc yes, many existing things can't be done with the new CLI yet. We decided not to make that a blocker to stabilisation, as it can be added later. Feel free to open an issue (please check for existing ones first!) or open a PR directly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants