vendoring: isolate the vendor directory

[vbatts]

Due to the new vendoring logic in the golang compiler it can cause
issues for projects that import a package that has vendored a package
that is used locally. See containers/image#223

This change moves the vendored sources to the package that uses them,
rather than for the whole project. Also is explictly is not vendoring
code repos from "github.com/opencontainers/". For now we'll consider
these non-remote. Though versioning may likely be future concern.

Fixes #527
Obsoletes #528

Signed-off-by: Vincent Batts [email protected]

[vbatts]

@runcom @mtrmac PTAL
@tianon is this what you were recommending? Seems to work well enough.

[wking]

On Thu, Jan 26, 2017 at 11:54:02AM -0800, Vincent Batts wrote: Due to the new vendoring logic in the golang compiler it can cause issues for projects that import a package that has vendored a package that is used locally… This change moves the vendored sources to the package that uses them, rather than for the whole project… … R schema/vendor/github.com/pkg/errors/LICENSE (0) …

Doesn't this just reduce your cross-section (from “all image-spec consumers” to “image-spec/schema consumers”)? image-tools (and likely other projects) are vendoring image-spec schema (e.g. for [1]), so I expect they'd still have issues after this change (buy I haven't tested to confirm). An alternative approach would be to rename vendor/ to _vendor/ (or whatever), and then drop in a symlink or rename (I'm not sure if Go supports symlinks) when you're running image-spec-local tests. [1]: https://github.com/opencontainers/image-tools/blob/121b8c90841b82a1c611d23f3c26bb4bda20ffa9/image/config.go#L28

[tianon]

@vbatts depends -- if external (ie, outside this repo) Go programs are intended to import github.com/opencontainers/image-spec/schema, then this won't work since it'll (partially) force these imports upon folks using them

Generally speaking, libraries shouldn't have vendor/, but package main can (should?), so for packages that have ./cmd/abc, ./cmd/xyz, etc where all their package main lives, then ./cmd/vendor/... is 💯, but at ./vendor/..., it'd force users of ./other/package to also have these vendors.

[wking]

On Thu, Jan 26, 2017 at 12:05:05PM -0800, Tianon Gravi wrote: Generally speaking, libraries shouldn't have `vendor/`, but `package main` should, so for packages that have `./cmd/abc`, `./cmd/xyz`, etc…

Only using cmd/vendor/ doesn't help library tests. For example, schema/spec_test.go imports the (vendored) github.com/russross/blackfriday, etc. The only ways I see to address that are: a. Renaming (or similar [1]). This is an annoying hack, but it would work. b. Moving tests to a separate package (schema-test/spec_test.go?). This is less annoying to work with, but means you can only test the public image-spec/schema API. [1]: #532 (comment)

[tianon]

IMO that's a reasonably easy problem to solve -- just have the CI which runs the tests rename vendor to something with a src subdirectory and add it directly to the GOPATH. That way, it's easy for developers to run the tests locally against other versions of the deps if they so choose.

I really wish the Go developers had kept .../src/... in this new vendoring spec so that we could continue to use a vendor directory as a direct GOPATH component. 😞

Edit: and this is complicated slightly further by the fact that go(1) (the tool) doesn't look kindly on symlinks, and so sometimes they work, but often times they break in very obscure ways

[vbatts]

k. so this thing hardly makes sense, and I did it wrong too.