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

--derivation considered harmful, remove it in lieu of ^ and new stuff #7261

Open
2 of 5 tasks
Ericson2314 opened this issue Nov 4, 2022 · 19 comments
Open
2 of 5 tasks
Labels
feature Feature request or proposal idea approved The given proposal has been discussed and approved by the Nix team. An implementation is welcome. new-cli Relating to the "nix" command UX The way in which users interact with Nix. Higher level than UI.

Comments

@Ericson2314
Copy link
Member

Ericson2314 commented Nov 4, 2022

Reasons it is bad

No single meaning

--derivation is a shared flag between all commands, but only some commands care about it at all:

src/nix/diff-closures.cc:        auto beforePath = Installable::toStorePath(getEvalStore(), store, Realise::Outputs, operateOn, before);
src/nix/diff-closures.cc:        auto afterPath = Installable::toStorePath(getEvalStore(), store, Realise::Outputs, operateOn, after);
src/nix/why-depends.cc:        auto packagePath = Installable::toStorePath(getEvalStore(), store, Realise::Outputs, operateOn, package);
src/nix/why-depends.cc:        auto dependencyPath = Installable::toStorePath(getEvalStore(), store, Realise::Derivation, operateOn, dependency);

Other commands force one way or the other:

 $ git grep OperateOn src/nix
src/nix/develop.cc:                getEvalStore(), store, Realise::Nothing, OperateOn::Output, {installable});
src/nix/develop.cc:            for (auto & path : Installable::toStorePaths(getEvalStore(), store, Realise::Outputs, OperateOn::Output, {bashInstallable})) {
src/nix/run.cc:        auto outPaths = Installable::toStorePaths(getEvalStore(), store, Realise::Outputs, OperateOn::Output, installables);

Still other commands force something like --derivation by hand, like nix log:

nix/src/nix/log.cc

Lines 45 to 52 in 499e99d

auto log = std::visit(overloaded {
[&](const DerivedPath::Opaque & bo) {
return logSub.getBuildLog(bo.path);
},
[&](const DerivedPath::Built & bfd) {
return logSub.getBuildLog(bfd.drvPath);
},
}, b.raw());

This makes for unpredictable behavior, which is a frustrating user experience.

Doesn't scale to CA derivations

In the CA derivation world, we cannot assume a path has one unique builder. This is true both for unstable floating CA derivations, and stable fixed CA derivations (fixed output derivations). --derivation fundamentally is meaningless, or non-determinstic (picking one of many builders) in an arbitrary way. This is no good.

nix build myFixedOutput0 # produces /nix/store/asdfasdfdasf-foo
nix build myFixedOutput1 # Also produces /nix/store/asdfasdfdasf-foo
nix log /nix/store/asdfasdfdasf-foo # does what?!

Doesn't scale to RFC 92

Currently, if one passes a BuiltPath derived path, --derivation (and nix log) always exacts the drvPath part. But with RFC 92 the output path could also be a derivation! This makes --derivation confusing for users.

It also leads to a loss of expressive power. Imagine if one wants --derivation for one argument but not another, e.g. for why-depends asking why a computed derivation depends on another derivation:

nix why-depends makeDrvs.anOutputWhichIsADrv gcc # oops cannot get gcc.drvPath which is what I want

What we should do instead

See also

@Ericson2314 Ericson2314 added the feature Feature request or proposal label Nov 4, 2022
@Ericson2314 Ericson2314 changed the title --derivation considered harmful, remove it in lieu of ^ and new commands --derivation considered harmful, remove it in lieu of ^ and new stuff Nov 4, 2022
@Ericson2314
Copy link
Member Author

Ericson2314 commented Nov 4, 2022

BTW if the nix log UX feels fishy, #3813 is an suggestion by me (abandoned after @edolstra thought it was too much work) that we should have a notion of build attempts which logs are keyed on instead. The idea is that one can have a variety of successful or failing build attempts of the same derivation and we can remember all their logs, rather than replacing the derivation-keyed log every time their is a new attempt. CA derivation trust map entries could also point to the (successful) attempt they came from.

In this case, instead of saying nix log should work on derivation or an output path, it needs to be given an attempt. One can look up attempts from derivations or (derivation, output) pairs. If the latter path is obtainable, we can restrict ourselves to just attempts that succeeded.

@fricklerhandwerk
Copy link
Contributor

Can you explain the difference between high-level and low-level installables?

@Ericson2314
Copy link
Member Author

Low level is store-only, so just store paths with or without ^. High level is everything else.

@Ericson2314
Copy link
Member Author

@Radvendii based on your nice fix in #7337, I thought you might be interested in this.

@Ericson2314
Copy link
Member Author

#7467 tracks some of the documentation work needed for this to have a good UX.

Ericson2314 added a commit to obsidiansystems/nix that referenced this issue Jan 14, 2023
See the change log for user-facing details.

Only two lines in the tests need changing, showing that the impact of
this is pretty light.

Progress towards NixOS#7261
Ericson2314 added a commit to obsidiansystems/nix that referenced this issue Jan 16, 2023
See the change log for user-facing details.

Only two lines in the tests need changing, showing that the impact of
this is pretty light.

Progress towards NixOS#7261

Co-authored-by: Travis A. Everett <travis.a.everett@gmail.com>
@Ericson2314
Copy link
Member Author

@7c6f434c you might take an interest in this.

Ericson2314 added a commit to obsidiansystems/nix that referenced this issue Jan 20, 2023
The release notes document the change in behavior, I don't include it
here so there is no risk to it getting out of sync.

> Motivation

>> Plumbing CLI should be simple

Store derivation installations are intended as "plumbing": very simple
utilities for advanced users and scripts, and not what regular users
interact with. (Similarly, regular Git users will use branch and tag
names not explicit hashes for most things.)

The plumbing CLI should prize simplicity over convenience; that is its
raison d'etre. If the user provides a path, we should treat it the same
way not caring what sort of path it is.

>> Scripting

This is especially important for the scripting use-case. when arbitrary
paths are sent to e.g. `nix copy` and the script author wants consistent
behavior regardless of what those store paths are. Otherwise the script
author needs to be careful to filter out `.drv` ones, and then run `nix
copy` again with those paths and `--derivation`. That is not good!

>> Surprisingly low impact

Only two lines in the tests need changing, showing that the impact of
this is pretty light.

Many command, like `nix log` will continue to work with just the
derivation passed as before. This because we used to:

- Special case the drv path and replace it with it's outputs (what this
  gets rid of).

- Turn those output path *back* into the original drv path.

Now we just skip that entire round trip!

> Context

Issue NixOS#7261 lays out a broader vision for getting rid of `--derivation`,
and has this as one of its dependencies. But we can do this with or
without that.

`Installable::toDerivations` is changed to handle the case of a
`DerivedPath::Opaque` ending in `.drv`, which is new: it simply doesn't
need to do any extra work in that case. On this basis, commands like
`nix {show-derivation,log} /nix/store/...-foo.drv` still work as before,
as described above.

Co-authored-by: Travis A. Everett <travis.a.everett@gmail.com>
Ericson2314 added a commit to obsidiansystems/nix that referenced this issue Feb 5, 2023
Per the old FIXME, this flag was on too many commands, and mostly
ignored. Now it is just on the commands where it actually has an effect.

Per NixOS#7261, I would still like to get
rid of it entirely, but that is a separate project. This change should
be good with or without doing that.
@thufschmitt thufschmitt added the idea approved The given proposal has been discussed and approved by the Nix team. An implementation is welcome. label Feb 27, 2023
@thufschmitt thufschmitt removed this from Nix team Feb 27, 2023
@fricklerhandwerk
Copy link
Contributor

Discussed in the Nix team meeting 2022-02-27:

  • consensus on the end goal and the proposed roadmap
  • idea approved

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/2023-02-27-nix-team-meeting-minutes-36/25890/1

@edolstra
Copy link
Member

I'm not convinced removing --derivation is a good idea. In general, commands should operate on output paths by default, since .drv files are an implementation detail that might even disappear in the future. For instance, nix path-info nixpkgs#hello should show info about the outputs of hello, since that's almost always what the user cares about. I shouldn't have to use some obscure syntax like nix path-info nixpkgs#hello^* to get the outputs. Instead, it's better in the rare case that I want to get info about the .drv, to ask for that explicitly using --derivation.

@Ericson2314
Copy link
Member Author

Ericson2314 commented Feb 28, 2023

@edolstra This issue does not disagree that

nix path-info nixpkgs#hello

should continue to work.

What it says is that one should do

nix path-info nixpkgs#hello.drvPath

to get information about the derivation file itself.

@fricklerhandwerk
Copy link
Contributor

.drv files are an implementation detail that might even disappear in the future.

And so are store paths, to some extent. But that's the reference type the Nix store happens to use, so we have to expose it at the nix store level. Maybe nix path-info should actually be nix store path-info? Why would the whole package management use case (for which I assume the top level nix command is the entry point) even care about paths except as an implementation detail of how environments are set up?

Ericson2314 added a commit to obsidiansystems/nix that referenced this issue Feb 28, 2023
The release notes document the change in behavior, I don't include it
here so there is no risk to it getting out of sync.

> Motivation

>> Plumbing CLI should be simple

Store derivation installations are intended as "plumbing": very simple
utilities for advanced users and scripts, and not what regular users
interact with. (Similarly, regular Git users will use branch and tag
names not explicit hashes for most things.)

The plumbing CLI should prize simplicity over convenience; that is its
raison d'etre. If the user provides a path, we should treat it the same
way not caring what sort of path it is.

>> Scripting

This is especially important for the scripting use-case. when arbitrary
paths are sent to e.g. `nix copy` and the script author wants consistent
behavior regardless of what those store paths are. Otherwise the script
author needs to be careful to filter out `.drv` ones, and then run `nix
copy` again with those paths and `--derivation`. That is not good!

>> Surprisingly low impact

Only two lines in the tests need changing, showing that the impact of
this is pretty light.

Many command, like `nix log` will continue to work with just the
derivation passed as before. This because we used to:

- Special case the drv path and replace it with it's outputs (what this
  gets rid of).

- Turn those output path *back* into the original drv path.

Now we just skip that entire round trip!

> Context

Issue NixOS#7261 lays out a broader vision for getting rid of `--derivation`,
and has this as one of its dependencies. But we can do this with or
without that.

`Installable::toDerivations` is changed to handle the case of a
`DerivedPath::Opaque` ending in `.drv`, which is new: it simply doesn't
need to do any extra work in that case. On this basis, commands like
`nix {show-derivation,log} /nix/store/...-foo.drv` still work as before,
as described above.

When testing older daemons, the post-build-hook will be run against the
old CLI, so we need the old version of the post-build-hook to support
that use-case.

Co-authored-by: Travis A. Everett <travis.a.everett@gmail.com>
Ericson2314 added a commit to obsidiansystems/nix that referenced this issue Feb 28, 2023
The release notes document the change in behavior, I don't include it
here so there is no risk to it getting out of sync.

> Motivation

>> Plumbing CLI should be simple

Store derivation installations are intended as "plumbing": very simple
utilities for advanced users and scripts, and not what regular users
interact with. (Similarly, regular Git users will use branch and tag
names not explicit hashes for most things.)

The plumbing CLI should prize simplicity over convenience; that is its
raison d'etre. If the user provides a path, we should treat it the same
way not caring what sort of path it is.

>> Scripting

This is especially important for the scripting use-case. when arbitrary
paths are sent to e.g. `nix copy` and the script author wants consistent
behavior regardless of what those store paths are. Otherwise the script
author needs to be careful to filter out `.drv` ones, and then run `nix
copy` again with those paths and `--derivation`. That is not good!

>> Surprisingly low impact

Only two lines in the tests need changing, showing that the impact of
this is pretty light.

Many command, like `nix log` will continue to work with just the
derivation passed as before. This because we used to:

- Special case the drv path and replace it with it's outputs (what this
  gets rid of).

- Turn those output path *back* into the original drv path.

Now we just skip that entire round trip!

> Context

Issue NixOS#7261 lays out a broader vision for getting rid of `--derivation`,
and has this as one of its dependencies. But we can do this with or
without that.

`Installable::toDerivations` is changed to handle the case of a
`DerivedPath::Opaque` ending in `.drv`, which is new: it simply doesn't
need to do any extra work in that case. On this basis, commands like
`nix {show-derivation,log} /nix/store/...-foo.drv` still work as before,
as described above.

When testing older daemons, the post-build-hook will be run against the
old CLI, so we need the old version of the post-build-hook to support
that use-case.

Co-authored-by: Travis A. Everett <travis.a.everett@gmail.com>
Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>
Ericson2314 added a commit to obsidiansystems/nix that referenced this issue Feb 28, 2023
The release notes document the change in behavior, I don't include it
here so there is no risk to it getting out of sync.

> Motivation

>> Plumbing CLI should be simple

Store derivation installations are intended as "plumbing": very simple
utilities for advanced users and scripts, and not what regular users
interact with. (Similarly, regular Git users will use branch and tag
names not explicit hashes for most things.)

The plumbing CLI should prize simplicity over convenience; that is its
raison d'etre. If the user provides a path, we should treat it the same
way not caring what sort of path it is.

>> Scripting

This is especially important for the scripting use-case. when arbitrary
paths are sent to e.g. `nix copy` and the script author wants consistent
behavior regardless of what those store paths are. Otherwise the script
author needs to be careful to filter out `.drv` ones, and then run `nix
copy` again with those paths and `--derivation`. That is not good!

>> Surprisingly low impact

Only two lines in the tests need changing, showing that the impact of
this is pretty light.

Many command, like `nix log` will continue to work with just the
derivation passed as before. This because we used to:

- Special case the drv path and replace it with it's outputs (what this
  gets rid of).

- Turn those output path *back* into the original drv path.

Now we just skip that entire round trip!

> Context

Issue NixOS#7261 lays out a broader vision for getting rid of `--derivation`,
and has this as one of its dependencies. But we can do this with or
without that.

`Installable::toDerivations` is changed to handle the case of a
`DerivedPath::Opaque` ending in `.drv`, which is new: it simply doesn't
need to do any extra work in that case. On this basis, commands like
`nix {show-derivation,log} /nix/store/...-foo.drv` still work as before,
as described above.

When testing older daemons, the post-build-hook will be run against the
old CLI, so we need the old version of the post-build-hook to support
that use-case.

Co-authored-by: Travis A. Everett <travis.a.everett@gmail.com>
Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>
Ericson2314 added a commit to obsidiansystems/nix that referenced this issue Feb 28, 2023
The release notes document the change in behavior, I don't include it
here so there is no risk to it getting out of sync.

> Motivation

>> Plumbing CLI should be simple

Store derivation installations are intended as "plumbing": very simple
utilities for advanced users and scripts, and not what regular users
interact with. (Similarly, regular Git users will use branch and tag
names not explicit hashes for most things.)

The plumbing CLI should prize simplicity over convenience; that is its
raison d'etre. If the user provides a path, we should treat it the same
way not caring what sort of path it is.

>> Scripting

This is especially important for the scripting use-case. when arbitrary
paths are sent to e.g. `nix copy` and the script author wants consistent
behavior regardless of what those store paths are. Otherwise the script
author needs to be careful to filter out `.drv` ones, and then run `nix
copy` again with those paths and `--derivation`. That is not good!

>> Surprisingly low impact

Only two lines in the tests need changing, showing that the impact of
this is pretty light.

Many command, like `nix log` will continue to work with just the
derivation passed as before. This because we used to:

- Special case the drv path and replace it with it's outputs (what this
  gets rid of).

- Turn those output path *back* into the original drv path.

Now we just skip that entire round trip!

> Context

Issue NixOS#7261 lays out a broader vision for getting rid of `--derivation`,
and has this as one of its dependencies. But we can do this with or
without that.

`Installable::toDerivations` is changed to handle the case of a
`DerivedPath::Opaque` ending in `.drv`, which is new: it simply doesn't
need to do any extra work in that case. On this basis, commands like
`nix {show-derivation,log} /nix/store/...-foo.drv` still work as before,
as described above.

When testing older daemons, the post-build-hook will be run against the
old CLI, so we need the old version of the post-build-hook to support
that use-case.

Co-authored-by: Travis A. Everett <travis.a.everett@gmail.com>
Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>
@maralorn
Copy link
Member

I just want to contribute, that this issue bit me today. I used nix copy /nix/store/...-foo.drv in a script of mine, which subsequently led to failures because that derivation was not actually copied to the host. Having the different paths /nix/store/...-foo.drv and /nix/store/...-foo do the same thing for nix copy but then using the flag --derivation to reintroduce their difference does not feel like intuitive UX to me.

@Ericson2314
Copy link
Member Author

@maralorn Happy to report that since #7600 was merged, that part is already done!

@roberth
Copy link
Member

roberth commented Mar 15, 2023

--derivation also makes it non-trivial to support why-depends chains that end in a source. It seems to require an otherwise unnecessary corner case.

Ericson2314 added a commit to obsidiansystems/nix that referenced this issue Jun 27, 2023
Since PR NixOS#7601, we've had enough flexibility in our types of
installables that it is no longer needed.

Progress towards NixOS#7261
Ericson2314 added a commit to obsidiansystems/nix that referenced this issue Jun 27, 2023
Since PR NixOS#7601, we've had enough flexibility in our types of
installables that it is no longer needed.

Progress towards NixOS#7261
Ericson2314 added a commit to obsidiansystems/nix that referenced this issue Jun 27, 2023
Since PR NixOS#7601, we've had enough flexibility in our types of
installables that it is no longer needed.

Progress towards NixOS#7261
Ericson2314 added a commit to obsidiansystems/nix that referenced this issue Jul 31, 2023
Since PR NixOS#7601, we've had enough flexibility in our types of
installables that it is no longer needed.

Progress towards NixOS#7261
Ericson2314 added a commit to obsidiansystems/nix that referenced this issue Oct 17, 2023
Since PR NixOS#7601, we've had enough flexibility in our types of
installables that it is no longer needed.

Progress towards NixOS#7261
Ericson2314 added a commit to obsidiansystems/nix that referenced this issue Oct 23, 2023
Since PR NixOS#7601, we've had enough flexibility in our types of
installables that it is no longer needed.

Progress towards NixOS#7261
Ericson2314 added a commit to obsidiansystems/nix that referenced this issue Oct 23, 2023
Since PR NixOS#7601, we've had enough flexibility in our types of
installables that it is no longer needed.

Progress towards NixOS#7261
@Ericson2314
Copy link
Member Author

This was discussed in today's Nix team meeting some more

  • Agreement we can do without new syntax at his time

  • Release notes must be good to cover cases

  • Backport making .drvPath work

  • --derivation can have error message with migration instructions

  • @edolstra: .drvPath and .inputDerivation seem ad hoc and imposes complexity on nixpkgs, and it's not as discoverable

  • @roberth: users don't need to learn --derivation

  • @Ericson2314: --expr and --file are very different from --derivation

  • @roberth: installable "modes" like --expr or --derivation can not be combined on the command line

  • @tomberek: we can address the discoverability concerns in the manual (referenced in the deprecation message) and see how people respond to the change

  • @Ericson2314: make sure we judge the feedback keeping in mind that the cost of having an unnecessary feature is a subtle long term cost

  • is this a regression considering (Redefine outputs in terms of language-level "package", not necessarily store-level derivation (RFC-92, and multi-drv packages, docs) #6507)? No.

    • @roberth: maybe for scriptability if expressions decide to put the drvPath in other places and the goal is to retrieve the drv of an output. In this case I'd prefer a syntax like nixpkgs#hello^.., because that can be mixed with non-^.. installables. However, this seems not applicable (yet?), because all packages have a drvPath today.
  • @Ericson2314: maybe we should have a separation such as between nix-instantiate and nix-build

  • @edolstra: nix-instantiate is a nice orthogonal design, but not user friendly

  • @Ericson2314: derivations are a plumbing-side thing

  • @edolstra: nix why-depends needs something like --derivation

  • @edolstra: plumbing commands are not a priority for new-cli work

  • @Ericson2314: keep the porcelain simple for new users, which is easier if we have good plumbing commands to support the advanced use cases

There is still some disagreement, but we agree on some next steps:

Steps

  1. Make .drvPath work

    • back port it to 2.19
  2. Document .drvPath as part of explaining --derivation

  3. Add support for ^..

    • ^.. / ^* can both be thought of as bonus/extra deriving path syntax.

Agreement on 1-3. Defer 4 until later.

  1. Some sort of warning on --derivation indicating (a) we're debating it (b) issue to look at (c) some other ways to do the same thing, etc.

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/2024-01-08-nix-team-meeting-minutes-114/38156/1

@roberth
Copy link
Member

roberth commented Jan 12, 2024

^* selects multiple paths, unlike any of the other syntax.
You could still construct it to be algebraic by throwing everything into a list, but then we'd lose the connection with the expression language, which does not, and must not do that.

It is therefore not part of the deriving path language, but a custom, CLI+scheduler-only language of deriving path sets. (sets: I don't think they're currently treated as lists)

@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/pre-rfc-implement-dependency-retrieval-primitive/43418/5

@nixos-discourse
Copy link

This issue 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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Feature request or proposal idea approved The given proposal has been discussed and approved by the Nix team. An implementation is welcome. new-cli Relating to the "nix" command UX The way in which users interact with Nix. Higher level than UI.
Projects
None yet
Development

No branches or pull requests

7 participants