Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

*: update dependencies #324

Merged
merged 3 commits into from
Jun 19, 2020
Merged

Conversation

cyphar
Copy link
Member

@cyphar cyphar commented Jun 10, 2020

We haven't done a dependency bump in quite a while, and there are a fair
few fixes and improvements we should get into umoci before the next
release.

% go get -u
go: github.com/apex/log upgrade => v1.4.0
go: github.com/cpuguy83/go-md2man/v2 upgrade => v2.0.0
go: github.com/golang/protobuf upgrade => v1.4.2
go: github.com/klauspost/compress upgrade => v1.10.9
go: github.com/klauspost/cpuid upgrade => v1.3.0
go: github.com/klauspost/pgzip upgrade => v1.2.4
go: github.com/konsorten/go-windows-terminal-sequences upgrade => v1.0.3
go: github.com/opencontainers/go-digest upgrade => v1.0.0
go: github.com/opencontainers/runtime-spec upgrade => v1.0.2
go: github.com/pkg/errors upgrade => v0.9.1
go: github.com/sirupsen/logrus upgrade => v1.6.0
go: github.com/vbatts/go-mtree upgrade => v0.5.0
go: golang.org/x/crypto upgrade => v0.0.0-20200604202706-70a84ac30bf9
go: golang.org/x/net upgrade => v0.0.0-20200602114024-627f9648deb9
go: golang.org/x/sys upgrade => v0.0.0-20200615200032-f1bc736245b1
go: google.golang.org/protobuf upgrade => v1.24.0

However there are two issues with this update:

  • We cannot update github.com/urfave/cli to anything later than
    v1.22.1 because of a bug when it comes to StringSliceFlag parsing that we
    hit in CI
    -- hence the new excludes block.

  • Updating github.com/klauspost/compress to anything later than v1.8.6 causes
    us to generate different gzip-compressed blobs due to an optimisation in
    their compression
    . Since this is generally a good change to have, we have
    to update our CI so that it works with the newest version (even if it's
    sub-optimal to generate different bytes between versions).

We need to temporarily hardcode go1.14 as a package name because of issues with
the openSUSE go metapackage
. We also need to add a dummy "rootless" user so
that we can avoid issues with Go being confused by where the GOCACHE should
live. There are also some minor improvements to the vendoring tests, and
we have to re-vendor the dependencies because vendor/ has changed under
Go 1.14.

Signed-off-by: Aleksa Sarai asarai@suse.de

@cyphar
Copy link
Member Author

cyphar commented Jun 18, 2020

Okay, so we've now entered the era of build errors that actually make no sense:

go build  -tags "" -buildmode=pie -ldflags "-s -w -X main.gitCommit="5925696bd511439db158f9dc779bdc4645bbb7d2-dirty" -X main.version=0.4.5+dev" -o ./umoci github.com/opencontainers/umoci/cmd/umoci
go: inconsistent vendoring in /home/travis/gopath/src/github.com/opencontainers/umoci:
	github.com/cpuguy83/go-md2man@v1.0.10: is explicitly required in go.mod, but not marked as explicit in vendor/modules.txt

Note that there isn't a github.com/cpuguy83/go-md2man dependency of any kind, and the only dependency of github.com/cpuguy83/go-md2man/v2 is indirect and is of v2.0.0 not v1.0.10. Also why the hell does the CI think that the tree is dirty? o.O

@tych0
Copy link
Member

tych0 commented Jun 18, 2020

Sadly in my experience go after 1.11 introduces a lot of these kinds of errors :(

When building my own clone locally, I get:

go build  -tags "" -buildmode=pie -ldflags "-s -w -X main.gitCommit="391b0bb74258612f57914b2df823e86333c91e90" -X main.version=0.4.5+dev" -o ./umoci github.com/opencontainers/umoci/cmd/umoci
can't load package: package github.com/opencontainers/umoci/cmd/umoci: cannot find package "github.com/opencontainers/umoci/cmd/umoci" in any of:
	/usr/local/go/src/github.com/opencontainers/umoci/cmd/umoci (from $GOROOT)
	/home/tycho/packages/go/src/github.com/opencontainers/umoci/cmd/umoci (from $GOPATH)

@cyphar
Copy link
Member Author

cyphar commented Jun 18, 2020

@tych0 Did you move the repo to $GOPATH/src/github.com/opencontainers/umoci? This PR auto-sets GO111MODULE=on in the Makefile which should avoid those types of build failures in the future.

@cyphar
Copy link
Member Author

cyphar commented Jun 19, 2020

I figured out the odd dependency issue. The problem is that with Go modules, go get inside a Go module directory results in the program being added to the go.mod instead of being installed. Travis runs inside the source repo, it turns out we were doing go get github.com/cpuguy83/go-md2man in the before_script which means it pulls the latest 1.x release (but in this PR we switched to the v2 branch of releases), and thus you end up with issues with vendoring. Should be fixed now. sigh

@cyphar cyphar force-pushed the update-dependencies branch 2 times, most recently from dc76216 to af02cb5 Compare June 19, 2020 08:14
We need to temporarily hardcode go1.14 as a package name because of issues with
the openSUSE go metapackage[1]. We also need to add a dummy "rootless" user so
that we can avoid issues with Go being confused by where the GOCACHE should
live. There are also some minor improvements to the vendoring tests, and
we have to re-vendor the dependencies because vendor/ has changed under
Go 1.14.

In addition, Go 1.14 now makes "go get" import code into the current
module which causes issues if you actually want to install the bloody
tool (causing "fun" issues like dependencies appearing out of nowhere
and then causing compilation issues becuase the dependency wasn't
vendored). So we have to set GO111MODULE=off in .travis.yml where
applicable.

[1]: https://bugzilla.opensuse.org/show_bug.cgi?id=1172608

Signed-off-by: Aleksa Sarai <asarai@suse.de>

squash! ci: actually use Go 1.14
We haven't done a dependency bump in quite a while, and there are a fair
few fixes and improvements we should get into umoci before the next
release.

  % go get -u
  go: github.com/apex/log upgrade => v1.4.0
  go: github.com/cpuguy83/go-md2man/v2 upgrade => v2.0.0
  go: github.com/golang/protobuf upgrade => v1.4.2
  go: github.com/klauspost/compress upgrade => v1.10.9
  go: github.com/klauspost/cpuid upgrade => v1.3.0
  go: github.com/klauspost/pgzip upgrade => v1.2.4
  go: github.com/konsorten/go-windows-terminal-sequences upgrade => v1.0.3
  go: github.com/opencontainers/go-digest upgrade => v1.0.0
  go: github.com/opencontainers/runtime-spec upgrade => v1.0.2
  go: github.com/pkg/errors upgrade => v0.9.1
  go: github.com/sirupsen/logrus upgrade => v1.6.0
  go: github.com/vbatts/go-mtree upgrade => v0.5.0
  go: golang.org/x/crypto upgrade => v0.0.0-20200604202706-70a84ac30bf9
  go: golang.org/x/net upgrade => v0.0.0-20200602114024-627f9648deb9
  go: golang.org/x/sys upgrade => v0.0.0-20200615200032-f1bc736245b1
  go: google.golang.org/protobuf upgrade => v1.24.0

However there are three issues with this update:

 * We cannot update github.com/urfave/cli to anything later than
   v1.22.1 because of a bug when it comes to StringSliceFlag parsing that we
   hit in CI[1] -- hence the new excludes block.

 * Updating github.com/klauspost/compress to anything later than v1.8.6 causes
   us to generate different gzip-compressed blobs due to an optimisation in
   their compression[2]. Since this is generally a good change to have, we have
   to update our CI so that it works with the newest version (even if it's
   sub-optimal to generate different bytes between versions).

 * Updating github.com/cpuguy83/go-md2man to v2 caused issues with our
   "go get" invocation. It turns out we were silently adding go-md2man
   (v1) to our go.mod file each time we ran a Travis build, and the
   switch to v2 uncovered this issue. This is easily fixed by setting
   GO111MODULE=off when doing 'go get' in .travis.yml.

[1]: urfave/cli#1152
[2]: klauspost/compress#105

Signed-off-by: Aleksa Sarai <asarai@suse.de>
This might make running the tests a little bit quicker (maybe even more
so if we add t.Parallel() calls in the future), and it removes the need
for collating cover-profile output for each unit test run.

Signed-off-by: Aleksa Sarai <asarai@suse.de>
@cyphar
Copy link
Member Author

cyphar commented Jun 19, 2020

LGTM. This should be ready for review now.

/ping @tych0 @vbatts

@tych0
Copy link
Member

tych0 commented Jun 19, 2020

Did you move the repo to $GOPATH/src/github.com/opencontainers/umoci? This PR auto-sets GO111MODULE=on in the Makefile which should avoid those types of build failures in the future.

It's outside my $GOPATH, so it should be on by default. I assume it's just some caching bug, fresh clones in a container work. But removing ~/.cache/go-build and $GOPATH/pkg wasn't enough /shrug.

@tych0
Copy link
Member

tych0 commented Jun 19, 2020

LGTM

@cyphar cyphar merged commit 9734388 into opencontainers:master Jun 19, 2020
@cyphar cyphar deleted the update-dependencies branch June 19, 2020 15:00
@cyphar
Copy link
Member Author

cyphar commented Jun 19, 2020

Huh, that's a bit strange. 🤷

On a side note, next time I should do go get -u all. It only affects transitive packages but ideally we should be using the latest version of everything. Oh well, I'll do that next time (the only changes we missed in this PR as to some miscellaneous pretty-printing libraries).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants