Don't vendor dependencies for etcd/client

[schmichael]

I have run into the same problem @reterVision did in #4904 when attempting to implement a fake etcd interface in a library.

I'm new to vendoring in Go, but my understanding is that using vendoring in libraries (that is: non-main non-internal non-test packages with public APIs) is discouraged* and that vendoring should be left up to the binaries (executables).

I'm unable to find direct guidance on the subject in the official vendoring doc, but the passage seems to indicate it's an open issue?

[This document] does not attempt to solve the problem of vendoring resulting in multiple copies of a package being linked into a single binary. Sometimes having multiple copies of a library is not a problem; sometimes it is. At least for now, it doesn’t seem that the go command should be in charge of policing or solving that problem.

Example

To give a concrete example of this issue:

• Metafora is a library which imports etcd/client
• Metafora's tests have a fake etcd implementation that implements etcd/client.KeysAPI
• Metafora does not vendor so the etcd/client.KeysAPI's context.Context arguments resolve to etcd/vendor/.../context while my fake implementation's context.Context arguments resolve to the global context package.
  • Therefore my fake implementation of KeysAPI does not build and my tests are broken.

(If context.Context were a struct instead of an interface my non-test code would have already run into the vendoring issue.)

As far as I can tell I have 4 options to resolve this build error:

1. vendor as instructed by the client's README
   • The obvious solution. However this (most likely?) forces vendoring onto all users of my library which... seems wrong? Perhaps all Go repos will include a vendor directory in this brave post-1.6 world?
2. Do not use a fake implementation of the KeysAPI interface
   • My tests would be difficult to duplicate against etcd directly as they'd be timing dependent
3. Implement a shim on the KeysAPI interface to fake instead of the actual KeysAPI interface.
   • Ugh, this means extra work in the actual code just to make faking possible in tests. Undesirable, but not out of the question.
4. Fork etcd/client to a new repo without vendoring. Also include local dependencies etcd/pkg/{pathutil,types} and rewrite imports (just to ensure vendoring is totally avoided).
   • Ugh again. Porting changes over would be an ongoing hassle unless automated with a github webhook and script.

If 1 - vendor everywhere - is the solution I'd just love some other examples in the Go community of libraries vendoring, or feedback from core developers that this is the correct path for all Go projects to take.

* My only source for this is some reputable members of #go-nuts who didn't give consent for me to name their names here. :)

[xiang90]

@schmichael The short answer is we are considering moving client/clientv3 to its own top level pkg.

[peterbourgon]

It is not only client packages. I am having the same problem when trying to implement various etcdserverpb.XServer interfaces. See this mailing list post on golang-nuts and this repo which demonstrates the problem.

This problem was raised earlier on golang-dev, see this post and Russ's reply immediately after, where it is stated

. . . vendored code is a private copy of something. That is, "vendor" is explicitly "internal" as well.

So, @schmichael's request generalizes to: don't have vendoring for any code which may be imported by other packages. This basically means anything other than package mains.

[peterbourgon]

I think the rule is

• If your repo contains any package that isn't package main, and
• If any of those packages expose types from any of their dependencies, and
• If it's possible that a third party would want to import any of those packages, then
• You must not use vendoring in that repo vendor any of those dependencies.

coreos/etcd satisfies all of the conditions. Therefore, one option could be splitting etcd into two repos:

• etcdbin, containing all of the package mains, with vendoring
• etcdlib, containing everything that isn't package main, with no vendoring

Another option would be removing all exported types from the vendor folder, starting with golang.org/x/net/context.

[peterbourgon]

@schmichael For the record your option 1 is definitely not a viable solution due to reasons outlined above.

[xiang90]

@peterbourgon Thanks for the extensive analysis. We are looking into this problem and will try all the possible options. We want everything inside one repo if possible and also want to ensure go tool works (like go get, go install). We will get back to you soon.

[peterbourgon]

@xiang90 There is extensive discussion on the mailing list and the Gopher slack #vendor channel, I recommend coming to the latter to get a proper framing/context for the discussion.

If you need one repo, then I think you will need to build your binaries with nonstandard tooling (e.g. glide, gb, etc.) and check in a vendor manifest file but not the vendor/ directory itself.

[schmichael]

Thanks! My library now builds without vendoring!

The solution seems a bit awkward (symlinking root into cmd/vendor), so I hope the discussion around vendoring best practices continues in the larger Go community.

Edit: Turns out my hopes already came true: https://groups.google.com/forum/m/#!topic/golang-dev/4FfTBfN2YaI

[xiang90]

@schmichael We are following that discussion. So the current solution might not be the final solution. We just try to make it work at least.