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

Document <drvPath>^<outputName> syntax for installables #9362

Closed
wants to merge 5 commits into from
Closed

Document <drvPath>^<outputName> syntax for installables #9362

wants to merge 5 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Nov 16, 2023

The ^ syntax for installables was added in these three PRs:

... but was not added to the manual. This commit documents the syntax for this fifth type of installable.

My earlier complaints about vagueness in #4543 are solved if /nix/store/xyz.drv^outname counts as an installable, since this syntax gives us a way to use installable-accepting commands (like nix copy) on a derivation without using the command on its outputs (or vice versa).

At the time of that discussion the ^ syntax had not yet reached the then-stable version of Nix, and I didn't find out about it until a few months ago. I think it is awesome. The --derivation flag always seemed like an ugly hack.

Anyways, I assume that the intent is that /nix/store/xyz.drv^outname counts as an installable. If so, this is my attempt at documenting that fact.

@github-actions github-actions bot added documentation new-cli Relating to the "nix" command labels Nov 16, 2023
@ghost ghost marked this pull request as ready for review November 16, 2023 08:31
@ghost ghost requested a review from fricklerhandwerk as a code owner November 16, 2023 08:31
The `^` syntax for installables was added in
404c222 and
e2a4e7a but was not added to the
manual.  This commit documents the syntax for this fifth type of
installable.
Copy link
Contributor

@fricklerhandwerk fricklerhandwerk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

I'm not sure about the installables part, @Ericson2314 will know better.

src/nix/nix.md Outdated Show resolved Hide resolved
doc/manual/src/glossary.md Outdated Show resolved Hide resolved
doc/manual/src/glossary.md Outdated Show resolved Hide resolved
doc/manual/src/glossary.md Outdated Show resolved Hide resolved
- Either:
- one of the store derivation's [output names](#gloss-output-name) or
- the character `*`

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
A plain store derivation path refers to the first output declared for that derivation.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hrm, @fricklerhandwerk did you mean to delete those lines in your suggestion, or did you intend to only add the new line?

@Ericson2314
Copy link
Member

Ericson2314 commented Nov 16, 2023

We should document "Deriving Paths" first. That is the underlying concept. The fact that those never got documented means people just see ad-hoc syntax and don't know how to think about it. That is the first problem.

This is the same thing I think should block #8595 too.

@Ericson2314
Copy link
Member

But yes you are right @amjoseph-nixpkgs, Store paths are a type of Deriving Path ("constant" deriving paths @roberth came up with the name), which in turn are type of installable.

We also support ^ on the other installables but I think I want to get rid of that. Maybe also get rid of ^ supporting multiple names when we can just use shell {a,b,c} too. (and that works for .output too).

@fricklerhandwerk
Copy link
Contributor

@Ericson2314 none of that should be blocking here. IMO this can be fixed up and merged.

@Ericson2314
Copy link
Member

@fricklerhandwerk I think it is blocking because it is quite annoying to get text that is inline and delete it later. Much better to have the small pages/outline so people know what they should document / what they should refer to to the get go.

https://github.com/NixOS/nix/compare/document-derivation-and-deriving-path I pushed some text there taken from my other branch. I also got rid of "store derivation" per @roberth's #6507. I think if we fix that stuff up it will take a little bit but not too long, and then we'll have a much better foundation for these other things.

It's really important to me that read don't waste neurons building independent mental models for:

  1. low-level "installables"
  2. derivation inputs
  3. string context elements

because they missed the fact that these are all essentially the same thing.

Adam Joseph and others added 4 commits November 17, 2023 04:17
Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>
Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>
Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>
Co-authored-by: Valentin Gagarin <valentin.gagarin@tweag.io>
@ghost
Copy link
Author

ghost commented Nov 17, 2023

We should document "Deriving Paths" first.

Okay but if that is not imminent we should at least merge a one-line change that adds the fifth bullet point to the definition of installable. Even if it's a placeholder. Otherwise there's a huge gap between the documented behavior and the actual behavior -- for nearly every command.

https://github.com/NixOS/nix/compare/document-derivation-and-deriving-path

Is there a PR to discuss this?

👍 I like the addition of the qualifier expression to derivation to make it clear when you're talking about something in the Nix language rather than something on the disk.

I pushed some text there taken from my other branch. I also got rid of "store derivation" per @roberth's #6507.

👎 I would leave it. Then we have two clearly-defined terms:

  • expression derivation
  • store derivation

Anybody who neglects to include one of the two qualifiers is asking the reader to infer the qualifier themselves... which is how we talk on github and irc but not appropriate for polished documentation.

Regarding #6507, that PR is correct that:

Nix doesn't really have a notion of "package".

... and I don't think that's going to change in the immediate future, or without some raging debates occurring. I think we should let sleeping dogs lie here and not try to define "package" in the Nix manual. At least for now.

We also support ^ on the other installables but I think I want to get rid of that.

Yeah I was going to mention that. It's one reason I picked "store derivation output reference"... it means we can define the parallel concept "flake attribute output reference" for the thing you get when you put ^outputName at the end of a flake attribute.

Maybe also get rid of ^ supporting multiple names when we can just use shell {a,b,c} too. (and that works for .output too).

... but not for ^* unfortunately.

@Ericson2314
Copy link
Member

@amjoseph-nixpkgs So on second thought in the Nix Team meeting I think I am scope creeping too much so It is better to just begin with your PR.

Can you add the glossary entry for deriving paths from #8595 and then reference that? That will do for now.

Comment on lines +62 to 63
- [Store derivation output reference](@docroot@/glossary.md#gloss-store-derivation-output-reference)
- [Store path](#store-path)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then both these become deriving path

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And down below the "Store path" description becomes "Deriving Path"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also see the "Derivation output selection" section down below.

> Reference to the `man` output of a given `bash-5.2-p15` derivation:
>
> `/nix/store/llil5bng8p7203l25mqb5lwdhlaya4d9-bash-5.2-p15.drv^man`

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
See also: The store path section of the [installables syntax](@docroot@/command-ref/new-cli/nix.html#store-path), where output references are used.

It'd be good to link or explain what this syntax is a part of and where it's used.

@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-11-17-nix-team-meeting-minutes-104/35753/1

@ghost ghost closed this by deleting the head repository Jan 23, 2024
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation new-cli Relating to the "nix" command
Projects
Status: 🏁 Review
Development

Successfully merging this pull request may close these issues.

4 participants