use vendoring instead of gx for quic-go

[marten-seemann]

Fixes #41.

[raulk]

Can we vendor only quic-go? Or is there a reason to also vendor:

• golang.org/x/crypto
• github.com/lucas-clemente/aes12
• github.com/hashicorp/golang-lru
• github.com/bifurcation/mint

[marten-seemann]

These are all dependencies of quic-go. I vendored quic-go (which itself vendors its dependencies) by running

govendor fetch github.com/lucas-clemente/quic-go@=v0.10.0-no-integrationtests

Apparently, this stripped all the dependencies from the vendor directory in quic-go, and added them as dependencies here.

Do you know any better way to vendor stuff?

[raulk]

Ah, I see that govendor flattens out nested vendor directories. I guess this avoids conflicts when your consumer code happens to use transitive dependencies (i.e. dependencies of vendored dependencies). Not sure it's useful in our case, as IMHO it creates more noise in the vendor/ directory than necessary. But I have no strong feelings.

Regarding the golang.org/x/crypto dependency, I think it's a mistake upstream. govendor allows you to exclude deps from vendoring, and unless I'm missing something, I feel this one should be excluded from vendoring upstream (in quic-go).

[marten-seemann]

Why do you think it’s a mistake? golang/x comes with no stability guarantee whatsoever (we once had to deal with the case that they renamed one of the packages without prior notice).

[Stebalien]

Why do you think it’s a mistake? golang/x comes with no stability guarantee whatsoever (we once had to deal with the case that they renamed one of the packages without prior notice).

gx should handle this. That is, unless I'm mistaken, gx will rewrite imports inside vendor as well. so the solution is to have go-libp2p-quic-transport import golang/x packages itself.

An alternative is to:

1. Vendor all of these anyways for now.
2. Use .gxignore to exclude the vendor directory from gx.
3. Keep the gx import.

I'd also seriously consider forking quic-go for now.

---

Note: what I don't want is two copies of the golang/x packages in in go-ipfs.

[marten-seemann]

I don't really like the idea of a fork. It would work, but seems like it's just a workaround for Go imports not being able to include anything other than the master branch.

While I disagree that it's a mistake to vendor golang/x in quic-go itself, I understand that we don't want to have two copies in go-ipfs.
@Stebalien's alternative would work, but require me to make gx releases of quic-go. I'm a bit scared of subtle bugs here if for some reason the vendored and the gxed version are not identical (i.e. if I mess up cutting a release).

[Stebalien]

alternative would work, but require me to make gx releases of quic-go. I'm a bit scared of subtle bugs here if for some reason the vendored and the gxed version are not identical (i.e. if I mess up cutting a release).

This package already uses a gxed version of quic-go. My suggestion was to:

1. Keep that.
2. Add a .gxignore file to this repo to ignore the vendor folder.

That way, the gx package will use the gxed quic-go but the regular package will use the vendored deps.

[marten-seemann]

This package already uses a gxed version of quic-go.

Of course, and it's the only way QUIC can be used. My worries with your suggestion is that we have two different ways to use QUIC, which makes it easier for errors to creep in. If we do so, we should definitely have Travis run the tests with and without gx.

[Stebalien]

Of course, and it's the only way QUIC can be used. My worries with your suggestion is that we have two different ways to use QUIC, which makes it easier for errors to creep in. If we do so, we should definitely have Travis run the tests with and without gx.

This is always true with gx and libp2p, given that libp2p doesn't mandate gx.

[whyrusleeping]

ping @karalabe, I can't sign into twitter right now, but I think this is the root of the thing you were talking about.

[karalabe]

@whyrusleeping I'm not really sure to be honest, though maybe someone here can point me to a good direction?

I'm trying to maintain a version of IPFS that plays nicely with stock Go tooling (i.e. go get and all that). It worked for almost a year, but ever since this QUIC work was merged in, it just went belly up with:

$ go run -v main.go 
vendor/github.com/marten-seemann/qtls/common.go:10:2: cannot find package "crypto/internal/cipherhw" in any of:
	/work/src/[...]/vendor/crypto/internal/cipherhw (vendor tree)
	/opt/google/go/src/crypto/internal/cipherhw (from $GOROOT)
	/work/src/crypto/internal/cipherhw (from $GOPATH)
vendor/github.com/marten-seemann/qtls/13.go:24:2: cannot find package "github_com/cloudflare/sidh/sidh" in any of:
	/work/src/[...]/vendor/github_com/cloudflare/sidh/sidh (vendor tree)
	/opt/google/go/src/github_com/cloudflare/sidh/sidh (from $GOROOT)
	/work/src/github_com/cloudflare/sidh/sidh (from $GOPATH)
vendor/github.com/marten-seemann/qtls/cipher_suites.go:18:2: cannot find package "golang_org/x/crypto/chacha20poly1305" in any of:
	/work/src/[...]/vendor/golang_org/x/crypto/chacha20poly1305 (vendor tree)
	/opt/google/go/src/golang_org/x/crypto/chacha20poly1305 (from $GOROOT)
	/work/src/golang_org/x/crypto/chacha20poly1305 (from $GOPATH)
vendor/github.com/marten-seemann/qtls/13.go:25:2: cannot find package "golang_org/x/crypto/curve25519" in any of:
	/work/src/[...]/vendor/golang_org/x/crypto/curve25519 (vendor tree)
	/opt/google/go/src/golang_org/x/crypto/curve25519 (from $GOROOT)
	/work/src/golang_org/x/crypto/curve25519 (from $GOPATH)

Notably, I see 2 horrible things in this log:

• github.com/marten-seemann/qtls imports crypto/internal/cipherhw, which doesn't exist on Go 1.11. Why would a library even import internal Go library stuff? How does upstream IPFS work around this? How can I fix it in my repo?
• golang_org A what? What's this? Again, how does upstream IPFS make heads or tails of this import path?

[karalabe]

I figured out half my problem. 8 months ago the IPFS repo added vendor to .gitignore. This resulted in my ungx-er correctly creating the go get-able repo, but when I pushed it upstream to github the vendor folder got lost (indeed I have a commit from 8 months ago deleting 1.5M lines). The builds succeeded on CI because it had the correct vendor, but failed when checking out because the vendor got lost.

The failures however only started to happen recently, since until then IPFS didn't refer to broken code (with non-broken versions). Now I need to figure out how to pin QUIC to a meaningfully workable version.

[karalabe]

And I've solved my last issue too. Apparently the upstream quic-go library is borked, and whilst IPFS does pin the working version via gx, my vendored repo used the broken one. Same for the cbor encoder which broke its own APIs. After including these hard in my sources everything's fine. Cheers @whyrusleeping .

Btw, for any oursider, I was talking about this project: https://github.com/ipsn/go-ipfs

[raulk]

@marten-seemann not intending to put a stick in the wheel of vendoring, but have you guys considered go mod upstream? (quic-go). Since quic-go upstreams might not use go mod, my understanding is that you can refer to specific commits of those dependencies in go.mod, much like a vendor.json file.

Of course, all this is unfeasible if you need to support Go versions < 1.11, but would IMHO be a step in the right direction, and would help us experiment with go mod in libp2p (we've been intending to do that for a while).

[marten-seemann]

@raulk: That would be a valid reason to drop Go < 1.11. I haven't played around with go mod yet. What advantage do you see over using go vendoring, other than this is where Go is apparently headed?

[marten-seemann]

@raulk: I'd like to resolve this issue soon (as soon as I find someone who can enable CircleCI for this repo, Travis is annoying...). It seems that you've already spent some time digging into go mod, so I'd appreciate your help here.

[marten-seemann]

I fixed the Travis build, and we're now building and testing a gx and a non-gx version version.
This PR should be ready for review now.

[marten-seemann]

@raulk Could you have another look at this PR please?

[raulk]

@marten-seemann sorry for the delay here. We've been in conversations re: gx and gomod elsewhere, and I wanted to see where those went before taking action here.

We are slowly phasing in support for gomod in IPFS and libp2p. Some modules like go-libp2p-kad-dht already have a go.mod file. Thanks to minimal version selection, we can roll gomod support gradually and lazily.

We are also maintaining gx support (as IPFS and others use it). Both dependency managers can co-exist.

Here's what I'm thinking re: QUIC:

• Adding support to quic-go for gomod. If the vendor directory is needed, it can be generated with go mod vendor. This way quic-go will be aligned and prepared with Go's dependency management direction. I'll send a PR for that.
• Adding a gomod file in go-libp2p-quic-transport, that references the last working commit of quic-go. This replaces vendoring as introduced in this PR.
• Adding gx support in quic-go, or forking quic-go into the gxed organisation. Right now I'm not sure where the gx'ed source tree is?

WDYT?

cc/ @anacrolix -- the gomod warrior/preacher 😛

[raulk]

FYI – I forked quic-go into gxed and produced the v0.10.0 release: https://github.com/gxed/quic-go/tree/v0.10.0. The hash is different due to changes in package.json.

[anacrolix]

I've enabled CircleCI for this repo. I have a branch that contains go mod files that pass the tests. However CircleCI refuses to run the tests, it locks up part way through: https://circleci.com/gh/libp2p/go-libp2p-quic-transport/4.

[anacrolix]

#47

[marten-seemann]

As I said in quic-go/quic-go#1788, my proposal is the following: For the next quic-go release I'll add go mod support to qtls, quic-go and this repository.
Until then, we have two options:

1. continue as we did before (i.e. not merging this PR)
2. merging this PR, since it makes go-libp2p-quic-transport go gettable

WDYT?

[raulk]

@marten-seemann I'd like us to solve the packaging issue ASAP, as it's blocking downstream adopters who are excited about experimenting with libp2p QUIC, and cannot because of packaging issues.

I think the correct way of resolving is to push the missing commit to https://github.com/marten-seemann/qtls. Right now, if you erase the vendor dir in https://github.com/lucas-clemente/quic-go/, you wouldn't be able to recreate it, and that's the root cause IMO.

Are you able to push this commit: marten-seemann/qtls-deprecated@591c71538704 to https://github.com/marten-seemann/qtls, and place it in a deprecated branch?

That would allow us to proceed with the long-term solution here (go mod), rather than vendoring now just to move to go mod later.

[raulk]

@marten-seemann I recovered the commit from the qtls-deprecated repo and pushed it to the deprecated branch on my fork of qtls: https://github.com/raulk/qtls/tree/deprecated. Could you:

1. fetch from my remote.
2. checkout the deprecated branch.
3. push it to your remote.

Then I think everything will work again.

[marten-seemann]

@raulk and @anacrolix Sorry for all the hassle caused, I didn't expect that moving this repo would cause any problems. And thanks a lot for figuring out this solution.
This is a git feature I read about a long time ago, but somehow forgot about. You can have multiple branches that don't share a single commit. I pushed my old repository into the deprecated branch of qtls, so now the commit should be available again.

[anacrolix]

I don't think a go-get-safe package is a goal here anymore? If the package is compatible with go-mod, it may not be worth the additional maintenance for the vendored stuff. Not only does it bloat the repo, it's also ignored by gx and go mod. However if that's not the case, I'll merge it. @marten-seemann do you still want this?

[raulk]

@marten-seemann thanks for cooperating! If you don't mind, I'm gonna close this PR without merging as we're favouring gomod everywhere.

If we really need to vendor because some downstreams don't use gomod, we can use go mod vendor to maintain the vendor tree instead of relying on third party tooling.

SGTY? Reopen if not.