ValidPathInfo: make ca field a proper datatype #3649
Conversation
I think it makes more sense to define the data model (derivations), before the operations (store api).
…son2314/nix into validPathInfo-ca-proper-datatype
…314/nix into validPathInfo-ca-proper-datatype
|
Oh, I thought I could edit this because it is from a repo I have access too, but I guess that is not the case. This contains a number of other PRs:
And the culmination of our work to tighten up the API replacing strings with proper data type. We're doing this to help root out bugs with the git objects work, but we hope it can be merged separately as #3455 as it cleans up the code regardless of whether git objects ever makes it in. |
|
This is still WIP because we need to actually write the |
src/libstore/derivations.hh
Outdated
| std::string hashAlgo; /* hash used for expected hash computation */ | ||
| std::string hash; /* expected hash, may be null */ | ||
| DerivationOutput(StorePath && path, std::string && hashAlgo, std::string && hash) | ||
| std::optional<FileSystemHash> hash; /* hash used for expected hash computation */ |
There was a problem hiding this comment.
That is, a derivation might not have any file ingestion at all. If it's not fixed output, I think it will only have file ingestion through its inputs.
There was a problem hiding this comment.
I would recommend just using the Hash type here.
There was a problem hiding this comment.
Very confusingly, the hashAlgo string also contained the r:. See #3439, where this comes from.
src/libstore/file-hash.hh
Outdated
|
|
||
| /// Pair of a hash, and how the file system was ingested | ||
| struct FileSystemHash { | ||
| FileIngestionMethod method; |
There was a problem hiding this comment.
this might need to be optional
There was a problem hiding this comment.
Mainly because of the above comment. Not all uses of FileSystemHash will have a real "FileIngestionMethod" defined. Consider derivations.
There was a problem hiding this comment.
Oh, we should rename the data type then. This is strictly for content-addressed things.
Co-authored-by: Matthew Bauer <mjbauer95@gmail.com>
meditans
force-pushed
the
validPathInfo-ca-proper-datatype
branch
from
3ff7f7d to
343c20a
Compare
This was a latent bug that just appeared because of the tests that were added. Remember to wait for CI! :)
…athInfo-ca-proper-datatype
…idPathInfo-ca-proper-datatype
Thanks Matt!
Fixed the rest of these before, but this one slipped through.
meditans
changed the title
… into validPathInfo-ca-proper-datatype
Ericson2314
force-pushed
the
validPathInfo-ca-proper-datatype
branch
from
babb27c to
2f0e395
Compare
| }; | ||
|
|
||
| /// Pair of a hash, and how the file system was ingested | ||
| struct FixedOutputHash { |
There was a problem hiding this comment.
I went with FixedOutputHash for this PR, now that it is not just used in derivations.
No description provided.