Replace Godep with Glide, introduce native Go vendoring

[s-urbaniak]

This PR introduces native Go vendoring and introduces Glide in favor of godep.

The following table shows a rkt-relevant feature comparison between both tools:

Feature	Godep	Glide
Restore depdencies into the GOPATH	godep restore	N/A, see [1]
Add a new dependency	go get github.com/fizz/buzz && git co ... && ./scripts/godep-save	glide get -u github.com/fizz/buzz#>=v1.0 && ./scripts/update-glide.sh, **
Update an existing dependency	git pull ... && godep update ...	(Optional) bump version in glade.yaml, $ ./scripts/update-glide.sh
Remove an existing dependency	Remove dep, then ./scripts/godep-save	glide rm github.com/fizz/buzz && ./scripts/update-glide.sh, see *

*: Note that glide in v0.10.2 currently has a bug with the --delete option. This is fixed by a small PR [2] we submitted.

**: While it is possible to fetch single packages, doing so glide really wants to update the whole tree too, see Masterminds/glide#321 for an explanation.

The general method of maintaining dependencies in Glide is to edit the glide.yaml and calling glide update. The glide.yaml specifies the desired state and declares not only concrete versions but potentially whole version ranges. Whenever glide update is being executed the tool then tries to bump to the newest version matching the specified version range and pins the actual commit IDs in a file called glide.lock. It takes care to bump to tagged versions only if a semver has been specified in glade.yaml. Glide supports the following two modes of operation:

1. Having the vendor directory not checked into the VCS, but only glide.yaml and glide.lock.
2. Having the vendor directory being checked in to the VCS. For this mode one has to specify the --update-vendored, -u flag to glide update.

For rkt option 2. is a must for having reproducible builds.

When invoking glide update it simply checks out the target repository without modifications into the vendor directory. One can specify the --strip-vcs, -s, and --strip-vendor, -v flags to remove VCS metadata and/or nested vendor and Godeps directories. Nevertheless the size of the vendor directories is still very big, even when combined with the -s, and -v flags. The glide-vc [3] tool for stripping more unneeded files has been created to keep the vendor directory size sane. Unfortunately for rkt glide-vc actually strips too much, so we submitted a small PR [4] against it.

Here are the directory sizes for rkt:

Godeps	glide	glide -v -s	glide + glide-vc
12Mb	648Mb	172Mb	11Mb

One can clearly see that the glide + glide-vc combo is what we are looking for. The smaller size actually surprised me, so I made a diff exposing the differences in [5].

There is an ongoing discussion in the Go community about the next "idiomatic" package/vendoring, most of the ideas captured in [6].

Also I submitted a question [7] regarding transitive dependency resolution.

[1] Masterminds/glide#366

[2] Masterminds/glide#453

[3] https://github.com/sgotti/glide-vc

[4] sgotti/glide-vc#13

[5] https://gist.github.com/s-urbaniak/10f800d20f4281edfcf8d43e5bad115f

[6] https://docs.google.com/document/d/1xrV9D5u8AKu1ip-A1W9JqhUmmeOhoI6d6zjVwvdn5mc

[7] Masterminds/glide#471

Fixes #2385

[s-urbaniak]

@alban @iaguis @jonboulle @lucab PTAL

[s-urbaniak]

@jellonek PTAL

[s-urbaniak]

@steveej PTAL

[s-urbaniak]

@sgotti PTAL

[alban]

diffstat from the command line:

 2269 files changed, 277710 insertions(+), 328997 deletions(-)

I guess the github "+20 −43,382" is wrong.

[s-urbaniak]

I think, I'm experiencing the Semafailure as #2718, testing TestFetch locally.

[s-urbaniak]

:-/ TestFetch* passes locally

[jellonek]

So it's still false-positive :/

[s-urbaniak]

That TestFetch* is flaky as hell, semaphore is green, hence this issue becomes reviewable

[jonboulle]

What exactly is flaking? Sounds like we need to prioritise a replacement.
Is it docker stuff? appc/docker2aci#172

On 2 June 2016 at 11:12, Sergiusz Urbaniak notifications@github.com wrote:

That TestFetch* is flaky as hell, semaphore is green, hence this issue
becomes reviewable

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#2735 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/ACewN_Bi8EIVUNMXPUGzhDuShxsv1m1Jks5qHp5xgaJpZM4Irofj
.

[s-urbaniak]

@jonboulle I saw --- FAIL: TestFetch failures as described in #2718

[s-urbaniak]

As discussed OOB with @lucab: His idea was to introduce a git submodule for the vendor directory to keep the size of the actual repo small. Apparently go has support for this starting from 1.6 (https://go-review.googlesource.com/#/c/17974/).

Some downstream maintainers (Debian) don't care about vendor but some do (Arch), where rkt is being build as described in the documentation being a single binary.

My suggestion is not to go forward with submodules due to the nature of it, but rather decide whether we strictly want to check in vendor or not.

Going with option 1. (not checking in the vendor directory) still puts a burden on a) downstream maintainers and b) every CI build who would need to pull ~650MB by calling glide install

[jonboulle]

I don't understand the proposal exactly - to keep a vendor/ directory in
another git repository? or to actually have a git submodule for every
single dependency? In either case, I don't think any components of rkt are
go gettable, so this doesn't really help, and in the latter case, it
doesn't help with upstream repositories disappearing or being unavailable.

I think we should just stick with a checked-in vendor directory that's as
slim as possible

rkt is being build as described in the documentation being a single
binary.

I don't really understand this bit.

A thought: what if we provide a simple bash script that would fully hydrate
the vendor directory (kinda like a godep restore) with unstripped
dependences, so people can easily verify that we are checking in an exact
subset of what we're claiming to. Would this help downstream concerns?

On 3 June 2016 at 17:15, Sergiusz Urbaniak notifications@github.com wrote:

As discussed OOB with @lucab https://github.com/lucab: His idea was to
introduce a git submodule for the vendor directory to keep the size of
the actual repo small. Apparently go has support for this starting from
1.6 (https://go-review.googlesource.com/#/c/17974/).

Some downstream maintainers (Debian) don't care about vendor but some do
(Arch), where rkt is being build as described in the documentation being
a single binary.

My suggestion is not to go forward with submodules due to the nature of
it, but rather decide whether we strictly want to check in vendor or not.

Going with option 1. (not checking in the vendor directory) still puts a
burden on a) downstream maintainers and b) every CI build who would need to
pull ~650MB by calling glide install

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#2735 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/ACewN7mq1pRx4v_nn5qtTBVtR3CEFnmWks5qIEUHgaJpZM4Irofj
.

[lucab]

@jonboulle it was more like a brainstorm discussion than an actual proposal, but in short: a single submodule for /vendor.

Glide manifest and lockfile already cover the reproducible part. Vendoring everything in git only prevents multiple network trips and prevents disappearing repos. The downside is increased repo size, cloning time (I don't have numbers here, but I guess this PR will weight a bit) and source tarball size. Plus some downstream will strip it anyway.

I was just wondering if the two concerns could be split here, but at this point it looks like it is not a great idea.

[s-urbaniak]

On Fri, Jun 3, 2016 at 5:49 PM, Jonathan Boulle notifications@github.com
wrote:

I don't understand the proposal exactly - to keep a vendor/ directory in
another git repository? or to actually have a git submodule for every
single dependency? In either case, I don't think any components of rkt are
go gettable, so this doesn't really help, and in the latter case, it
doesn't help with upstream repositories disappearing or being unavailable.

I think we should just stick with a checked-in vendor directory that's as
slim as possible

rkt is being build as described in the documentation being a single
binary.

I don't really understand this bit.

Nevermind, I don't understand it either, my sentence doesn't make any sense
;-) What I wanted to say: Arch builds rkt as we do locally according to the
documentation using the provided vendor directory.

A thought: what if we provide a simple bash script that would fully hydrate
the vendor directory (kinda like a godep restore) with unstripped
dependences, so people can easily verify that we are checking in an exact
subset of what we're claiming to. Would this help downstream concerns?

On 3 June 2016 at 17:15, Sergiusz Urbaniak notifications@github.com
wrote:

As discussed OOB with @lucab https://github.com/lucab: His idea was to
introduce a git submodule for the vendor directory to keep the size of
the actual repo small. Apparently go has support for this starting from
1.6 (https://go-review.googlesource.com/#/c/17974/).

Some downstream maintainers (Debian) don't care about vendor but some do
(Arch), where rkt is being build as described in the documentation being
a single binary.

My suggestion is not to go forward with submodules due to the nature of
it, but rather decide whether we strictly want to check in vendor or not.

Going with option 1. (not checking in the vendor directory) still puts a
burden on a) downstream maintainers and b) every CI build who would need
to
pull ~650MB by calling glide install

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#2735 (comment), or
mute
the thread
<
https://github.com/notifications/unsubscribe/ACewN7mq1pRx4v_nn5qtTBVtR3CEFnmWks5qIEUHgaJpZM4Irofj

.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#2735 (comment), or mute
the thread
https://github.com/notifications/unsubscribe/AAW8MCnS4A3XAH4PJMsN2L9pr1k1sj8Qks5qIE0hgaJpZM4Irofj
.

[sgotti]

@s-urbaniak good to see moving to vendor (and committing it). Probably the vendor directory size (while keeping legal files) should be (I haven't checked all the vendored packages) further reduced since some not required packages may be kept (glide-vc currently used the glide.lock file to get the required packages list but it can contain unneeded packages: see sgotti/glide-vc#14)

[s-urbaniak]

@sgotti thanks for the update!

Let's move this to the next release. glide-vc does not have sgotti/glide-vc#13 merged yet, this needs a rebase, Godeps changed, and I'm between planes travelling.

[jonboulle]

I think this should land first after 1.9.0 is cut.

[lucab]

Conflicting with #2837. Please take go-systemd bump into account when rebasing.

[jonboulle]

@squeed review?

[s-urbaniak]

@lucab addressed review comments, PTAL
@squeed PTAL :-)

[s-urbaniak]

merged changes from #2758

[lucab]

@s-urbaniak I'm fine with the changes. Tested the workflow before and was already working ok. If @squeed is ok with it, I think we can land it once CI tests are over.

[squeed]

This looks good to me!

I am wondering if we should have a script that warns / fails on unpinned packages. Unless I am not understanding this correctly, it seems possible that a glide update could have unintended consequences that would be lost in a large git diff.

[s-urbaniak]

@squeed We have quite a lot of transitive unpinned dependencies, also in Godeps. We can surely make more tooling to find the complement set in glide.lock compared to glide.yaml.

Before merging I'd like to have at least yet @alban and/or @iagus have a look at it.

[lucab]

@squeed I think it is the same discussion I split to Masterminds/glide#485

[squeed]

Those are interesting discussions, but I was thinking a bit simpler :-). I was trying to think of a way to tool around the issue in Masterminds/glide#252 by enforcing that all (non-transitive) dependencies are pinned..

It probably makes sense to just drop this and wait on 252.

[s-urbaniak]

reconfirmed OOB with @alban to merge it now