WIP: Git with refs types refactor #53
Conversation
Header is valid, but rest of the code needs to be adapted.
…t-with-refs-strawman
And get content-address.o working
ensure-ca and path-info will do, soon
| typedef std::variant< | ||
| TextInfo, | ||
| FixedOutputInfo, | ||
| IPFSHashWithOptValue<IPFSGitTreeNode> |
There was a problem hiding this comment.
Doesn't this duplicate data because IPFSHashWithOptValue has a hash, but we can also compute the hash from IPFSGitTreeNode?
There was a problem hiding this comment.
It might be better to make IPFSHashWithOptValue a variant of either a hash or a value.
src/libstore/content-address.cc
Outdated
| + fsh.hash.to_string(Base32, true); | ||
| [](IPFSHash<IPFSGitTreeNode> ih) { | ||
| // FIXME do Base-32 for consistency | ||
| return "ipfs-git:" + ih.to_string(); |
There was a problem hiding this comment.
I don't think we need to restrict ourselves to git. The cid could be any object, and we might decide later on that we want to use nar instead of git, given the 2MB git limit and that we have to convert to and from nar for the store api calls a lot.
|
|
||
| struct ContentAddress { | ||
| template<typename Underlying> | ||
| struct ContentAddressT { |
There was a problem hiding this comment.
I think a little duplication here would be much better than doing this. What even is ContentAddressT? If you really want to use it, you should call it something like NameWrapper or something.
| }); | ||
| } | ||
| // FIXME: ipfs sort order is weird, it always puts type before | ||
| // references, so we rename it to qtype so it always comes |
There was a problem hiding this comment.
There's no qtype anymore. Shouldn't we have something telling ipfs what type we have?
src/nix/make-content-addressable.cc
Outdated
| auto ca_ = store->queryPathInfo(ref)->fullContentAddressOpt(*store); | ||
| assert(ca_); | ||
| ipfsRefs.insert(IPFSRef(computeIPFSCid(*ca_), ref.name())); | ||
| rewritingSink.flush(); |
There was a problem hiding this comment.
this got merged wrong - it should have been deleted i think
src/libutil/ipfs.cc
Outdated
| IPFSHash<Deref> fromString(std::string_view cid) { | ||
| auto prefix = cid.substr(0, 9); | ||
| HashType algo = prefix == "f01711220" ? htSHA256 | ||
| : prefix == "f01781114" ? htSHA1 |
There was a problem hiding this comment.
f0178 means it's the git codec, which should be an error for the "full" content address at least
| // References for the paths | ||
| PathReferences<IPFSRef> references; | ||
| PathReferences<ContentAddressT<Ref<IPFSGitTreeNodeT<Ref>>>> references; |
There was a problem hiding this comment.
Why would you ever need more than one level of PathReferences? We can always query the daemon once we have the cid of a reference.
There was a problem hiding this comment.
More importantly, when would we ever fill in more than one layer of references? We handle each path on its own, so no room to share information like this between queryPathInfo calls.
| std::shared_ptr<Deref> value = nullptr; | ||
|
|
||
| // Will calculate and store hash from value | ||
| static IPFSHashWithOptValue fromValue(Deref value); |
There was a problem hiding this comment.
once you have this, you don't need the value
There was a problem hiding this comment.
so this should just be:
IPFSHash fromValue(Deref value);
…t-with-refs-types-refactor
Just some link errors for knowingly-missing functions
…t-with-refs-types-refactor
…fs-types-refactor
No description provided.