As discussed on IRC, add a flag for ipfs add (and possibly other commands) to use CIDv1. cc @whyrusleeping
This should probably be similar to how block put works. Allowing to specify hash function, base and ...
Okay, I will take this one. Seams easy enough. @whyrusleeping if you want to just implement this yourself ping me on IRC first so there isn't any wasted effort.
I am thinking that this should be a global option, so that to control how Cid's are displayed though the API. For now a simple switch to enable CidV1 output. Later, it could be more involved and allow additional customizations such as specifying the base.
For now I can just add a flag to add, but I am thinking that this flag is going to eventually get added to nearly all the commands that output Cids in some form.
(Note: I am also assuming that if you specify a hash in CidV1 the format and it is stored in CidV0 it will still work, the same for the other way around, if not perhaps this should be fixed first.)
@whyrusleeping @Kubuxu @diasdavid what do you think?
@kevina I think this is a little bit more tricky than just adding a simple switch to ipfs add. I think the easiest way to accomplish this would be to add a flag to ipfs daemon like: --enable-cidv1-experiment or something (Used 'experiment' there to fit in with the current convention we have for daemon enabled new feature flags). And have it modify the defaultCidPrefix variable in merkledag/node.go to set version to 1.
This is also a dupe of #3243
@whyrusleeping so you are saying you do not want to allow the user to select the form of the Cid returned on a per-instance bases?
If we go with the daemon flag how will the case when the daemon is not running be handled. If you want to go that route a config option seams a better option.
Just as a note, this will break a lot of our sharness testing infrastructure. When we start working with it (and preparing for it to be default), we should do something that would make future changes like that less painful. For example: have file with generated hash and steps to reproduce it, then when change like that is done, the reproduction steps can be executed and new target hash can be generated for tests.
@Kubuxu in some cases that is a lot easier said then done. Can we create a separate issue for that (sharness testing infrastructure).
@kevina Yeah, a config option would probably be a better solution here
@whyrusleeping also, unless I am missing something, I don't think it should be too hard to reformat a Cid in one format to another based on a flag. Is there a reason you don't want to do this?
@kevina because just switching the output format of the root cid won't actually accomplish what we want. The cids written in the links to child objects would still be version 0
@kevina we could still do it on a per add basis, but it will be more tricky as it will require modifying a lot of wiring down into the adder/importer code and also touching the merkledag package to change how specifying a CID version on creation of a merkledag.ProtoNode
@whyrusleeping it might be more tricky but a global option to always use CidV1 could break a lot of things and will be difficult to test. I per add flag on the other hand will be fairly easy to test.
It should not be required to modify the merkledag package. Just set the Prefix field in ProtoNode immensely after the creation. For the rest we can start with a similar path used to enable RawLeaves that is add an option to add maybe call it --cid-version with valid values of 0 or 1 and a default of 0; then push the option down into DagBuilderHelper.
After that is done add two new methods:
(1) NewUnixfsNode() to DagBuilderHelper
func (db *DagBuilderHelper) NewUnixfsNode() *UnixfsNode {
node := h.NewUnixfsNode()
node.SetCidVersion(db.cidVersion)
return node
}
and (2) SetCidVersion() to UnixfsNode which will set the Prefix to the correct value based on the version.
And finally modify the dag builders to call NewUnixfsNode in DagBuilderHelper instead of creating a node directly from the helpers package.
So it will touch some code but only slightly more than adding the RawLeaves does.
Sound Good?
@kevina sounds good to me, thanks for the writeup!
Should have it implemented by the end of the day.
FYI: As I was afraid my outline above was incomplete. It only handled the case when adding a single file. Handling directories will require a few additional changes. I will describe the additional changes in the p.r., which should be ready by the end of today.