-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Simplify ContentAddress
#8650
Simplify ContentAddress
#8650
Conversation
Whereas `ContentAddressWithReferences` is a sum type complex because different varieties support different notions of reference, and `ContentAddressMethod` is a nested enum to support that, `ContentAddress` can be a simple pair of a method and hash. `ContentAddress` does not need to be a sum type on the outside because the choice of method doesn't effect what type of hashes we can use. Co-Authored-By: Cale Gibbard <cgibbard@gmail.com>
5ab2afe
to
903700c
Compare
FixedOutputHash LocalStore::hashCAPath( | ||
const FileIngestionMethod & method, const HashType & hashType, | ||
ContentAddress LocalStore::hashCAPath( | ||
const ContentAddressMethod & method, const HashType & hashType, | ||
const StorePath & path) | ||
{ | ||
return hashCAPath(method, hashType, Store::toRealPath(path), path.hashPart()); | ||
} | ||
|
||
FixedOutputHash LocalStore::hashCAPath( | ||
const FileIngestionMethod & method, | ||
ContentAddress LocalStore::hashCAPath( | ||
const ContentAddressMethod & method, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These I just generalized to not use the deleted type. And this gets us closer in fact to being able to deduplicate this with some build/local-derivation-goal.cc
stuff later.
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/2023-07-10-nix-team-meeting-minutes-70/30471/1 |
Motivation
Whereas
ContentAddressWithReferences
is a sum type complex because different varieties support different notions of reference, andContentAddressMethod
is a nested enum to support that,ContentAddress
can be a simple pair of a method and hash.ContentAddress
does not need to be a sum type on the outside because the choice of method doesn't effect what type of hashes we can use.Context
#3746 and #3959 got the data types in their current form, but I did not notice that this simplification became possible.
@amjoseph-nixpkgs in #3959 (comment) raised the goal of flattening the content address data types / making text less "the odd one out". I agree, and this gets us closer.
Checklist for maintainers
Maintainers: tick if completed or explain if not relevant
tests/**.sh
src/*/tests
tests/nixos/*
Priorities
Add 👍 to pull requests you find important.