Fix building CA derivations with and eval store #9589
Conversation
github-actions
bot
added
with-tests
| if (&worker.evalStore != &worker.store) { | ||
| RealisedPath::Set inputSrcs; | ||
| for (auto & i : drv->inputSrcs) | ||
| inputSrcs.insert(i); | ||
| if (worker.evalStore.isValidPath(i)) |
There was a problem hiding this comment.
Do we have a temporary root for it in the evalStore?
There was a problem hiding this comment.
I think it is rooted by the derivation in evalStore (if that is rooted), if the derivation is in store then this whole thing is a no-op
| @@ -604,7 +617,13 @@ void DerivationGoal::inputsRealised() | |||
| return *outPath; | |||
| } | |||
| else { | |||
| auto outMap = worker.evalStore.queryDerivationOutputMap(depDrvPath); | |||
| auto outMap = [&]{ | |||
| for (auto * drvStore : { &worker.evalStore, &worker.store }) | |||
There was a problem hiding this comment.
thought: {,} is a hidden overlay store
tomberek
added
the
idea approved
|
🎉 All dependencies have been resolved ! |
I don't love the way this code looks. There are two larger problems: - eval, build/scratch, destination stores (NixOS#5025) should have different types to reflect the fact that they are used for different purposes and those purposes correspond to different operations. It should be impossible to "use the wrong store" in my cases. - Since drvs can end up in both the eval and build/scratch store, we should have some sort of union/layered store (not on the file sytem level, just conceptual level) that allows accessing both. This would get rid of the ugly "check both" boilerplate in this PR. Still, it might be better to land this now / soon after minimal cleanup, so we have a concrete idea of what problem better abstractions are supposed to solve.
Ericson2314
force-pushed
the
floating-content-addressing-derivations-eval-store
branch
from
36f366b
to
9f39dda
Compare
Ericson2314
deleted the
floating-content-addressing-derivations-eval-store
branch
|
Well put! :) |
|
@Ericson2314 I'm trying to get IFD to use |
|
@shlevy I don't think there is anything called a
|
|
@Ericson2314 |
|
Oh hah! I never noticed that. |
…ddressing-derivations-eval-store Fix building CA derivations with and eval store (cherry picked from commit dfc0cee) Change-Id: I28feb5a36d4fe75f0ed3e3e2db6eb56b67d0f371
Motivation
Fix the bug! Add the test!
Context
I don't love the way this code looks. There are two larger problems:
eval, build/scratch, destination stores (Separate stores for evaluation, building and storing the result #5025) should have different types to
reflect the fact that they are used for different purposes and those
purposes correspond to different operations. It should be impossible
to "use the wrong store" in my cases.
Since drvs can end up in both the eval and build/scratch store, we
should have some sort of union/layered store (not on the file sytem
level, just conceptual level) that allows accessing both. This would
get rid of the ugly "check both" boilerplate in this PR.
Still, it might be better to land this now / soon after minimal cleanup,
so we have a concrete idea of what problem better abstractions are
supposed to solve.
Depends on #9588
Priorities
Add 👍 to pull requests you find important.