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

ci: Require go.mod to be tidy #128

Merged
merged 1 commit into from
May 19, 2021
Merged

Conversation

rhcarvalho
Copy link
Contributor

This helps us prevent accidentally having an inconsistent go.mod file
after changes to dependencies.

@rhcarvalho
Copy link
Contributor Author

Waiting to see a Travis failure -- currently our go.mod is not tidy.
Will update the PR after confirming the .travis.yml config is good to catch future cases of untidiness :)

@rhcarvalho
Copy link
Contributor Author

rhcarvalho commented Dec 20, 2019

Travis job: https://travis-ci.com/getsentry/sentry-go/builds/141893202

  • Builds with GOMODULE111=on failed as expected (go.mod not tidy) except for master, in which go mod tidy didn't change go.mod but found a data race in TestNewDsn. (update: this is not a data race in sentry-go, but a problem in github.com/google/go-cmp, see Bump github.com/google/go-cmp to 0.4.0 (required for Go 1.14) #129)
  • Builds with GOMODULE111=off failed because it is wrong to run go mod tidy when modules are disabled.

TODO:

  • Do not run go mod tidy when GOMODULE111=off.

@rhcarvalho rhcarvalho force-pushed the tidy-gomod branch 2 times, most recently from a91b5a8 to f14cfad Compare December 20, 2019 15:14
@rhcarvalho
Copy link
Contributor Author

Travis job: https://travis-ci.com/getsentry/sentry-go/builds/141935681

  • GO111MODULE=off: all green.
  • GO111MODULE=on: Go 1.11, 1.12 and 1.13 red because master has a go.mod tidy issue -- fail as expected (or is it go mod tidy that has an issue? Seems likely for Go < 1.14).
    Go@master fails because of a test dependency -- go mod tidy did not change go.mod. Looks like its behavior is going to change in Go 1.14.
    I think the difference is attributable to cmd/go: 'mod tidy' chooses points in requirement cycles that differ from other subcommands golang/go#34086. In fact, with Go 1.13, running go mod tidy in sentry-go@master changes go.mod and then running go build reverts the change!
    Changing go.mod to match what go mod tidy wants would break the build/tests:
$ go mod tidy
$ git diff
diff --git a/go.mod b/go.mod
index a5a446f..ed7a7b2 100644
--- a/go.mod
+++ b/go.mod
@@ -22,7 +22,7 @@ require (
        github.com/pkg/errors v0.8.1
        github.com/sergi/go-diff v1.0.0 // indirect
        github.com/smartystreets/goconvey v1.6.4 // indirect
-       github.com/ugorji/go v1.1.7 // indirect
+       github.com/ugorji/go/codec v1.1.7 // indirect
        github.com/urfave/negroni v1.0.0
        github.com/valyala/fasthttp v1.6.0
        github.com/xeipuuv/gojsonschema v1.2.0 // indirect
$ GOFLAGS=-mod=readonly go build
go: updates to go.mod needed, disabled by -mod=readonly
$ GOFLAGS=-mod=readonly go test
go: updates to go.mod needed, disabled by -mod=readonly

@rhcarvalho
Copy link
Contributor Author

Once Go 1.14 gets released, its go mod tidy will possibly disagree with 1.13 and earlier.
That would make running go mod tidy in CI awkward.

Putting this on hold to evaluate the situation better.

@rhcarvalho rhcarvalho force-pushed the tidy-gomod branch 2 times, most recently from c11deeb to 0403321 Compare May 19, 2021 14:50
This helps us prevent accidentally having an inconsistent go.mod file
after changes to dependencies.
@rhcarvalho rhcarvalho marked this pull request as ready for review May 19, 2021 14:59
@rhcarvalho
Copy link
Contributor Author

Revived this.

We can have a check for "tidiness" as long as we agree on a single authoritative Go version to use.

One day this might become part of the go command, see https://togithub.com/golang/go/issues/27005.

@rhcarvalho rhcarvalho enabled auto-merge (squash) May 19, 2021 15:20
@rhcarvalho rhcarvalho merged commit 20c242c into getsentry:master May 19, 2021
@rhcarvalho rhcarvalho deleted the tidy-gomod branch May 19, 2021 15:26
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