Skip to content
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

Closed
Kubuxu opened this issue May 13, 2017 · 14 comments
Closed

Improve DagModifier to fully support CIDv1 #3921

Kubuxu opened this issue May 13, 2017 · 14 comments
Assignees
Labels
kind/enhancement A net-new feature or improvement to an existing feature

Comments

@Kubuxu
Copy link
Member

Kubuxu commented May 13, 2017

Currently DagModifier lacks proper support of CIDv1.
We should investigate what is lacking and improve it.

List:

@Kubuxu Kubuxu added the kind/enhancement A net-new feature or improvement to an existing feature label May 13, 2017
@kevina
Copy link
Contributor

kevina commented May 18, 2017

@Kubuxu @whyrusleeping this sounds like something I can take on if no one else is working on it...

@whyrusleeping
Copy link
Member

@kevina I think @Kubuxu is working on this, I also had a branch pushed that started to address the issue, see #3901

@Kubuxu
Copy link
Member Author

Kubuxu commented May 18, 2017

Yeah, I started working on it but there isn't much done. I just added test to FilesAPI that exposes the problem.

@kevina
Copy link
Contributor

kevina commented Jun 19, 2017

@Kubuxu can you push your tests somewhere, I am working on this now in connection with #3989.

@whyrusleeping
Copy link
Member

@kevina I think its this: #3901

@kevina
Copy link
Contributor

kevina commented Jul 1, 2017

There are three possible aspects to this issue

  1. Fully support raw leaves, Add full support for CidV1 in Files API and Dag Modifier #4026 makes progress there
  2. Support the use of Cidv1 including support for alternative hash functions, I will work on this next. The easiest way to do this is to allow passing in a custom prefix like is now supported in add
  3. Allow non ProtoBuf/Raw nodes to be part of the files API dag.

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.

@whyrusleeping
Copy link
Member

@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.

@kevina
Copy link
Contributor

kevina commented Aug 9, 2017

Here is my planned strategy for support CidV1 and alternative hash functions.

  1. When a file or directory is modified the new hash and any new interior nodes in the dag will use the same prefix (which will preserve the Cid version and hash function).
  2. When a new file or directory is created it will use the same prefix as the directory it is added to, by default. The --cid-version and --hash option can be used to specify a different prefix instead.
  3. If the Cid version is 0 legacy leaf nodes will be used, otherwise raw leaves will be used. This behavior can be changed by using the --raw-leaves option when modifying a file.
  4. There should be some way to change the prefix of the MFS root. I am thinking about adding a rehash sub-command to change the prefix of an empty root. The idea behind this command is that in the future it can be used to change the prefix of any type of mfs-object.

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.

@kevina
Copy link
Contributor

kevina commented Aug 13, 2017

CidV1 support as outlined above (with minor modifications) is implemented in #4026.

@whyrusleeping
Copy link
Member

whyrusleeping commented Aug 14, 2017

@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?

@kevina
Copy link
Contributor

kevina commented Aug 26, 2017

@whyrusleeping done, see #4171

@whyrusleeping
Copy link
Member

@kevina could you provide a progress update here?

@kevina
Copy link
Contributor

kevina commented Sep 2, 2017

@whyrusleeping, per my earlier comment this is is done please see #4026, or am I misunderstanding the question?

@kevina
Copy link
Contributor

kevina commented Nov 28, 2017

#4026 is now merged, closing

@kevina kevina closed this as completed Nov 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement A net-new feature or improvement to an existing feature
Projects
None yet
Development

No branches or pull requests

3 participants