-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Introduce nix derivation instantiate
#11506
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks overall like a clean subset of nix-instantiate
.
However some integration test would be nice and should be easy to add (with and without JSON).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think instantiate
should not be placed under the derivation
namespace. As I see it, derivation
commands should operate on derivations and should not have to know about expressions/flakes/installables. This command however operates on installables to produce a derivation.
See also #7868 and #7903, cc @Ericson2314
@fgaz All right, but where then ? |
The best place I can think of right now is |
Both look fine to me. Another option is
I think for now we can just wait for a nix maintainer to give their opinion. |
I think BTW, the reason that the new CLI doesn't have an equivalent for |
We're already doing a good job at this, but in this new scenario, we'll still need the functionality of writing |
Should it be named Should I add the feature to the old nix-instantiate command instead ? |
Hold on, shouldn't |
No, I think commands should work on as many installable types as possible. We definitely want people to be able to say |
Discussed in meeting:
|
@tomberek Thanks for making a decision. I updated the PR to reflect the naming choice. |
b8d1965
to
c7ad17d
Compare
Oh, just rebased, that was probably the second point. |
|
No. There was none. |
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 |
Also, derivations provide some natural workflows for build-dependency GC-pinning. |
return res; | ||
} | ||
|
||
// TODO deduplicate with other code also setting such out links. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be nice to deduplicate this, but that can also be done in a future PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See 7f6d006 which factors out createOutLinks()
.
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/2024-10-07-nix-team-meeting-minutes-184/54046/1 |
This is simmilar to `nix-instantiate` with support for installables (and flakes).
c7ad17d
to
3edc164
Compare
I have reworked output. It now uses a list instead of a set. Documentation and tests updated accordingly. (also rebased, because I thought that may fix running tests in meson, but no. I still need to run |
|
This clashes with RFC 92 a bit, because it is unclear what it should do in the case of derivations that are themselves the build outputs of derivations. We don’t want to do this now in light of those uncertainties, and would much prefer it if you could work on the plan outlined in #7261 We would also appreciate advice on how to make issues like this more prominant so contributors like you don’t end up spending a bunch of time implementing something that overlaps with an agreed-upon plan that’s just waiting to be implemented. Sorry for the confusing discussion response from us on this, and the delayed disapproval. |
[edit] I do not think this message was constructive or useful [/edit] |
In merely linking #7261 I also buried the lede: with the
should be the same thing as
The latter is, we freely admit, not so discoverable, and reveals once again how the
which are not expressible with this command. |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/2024-10-09-nix-team-meeting-minutes-185/54335/1 |
This is similar to
nix-instantiate
with support for installables (and flakes).Motivation
The ability to obtain a derivation with the new cli has never been really implemented. #3908 (comment) is the best work-around, but it does not add a gc root for the generated paths.
This brings a dedicated entry-point with the right semantics instead of the workaround above.
Context
Fixes #3908
To some extent, this also addresses #7138 because outputs can easily be rooted. This brings that capability to store derivations.
Priorities and Process
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.