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: bootstrapped vet tool for 1.23 lacks GODEBUG=gotypesalias=1 #68797

Closed
findleyr opened this issue Aug 8, 2024 · 9 comments
Closed

cmd/go: bootstrapped vet tool for 1.23 lacks GODEBUG=gotypesalias=1 #68797

findleyr opened this issue Aug 8, 2024 · 9 comments
Assignees
Labels
FixPending Issues that have a fix which has not yet been reviewed or submitted. NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone

Comments

@findleyr
Copy link
Contributor

findleyr commented Aug 8, 2024

As reported in #68796, it appears that the cmd/vet built by make.bash lacks GODEBUG=gotypesalias=1, which should have been the default in 1.23. See below: the bug of #68796 only appears if GODEBUG=gotypesalias=1 is explicitly set in the environment, or if cmd/vet is reinstalled (apparently, reinstalling picks up the correct GODEBUG defaults).

While I think #68796 is a release blocker, this may not be a release blocker: if cmd/vet works as expected, it doesn't really matter which gotypesalias value it observes.

> go version
go version go1.23rc2 linux/amd64
> go vet bad.go 
# command-line-arguments
# [command-line-arguments]
./bad.go:6:2: fmt.Printf format %s has arg 123 of wrong type int
./bad.go:7:2: command-line-arguments.wrapf format %s has arg 123 of wrong type int
> GODEBUG=gotypesalias=1 go vet bad.go 
# command-line-arguments
# [command-line-arguments]
./bad.go:6:2: fmt.Printf format %s has arg 123 of wrong type int
> go install cmd/vet
> go vet bad.go
# command-line-arguments
# [command-line-arguments]
./bad.go:6:2: fmt.Printf format %s has arg 123 of wrong type int

CC @matloob @samthanawalla

@findleyr findleyr added this to the Go1.23 milestone Aug 8, 2024
@findleyr
Copy link
Contributor Author

findleyr commented Aug 8, 2024

@adonovan notices the following:

> go version -m $(which go)
/usr/local/google/home/rfindley/src/go/bin/go: go1.23rc2
        path    cmd/go
        build   -buildmode=exe
        build   -compiler=gc
        build   -gcflags=cmd/...=-dwarf=false
        build   -trimpath=true
        build   DefaultGODEBUG=asynctimerchan=1,gotypesalias=0,httplaxcontentlength=1,httpmuxgo121=1,httpservecontentkeepheaders=1,netedns0=0,panicnil=1,tls10server=1,tls3des=1,tlskyber=0,tlsrsakex=1,tlsunsafeekm=1,winreadlinkvolume=0,winsymlink=0,x509keypairleaf=0,x509negativeserial=1
        build   CGO_ENABLED=0
        build   GOARCH=amd64
        build   GOOS=linux
        build   GOAMD64=v1

Note the value of DefaultGODEBUG has gotypesalias=0. That is wrong. Marking as a release blocker until we understand better.

@dmitshur dmitshur added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Aug 8, 2024
@ianlancetaylor
Copy link
Contributor

I know I'm confused, but I thought that gotypesalias=1 was going to be the default for 1.24, not for 1.23.

@adonovan
Copy link
Member

adonovan commented Aug 9, 2024

I know I'm confused, but I thought that gotypesalias=1 was going to be the default for 1.24, not for 1.23.

Typically when you say you're confused, it's because I am mistaken and have confused you, but not in this instance:

https://tip.golang.org/doc/go1.23#gotypespkggotypes:

By default, go/types now produces Alias type nodes for type aliases. This behavior can be controlled by the GODEBUG gotypesalias flag. Its default has changed from 0 in Go 1.22 to 1 in Go 1.23.

I had to check to convince myself. ;-)

@hyangah
Copy link
Contributor

hyangah commented Aug 9, 2024

@adonovan notices the following:

> go version -m $(which go)
/usr/local/google/home/rfindley/src/go/bin/go: go1.23rc2
        path    cmd/go
        build   -buildmode=exe
        build   -compiler=gc
        build   -gcflags=cmd/...=-dwarf=false
        build   -trimpath=true
        build   DefaultGODEBUG=asynctimerchan=1,gotypesalias=0,httplaxcontentlength=1,httpmuxgo121=1,httpservecontentkeepheaders=1,netedns0=0,panicnil=1,tls10server=1,tls3des=1,tlskyber=0,tlsrsakex=1,tlsunsafeekm=1,winreadlinkvolume=0,winsymlink=0,x509keypairleaf=0,x509negativeserial=1
        build   CGO_ENABLED=0
        build   GOARCH=amd64
        build   GOOS=linux
        build   GOAMD64=v1

Note the value of DefaultGODEBUG has gotypesalias=0. That is wrong. Marking as a release blocker until we understand better.

I am confused.
Isn't it expected when the go binary is built with the bootstrap go (<go1.23)? Or are toolchain binaries of go1.N supposed to be rebuilt with go1.N to pick up new GODEBUG (some may not be known to the bootstrap go)?

@dmitshur
Copy link
Contributor

dmitshur commented Aug 9, 2024

I believe the latter is intended, see here and here.

My current (and possibly mistaken or only partial) understanding of what's happening is that the cmd/go binary is built in a temporary work directory that has a go.mod file (to fix #44209), and that go.mod file doesn't contain a go directive, so cmd/go ends up falling back to the default of 1.16. At tip, including "go 1.24" in that temporary go.mod file seems to be enough to fix the original reproducer.

 workdir = xworkdir()
-if err := os.WriteFile(pathf("%s/go.mod", workdir), []byte("module bootstrap"), 0666); err != nil {
+if err := os.WriteFile(pathf("%s/go.mod", workdir), []byte("module bootstrap\ngo 1.24\n"), 0666); err != nil {
 	fatalf("cannot write stub go.mod: %s", err)
 }

@findleyr
Copy link
Contributor Author

findleyr commented Aug 9, 2024

Based on internal discussions, it sounds like we're comfortable with the status quo for 1.23, since this GODEBUG issue has existed since Go 1.17. We can fix for 1.24. Then we also don't need to backport the fix for #68796 to 1.23. Thanks everyone for your help with the urgent investigation here.

@findleyr findleyr modified the milestones: Go1.23, Go1.24 Aug 9, 2024
@ianlancetaylor
Copy link
Contributor

Thanks for the explanations, sorry for the noise.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/604799 mentions this issue: cmd/dist: set go version in bootstrap go.mod file

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. FixPending Issues that have a fix which has not yet been reviewed or submitted. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Aug 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FixPending Issues that have a fix which has not yet been reviewed or submitted. NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Projects
Status: Done
Development

No branches or pull requests

8 participants