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

x/build: update dependencies in go.mod #30833

Closed
dmitshur opened this issue Mar 14, 2019 · 12 comments
Closed

x/build: update dependencies in go.mod #30833

dmitshur opened this issue Mar 14, 2019 · 12 comments
Labels
Builders x/build issues (builders, bots, dashboards) FrozenDueToAge modules NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@dmitshur
Copy link
Contributor

dmitshur commented Mar 14, 2019

The latest version of the golang.org/x/build module requires some older dependencies. Doing go get -u -m inside it currently fails with:

$ go get -u -m
...
go: github.com/golang/lint@v0.0.0-20190313153728-d0100b6bd8b3: parsing go.mod: unexpected module path "golang.org/x/lint"
...
go get: error loading module requirements
$ echo $?
1

See golang/lint#436 (comment).

/cc @bcmills @matloob @broady

@dmitshur dmitshur added Builders x/build issues (builders, bots, dashboards) NeedsFix The path to resolution is known, but the work has not been done. modules labels Mar 14, 2019
@gopherbot gopherbot added this to the Unreleased milestone Mar 14, 2019
@dmitshur
Copy link
Contributor Author

The build list of the latest version of the golang.org/x/build module includes the github.com/openzipkin/zipkin-go module at version v0.1.1:

$ go list -m all | grep zipkin
github.com/openzipkin/zipkin-go v0.1.1

That version is fine, but the the current latest version of github.com/openzipkin/zipkin-go module, v0.1.5, requires a bad version of grpc (see openzipkin/zipkin-go#114). I suspect when you do go get -u -m on x/build, it ends up trying to update the zipkin-go module to its latest version, which in turn brings in the bad grpc version and the incorrect github.com/golang/lint module that it requires.

I think that when zipkin-go makes a newer semver tag that includes the fix from openzipkin/zipkin-go#115 and in turn resolves openzipkin/zipkin-go#117, then this issue will get resolved too.

An alternative path to resolution here is to remove dependencies from x/build such that the zipkin-go module is no longer in the build list of x/build (but that's probably not easy to do).

This assumes that no modules in the build list explicitly require github.com/openzipkin/zipkin-go v0.1.5 or v0.1.4, the two bad versions (v0.1.3 and earlier are okay, they don't require a bad version of grpc). So far, I haven't seen either of those two versions explicitly required anywhere.

@dmitshur
Copy link
Contributor Author

I'll send a CL to remove the indirect dependency on grpc v1.16.0, we want to remove that as soon as possible.

Then, this module requires the cloud.google.com/go module, which in turn currently requires golang.org/x/build as well (this is tracked in googleapis/google-cloud-go#1344).

So, to make progress, x/build should drop its indirect requirement on grpc v1.16.0. Then the cloud.google.com/go module needs to be updated either to latest x/build pseudo-commit (without grpc v1.16.0), or not to depend on it at all. After that, x/build can be updated to depend on the new cloud.google.com/go version.

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/167597 mentions this issue: go.mod: drop indirect requirement of bad grpc version

gopherbot pushed a commit to golang/build that referenced this issue Mar 14, 2019
Version v1.16.0 of the grpc module is known to be bad (because it
doesn't include the fix from grpc/grpc-go#2393), so we don't want
to require it, even indirectly.

Commands run:

	go mod edit -droprequire=google.golang.org/grpc
	go mod tidy
	go test ./...

Updates golang/go#30833

Change-Id: I549dd5276ae19aa89c18cf2e9c981e81b8ffdd11
Reviewed-on: https://go-review.googlesource.com/c/build/+/167597
Run-TryBot: Dmitri Shuralyov <dmitshur@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
@jeanbza
Copy link
Member

jeanbza commented Mar 14, 2019

CL that bumps us to latest x/build: https://code-review.googlesource.com/c/gocloud/+/39010

Will tag that after it's in.

gopherbot pushed a commit to googleapis/google-cloud-go that referenced this issue Mar 14, 2019
gax@v2.0.4 upgrades its grpc dependency from v1.16.0 to v1.19.0: https://github.com/googleapis/gax-go/releases/tag/v2.0.4

golang.org/x/build v0.0.0-20190314133821-5284462c4bec removes an
indirect dependency on grpc@v1.16.0: golang/go#30833

This CL was generated by manually deleting the gax and x/build
dependencies in go.mod and running `go mod tidy`.

This CL will be tagged v0.37.1.

Relevant issue: #1359

Change-Id: I68dbfa8b0f58825c74092247a7825a296f3aac79
Reviewed-on: https://code-review.googlesource.com/c/gocloud/+/39010
Reviewed-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
@dmitshur
Copy link
Contributor Author

dmitshur commented Mar 14, 2019

@jadekler I'm beginning to realize the process I described in #30833 (comment) may not work.

https://code-review.googlesource.com/c/gocloud/+/39010 updates to the latest x/build pseudo-version, but that pseudo-version still requires a previous version of cloud, which in turn requires more old things...

Maybe the way to solve this is for x/build to issue a new pseudo-version (a new commit) that preemptively updates to the upcoming cloud tag. Then that pseudo-version of x/build can be used in the upcoming cloud tagged release. Does that make sense?

E.g., I'm spotting github.com/golang/lint in go.sum of that CL, which makes me think this won't fix the problem fully.

@jeanbza
Copy link
Member

jeanbza commented Mar 14, 2019

Ack. I'm happy to try it if you'd like, or we can punt. I think that fixing the core issue - #30831 - is probably the only "sane" solution heh. :)

@dmitshur
Copy link
Contributor Author

dmitshur commented Mar 14, 2019

I think that fixing the core issue - #30831 - is probably the only "sane" solution heh. :)

Sure, but a fix for that is unlikely to land before Go 1.13, which is still some time off. It'll prevent this general problem from re-occurring, but I imagine we can resolve this specific instance related to x/lint well before then.

Ack. I'm happy to try it if you'd like, or we can punt.

I've realized that current go.mod of x/build requires cloud.google.com/go@v0.31.0, which did not yet have a go.mod file, and therefore doesn't require an older x/build version.

That means you should be okay to proceed with tagging CL https://code-review.googlesource.com/c/gocloud/+/39010 (commit googleapis/google-cloud-go@88dee6a) as v0.37.1, and it will be a helpful change towards resolving the go get -u issue affecting the cloud module.

After you do that, I think I'm finding a relatively short path to resolve the rest of the problem in the cloud module, but it can be a separate follow up. I'll talk more about that in googleapis/google-cloud-go#1359.

gregoryduquesnoy pushed a commit to gregoryduquesnoy/google-cloud-go that referenced this issue Mar 20, 2019
gax@v2.0.4 upgrades its grpc dependency from v1.16.0 to v1.19.0: https://github.com/googleapis/gax-go/releases/tag/v2.0.4

golang.org/x/build v0.0.0-20190314133821-5284462c4bec removes an
indirect dependency on grpc@v1.16.0: golang/go#30833

This CL was generated by manually deleting the gax and x/build
dependencies in go.mod and running `go mod tidy`.

This CL will be tagged v0.37.1.

Relevant issue: googleapis#1359

Change-Id: I68dbfa8b0f58825c74092247a7825a296f3aac79
Reviewed-on: https://code-review.googlesource.com/c/gocloud/+/39010
Reviewed-by: kokoro <noreply+kokoro@google.com>
Reviewed-by: Jonathan Amsterdam <jba@google.com>
@jeanbza
Copy link
Member

jeanbza commented Mar 20, 2019

I've released 0.37.1. Is the next step to release a version of x/build which depends on something later than 0.31.0?

@dmitshur
Copy link
Contributor Author

@thepudds Did you close this issue unintentionally?

It's not yet fully resolved; doing go get -u -m on the x/build module still produces an error.

@jadekler Thanks for the update!

I don't think it's necessary to update x/build to require a newer version of cloud, because v0.31.0 did not yet have a go.mod file added yet, so it's not a bad version (because it doesn't require any bad versions of other modules).

@dmitshur dmitshur reopened this Mar 21, 2019
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/169517 mentions this issue: [dev.boringcrypto] misc/boring: add go1.12.1b4 and update build scripts

@thepudds
Copy link
Contributor

@dmitshur sorry, not sure what I did, but I did not mean to close this.

gopherbot pushed a commit that referenced this issue Mar 28, 2019
The inliner seems to have gotten a bit too smart in 1.12 and it made
sha1.boringNewSHA1 disappear. Replace it with the proper
crypto/internal/boring/sig.BoringCrypto signature. Also, switch the
negative signature to sha256.(*digest), since SHA-256 is used for sure
by cmd/go. Not using crypto/internal/boring/sig.StandardCrypto just to
be safe, in case the crypto/internal/boring/sig mechanism breaks.

Also, had to fight #30833 and #30515 to get
golang.org/x/build/cmd/release to build in modules mode.

Change-Id: I46f1471582fd77daae47d00baab975109902052d
Reviewed-on: https://go-review.googlesource.com/c/go/+/169517
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
Yawning added a commit to oasisprotocol/oasis-core that referenced this issue Apr 1, 2019
The libp2p overrides have been removed as the libraries appear to have
sane tagging now.  Process done semi-maually instead of via `go get -u`
because of golang/go#30833.
@dmitshur
Copy link
Contributor Author

dmitshur commented Apr 8, 2019

By now, none of the required module versions in x/build's go.mod requirement list are problematic. However, there were three remaining modules whose latest released version needed to be fixed (and until it was fixed, running go get -u on x/build would upgrade to them, triggering the problem).

The latest versions (released today) of the modules cloud.google.com/go, google.golang.org/api, and go.opencensus.io (v0.37.3+, v0.3.1+, and v0.20.1+, respectively) have expunged the problematic github.com/golang/lint module path from their build list.

As a result, this go get -u problem has been resolved in x/build as well.

Doing the following results in successful output:

$ cd $(mktemp -d)
$ git clone https://go.googlesource.com/build .
$ export GO111MODULE=on
$ go get -u -m
...
$ echo $?
0

Thank you very much @jadekler and everyone else involved for your work on resolving this problem!

@dmitshur dmitshur closed this as completed Apr 8, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Builders x/build issues (builders, bots, dashboards) FrozenDueToAge modules NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

No branches or pull requests

4 participants