Dedup some derivation initialization logic, and test#14560
Dedup some derivation initialization logic, and test#14560Ericson2314 merged 2 commits intoNixOS:masterfrom
Conversation
github-actions
bot
added
new-cli
Ericson2314
force-pushed
the
fill-in-outputs
branch
from
e8532fc to
d34ee4e
Compare
| * | ||
| * @param store The store to use for validation | ||
| */ | ||
| void checkInvariants(Store & store) const; |
There was a problem hiding this comment.
Why is this version needed now?
There was a problem hiding this comment.
It is used in parseJsonAndValidate, we validate the derivation before we compute its drvPath now. The drvPath was just used for error messages, but it we can write a better error message --- since we just computed the drv path, it won't mean anything to the human reading the error anyways where ("derivation from JSON" or similar will)
|
|
||
| # If we remove the input addressed to make it a deferred derivation, we | ||
| # still get the same result because Nix will see that need not be | ||
| # deferred and fill in the right input address for us. |
There was a problem hiding this comment.
Why is this a good thing? From reading checkInvariants, it looks like this is new behavior.
There was a problem hiding this comment.
checkInvariants doesn't do this, but fillInOutputPaths does. It is new behavior in the nix derivation add code path (which is unstable), but not new behavior in the builtins.derivation code path.
src/libstore/derivations.cc
Outdated
| return; | ||
| } | ||
| auto & value = j->second; | ||
| if (value == "") { |
There was a problem hiding this comment.
Can this actually happen - I did not see it in the old code?
There was a problem hiding this comment.
The user-submitted JSON is arbitrary data, so yes it can happen. It also does happen in the builtins.derivationStrict code path already.
Ericson2314
force-pushed
the
fill-in-outputs
branch
2 times, most recently
from
8149291 to
745c307
Compare
Ericson2314
force-pushed
the
fill-in-outputs
branch
7 times, most recently
from
7f3f8f1 to
79cf901
Compare
These do a read/write test in the middles of some computation. They are an imperative way to test intermediate values rather than functionally testing end outputs.
`nix derivation add`, and its C API counterpart, now works a bit closer to `builtins.derivation` in that they don't require the user to fill-in input addressed paths correctly ahead of time. The logic for this is carefully deduplicated, between all 3 entry points, and also between the existing `checkInvariants` function. There are some more functional tests, and there are also many more unit tests. Co-authored-by: Sergei Zimmerman <sergei@zimmerman.foo> Co-authored-by: edef <edef@edef.eu>
Ericson2314
force-pushed
the
fill-in-outputs
branch
from
9962fc3 to
620a694
Compare
| auto drv = static_cast<Derivation>(json); | ||
| auto drv = Derivation::parseJsonAndValidate(*store, json); | ||
|
|
||
| auto drvPath = writeDerivation(*store, drv, NoRepair, /* read only */ dryRun); |
There was a problem hiding this comment.
why do we actually have writeDerivation twice here?
5 participants
Motivation
nix derivation add, and its C API counterpart, now works a bit closer tobuiltins.derivationin that they don't require the user to fill-in input addressed paths correctly ahead of time.The logic for this is carefully deduplicated, between all 3 entry points, and also between the existing
checkInvariantsfunction. There are some more functional tests, and there are also many more unit tests.Context
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.