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

cmd/go: add a 'go mod tidy' flag to provide checksums needed by older versions of the toolchain #46141

Closed
bcmills opened this issue May 12, 2021 · 5 comments
Labels
FrozenDueToAge modules NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone

Comments

@bcmills
Copy link
Contributor

bcmills commented May 12, 2021

While working on CL 318809, @stamblerre noticed an interesting wrinkle with go mod tidy in Go 1.17 modules (#36460).

go mod tidy in a Go 1.17 module preserves only the checksums needed for Go 1.17 itself to be able to build in the main module. However, we also build (and test) the golang.org/x/tools repo using Go 1.15 and 1.16.

The go.mod file for a 1.17 module is (intentionally) reasonable for Go 1.15 and 1.16 consumers, but the go.sum file that go mod tidy currently generates is too small for builds using older versions: it lists only the checksums needed by Go 1.17 (with pruned module graphs), not 1.16 and earlier (with transitive-closure module graphs).

That's still ok in general for Go 1.15 and 1.16 users working with public dependencies, because they can download any missing checksums from the checksum server. However, there are a few more wrinkiles:

  • In Go 1.16 we changed the default for the -mod flag to -mod=readonly (cmd/go: default to & improve -mod=readonly #40728), which intentionally also avoids downloading missing checksums (cmd/go: go build -mod=readonly should disallow go.sum updates #30667).

  • When we run Go project tests using Go 1.15 and earlier, we intentionally firewall off network access to prevent tests from unintentionally relying on external data. That firewall prevents access to sum.golang.org. I think it is reasonable for Go users in general to similarly firewall off their tests, and we certainly do encourage Go users to test against all Go versions their project intends to support.

Plan

I discussed this problem with @jayconrod, @matloob, and @rsc, and we concluded that the most ergonomic fix is:

  1. Add a -compat flag to go mod tidy, parallel to the -go flag (cmd/go: add a '-go' flag to 'go mod tidy' #45094), to set the minimum Go version with which builds invoked within the main module should be able to succeed. go mod tidy will make a best effort to preserve checksums needed by the indicated version of the go command, but any errors in otherwise-unnecessary transitive dependencies will be ignored.

  2. Default that flag to 1.16 or earlier until some future release, so that Go users will by default save enough checksums to allow older Go versions to build packages within their module. (I'm not 100% sure about this part.)

The above will fix builds for users with older versions of the go command provided that the selected versions of explicitly-required modules in the Go 1.17 dependency graph are identical to the versions selected for those modules in the pre-1.17 dependency graph. (If the Go 1.17 versions are lower, users on Go 1.16 and lower will need to either 1.17 modules using -mod=mod so that the versions can float upward, using -mod=vendor so that the module graph is ignored, or will need the additional change described in #46137.)

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/320172 mentions this issue: cmd/go: add tests illustrating what happens when Go 1.16 is used in a Go 1.17 main module

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/321069 mentions this issue: cmd/go/internal/modcmd: factor out a type for flags whose arguments are Go versions

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/321070 mentions this issue: cmd/go/internal/modload: set the default GoVersion in a single location

@gopherbot
Copy link
Contributor

Change https://golang.org/cl/321071 mentions this issue: cmd/go: add a -compat flag to 'go mod tidy'

gopherbot pushed a commit that referenced this issue May 24, 2021
…re Go versions

For #46141
Updates #45094

Change-Id: I6553600c69273762a81795ef021c66f4e0872b6b
Reviewed-on: https://go-review.googlesource.com/c/go/+/321069
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
gopherbot pushed a commit that referenced this issue May 24, 2021
For #46141
Updates #36460

Change-Id: Ie4c13c73a451650d1e8abb8e5cebfc30d0a71a70
Reviewed-on: https://go-review.googlesource.com/c/go/+/321070
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
gopherbot pushed a commit that referenced this issue May 24, 2021
… Go 1.17 main module

For #46141
Updates #46160

Change-Id: Ib22435b8051aaf3fa74d43d3b7f2d091e67f05e2
Reviewed-on: https://go-review.googlesource.com/c/go/+/320172
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Jay Conrod <jayconrod@google.com>
@gopherbot
Copy link
Contributor

Change https://golang.org/cl/325910 mentions this issue: doc/go1.17: add a release note for the '-compat' flag to 'go mod tidy'

gopherbot pushed a commit that referenced this issue Jun 8, 2021
Updates #46141

Change-Id: I7a6a84f816e3db19bb492f862366a29dc46ed2ee
Reviewed-on: https://go-review.googlesource.com/c/go/+/325910
Trust: Bryan C. Mills <bcmills@google.com>
Run-TryBot: Bryan C. Mills <bcmills@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
@golang golang locked and limited conversation to collaborators Jun 7, 2022
@rsc rsc unassigned bcmills Jun 23, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge modules NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Projects
None yet
Development

No branches or pull requests

2 participants