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: TestScript expects go.env to contain GOTOOLCHAIN=auto #60685

Closed
qmuntal opened this issue Jun 8, 2023 · 7 comments
Closed

cmd/go: TestScript expects go.env to contain GOTOOLCHAIN=auto #60685

qmuntal opened this issue Jun 8, 2023 · 7 comments
Labels
GoCommand cmd/go NeedsFix The path to resolution is known, but the work has not been done. release-blocker Testing An issue that has been verified to require only test changes, not just a test failure.
Milestone

Comments

@qmuntal
Copy link
Contributor

qmuntal commented Jun 8, 2023

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

$ go version
go version devel go1.21-b9baf4452f Thu Jun 8 01:52:35 2023 +0000 windows/amd64

Does this issue reproduce with the latest release?

Only with tip.

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

go env Output
$ go env

set GO111MODULE=
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users*\AppData\Local\go-build
set GOENV=C:\Users*
\AppData\Roaming\go\env
set GOEXE=.exe
set GOEXPERIMENT=
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOINSECURE=
set GOMODCACHE=C:\Users*\go\pkg\mod
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Users*
\go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=C:\Users*\code\golang-go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLCHAIN=local
set GOTOOLDIR=C:\Users*
\code\golang-go\pkg\tool\windows_amd64
set GOVCS=
set GOVERSION=devel go1.21-b9baf4452f Thu Jun 8 01:52:35 2023 +0000
set GCCGO=gccgo
set GOAMD64=v1
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=NUL
set GOWORK=
set CGO_CFLAGS=-O2 -g
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-O2 -g
set CGO_FFLAGS=-O2 -g
set CGO_LDFLAGS=-O2 -g
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -Wl,--no-gc-sections -fmessage-length=0 -ffile-prefix-map=C:\Users***\AppData\Local\Temp\go-build4262901046=/tmp/go-build -gno-record-gcc-switches

What did you do?

  1. Update GOROOT/go.env, replacing GOTOOLCHAIN=auto with GOTOOLCHAIN=local.
  2. Run go tool dist test -run cmd/go

What did you expect to see?

All tests passed.

What did you see instead?

Some tests failed.

=== Failed
=== FAIL: cmd/go TestScript/work_goline_order (0.15s)
=== FAIL: cmd/go TestScript/mod_go_version (0.04s)
=== FAIL: cmd/go TestScript/mod_get_future (0.08s)
=== FAIL: cmd/go TestScript/gotoolchain_modcmds (0.13s)
=== FAIL: cmd/go TestScript/gotoolchain_local (0.05s)
=== FAIL: cmd/go TestScript/goline_order (0.10s)
=== FAIL: cmd/go TestScript (0.33s)

As per CL 462198, the original intent of having go.env was this:

Various Linux distributions edit cmd/go/internal/cfg/cfg.go to change
the default settings of GOPROXY and GOSUMDB. Make it possible for
them to do this without editing the go command source code by
introducing GOROOT/go.env and moving those defaults there.

I'm trying to exercise this use case, changing the GOTOOLCHAIN default value instead (see #57001), and I can't get all tests to pass without patching a bunch of script tests.

@rsc I might be missing something, as this feature is quite new, but I'll tentatively label this as release-blocker because it is a potential blocker for Linux distros, at least it is for the one I support at Microsoft.

@qmuntal qmuntal changed the title cmd/go: TestScript expect go.env to contain GOTOOLCHAIN=auto cmd/go: TestScript expects go.env to contain GOTOOLCHAIN=auto Jun 8, 2023
@qmuntal qmuntal added this to the Go1.21 milestone Jun 8, 2023
@dr2chase
Copy link
Contributor

dr2chase commented Jun 8, 2023

@bcmills @rsc

@dr2chase dr2chase added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 8, 2023
@bcmills
Copy link
Member

bcmills commented Jun 8, 2023

@qmuntal, does setting GOTOOLCHAIN=auto in the default script environment help?

(If not, perhaps we need a test hook to override the default GOROOT/go.env.)

@bcmills bcmills added Testing An issue that has been verified to require only test changes, not just a test failure. GoCommand cmd/go labels Jun 8, 2023
@qmuntal
Copy link
Contributor Author

qmuntal commented Jun 8, 2023

@qmuntal, does setting GOTOOLCHAIN=auto in the default script environment help?

(If not, perhaps we need a test hook to override the default GOROOT/go.env.)

Almost, the only remaining failure is TestScript/gotoolchain_local:

script_test.go:158: FAIL: testdata/script/gotoolchain_local.txt:11: stdout auto: no match for `(?m)auto` in stdout

@bcmills
Copy link
Member

bcmills commented Jun 8, 2023

That one seems like just an erroneous expectation in the test. Want to send a CL to clean it up?

@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 Jun 9, 2023
@qmuntal
Copy link
Contributor Author

qmuntal commented Jun 9, 2023

That one seems like just an erroneous expectation in the test. Want to send a CL to clean it up?

Suro + done!

@gopherbot
Copy link

Change https://go.dev/cl/502035 mentions this issue: cmd/go: fix TestScript/gotoolchain* when go.env doesn't set GOTOOLCHAIN=auto

@bcmills
Copy link
Member

bcmills commented Jun 9, 2023

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
GoCommand cmd/go NeedsFix The path to resolution is known, but the work has not been done. release-blocker Testing An issue that has been verified to require only test changes, not just a test failure.
Projects
None yet
Development

No branches or pull requests

5 participants