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: mod tidy result different from previous go version #47847

Closed
andig opened this issue Aug 20, 2021 · 6 comments
Closed

cmd/go: mod tidy result different from previous go version #47847

andig opened this issue Aug 20, 2021 · 6 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Milestone

Comments

@andig
Copy link
Contributor

andig commented Aug 20, 2021

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

$ go version
go version go1.17 darwin/arm64

Does this issue reproduce with the latest release?

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

go env Output
$ go env
GO111MODULE=""
GOARCH="arm64"
GOBIN=""
GOCACHE="/Users/andig/Library/Caches/go-build"
GOENV="/Users/andig/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="arm64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/andig/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/andig/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.17"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/andig/htdocs/evcc/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/sv/rs_453y57xj86xsbz3kw1mbc0000gn/T/go-build667507156=/tmp/go-build -gno-record-gcc-switches -fno-common"
GOROOT/bin/go version: go version go1.17 darwin/arm64
GOROOT/bin/go tool compile -V: compile version go1.17
uname -v: Darwin Kernel Version 20.6.0: Wed Jun 23 00:26:27 PDT 2021; root:xnu-7195.141.2~5/RELEASE_ARM64_T8101
ProductName:	macOS
ProductVersion:	11.5.2
BuildVersion:	20G95
lldb --version: lldb-1205.0.28.2
Apple Swift version 5.4.2 (swiftlang-1205.0.28.2 clang-1205.0.19.57)

What did you do?

Running go mod tidy on top of evcc-io/evcc@f20f97a does not make changes. Going back to go 1.16.7, which is running on CI, I get

diff --git a/go.sum b/go.sum
index 5f514c59..59f5910b 100644
--- a/go.sum
+++ b/go.sum
@@ -1157,7 +1157,6 @@ honnef.co/go/tools v0.0.1-2020.1.3/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9
 honnef.co/go/tools v0.0.1-2020.1.4/go.mod h1:X/FiERA/W4tHapMX5mGpAtMSVEeEUOyHaw9vFzvIQ3k=
 mvdan.cc/sh v2.6.4+incompatible/go.mod h1:IeeQbZq+x2SUGBensq/jge5lLQbS3XT2ktyp3wrt4x8=
 nhooyr.io/websocket v1.6.5/go.mod h1:F259lAzPRAH0htX2y3ehpJe09ih1aSHN7udWki1defY=
-nhooyr.io/websocket v1.8.6 h1:s+C3xAMLwGmlI31Nyn/eAehUlZPwfYZu2JXM621Q5/k=
 nhooyr.io/websocket v1.8.6/go.mod h1:B70DZP8IakI65RVQ51MsWP/8jndNma26DVA/nFSCgW0=
 rsc.io/binaryregexp v0.2.0/go.mod h1:qTv7/COck+e2FymRvadv62gMdZztPaShugOCi3I+8D8=
 rsc.io/quote/v3 v3.1.0/go.mod h1:yEA65RcK8LyAZtP9Kv3t0HmxON59tX3rD+tICJqUlj0=

What did you expect to see?

Consistent behaviour while required go version is not changed.

What did you see instead?

Different treatment of go.sum in go 1.17

@mknyszek mknyszek added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 20, 2021
@mknyszek mknyszek added this to the Backlog milestone Aug 20, 2021
@mknyszek
Copy link
Contributor

As far as I'm aware, this is expected. See https://golang.org/doc/go1.17#go-command. Closing for now. Let me know if there's something else and I'll reopen.

@andig
Copy link
Contributor Author

andig commented Aug 20, 2021

I cannot find anything that would apply when required go version remains at 1.16? Imho reopen.

@andig
Copy link
Contributor Author

andig commented Aug 20, 2021

If a module specifies go 1.17 or higher…

does not apply here. It‘s still 1.16

@mknyszek
Copy link
Contributor

You're right. This paragraph

By default, go mod tidy verifies that the selected versions of dependencies relevant to the main module are the same versions that would be used by the prior Go release (Go 1.16 for a module that specifies go 1.17), and preserves the go.sum entries needed by that release even for dependencies that are not normally needed by other commands.

suggested to me that go mod tidy would ensure the same versions are selected, but the algorithm could still be different (so unnecessary dependencies or something could get removed just due to algorithm changes, but not pruned to the same extent as the module pruning capabilities of a go1.17 file).

That being said, I'm not actually sure if go mod tidy guarantees that it will generate exactly the same go.sum across releases. If it does, then that's a bug. If not, then I'm not sure there's anything to do here. Reopening for now anyway.

CC @bcmills @jayconrod @matloob

@mknyszek mknyszek reopened this Aug 20, 2021
@mknyszek mknyszek modified the milestones: Backlog, Go1.18 Aug 20, 2021
@jayconrod
Copy link
Contributor

I tried this out, but I'm seeing both Go 1.16.7 and 1.17 remove that line from go.sum. It doesn't seem like any packages in it are needed to build packages in the main module or their tests, so it's correct for go mod tidy to delete that sum.

@andig What does go mod why -m nhooyr.io/websocket show for 1.16.7? For 1.17? Does go mod verify indicate any problem?

@jayconrod jayconrod added the WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided. label Aug 20, 2021
@andig
Copy link
Contributor Author

andig commented Aug 21, 2021

This is very strange. Yesterday, 2 different developers were able to reproduce that the sum entry is NOT removed with go1.17 locally, while it was on CI with go1.16.7 (which is how we've found this). I was also able to repro that it IS removed with 1.16.6 locally.

Today, I've rerun everything for 1.16.6+7 and 1.17 and do locally get consistent behaviour- the entry IS always removed, even on go1.17.

Either there is flaky behaviour in 1.17 (unlikely) or we've both screwed up yesterday (likely).

I'll close for now and get back if I see this again- thank you!

@andig andig closed this as completed Aug 21, 2021
@golang golang locked and limited conversation to collaborators Aug 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. WaitingForInfo Issue is not actionable because of missing required information, which needs to be provided.
Projects
None yet
Development

No branches or pull requests

4 participants