A tricky (impossible?) situation with common packages and vendoring

[Peter Bourgon]

I need to create a type which implements the etcdserverpb.KVServer
interface[0]. Note that package context is imported by its canonical
import path[1]. Also note that etcd has a vendor folder in its root
which includes the context package[2].

[0] https://github.com/coreos/etcd/blob/de801b5/etcdserver/etcdserverpb/rpc.pb.go#L1724-L1743
[1] https://github.com/coreos/etcd/blob/de801b5/etcdserver/etcdserverpb/rpc.pb.go#L18
[2] https://github.com/coreos/etcd/tree/de801b500b1f2ab237757052edd87901a5906153/vendor/golang.org/x/net/context

The etcd repo contains packages that are intended to be imported by
other packages, such as this one. But it also contains binaries, so I
think the presence of the vendor folder is correct. Here is my
code[3]. It fails to build because etcdserverpb.KVServer.Compact
expects "github.com/coreos/etcd/vendor/golang.org/x/net/context".Context
rather than "golang.org/x/net/context".Context[4], which as far as I
know I cannot possibly provide.

[3] https://gist.github.com/peterbourgon/79f44f89d5c334b5f05a291f25fe6bd8
[4] https://gist.github.com/peterbourgon/304c21802b3531cd8d0b50247ca41522

What went wrong, and how can I fix it?

Cheers,
Peter.

[jonathan...@gmail.com]

A bit of shell in your build routine to move up any nested vendor packages if your putting any packages in vendor at build time. Otherwise vendoring tools like govendor do this already.

[Sam Boyer]

AFAIK, you can't fix it without hacking etcd's vendor - assuming that etcd is under your project's vendor, you'd need to move x/net/context from under etcd/vendor to your own vendor/ directory. We've been referring to that process as 'flattening'.

While vendor does allow you to use the canonical import path in the source code (yaaaaay), the compiler still rewrites those paths to what you see there ("github.com/coreos/etcd/vendor/golang.org/x/net/context"). Thus, any shared deps have to be pushed up to the topmost vendor directory.

Most of the package management tools are working on solutions to this. I'm working on a comprehensive solver (https://github.com/sdboyer/vsolver) - with glide as the first integration target - that'll be able to figure out if it's safe to flatten in such a way, and explain why if it's not.

[Peter Bourgon]

Thanks for the replies. My sample code was misleading, the type that
implements etcdserverpb.KVServer is itself in a library, so its repo
has no vendor folder by design. My users should be able to `go get` my
code, or use it in their own projects, without requiring any tricks in
their build process. Does that make the problem intractable?

[Sam Boyer]

If you can't assume a package manager, then yup, I think it's intractable. 

This exemplifies why "just committing vendor" is kind of a shortsighted solution. And, because this can easily happen recursively (e.g. etcd could import a project with main+lib packages that itself chooses to vendor), it's one that increases in harmfulness to the ecosystem as adoption of it increases.

[Peter Bourgon]

Thanks. Committing vendor is fine, I think, as long as nobody imports
your repo. That is, as long as you're exclusively a package main. As
soon as you have something that others may want to import, all bets
are off.

This is very disappointing. It makes my project significantly more
difficult to use as intended. Maybe so difficult that it will prevent
widespread use.

[Sam Boyer]

Sure. And, right, sorry, I should've been clearer - if your repo only has main package(s), there's no problem in the case where you've committed your vendor. When I say '"just committing vendor" is kind of shortsighted,' it's a bigger-picture guess about how having that easy approach available might drive some systemic things:

- plenty of folks just won't be aware of the harm, and will go ahead and publish split main/lib repos with committed vendor dirs

- folks who are aware of it will be discouraged from splitting useful parts out of main packages, because the sudden steep increase in maintenance difficulty that presents

- wary of these problems, folks may become even less inclined to rely on other deps than they are now. (ironic, given that vendor dirs were supposed to do the opposite)

and people end up in the spot you're in now - a bit despondent because sharing code just got a little more infeasible.

that said, it's not at all intractable in general. we just need the right tooling. i, for one, am working feverishly to that end :)

[Daniel Theophanes]

Everything is working as intended.

You need to copy the vendor packages to your own vendor folder to "flatten" them. Don't use git sub-modules: see https://www.reddit.com/r/golang/comments/4cptba/best_practice_for_vendoring_in_libraries/

In this case because etcd copies packages locally, you need to use a vendor tool when consuming those packages.

I would personally suggest github.com/kardianos/govendor, but any tool that "flattens" the vendor repo will functionally work. 

[Peter Bourgon]

Upon further thought, I think this is actually a very serious problem.
The vendoring in coreos/etcd makes it impossible for other packages to
implement its interfaces. This generalizes to: vendoring in •any repo•
makes it •impossible• to interact with packages in that repo in ways
that require type identity with vendored deps.

I've created https://github.com/peterbourgon/wtf as a minimal example
to demonstrate this problem.

I'd appreciate it if someone involved with the design of
GO15VENDOREXPERIMENT could comment. Russ?

[Konstantin Shaposhnikov]

I asked the same question some time ago on golang-dev: https://groups.google.com/d/msg/golang-dev/WebP4dLV1b0/Lhk4hpwJEgAJ. Read the thread to see Russ's replies.

[Peter Bourgon]

Thanks, Konstantin. Money quote there from Russ seems to be

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

I am trying to boil this down to a rule. If

- Your repo contains any package that isn't package main, and
- Any of those packages expose types from any of their dependencies, and
- It's •possible• that a third party would want to import any of those packages,

Then you •must not• use vendoring in that repo.

Is that sufficiently expressed?

[Pierre Durand]

Or don't rewrite import path.

[Peter Waller]

I have hit this exact situation and spent some time thinking about it.

I wanted to vendor some code from elsewhere without copying the source, modifying the source, or using external vendoring tools. `git submodule` works just fine for everything we use so far, a fairly substantial number of dependencies. It has a lot of nice properties. But then I hit this case where there was some perfectly good code that I wanted to use, but couldn't, without violating the rule "just submodule it".

The structure looked something like:

/cmd/foo

/pkg/foo

/vendor/bar

Upstream was unsympathetic and didn't want to change anything. Which is fair enough. Thinking about it further though, they couldn't have fixed it for me, within their repository. They would have had to introduce a separate repository for main to live in.

My first thought was that they should move /vendor/bar to /cmd/vendor/bar. But only when I tried to do it in another circumstance did I realise I had missed the obvious: When you do that, the vendoring doesn't apply to /pkg/bar anymore. So your only choice if you want /pkg/foo to be externally importable is to have another repository for /cmd/, which contains both /vendor/pkg/foo and /vendor/bar.

One style of solution I thought of is some sort of vendor package directory masking. If I use vendoring, I may want to "hide" a vendor directory (nested within a vendor directory) from the build system, so that I may put my own vendor'd packages in its place. I even started prototyping this behaviour with a fuse go program.

But in the end, this was all massively overkill for my problem and I found another solution which didn't require importing the module at all.

I do fear though that the more vendoring is used by packages I may want to import, the less I can do so.

[Dave Cheney]

I think it could be more succinctly expressed as:

Libraries must not vendor code.

What's a library? You're code is, if some other project imports it.

[Peter Waller]

On 1 April 2016 at 10:01, Dave Cheney <da...@cheney.net> wrote:

Libraries must not vendor code.

What's a library? You're code is, if some other project imports it.

 

Some open questions:

(Let's say I have something intended to be used as both a library and a program :)

Is it OK to have a library /pkg/foo and a program /cmd/foo in the same repository? (My intuitive guess: a reasonable thing to want?)

Is /cmd/foo allowed to vendor /pkg/foo's dependencies? (yes)

Must that program also vendor that /pkg/foo? (seems like it might be required if you want to use vendoring in /cmd/foo)

Can you suggest a directory structure? (seems like you need a copy of /pkg/foo in the repository, which lives under /cmd/vendor or /cmd/foo/vendor)

Something like:

/cmd/foo

/cmd/vendor/pkg/foo

/cmd/vendor/foo-dependency

/pkg/foo

The copy at /cmd/vendor/pkg/foo (or /pkg/foo depending on your perspective) is unfortunate, but I don't currently see another way. I'd love for there to be one :)

What this achieves is it puts a package at /pkg/foo which is importable from the outside world which doesn't pull in any vendor'd packages.

[Ian Davis]

The corollary to your rule is:

Repositories that contain vendored code should not be imported.

(since if it were a library intended for importing then the author would
not be using vendoring)

I'm being facetious here, but my growing feeling is that the current
vendoring approach for Go is harmful for code reuse.

[Peter Waller]

On 1 April 2016 at 10:17, Peter Waller <pe...@pdftables.com> wrote:

What this achieves is it puts a package at /pkg/foo which is importable from the outside world which doesn't pull in any vendor'd packages.

What is especially unfortunate about this state of affairs is that essentially everyone who ever makes repositories which are both cmd/ and pkg/ (and use vendoring) has to get this "right"*. Otherwise `go get` (and, e.g, pinning upstream with git submodules) won't work without further intervention. As an importer of code there is nothing I can do except something more complicated.

* Not to claim that what I suggested in the previous post is right.

[Dave Cheney]

On Friday, 1 April 2016 20:18:11 UTC+11, Peter Waller wrote:

On 1 April 2016 at 10:01, Dave Cheney <da...@cheney.net> wrote:

Libraries must not vendor code.

What's a library? You're code is, if some other project imports it.

 

Some open questions:

(Let's say I have something intended to be used as both a library and a program :)

Don't do that.

 

Is it OK to have a library /pkg/foo and a program /cmd/foo in the same repository? (My intuitive guess: a reasonable thing to want?)

Yes, the advice for using /vendor is it should go at the top of your repository so it's scope covers all the code in the repository, ie

github.com/you/foo/vendor

 

Is /cmd/foo allowed to vendor /pkg/foo's dependencies? (yes)

Sounds like a bad idea.

 

Must that program also vendor that /pkg/foo? (seems like it might be required if you want to use vendoring in /cmd/foo)

Not if they are in the same repository, and the should be, because they are tightly coupled.

 

Can you suggest a directory structure? (seems like you need a copy of /pkg/foo in the repository, which lives under /cmd/vendor or /cmd/foo/vendor)

Something like:

/cmd/foo

/cmd/vendor/pkg/foo

/cmd/vendor/foo-dependency

/pkg/foo

The copy at /cmd/vendor/pkg/foo (or /pkg/foo depending on your perspective) is unfortunate, but I don't currently see another way. I'd love for there to be one :)

That's too complicated IMO, vendoring is per repo (per project), so 

github.com/you/foo/vendor

 

What this achieves is it puts a package at /pkg/foo which is importable from the outside world which doesn't pull in any vendor'd packages.

Don't do that, you're making it to hard on yourself. If it's a library; designed to be imported by _other_ repositories, put it in it's own repo. If it's a library that is part of the final application (what gb calls a project), then don't split it into it's own repo, that's just making it harder than it needs to be. 

[Peter Waller]

On 1 April 2016 at 11:15, Dave Cheney <da...@cheney.net> wrote:

Don't do that, you're making it to hard on yourself. If it's a library; designed to be imported by _other_ repositories, put it in it's own repo. If it's a library that is part of the final application (what gb calls a project), then don't split it into it's own repo, that's just making it harder than it needs to be.

The problem isn't that I do that - in itself - but that there are packages in the wild that do this that I may want to import.

I'm willing to change my behaviour where I become aware that is it wrong.

On the other hand, I don't see what's wrong with having both a library and a main function in one package. I mean, etcd does it, right? Are they doing it wrong? If so, please tell them for me :)

I write much of my code so that main() is really a tiny program which just calls a library. Are you saying this has to live in its own repository, or am I misunderstanding?

[Peter Waller]

On 1 April 2016 at 11:15, Dave Cheney <da...@cheney.net> wrote:

On Friday, 1 April 2016 20:18:11 UTC+11, Peter Waller wrote:

Something like:

/cmd/foo

/cmd/vendor/pkg/foo

/cmd/vendor/foo-dependency

/pkg/foo

The copy at /cmd/vendor/pkg/foo (or /pkg/foo depending on your perspective) is unfortunate, but I don't currently see another way. I'd love for there to be one :)

That's too complicated IMO, vendoring is per repo (per project)

FWIW, this is not something I was seriously suggesting, but intended to be a straw man.

[Dave Cheney]

On Friday, 1 April 2016 21:20:45 UTC+11, Peter Waller wrote:

On 1 April 2016 at 11:15, Dave Cheney <da...@cheney.net> wrote:

Don't do that, you're making it to hard on yourself. If it's a library; designed to be imported by _other_ repositories, put it in it's own repo. If it's a library that is part of the final application (what gb calls a project), then don't split it into it's own repo, that's just making it harder than it needs to be.

The problem isn't that I do that - in itself - but that there are packages in the wild that do this that I may want to import.

I'm willing to change my behaviour where I become aware that is it wrong.

On the other hand, I don't see what's wrong with having both a library and a main function in one package. I mean, etcd does it, right? Are they doing it wrong? If so, please tell them for me :)

 

This is just my opinion, feel free to return it for the complete purchase amount, or simply discard if you don't agree with it. 

 

I write much of my code so that main() is really a tiny program which just calls a library. Are you saying this has to live in its own repository, or am I misunderstanding?

If the library and the code that calls it are related, then they should be in the same repository. 

If the library has many consumers then you should not also place vendored code in that repository -- otherwise consumers of your library will find themselves in the same position as Peter Bourgon has found himself.

[Peter Waller]

On 1 April 2016 at 11:26, Dave Cheney <da...@cheney.net> wrote:

If the library has many consumers then you should not also place vendored code in that repository -- otherwise consumers of your library will find themselves in the same position as Peter Bourgon has found himself.

An interesting thing about this is that it places the choice in the hands of the producer, and as a consumer my life is harder.

They have to knowingly make the "correct" choice, and also not work against my interests as a consumer.

[Alex Bligh]

Does that mean if your code is a library (you want others to import it)
you cannot have within it a test suite which requires vendored code?
If so, that's a bit unfortunate for interoperability tests.

--
Alex Bligh

[Dave Cheney]

Based on Peter's findings, yes, that appears to be the case.

>
> --
> Alex Bligh
>
>
>
>

[Jakob Borg]

As I understand it, vendored dependencies themselves are not the
problem here, but *exposing* them such as returning or taking as
arguments types from them. So vendoring x/net/context and having
methods accept a context.Ctx from the outside is a no-no. But a
vendored dependency for tests, or internal use only, or as a
dependency to some cmd/* package in the same repo ought to be
acceptable.

I don't think this changes anything for the base rule of "libraries
should not vendor" - that still holds.

//jb

[Konstantin Shaposhnikov]

Does that mean if your code is a library (you want others to import it)
you cannot have within it a test suite which requires vendored code?
If so, that's a bit unfortunate for interoperability tests.
 

Only if public (exported) API of your library uses types from its dependant packages (like in Peter's example).

I agree that this situation is unfortunate.

[Jakob Borg]

2016-04-01 13:02 GMT+02:00 Jakob Borg <ja...@nym.se>:
> So vendoring x/net/context and having
> methods accept a context.Ctx from the outside is a no-no.

Adding another thought here, this specific case only sounds even
vaguely acceptable because it's - at this point - such a common type
that it "feels" like the standard library. I.e. we commonly accept
io.Writers so why not context.Contexts? But in most other cases you
would not expose a method that takes some interface from an unrelated
package - you'd declare a matching interface of your own to avoid the
dependency to begin with. That's rather the point of having implicitly
satisfied interfaces after all?

//jb

[Sam Boyer]

yep, this is the recursive nastiness i was referencing earlier.

i really doubt there's any kind of static filesystem magic we could work out that would solve this problem for folks. it's tooling, or pain.

[Konstantin Shaposhnikov]

>
> I don't think this changes anything for the base rule of "libraries
> should not vendor" - that still holds.
>

I wouldn't say that this rule is absolute though. There are some cases
when it makes sense for a library to vendor some or all of its
dependencies. For example Go standard library (when Go 1.7 is out)
vendors golang.org/x/net/http2/hpack

[Ian Davis]

But this implies that libraries should wrap all their dependencies which
is not always practical. For example, imagine a game library that relies
on OpenGL. Because it can't possibly cater for every use case it's
likely the library will want to expose hooks to directly access some of
the raw OpenGL functionality, e.g. handles to buffers. If the game
library has to alias those types then Go's own lexical matching suggests
it would also need to provide compatible aliases for the entire OpenGL
API.

[Sam Boyer]

FWIW, IMO this is the danger I referenced earlier - vendoring troubles discouraging people from having anything other than a main package. It's harmful to the ecosystem.

[Sam Boyer]

On Friday, April 1, 2016 at 5:18:11 AM UTC-4, Peter Waller wrote:

On 1 April 2016 at 10:01, Dave Cheney <da...@cheney.net> wrote:

Libraries must not vendor code.

What's a library? You're code is, if some other project imports it.

 

Some open questions:

(Let's say I have something intended to be used as both a library and a program :)

I disagree with Dave - It's perfectly fine to do this. glide supports it - you can have carefully-controlled dependencies, but needn't commit your vendor directory. (If you can rely on downstream folks to use glide, then it's safe for you to commit vendor, if you want).

[Dave Cheney]

That sounds utterly miserable. Both you, the library author, and the other you, the library consumer are walking on eggshells; one is petrified of accidentally leaking -- I don't think that concept is well understood -- it's never been a think that Go developers have had to worry about because there was only every one type for a given import path before -- and the other is petrified of being passed a type which will could fail an equality check when code is refactored, or a dependency updated.

My suggestion above may not be absolute, i'm sure with scrupulous attention to detail by a very dedicate development team, backed by tooling which hasn't been developed yet, it could be made to work. But it seems simpler, and far more explainable, to adopt a "only main packages vendor" maxim.

[Konstantin Shaposhnikov]

I wonder if extending "go test" to use dependencies from vendor_test
directory in some cases would help? This is just an idea though, I
haven't thought through all the details.

> --
> You received this message because you are subscribed to a topic in the
> Google Groups "golang-nuts" group.
> To unsubscribe from this topic, visit
> https://groups.google.com/d/topic/golang-nuts/AnMr9NL6dtc/unsubscribe.
> To unsubscribe from this group and all its topics, send an email to
> golang-nuts...@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

[Ian Davis]

On Fri, Apr 1, 2016, at 12:02 PM, Jakob Borg wrote:
> 2016-04-01 12:51 GMT+02:00 Alex Bligh <al...@alex.org.uk>:
> >
> > On 1 Apr 2016, at 10:01, Dave Cheney <da...@cheney.net> wrote:
> >
> >> I think it could be more succinctly expressed as:
> >>
> >> Libraries must not vendor code.
> >>
> >> What's a library? You're code is, if some other project imports it.
> >
> > Does that mean if your code is a library (you want others to import it)
> > you cannot have within it a test suite which requires vendored code?
> > If so, that's a bit unfortunate for interoperability tests.
>
> As I understand it, vendored dependencies themselves are not the
> problem here, but *exposing* them such as returning or taking as
> arguments types from them. So vendoring x/net/context and having
> methods accept a context.Ctx from the outside is a no-no. But a
> vendored dependency for tests, or internal use only, or as a
> dependency to some cmd/* package in the same repo ought to be
> acceptable.

Vendoring for internal use could lead to duplicate instances of packages
ending up in the final binary. If one of those packages initializes some
external resource via an init function and expects exclusive access to
it then that would be a problem.

[Jakob Borg]

2016-04-01 13:13 GMT+02:00 Ian Davis <m...@iandavis.com>:
> On Fri, Apr 1, 2016, at 12:07 PM, Jakob Borg wrote:
>> 2016-04-01 13:02 GMT+02:00 Jakob Borg <ja...@nym.se>:
>> > So vendoring x/net/context and having
>> > methods accept a context.Ctx from the outside is a no-no.
>>
>> Adding another thought here, this specific case only sounds even
>> vaguely acceptable because it's - at this point - such a common type
>> that it "feels" like the standard library. I.e. we commonly accept
>> io.Writers so why not context.Contexts? But in most other cases you
>> would not expose a method that takes some interface from an unrelated
>> package - you'd declare a matching interface of your own to avoid the
>> dependency to begin with. That's rather the point of having implicitly
>> satisfied interfaces after all?
>
> But this implies that libraries should wrap all their dependencies which
> is not always practical. For example, imagine a game library that relies
> on OpenGL.

No, I'm just saying a small interface is not necessarily the best
thing to have an external dependency for. In the context case this has
become practice anyway for reasons of tooling etc. In most other cases
I don't think we'd import, never mind *vendor*, a package just for a
small interface. See also left-pad... :)

For something like OpenGL, by all means do expose the necessary types
but then don't vendor the packages in your library.

//jb

[Sam Boyer]

On Friday, April 1, 2016 at 7:02:55 AM UTC-4, Jakob Borg wrote:

2016-04-01 12:51 GMT+02:00 Alex Bligh <al...@alex.org.uk>:
>
> On 1 Apr 2016, at 10:01, Dave Cheney <da...@cheney.net> wrote:
>
>> I think it could be more succinctly expressed as:
>>
>> Libraries must not vendor code.
>>
>> What's a library? You're code is, if some other project imports it.
>
> Does that mean if your code is a library (you want others to import it)
> you cannot have within it a test suite which requires vendored code?
> If so, that's a bit unfortunate for interoperability tests.

As I understand it, vendored dependencies themselves are not the
problem here, but *exposing* them such as returning or taking as
arguments types from them. So vendoring x/net/context and having
methods accept a context.Ctx from the outside is a no-no. But a
vendored dependency for tests, or internal use only, or as a
dependency to some cmd/* package in the same repo ought to be
acceptable.

Yep. There's an analysis I intend to put in the solver engine I'm working on (and, thus, into glide) that determines whether or not a types from a dependent package escape from a package. More or less like pointer escape analysis. Hard to do completely because of interfaces, but still gets us a lot of the way there.

 

I don't think this changes anything for the base rule of "libraries
should not vendor" - that still holds.

 

I do wish we could stop using the word "vendor" to refer to just "committing your vendor directory." That is one approach, but it is not the only approach (though I know Dave would prefer it to be). Conflating the word with just the one approach distorts the discussion.

//jb

[Ian Davis]

 

On Fri, Apr 1, 2016, at 12:20 PM, Dave Cheney wrote:

My suggestion above may not be absolute, i'm sure with scrupulous attention to detail by a very dedicate development team, backed by tooling which hasn't been developed yet, it could be made to work. But it seems simpler, and far more explainable, to adopt a "only main packages vendor" maxim.

 

I agree that this is a natural consequence of the vendor experiment.

It means vendoring doesn't help me, as a library author, when I want to provide tests and example code that interact with my library and its dependencies. I will have to provide those in a separate repository with a vendor directory. And ironically I should actually vendor my library into that repo too.

Possibly an alternative is to copy the packages I need and convert them to internal packages in my library. I'm not sure that's a good idea.

 

 

[Sam Boyer]

On Friday, April 1, 2016 at 7:20:09 AM UTC-4, Dave Cheney wrote:

That sounds utterly miserable. Both you, the library author, and the other you, the library consumer are walking on eggshells; one is petrified of accidentally leaking -- I don't think that concept is well understood -- it's never been a think that Go developers have had to worry about because there was only every one type for a given import path before -- and the other is petrified of being passed a type which will could fail an equality check when code is refactored, or a dependency updated.

My suggestion above may not be absolute, i'm sure with scrupulous attention to detail by a very dedicate development team, backed by tooling which hasn't been developed yet, it could be made to work. But it seems simpler, and far more explainable, to adopt a "only main packages vendor" maxim.

It's a performable analysis, and not one that has to be that difficult.

You've expressed to me previously that you chose to focus specifically on main packages with gb because it seemed like a reasonable first step - not because it was all there was. Have you changed your view? This language seems more absolute than what you've used in the past.