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
Allow the copy of ca derivation outputs #4174
Closed
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Still only implemented for (and used by) `LocalStore` atm
Copying a derivation output is equivalent to copy the output path, except that it will also link the output to the path (with `linkDeriverToPath` on the remote store)
Commands that accept store paths or installables as input can now take a derivation path with outputs. Like when the input is a nix expr, the derivation will be realised if needed.
Generalises `StorePathsCommand` to allow manipulating `Buildables` rather than just store paths.
Not really useful atm as there's no way to retrieve this information, but at least it's there
Build everything on the fly if `operateOn` is set to `Output`. Also fix the computation of the closure in case `operateOn` is set to `derivation`
Still needs a way to compute the closure of a derivation output
Rather than just storing the mapping `foo!out -> pathOf(foo!out)`, we also record the dependencies of `foo!out`. This comes with a set of needed changes - New `drv-output-info` module declaring the types needed for describing these dependencies - `linkDeriverToPath` is replaced by `registerDrvOutput` which has a slightly different signature - new `queryDrvOutputInfo` to retrieve the informations for a derivations This introcudes some redundancy on the remote-store side between `wopQueryDerivationOutputMap` and `wopQueryDrvOutputInfo`. However we might need to keep both (regardless of backwards compat) because we sometimes need to get some infos for all the outputs of a derivation (where `wopQueryDerivationOutputMap` is handy), but all the stores can't implement it − because listing all the outputs of a derivation isn't really possible for binary caches where the server doesn't allow to list a directory.
Because it's legitimate to call it on an un-realised drv output, in which case there's nothing to return
The previous version was assuming that only direct inputs of the drv could be direct dependencies of drv outputs, which is generally false (for example most c programs will depend on glibc while it won't be provided as a direct dependency but as a transitive one through stdenv)
Otherwise the drvOutput->outputPath link can't be done on the remote cache when using `nix copy` which breaks remote caching for CA derivations
…d path Fix NixOS#4138 and allows copying drv outputs without having to copy the whole drv closure
zimbatm
reviewed
Oct 24, 2020
for (auto& [outputName, status] : initialOutputs) { | ||
if (!status.wanted) | ||
continue; | ||
auto drvSubstitution = getEnv("USE_DRV_SUBSTITUTION"); |
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.
Is it possible to use a setting instead? The proliferation of environment variables that changes the behaviour of Nix can be problematic.
Oops, that was just some debugging code that wasn't supposed to be pushed 😇
There shouldn't be a need for such an option once it works properly because the new behavior fallbacks on the old one as needed
|
thufschmitt
force-pushed
the
copy-ca-derivations
branch
from
October 26, 2020 08:58
bb59326
to
edb6a15
Compare
So that we can add another implementation for drv outputs
thufschmitt
force-pushed
the
copy-ca-derivations
branch
from
October 27, 2020 06:25
edb6a15
to
b915a9a
Compare
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Big ugly PR adding support for moving CA derivation outputs around.
Not to be merged as-it-is, but can work as a POC.
It's obviously already possible to move the output paths around, but there's no way to copy the mapping
(drvPath, outputName) -> outputPath
which is essential for ca derivations. That's what this PR enables. More precisely, it(drvPath, outputName, outputPath)
along with some meta information (currently a set of dependencies, should probably be extended at least with a signature),drvPath!outputName
that uniquely (modulo non-determinism) identifies a derivation output,(this is very badly named because it's a general identifier for the elements of the store and not just the thing that's fed to derivations).
Store::registerDrvOutput(DrvOutputId&, DrvOutputInfo)
method which generalizesLocalStore::linkDeriverToPath
Store
class (so that every store can implement it rather than justLocalStore
)DrvOutputId->outPath
mapping, but also all the meta-information that might be neededcopyPaths(...)
and thenix copy
command to work on derivation outputsnix copy nixpkgs#hello
will now copy the derivation outputs ofnixpkgs#hello
rather than just the output paths.nix copy /nix/store/....-hello.drv!out
to manually specify a derivation outputnix copy /nix/store/....-hello
will just copy the output path like it used to doStill to do:
SubstitutionGoal
which I haven't done yetTo allow properly copying derivation outputs we need to compute their dependency closure (which is different from the closure of the output path).
Currently there's a (fragile and probably not exact) logic to recompute it on-the-fly for the local store, but we should rather store all the dependencies of a drv output in a new table in the db
/cc @edolstra @Ericson2314