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

Allow the copy of ca derivation outputs #4174

Closed
wants to merge 20 commits into from

Conversation

@regnat
Copy link
Contributor

@regnat regnat commented Oct 21, 2020

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

  • Define (in code) the notions of:
    • derivation output: the triplet (drvPath, outputName, outputPath) along with some meta information (currently a set of dependencies, should probably be extended at least with a signature),
    • derivation output id: The tuple drvPath!outputName that uniquely (modulo non-determinism) identifies a derivation output,
    • derivation input: either a plain store path or 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).
  • Add a Store::registerDrvOutput(DrvOutputId&, DrvOutputInfo) method which generalizes LocalStore::linkDeriverToPath
    1. By making it a virtual method of the Store class (so that every store can implement it rather than just LocalStore)
    2. By allowing to not just register the DrvOutputId->outPath mapping, but also all the meta-information that might be needed
  • Allow copyPaths(...) and the nix copy command to work on derivation outputs
    • nix copy nixpkgs#hello will now copy the derivation outputs of nixpkgs#hello rather than just the output paths.
    • It's now possible to run nix copy /nix/store/....-hello.drv!out to manually specify a derivation output
    • nix copy /nix/store/....-hello will just copy the output path like it used to do
  • Fix #4138 (because otherwise copying drv outputs to a remote store would require copying the whole drv closure too which is painful).

Still to do:

  • Allow signing drv outputs (which will be needed as it's the bit that has to be trusted now)
  • Implement transparent substitution (needs hooking into the SubstitutionGoal which I haven't done yet
  • Properly register the dependencies of a drv output in the database.
    To 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

regnat added 17 commits Oct 20, 2020
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
for (auto& [outputName, status] : initialOutputs) {
if (!status.wanted)
continue;
auto drvSubstitution = getEnv("USE_DRV_SUBSTITUTION");
Copy link
Member

@zimbatm zimbatm Oct 24, 2020

Is it possible to use a setting instead? The proliferation of environment variables that changes the behaviour of Nix can be problematic.

@regnat
Copy link
Contributor Author

@regnat regnat commented Oct 26, 2020

@regnat regnat force-pushed the copy-ca-derivations branch from bb59326 to edb6a15 Oct 26, 2020
@regnat regnat force-pushed the copy-ca-derivations branch from edb6a15 to b915a9a Oct 27, 2020
@nixos-discourse
Copy link

@nixos-discourse nixos-discourse commented Nov 5, 2020

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

https://discourse.nixos.org/t/tweag-nix-dev-update-4/9862/1

@edolstra edolstra marked this pull request as draft Nov 26, 2020
@regnat regnat added this to the ca-derivations-mvp milestone Dec 11, 2020
@regnat regnat closed this Jan 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants