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

Use DerivedPath for buildPaths and ensurePath #4594

Merged
merged 10 commits into from
Apr 5, 2021

Conversation

Ericson2314
Copy link
Member

@Ericson2314 Ericson2314 commented Mar 2, 2021

his avoids an ambiguity where the StorePathWithOutputs { drvPath, {} } could mean "build brvPath" or "substitute drvPath" depending on context.

It also brings the internals closer in line to the new CLI, by generalizing the Buildable type is used there and makes that distinction already.

In doing so, relegate StorePathWithOutputs to being a type just for backwards compatibility (CLI and RPC).

  • Better names BuildGoal, BuildGoalWithOptOutputName? I went with DerivedPath
  • No templating, maybe we can just get rid of the old one anyways.
  • Comments describing both, including we probably don't want the old one

@Ericson2314 Ericson2314 marked this pull request as ready for review March 2, 2021 21:20
@Ericson2314 Ericson2314 changed the title WIP: Use BuildableReq for buildPaths and ensurePath Use BuildableReq for buildPaths and ensurePath Mar 2, 2021
In the following commits it will become less prevalent.
These are by no means part of the notion of a store, but rather are
things that happen to use stores. (Or put another way, there's no way
we'd make them virtual methods any time soon.) It's better to move them
out of that too-big class then.

Also, this helps us remove StorePathWithOutputs from the Store interface
altogether next commit.
This avoids an ambiguity where the `StorePathWithOutputs { drvPath, {}
}` could mean "build `brvPath`" or "substitute `drvPath`" depending on
context.

It also brings the internals closer in line to the new CLI, by
generalizing the `Buildable` type is used there and makes that
distinction already.

In doing so, relegate `StorePathWithOutputs` to being a type just for
backwards compatibility (CLI and RPC).
This makes for better types errors and allows us to give it methods.
This allows us to namespace its constructors under it.
@Ericson2314 Ericson2314 changed the title Use BuildableReq for buildPaths and ensurePath Use DerivedPath for buildPaths and ensurePath Apr 5, 2021
@edolstra edolstra merged commit 4bf3eb2 into NixOS:master Apr 5, 2021
@Ericson2314 Ericson2314 deleted the lots-of-buildable branch April 6, 2021 05:17
matthewbauer added a commit to matthewbauer/nix that referenced this pull request May 5, 2021
This was previously done in NixOS#4515 but
got clobbered away in NixOS#4594.

--------------------------------------------------------------------------------

This fixes an issue where derivations with a primary output that is
not "out" would fail with:

$ nix profile install nixpkgs#sqlite
error: opening directory '/nix/store/2a2ydlgyydly5czcc8lg12n6qqkfz863-sqlite-3.34.1-bin': No such file or directory

This happens because while derivations produce every output when
built, you might not have them if you didn't build the derivation
yourself (for instance, the store path was fetch from a binary cache).
This uses outputName provided from DerivationInfo which appears to
match the first output of the derivation.
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.

3 participants