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.

Sign up for GitHub

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

Jump to bottom

cmd/compile: Go 1.22 changes support for modules that declare go 1.0 #65528

Closed
dmitshur opened this issue Feb 5, 2024 · 6 comments
Closed

cmd/compile: Go 1.22 changes support for modules that declare go 1.0 #65528

dmitshur opened this issue Feb 5, 2024 · 6 comments
Assignees
@mdempsky
Labels
compiler/runtime Issues related to the Go compiler and/or runtime. 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
Go1.23

Comments

@dmitshur
Copy link
Contributor

dmitshur commented Feb 5, 2024 •
edited

Go version

go version go1.22rc2 darwin/arm64

Output of go env in your module/workspace:

(Shouldn't be relevant; happy to provide otherwise.)

What did you do?

I used a Go 1.22-ish toolchain, instead of a 1.21-ish one, to install the command go.chromium.org/luci/cipd/client/cmd/cipd@latest. It failed because one of its dependencies has a go module directive with an unusually-low value of "1.0" here, reporting a build error as:

# github.com/danjacques/gofslock/fslock
compile: invalid value "go1" for -lang: should be something like "go1.12"

Here's a smaller reproducer:

$ cd $(mktemp -d)
$ go mod init test
$ go get go@1.21 toolchain@none
$ echo 'package p; import _ "github.com/danjacques/gofslock/fslock"' > p.go
$ go get . github.com/danjacques/gofslock@v0.0.0-20230728142113-ae8f59f9e88b

What did you see happen?

$ go test
# github.com/danjacques/gofslock/fslock
compile: invalid value "go1" for -lang: should be something like "go1.12"
FAIL	test [build failed]

$ GOTOOLCHAIN=go1.21.6 go test
?   	test	[no test files]
$ echo $?
0

What did you expect to see?

I'm not sure if this edge-case should work or not.

I read over https://go.dev/doc/toolchain#version, which includes:

The syntax ‘1.N’ is called a “language version”. It denotes the overall family of Go releases implementing that version of the Go language and standard library.

But doesn't seem to specify what the minimum value of N is.

The new go/version package has IsValid, which reports true for "go1.0" and "go1". (https://go.dev/play/p/82xhtZbE9N9?v=gotip)
@bcmills bcmills changed the title cmd/go: Go 1.22 changes support for very old Go language versions cmd/go,cmd/compile: Go 1.22 changes support for modules that declare go 1.0 Feb 5, 2024
@gopherbot gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Feb 5, 2024
@bcmills bcmills added the GoCommand cmd/go label Feb 5, 2024
@bcmills
Copy link
Contributor

bcmills commented Feb 5, 2024

Both gover.Lang and version.Lang have tests that a bare "1" is considered a valid language version:

On the other hand, cmd/compile expects to parse the -lang flag using a regular expression:

So I think two things need to happen here:

  1. We should double check (with @rsc) that version.Lang("go1.0") really is intended to be go1 and not go1.0, and
  2. assuming that it is, cmd/compile should be updated so that the -lang flag accepts all versions for which version.Lang(x) == x.

@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 5, 2024
@bcmills bcmills added this to the Go1.23 milestone Feb 5, 2024
@bcmills bcmills added release-blocker and removed GoCommand cmd/go labels Feb 5, 2024
@bcmills bcmills changed the title cmd/go,cmd/compile: Go 1.22 changes support for modules that declare go 1.0 cmd/compile: Go 1.22 changes support for modules that declare go 1.0 Feb 7, 2024
@rsc
Copy link
Contributor

rsc commented Feb 7, 2024

Lang("go1.0") = "go1" is correct; the compiler should accept "go1" in addition what it currently accepts namely "go1.0".

@gopherbot
Copy link

Change https://go.dev/cl/562322 mentions this issue: cmd/compile: accept -lang=go1 as -lang=go1.0

@bcmills
Copy link
Contributor

bcmills commented Feb 8, 2024

@gopherbot, please backport to Go 1.22. This was a regression in 1.22.0 and the fix is trivial.

@gopherbot
Copy link

Backport issue(s) opened: #65619 (for 1.22).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.

@gopherbot gopherbot mentioned this issue Feb 8, 2024
Closed
@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 Feb 8, 2024
@mdempsky
Copy link
Member

mdempsky commented Feb 8, 2024 •
edited

Lang("go1.0") = "go1" is correct; the compiler should accept "go1" in addition what it currently accepts namely "go1.0".

On go.dev/cl/562322, I pointed out that cmd/go does not support "go 1" in go.mod files. @bcmills says this is expected because "go 1.0 is not the canonical name for that version."

I'll also point out that go mod edit -go=1 fails. (Both -toolchain=go1 and -toolchain=go1.0 are supported though.)

If cmd/go won't let its users spell Go language version "1.0" as "1", why did it start spelling "-lang=go1.0" as "-lang=go1"? Why is it cmd/compile's responsibility to now accept either spelling? Can cmd/go instead revert to the original spelling that it still requires from users?

Thanks.

kxxt added a commit to kxxt/gofslock that referenced this issue Feb 11, 2024
@kxxt
fix go version in go.mod
e196ad6 
Go 1.22 breaks modules that declare go 1.0: golang/go#65528

This PR modifies the Go version in go.mod to 1.14 because
module must run on Go version 1.14 or later, as documented in https://go.dev/doc/modules/gomod-ref
@kxxt kxxt mentioned this issue Feb 11, 2024
Merged
ezz-no pushed a commit to ezz-no/go-ezzno that referenced this issue Feb 18, 2024
@mdempsky @ezz-no
cmd/compile: accept -lang=go1 as -lang=go1.0
b796c40 
Fixes golang#65528.

Change-Id: I55fef8cf7be4654c7242462d45f12999e0c91c02
Reviewed-on: https://go-review.googlesource.com/c/go/+/562322
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Bryan Mills <bcmills@google.com>
Arno500 added a commit to Arno500/plex-richpresence that referenced this issue Feb 19, 2024
@Arno500
Fix CI by forcing Go 1.21
ddc2e32 
golang/go#65528
bobg added a commit to bobg/mingo that referenced this issue Mar 11, 2024
@bobg
Remove workaround now that a fix for golang/go#65528 is backported to…
0a0831b 
… Go 1.22.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mdempsky mdempsky

Labels
compiler/runtime Issues related to the Go compiler and/or runtime. 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
None yet
Milestone
Go1.23
Development

No branches or pull requests
5 participants
@mdempsky @rsc @dmitshur @bcmills @gopherbot