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

Substitute drv outputs #4238

Closed

Conversation

thufschmitt
Copy link
Member

@thufschmitt thufschmitt commented Nov 9, 2020

Depends on #4225, only commits after 97aca21 are new.

Depends on #4487

Changes the build loop to allow substituting derivation outputs.

logger->startWork();
auto outputId = DrvOutputId::parse(readString(from));
auto outputPath = StorePath(readString(from));
store->registerDrvOutput(outputId, DrvOutputInfo{outputPath});
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this require trusted user or signature verification?

Copy link
Member Author

Choose a reason for hiding this comment

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

It definitely should, yes, and that's one of the necessary follow-ups.

I think it's okay to keep it for a subsequent PR though, as long as it is behind the ca-derivations feature flag (which it isn't atm, I'll have to add it)

Copy link
Member

Choose a reason for hiding this comment

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

Where do you track these problems?

Copy link
Member Author

Choose a reason for hiding this comment

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

Curently mostly on a sheet of paper on my desk. Opening proper issues is still on my todo-list

Copy link
Member Author

Choose a reason for hiding this comment

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

I've fixed it in 3be2780 (as part of #4225). Now registerDrvOutput requires the experimental feature ca-derivations to be enabled if we can't prove that it's indeed a valid output of an input-addressed derivation.

I've also opened #4248 to track the signature problem

@thufschmitt thufschmitt force-pushed the substitute-drv-outputs branch 2 times, most recently from a9090ae to b2a19e0 Compare November 12, 2020 12:15
@thufschmitt thufschmitt added the ca-derivations Derivations with content addressed outputs label Nov 16, 2020
@thufschmitt thufschmitt force-pushed the substitute-drv-outputs branch 3 times, most recently from 20a3ccc to 4f0541e Compare November 18, 2020 09:38
@thufschmitt thufschmitt force-pushed the substitute-drv-outputs branch 4 times, most recently from 9e86318 to 081b0bf Compare November 24, 2020 10:23
@thufschmitt thufschmitt force-pushed the substitute-drv-outputs branch 3 times, most recently from 661b315 to e10741c Compare December 1, 2020 16:23
@thufschmitt thufschmitt added this to the ca-derivations-mvp milestone Dec 11, 2020
@dpulls
Copy link

dpulls bot commented Jan 28, 2021

🎉 All dependencies have been resolved !

@thufschmitt thufschmitt force-pushed the substitute-drv-outputs branch 2 times, most recently from ddc348a to 0d77839 Compare January 28, 2021 16:34
Change the `nix-build` logic for linking/printing the output paths to allow for
some outputs to be missing. This might happen when the toplevel
derivation didn't have to be built, either because all the required
outputs were already there, or because they have all been substituted.
Simple test to ensure that `nix-build && nix-collect-garbage &&
nix-build -j0` works as it should
Once a build is done, get back to the original derivation, and register
all the newly built outputs for this derivation.

This allows Nix to work properly with derivations that don't have all
their build inputs available − thus allowing garbage collection and
(once it's implemented) binary substitution
Where a `RealisedPath` is a store path with its history, meaning either
an opaque path for stuff that has been directly added to the store, or a
`Realisation` for stuff that has been built by a derivation

This is a low-level refactoring that doesn't bring anything by itself
(except a few dozen extra lines of code :/ ), but raising the
abstraction level a bit is important on a number of levels:

- Commands like `nix build` have to query for the realisations after the
  build is finished which is fragile (see
  27905f1 for example). Having them
  oprate directly at the realisation level would avoid that
- Others like `nix copy` currently operate directly on (built) store
  paths, but need a bit more information as they will need to register
  the realisations on the remote side
That way we can copy the realisations too (in addition to the store
paths themselves)
@thufschmitt thufschmitt changed the base branch from master to ca/copy-drv-outputs January 28, 2021 16:46
@Ericson2314
Copy link
Member

@regnat I don't think this depends on anything anymore?

Also, there is another place we want to substitute resolutions: when resolving CA derivations. In that case we don't want to fetch the data. That means we would need to not have resolution goals spawn path goals, at least not unconditionally.

@Ericson2314
Copy link
Member

Getting into the weeds, I think some of the inputsRealised logic on resolving would need to be factored out so it can also be used twice`.

Basically we end up with:

  1. Try to fetch the outputs.
  2. Try to resolve the inputs. If we can, then resolve the derivation, and delegate to building that (call bits factored out from inputsRealised call)
  3. Try to download the inputs (same)
  4. Resolve the derivation (inputsRealised, and same logic, but now calls factored-out bits to do it)

This can be done in a follow-up PR, of course.

@thufschmitt
Copy link
Member Author

thufschmitt commented Jan 29, 2021

@regnat I don't think this depends on anything anymore?

It still depends #4487 which itself depends on #4340 and #4372 (unless we want to merge it into #4487. I don't really care about the order as long as everything ends-up merged)

Also, there is another place we want to substitute resolutions: when resolving CA derivations. In that case we don't want to fetch the data. That means we would need to not have resolution goals spawn path goals, at least not unconditionally.
Getting into the weeds, I think some of the inputsRealised logic on resolving would need to be factored out so it can also be used twice`.

Fully agree. I've opened #4493 to track that (sorry, I though there was already an issue about it)

@dpulls
Copy link

dpulls bot commented Feb 26, 2021

🎉 All dependencies have been resolved !

@edolstra edolstra closed this Feb 26, 2021
@edolstra edolstra deleted the branch NixOS:ca/copy-drv-outputs February 26, 2021 11:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ca-derivations Derivations with content addressed outputs
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants