Mischellaneous changes to prepare for CA drvs -- contains #3807, #3424, and #3418 #3830
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
Today's fixed output derivations and regular derivations differ in a few ways which are largely orthogonal. This replaces `isFixedOutput` with a `type` that returns an enum of possible combinations.
Thanks @asymmetric! Co-Authored-By: asymmetric <lorenzo@mailbox.org>
Thanks @asymmetric I failed to do them all in one batch Co-Authored-By: asymmetric <lorenzo@mailbox.org>
See documentattion in header and comments in implementation for details. This is actually done in preparation for floating ca derivations, not multi-output fixed ca derivations, but the distinction doesn't yet mattter. Thanks @cole-h for finding and fixing a bunch of typos.
Co-Authored-By: Cole Helbling <cole.e.helbling@outlook.com>
I think this is clearer
When we merge with master, the new lack of string types make this case impossible (after parsing). Later, when we actually implemenent CA-derivations, we'll change the types to allow that.
N.B. not using `std::visit` for fetchurl because there is no attempt to handle all the cases (e.g. no `else`) and lambda complicates early return.
We've added the variant to `DerivationOutput` to support them, but made `DerivationOutput::path` partial to avoid actually implementing them. With this chage, we can all collaborate on "just" removing `DerivationOutput::path` calls to implement CA derivations.
…idiansystems/nix into ca-derivation-data-types
…n2314/nix into misc-ca
This reverts commit 3804e3d.
This reverts commit 74b251b.
This reverts commit 3a9e4c3.
edolstra
reviewed
Jul 20, 2020
Ericson2314
commented
Jul 20, 2020
src/libstore/derivations.hh
Outdated
| std::optional<FixedOutputHash> hash; /* hash used for expected hash computation */ | ||
| }; | ||
|
|
||
| struct DerivationOutputFixed |
There was a problem hiding this comment.
Should the variant be only of new types, or should it include old ones if they are 1-1?
Ericson2314
commented
Jul 20, 2020
Comment on lines
+32
to
+33
| FileIngestionMethod method; | ||
| HashType hashType; |
There was a problem hiding this comment.
Should we actually have this flexibility, or should we just enforce NAR-sha256?
There was a problem hiding this comment.
If we do remove these fields, we can still encode drv files in a way that allows for adding them later
…/nix into misc-ca
…systems/nix into misc-ca
…systems/nix into ca-derivation-data-types
|
Conflicts fixed. |
|
Conflicts fixed |
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.
This doesn't actually implement any new features, 😲. What it does do is adjusting existing code to better take advantage of the new variants. I hope by essentially "pushing down" the
throw Error("not implemented")closer to where the algorithsm actually need to be generalized, it becomes easier for people to tackle them independently.