Add 'store add-gc-root' command to manually add indirect roots

[layus]

Motivation

This is a PoC fix for #7138. Now that I implemented it, I do not think it is the most elegant way to do it, as there is no way to ensure that the paths are not deleted during this operation. The command could fail however if not all the paths were successfully rooted.

Context

#7138

Priorities and Process

Add 👍 to pull requests you find important.

The Nix maintainer team uses a GitHub project board to schedule and track reviews.

[Mic92]

Intuitively I would have expected such as subcommand here:

Suggested change

	$ nix add-root foo
	$ nix store add-gc-root foo

[layus]

I wanted to add it in nix gc add-root, but nix gc seems mostly unused and intended to start a gc.

[roberth]

We currently don't have a top-level nix gc subcommand group, but a nix store gc command.

To help keep some consistency between commands, the subcommands are ideally nix <noun> <verb> with the exception of a small number of frequently used commands.
gc is a verb, since the "garbage" part is subordinate to the "collect" part (as a noun adjunct to be technical).
Since our commands end in verbs, this would create the expectation that nix gc is not a subcommand group, but a single action.

I think nix store add-root would be ok, but perhaps nix store add-gc-root would be better, as I think the hint about gc would be welcomed when reading scripts that call this command.
Interactive use should not be common, because that would usually come after nix build, which already produces a root.

[layus]

Who makes final decisions about that ?

[Mic92]

Nix team members, I would say. Often happens in those regular meetings.

[Mic92]

So we discussed this during the meeting:

Our question: Does the nix build command work for you instead of this nix store add-gc-root?

It also works with store paths, but also with installables:

$ nix build --out-link /tmp/result /nix/store/yb84nwgvixzi9sx9nxssq581pc0cc8p3-hello-2.12.1
$ ls -la /tmp/result
lrwxrwxrwx joerg users 56 B now /tmp/result@ ⇒ /nix/store/yb84nwgvixzi9sx9nxssq581pc0cc8p3-hello-2.12.1

The advantage of adding gcroot in general as part of the build is that it race-free.

[layus]

Changed name, added tests.

[roberth]

Triaged in Nix meeting 181:

We think that by improving the documentation we can solve the problem of the discoverability of the --out-link flag.

@layus, are you aware of a use case that is not already covered by nix build?

[nixos-discourse]

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

https://discourse.nixos.org/t/2024-09-25-nix-team-meeting-minutes-181-180/52712/1

[layus]

@roberth I should have been more explicit, but the whole point of #7138 is to add gc roots for .drv files themselves. There is currently no way to do that at all. Of course #11506 would be a cleaner solution, but I think this one can still be useful in many a case. If only because it is very fast and efficient.

[Mic92]

Ah, you are right. nix-build will actually tries to build them:

~/git/nixpkgs main ⇣⇡
% nix build --out-link /tmp/foo '/nix/store/af3rc6phyv80h7aq4y3d08awnq2ja8fp-hello-2.12.1.drv^*'

~/git/nixpkgs main ⇣⇡
% ls -la /tmp/foo
lrwxrwxrwx joerg users 56 B now  /tmp/foo@ ⇒ /nix/store/yb84nwgvixzi9sx9nxssq581pc0cc8p3-hello-2.12.1

[roberth]

nix build --out-link /tmp/foo '/nix/store/af3rc6phyv80h7aq4y3d08awnq2ja8fp-hello-2.12.1.drv^*'

It does work when you remove ^*. That was kind of the point of adding ^*, to make this kind of thing explicit.

$ nix build --out-link /tmp/foo '/nix/store/af3rc6phyv80h7aq4y3d08awnq2ja8fp-hello-2.12.1.drv'
warning: The interpretation of store paths arguments ending in `.drv` recently changed. If this command is now failing try again with '/nix/store/af3rc6phyv80h7aq4y3d08awnq2ja8fp-hello-2.12.1.drv^*'

$ readlink /tmp/foo
/nix/store/af3rc6phyv80h7aq4y3d08awnq2ja8fp-hello-2.12.1.drv

So while this works, in practice this still has problems though:

• It still produces the warning about the breaking change that added the ^* requirement for building outputs. I think we can remove this, as we've been warning for at least a year now.
• nix build on a .drv does not realise its outputs - is still a bit counterintuitive. I don't think I (as a NIx maintainer!) would have the awareness of that when reading nix build $drv in a script.

And then we have a more design related issue:
nix build, as implemented, almost "declares" the existence of something, considering the way it treats its arguments, and --out-link is the action. Considering the CLI guidelines, this suggests that the roles should be switched, e.g.

• e.g. nix link nixpkgs#hello --path /tmp/foo
  • build is actually quite irrelevant
  • link is the action - the verb
  • --path is a proper flag, because it is about how. The default could be result, as it is historically (though I thoroughly dislike its output name-based behavior, off-topic)
    • make it explicit? --path out=/tmp/foo --path dev=/tmp/headers

So in conclusion, we could make nix build work slightly better by removing the warning, but the UX isn't solved by that change alone.

[layus]

Okay, it seems I have been living in the past at work, being stuck at 2.14, but not on my own machine which brought some confusion.

@roberth, @Mic92 Passing a .drv path on the command line is not the only way to call nix build. I think nix build was initially designed to receive flake+attr path. And there is still no way to directly add a gc-root to the derivation of a flake output. Summarizing, there is no difference between nix build nixpkgs#xorg.xclock^out and nix build nixpkgs#xorg.xclock.

[layus@uberwald:~]$ nix build nixpkgs#xorg.xclock --print-out-paths
/nix/store/sxxw8d9iicf8rpc4hckpckb5vhpyvbwn-xclock-1.1.1

[layus@uberwald:~]$ nix build nixpkgs#xorg.xclock^out --print-out-paths
/nix/store/sxxw8d9iicf8rpc4hckpckb5vhpyvbwn-xclock-1.1.1

I did not take part in the discussions about the new cli, but my understanding was always that the verb was about what we want to build. build was for obtaining a package, and instantiate to obtain a store derivation.

For consistency, I think nix build --out-link /tmp/foo '/nix/store/af3rc6phyv80h7aq4y3d08awnq2ja8fp-hello-2.12.1.drv' should build hello and add a root to the result (yes, despite the new meaning of .drv) because that is what build does. take something that describes one or more build output and build them.

That was kind of the point of adding ^*, to make this kind of thing explicit.

I thought the point was to avoid having a --derivation flag where the distinction was needed. But nix build never needed that distinction.

For nix path-info it was needed. But again, while it works when passing a .drv, it does not when passing a flake output.

[layus@uberwald:~]$ nix path-info nixpkgs#xorg.xclock^out
/nix/store/sxxw8d9iicf8rpc4hckpckb5vhpyvbwn-xclock-1.1.1

[layus@uberwald:~]$ nix path-info nixpkgs#xorg.xclock
/nix/store/sxxw8d9iicf8rpc4hckpckb5vhpyvbwn-xclock-1.1.1

[layus@uberwald:~]$ nix-store -q --deriver /nix/store/sxxw8d9iicf8rpc4hckpckb5vhpyvbwn-xclock-1.1.1
/nix/store/82w6jak6c7zldgvxyq5nwhclz3yp85zp-xclock-1.1.1.drv

[layus@uberwald:~]$ nix path-info /nix/store/82w6jak6c7zldgvxyq5nwhclz3yp85zp-xclock-1.1.1.drv
warning: The interpretation of store paths arguments ending in `.drv` recently changed. If this command is now failing try again with '/nix/store/82w6jak6c7zldgvxyq5nwhclz3yp85zp-xclock-1.1.1.drv^*'
/nix/store/82w6jak6c7zldgvxyq5nwhclz3yp85zp-xclock-1.1.1.drv

[layus@uberwald:~]$ nix path-info /nix/store/82w6jak6c7zldgvxyq5nwhclz3yp85zp-xclock-1.1.1.drv^out
/nix/store/sxxw8d9iicf8rpc4hckpckb5vhpyvbwn-xclock-1.1.1

Now I am really confused.

Not to mention

[layus@uberwald:~]$ nix path-info nixpkgs#xorg.xclock.drvPath
error:
       … while evaluating the flake output attribute 'legacyPackages.x86_64-linux.xorg.xclock.drvPath'

       error: string '/nix/store/82w6jak6c7zldgvxyq5nwhclz3yp85zp-xclock-1.1.1.drv' has a context which refers to a complete source and binary closure. This is not supported at this time

[layus@uberwald:~]$ nix build nixpkgs#xorg.xclock.drvPath
error:
       … while evaluating the flake output attribute 'legacyPackages.x86_64-linux.xorg.xclock.drvPath'

       error: string '/nix/store/82w6jak6c7zldgvxyq5nwhclz3yp85zp-xclock-1.1.1.drv' has a context which refers to a complete source and binary closure. This is not supported at this time

[roberth]

The nix build UX has bothered me and I've thought about this some more during the past days, and I think nix build (as implemented) should have been nix create (ignoring the expectation that a nix build command exists, which is simultaneously true).

Zooming out from the CLI a bit, instantiation and realisation are both creation operations, and which one(s) it turns out to be is controlled by the structure of the installable as a DerivingPath or set of DerivingPaths. (cc @Ericson2314 are we still intent to rename to DerivingPath?)

flowchart BT
  realise["realise"]
  instantiate["instantiate"]
  
  build --> realise
  substitute --> realise
  realise  --> create
  instantiate --> create

Loading

nix generate or materialize may be suitable synonyms for create if we prefer those instead.

[roberth]

I don't like to further disturb such a fundamental thing as nix build, but what if we got the fundamental thing wrong?

@layus I'm glad you got this conversation started from a different angle, but I can imagine you didn't come here for this kind of conversation, or maybe you do? We could move this elsewhere if you prefer.

[layus]

@roberth I like deep discussions, and I think the cli deserves a good design, but I came here to address #7138 (which you opened apparently :-) ).

Right now I am actually using the hack I describe in #7138 (comment), which is not safe and pretty ugly.

I need a way to eval a flake output, and get a gc-root on the .drv file in return. This is a basic feature given how the cli warns about missing roots for all the produced paths, but not for .drv.
I understand that the new CLI is opinionated, and would like to get rid of .drv's ultimately. But right now they are a core part of the design at work, and I see no good alternative in the future. The idea of storing .drvs as json data outside the store is nice, but has some way to go before becoming production ready.

Right now, I want a cli entry point to get a rooted .drv for a given flake output (The whole point of #11506). This one is just a cherry on top, as I was working on the CLI anyway, and it felt simple enough to implement, fixing the ticket you opened.

[nixos-discourse]

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

https://discourse.nixos.org/t/2024-09-30-nix-team-meeting-minutes-182/53814/1

[kjeremy]

What I want out of this functionality is to be able to create a root on a remote store using --store

[layus]

This PR does not enable that. The biggest question is where you want to create the remote indirect root. Because rn the command takes a symlink to a store path. Remote roots need a proper design and a robust implementation. Le 2 octobre 2024 22:38:44 GMT+02:00, Jeremy Kolb ***@***.***> a écrit :

…

What I want out of this functionality is to be able to create a root on a remote store using `--store` -- Reply to this email directly or view it on GitHub: #11505 (comment) You are receiving this because you were mentioned. Message ID: ***@***.***>