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: 'go list -m -u all' in 1.17 may fail due to disallowed go.sum updates #47377

Closed
dmitshur opened this issue Jul 24, 2021 · 6 comments
Closed
Labels
FrozenDueToAge GoCommand cmd/go modules NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone

Comments

@dmitshur
Copy link
Contributor

dmitshur commented Jul 24, 2021

What version of Go are you using (go version)?

$ go version
go version go1.17rc1 darwin/arm64

(It also reproduces with latest master commit, 0914646).

Does this issue reproduce with the latest release?

Not with Go 1.16.6.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
GO111MODULE=""
GOARCH="arm64"
GOBIN=""
GOCACHE="/Users/dmitri/Library/Caches/go-build"
GOENV="/Users/dmitri/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="arm64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/dmitri/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/dmitri/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/darwin_arm64"
GOVCS=""
GOVERSION="go1.17rc1"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/dmitri/Dmitri/go/dmitri.shuralyov.com/website/gido/go.mod"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/folders/_0/h0671fcn4rgb5pn9c745dx2h0000gn/T/go-build3781199445=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

Using a fairly default configuration of Go (i.e., I'm not overriding -mod value or anything), I ran go list -m -u all in a tidy module to find information about available upgrades.

(Its go.mod and go.sum files were updated to select modern dependency versions as of a few months ago, followed by a very recent run of go mod tidy -go=1.17.)

Complete reproducer (non-minified)
$ export GOPATH=$(mktemp -d)
$ cd $(mktemp -d)
$ git clone https://dmitri.shuralyov.com/website/gido
$ cd gido
$ git checkout f460621dc784d8a8f3a1593a068115b62baf214d
$ curl -sO 'https://gist.githubusercontent.com/dmitshur/c99a2233e5111713338cc2fb66dea5be/raw/c4ae92ac158c71b3a0b58ebf0e603103132498be/go.mod'
$ curl -sO 'https://gist.githubusercontent.com/dmitshur/c99a2233e5111713338cc2fb66dea5be/raw/c4ae92ac158c71b3a0b58ebf0e603103132498be/go.sum'
$ go list -m -u all
go: updates to go.sum needed, disabled by -mod=readonly

What did you expect to see?

Successful operation without needing to take additional action. E.g.:

$ go list -m -u all
dmitri.shuralyov.com/website/gido
cloud.google.com/go v0.54.0 [v0.88.0]
cloud.google.com/go/bigquery v1.4.0 [v1.19.0]
cloud.google.com/go/datastore v1.1.0 [v1.5.0]
cloud.google.com/go/pubsub v1.2.0 [v1.13.0]
cloud.google.com/go/storage v1.6.0 [v1.16.0]
contrib.go.opencensus.io/exporter/prometheus v0.3.0
[...]

What did you see instead?

$ go list -m -u all
go: updates to go.sum needed, disabled by -mod=readonly
$ echo $?
1

This is a behavior change in 1.17 compared to 1.16. I'm not completely sure whether it's an unintentional regression or an intentional change, so reporting as early as I ran into it in case it's helpful.

CC @bcmills, @jayconrod, @matloob.

@dmitshur dmitshur added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. GoCommand cmd/go modules labels Jul 24, 2021
@dmitshur dmitshur added this to the Go1.17 milestone Jul 24, 2021
@dmitshur dmitshur changed the title cmd/go: 'go get -m -u all' in 1.17 may fail due to disallowed go.sum updates cmd/go: 'go list -m -u all' in 1.17 may fail due to disallowed go.sum updates Jul 24, 2021
@jayconrod jayconrod self-assigned this Jul 26, 2021
@jayconrod
Copy link
Contributor

Reproduced with this new test case. The module is in the cmd/go test suite, not a real module.

go list -m -u rsc.io/breaker

-- go.mod --
module m

go 1.16

require rsc.io/breaker v1.0.0
-- go.sum --
rsc.io/breaker v1.0.0/go.mod h1:s5yxDXvD88U1/ESC23I2FK3Lkv4YIKaB1ij/Hbm805g=

The sum that was causing the error was github.com/hpcloud/tail v1.0.0. There was already a sum for github.com/hpcloud/tail v1.0.0/go.mod, since that's the currently required version. go list -m -u should not add sums for zip files. Actually, it shouldn't even fetch zip files.

Debugging a bit, it looks like github.com/hpcloud/tail has another version, v2.10.6-bug100770-inotify-leak+incompatible. This would be considered the latest version shown by go list -m -u if v1.0.0 did not have a go.mod file. As part of the query process, when there are incompatible versions, the go command looks at the highest version for each major and checks whether it has a go.mod file. Higher major versions are assumed to be incorrectly tagged or unwanted.

The function that checks whether a version has a go.mod file is actually downloading the zip (hitting the .zip endpoint on the proxy). It seems faster to hit the .mod endpoint instead and check whether that file looks synthetic. If the original commit didn't have a go.mod file, the go command acts as if it had a go.mod file that only contains a module directive (no go directive or requirements). I'll try making that change tomorrow. Might make go list -m -u faster in addition to fixing this.

@gopherbot
Copy link

Change https://golang.org/cl/337850 mentions this issue: cmd/go: use .mod instead of .zip to determine if version has go.mod file

@cagedmantis
Copy link
Contributor

cagedmantis commented Jul 27, 2021

The release team is working to include this in the upcoming RC2.

@toothrot
Copy link
Contributor

toothrot commented Jul 27, 2021

I also verified that this CL fixes this issue for x/build's dependencies as well.

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jul 28, 2021
@cagedmantis
Copy link
Contributor

Is it possible that this fix introduced a test failure in the post-submit Linux longtest builder runs? I'm seeing failures in linux-386-longtest and linux-amd64-longtest.

@jayconrod
Copy link
Contributor

@cagedmantis Very likely. I've opened #47444 to track. Will fix or revert soon.

@golang golang locked and limited conversation to collaborators Jun 23, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge GoCommand cmd/go 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

5 participants