-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
Improve DagModifier to fully support CIDv1 #3921
Comments
@Kubuxu @whyrusleeping this sounds like something I can take on if no one else is working on it... |
Yeah, I started working on it but there isn't much done. I just added test to FilesAPI that exposes the problem. |
There are three possible aspects to this issue
I am not sure if (3) should is considered part of this issue, but it would seam like something that will be useful to have. I guess support for that depends on how well ProtoBuf directory entries interact with links to non-Protobuf node. In particular the Size entry in Link is support to be for "FileSize" which may not always have a meaningful value. |
@kevina sounds right to me. number 3 should probably just not freak out about having non-unixfs nodes in the graph. read and write shouldnt be supported on those nodes, and probably also don't want to support traversing them as directories in that context. |
Here is my planned strategy for support CidV1 and alternative hash functions.
I am planning on implementing this as part of #4026 however if any parts of this plan or controversial I can factor them out of that p.r. |
CidV1 support as outlined above (with minor modifications) is implemented in #4026. |
@kevina your comment (two above) sounds like the right way forward to me. The only weird part is (4). Reformatting the root (or any arbitrary object) seems like something we want to be able to do (I can imagine wanting to turn an old cidv0 file into a new object with raw leaves and blake2b). Maybe we should open a new issue to discuss how that interface should work? |
@whyrusleeping done, see #4171 |
@kevina could you provide a progress update here? |
@whyrusleeping, per my earlier comment this is is done please see #4026, or am I misunderstanding the question? |
#4026 is now merged, closing |
Currently DagModifier lacks proper support of CIDv1.
We should investigate what is lacking and improve it.
List:
The text was updated successfully, but these errors were encountered: