reconciling fileccoin/ipfs go-ipld-cbor incompatibility

[phritz]

Dumping state for why, this work is still in progress.

As part of https://github.com/filecoin-project/go-filecoin/issues/599 we made a small change to go-cid and released a new version (0.7.22), with the aim of propagating the change through to go-filecoin. Since ~everything depends on go-cid, @stebalien updated a huge number of ipfs repos dependent on go-cid to the new version and released them. What remains is to update go-filecoin's dependencies to those newly released packages.

However! go-filecoin and ipfs use different versions of go-ipld-cbor. ipfs uses 1.2.13 which is built from https://github.com/ipfs/go-ipld-cbor and filecoin uses 1.4.1 which is built from a fork that lives at https://github.com/filecoin-project/go-ipld-cbor (the fork came from the refmt branch in the original repo).

We have basically two options:

1. Update ipfs to use the new go-cid-based dependencies, release it (for filecoin), and then have filecoin depend on this new version. https://github.com/ipfs/go-ipfs/pull/5243 is a WIP that updates ipfs to use the new dependencies. In order to bring it over the finish line we need to resolve the issue that ipfs and filecoin depend on incompatible versions of go-ipld-cbor: either ipfs needs to use the refmt-based fork or filecoin needs to go back to the non-refmt-based version. Not sure what would need to happen to take the new deps but make ipfs master stay dependent on 1.2.13 while moving a branch for filecoin to 1.4.5 (there's a PR out for this release) -- that's maybe the next step to investigate here.

2. Update the ipfs branch that go-filecoin depends on (release-0.4.15-rc1) to use the new go-cid-based dependencies. I started on this in https://github.com/ipfs/go-ipfs/pull/5244, i think i almost have all the deps updated but is revealing a lot of code incompatibilities: the release branch is months older than master against which the deps were presumably tested. This PR also moves ipfs from 1.2.12 to 1.4.5 (https://github.com/filecoin-project/go-ipld-cbor/pull/4 -- not merged yet bc i wanted to have a better handle on what's going on here), I'm guessing we'll need to do some fixups.

Based on how painful 2 is looking and the fact that filecoin shouldn't depend on too-stale code, I think we want to do 1.

ps Something I haven't looked at yet is how filecoin works right now given that it depends on a version of ipfs that uses a different go-ipld-cbor version than it. I'm just dumping state so why can take a look.

[Stebalien]

Is anything blocking using the refmt cbor branch?

[phritz]

Is anything blocking using the refmt cbor branch?

Let me get a concrete answer to that. I dumped state above in case why had some wisdom, I was half-done investigating.

[phritz]

OK @Stebalien @whyrusleeping in https://github.com/ipfs/go-ipfs/pull/5243/ I've updated all the ipfs deps to use the new versions that use the new go-cid. I also updated it to use go-ipld-cbor v1.4.5 which is basically the refmt branch. The only change I had to make to get it to pass tests was trivial. So to answer the question "is anything blocking using the refmt cbor branch" there are three things I can think of:

1. We'd want to merge the changes made to the go-filecoin fork of go-ipld-cbor back upstream. These changes are small so I don't forsee a problem doing that.
2. The only confidence that I have is that this is WAI is that the tests pass. I'm not sure how much confidence that should give us. Someone who knows IPFS probably needs to look at the diff between refmt and master to say whether the changes are backwards-compatible. In particular, this change makes me wonder.
3. @frrist (or why?) made a comment here to the effect that "go-ipld-cbor 1.3 changes the underlying cbor impl and was not ready to be used in ipfs last time I spoke with why", so that would be another thing to consider.

[whyrusleeping]

@phritz I've tasked @warpfork with getting that merged up. Primary concerns are on getting the issues found by fuzzing fixed, and writing some performance regression tests for ipld perf in go-ipfs. This should happen relatively soon (i'd say in the space of a couple weeks).

[Stebalien]

My one issue is that all CIDs become cid.Cid instead of *cid.Cid. Unfortunately, everything else uses cid.Cid so this would be a painful change. See my comments on the refmt PR.

[dignifiedquire]

Finding more dependency issues, I wanted to see if we can switch to gossipsub, now that it got merged, but the go-libp2p-floodsub version depends on a different multiaddr than the version of go-libp2p than the one we are using..

[phritz]

Does anyone else think that it might be beneficial to step back and take a look at how we might re-architect the dependency graph so that it is less painful? I'm wondering if for example we couldn't group some of the lowest-level dependencies together into one repo.

[whyrusleeping]

thats kinda what go-libp2p is. We should just be depending on go-libp2p, and not the plethora of other sub-dependencies. That way fixing this is just:

gx update github.com/libp2p/go-libp2p
gx update github.com/libp2p/go-floodsub

and done, but as it stands we have to go update each of the child dependencies, because they are explicitly in the package.json for some reason.

[phritz]

Can I go and make this change?

[dignifiedquire]

[phritz]

@whyrusleeping what's required here is:

1. for each dependency that both libp2p and filecoin have remove that dependency from go-filecoin's package.json
2. verify that this works by undo-ing the gx rewrite, and then doing it again so we can see it picking up the right deps from libp2p
3. get that change in
4. update to the new libp2p versions

Is that about right?

[phritz]

I'm seeing something I don't understand with gx. Before starting on the above I wanted to make sure that rewrite worked like I thought it worked and that if on master i did a gx-go rewrite --undo followed by a gx-go rewrite I'd see no change. That is, they should be inverses. But that's not what I see. This is the state afterwards: https://github.com/filecoin-project/go-filecoin/pull/661/files

It's picking up versions of go-ipld-cbor, go-log, and go-ipfs-cmds that are different from what's explicitly listed in go-filecoin's package.json.

[whyrusleeping]

@phritz that seems off... Something is wrong in the dep tree. Try running gx deps dupes

[phritz]

When I run it on master I see:

nb:go-filecoin fritz$ gx deps dupes
package go-colorable imported as both QmTsHcKgTQ4VeYZd8eKYpTXeLW7KNwkRD9wjnrwsV2sToq and QmVrtgFpU2Q7FmuqHuiB62BZJrAtatBAxwFSmwYQpzA1eA
package sys imported as both QmVGjyM9i2msKvLXwh9VosCTgP4mL91kC7hDmqnwTTx6Hu and QmTsTrxKNXu8sZLv7FP6p884CHoDbT9upKA1j4FkCy5V7T
package go-ipfs-cmds imported as both QmUf5GFfV2Be3UtSAPKDVkoRd1TwEBTmx9TSSCFGGjNgdQ and QmTjNRVt2fvaRFu93keEC7z5M1GS1iH6qZ9227htQioTUY
package protobuf imported as both QmT6n4mspWYEya864BhCUJEgyxiRfmiSY9ruQwTUNpRKaM and QmXSs8cccbT4zDR95c1iRpYKDqVMzqeF1J6iZcavgE6eNw
package go-isatty imported as both QmWVGFCGGTAGrT3adV261k1h6nntcyDhGVszGV2i2pzwPe and QmRRr1zSEeFmjPWJeDAdhhQBRM2kYuPFC4T1QVwXKg7UrG
package go-log imported as both QmSpJByNKFX1sCsHBEp3R73FL4NF6FnQTEGyNAXHm2GS52 and QmRb5jh8z2E8hMGN2tkvs1yHynUanqnZ3UeKwgN1i9P1F8
package go-colorable imported as both QmTsHcKgTQ4VeYZd8eKYpTXeLW7KNwkRD9wjnrwsV2sToq and QmdvecVcFhbo5x4f3arqmfxyE3NzqwWyp77KzA68EKXJeX
package go-log imported as both QmSpJByNKFX1sCsHBEp3R73FL4NF6FnQTEGyNAXHm2GS52 and QmPuosXfnE2Xrdiw95D78AhW41GYwGqpstKMf4TEsE4f33
package sys imported as both QmVGjyM9i2msKvLXwh9VosCTgP4mL91kC7hDmqnwTTx6Hu and QmPXvegq26x982cQjSfbTvSzZXn7GiaMwhhVPHkeTEhrPT
package go-detect-race imported as both Qmf7HqcW7LtCi1W8y2bdx2eJpze74jkbKqpByxgXikdbLF and QmQHGMVmrsgmqUG8ih3puNXUJneSpi13dkcZpzLKkskUkH
package go-ipld-cbor imported as both QmNRz7BDWfdFNVLt7AVvmRefkrURD25EeoipcXqo6yoXU1 and QmRiRJhn427YVuufBEHofLreKWNw7P7BWNq86Sb9kzqdbd
package iptb imported as both QmYMTCRFe7Xgw2v47vqcxwDCPvkqafvdEbrZ2fFGK7u2VR and QmTD9SdvPDfzdQrBq4iSD3LDDaM3dKHtvvuGbqzMGUHkat
package go-ctrlnet imported as both QmRebo5SY2EajaMx2Fnom21KHPWSivJRJHYwe58nZ5N8yC and QmfEZa44SyWfyXpkbVfi19H1QpY73DU6E5omK2HbKXwqR6
package sys imported as both QmVGjyM9i2msKvLXwh9VosCTgP4mL91kC7hDmqnwTTx6Hu and QmTq8ag5pgTCqtGDtmpm1F5TPE2i1H8bcU6295WFKTc5ie
package go-log imported as both QmSpJByNKFX1sCsHBEp3R73FL4NF6FnQTEGyNAXHm2GS52 and QmQCqiR5F3NeJRr7LuWq8i8FgtT65ypZw5v9V6Es6nwFBD

[phritz]

@whyrusleeping is it the case that there should be zero dupes in our dependencies? For example if we depend on sys and fuse, and fuse depends on sys then versions of sys must always match? I know that there are situations where filecoin just won't build unless they match because packages are interoperating, but what about the case where they are not?

Re deps in general I'm wondering about things like:

• seems like we can only safely update deps at the speed of the slowest of our deps to release. If we depend on go-ipfs for example and the vetting process for a new release takes time, we can't safely update until that process has completed.
• seems like we could easily get into a situation where we have mutually exclusive dependency requirements with something like go-ipfs.
• with deep dependency trees the amount of work to test out a change deep in the tree is really painful. For example in order to test the update to go-cid you have to propagate the change up through 3-4 layers of dependencies, and each layer can branch to new dependents which also have to be updated, before you can even test that the change works in go-filecoin.
• seems like so much work to update a dep: we have to update all deps that depend on it, as well as their deps. And by "update" I mean update that code, release it, and propagate the change to the next level.

[whyrusleeping]

For example if we depend on sys and fuse, and fuse depends on sys then versions of sys must always match?

They don't always need to match, no. Ones that don't ever touch are fine to be different versions.

However, our gx plans for this quarter include making this problem go away a bit.

But:

For example in order to test the update to go-cid you have to propagate the change up through 3-4 layers of dependencies

For this, you can grab the current hash of go-cid that we are depending on, and run gx-go link <hash>. It will replace the gx dep we're depending on with the github pathed version, so any changes you make to $GOPATH/src/github.com/ipfs/go-cid will reflect in your builds, throughout the dependency tree.

seems like so much work to update a dep: we have to update all deps that depend on it, as well as their deps. And by "update" I mean update that code, release it, and propagate the change to the next level.

Yeah, it is. We're working on improving it.

[phritz]

I've spun two sub-issues out: https://github.com/filecoin-project/go-filecoin/issues/664 https://github.com/filecoin-project/go-filecoin/issues/663

@dignifiedquire unclear if these address your issue in https://github.com/filecoin-project/go-filecoin/issues/643#issuecomment-407090982 but I suspect so.

[phritz]

Also for the record:

• the move to refmt is happening in https://github.com/ipfs/go-ipld-cbor/issues/29.
• gx improvements relevant to deps issues above: https://github.com/whyrusleeping/gx/issues/179
• extraction of packages from go-ipfs: https://github.com/filecoin-project/go-filecoin/issues/665

[whyrusleeping]

In my most recent update, i used mainline go-ipld-cbor instead of our fork.

[phritz]

sweeeet