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/go/internal/modfetch: erroneously resolves a v2.…+incompatible version when a v2/go.mod file exists #51324

Closed
bcmills opened this issue Feb 22, 2022 · 10 comments
Closed

cmd/go/internal/modfetch: erroneously resolves a v2.…+incompatible version when a v2/go.mod file exists #51324

bcmills opened this issue Feb 22, 2022 · 10 comments
Labels
FrozenDueToAge modules NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Milestone
Go1.18

Comments

@bcmills
Copy link
Contributor

bcmills commented Feb 22, 2022

CL 378400 appears to have introduced a regression for modules that contain a v2/go.mod file and not go.mod file at the repo root.

github.com/sacloud/libsacloud at commit b491c1c2c6bd6e18fd0b55ef6770ecd1e66f1419 is one such example.
At that revision, it contains a v2/go.mod file that correctly declares module github.com/sacloud/libsacloud/v2, and no go.mod file whatsoever at the module root. The corresponding v2 pseudo-version should therefore apply only to the v2 module (github.com/sacloud/libsacloud/v2) — it should not also be valid as a +incompatible version for the module at the root of the repo.

Go 1.17 as originally released correctly rejected that version for the repo root. go1.18rc1 does not.

$ export GOPRIVATE=github.com/sacloud/libsacloud

$ go clean -modcache

$ gotip list -m github.com/sacloud/libsacloud@v2.25.2-0.20210930043551-b491c1c2c6bd
github.com/sacloud/libsacloud v2.25.2-0.20210930043551-b491c1c2c6bd+incompatible

$ go clean -modcache

$ go1.17 list -m github.com/sacloud/libsacloud@v2.25.2-0.20210930043551-b491c1c2c6bd
go list -m: github.com/sacloud/libsacloud@v2.25.2-0.20210930043551-b491c1c2c6bd: invalid version: should be v0 or v1, not v2

(Found by @heschi during Go 1.18 pre-release testing.)

CC @matloob
@bcmills bcmills added modules NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker labels Feb 22, 2022
@bcmills bcmills self-assigned this Feb 22, 2022
@bcmills bcmills added this to the Go1.18 milestone Feb 22, 2022
@bcmills
Copy link
Contributor Author

bcmills commented Feb 22, 2022 •
edited
Loading

One more data point: apparently go1.17 did, in fact, allow a +incompatible suffix on the root module if it was requested explicitly:

$ go clean -modcache

$ go1.17 list -m github.com/sacloud/libsacloud@v2.25.2-0.20210930043551-b491c1c2c6bd+incompatible
github.com/sacloud/libsacloud v2.25.2-0.20210930043551-b491c1c2c6bd+incompatible

So the recent change in behavior is just that we now resolve the +incompatible version automatically instead of erring out.

@bcmills
Copy link
Contributor Author

bcmills commented Feb 23, 2022

@gopherbot, please backport to Go 1.16 and Go 1.17. This is a regression introduced in a security release (for #35671), and can cause confusion if a Go user accidentally passes the wrong module path to go get.

This was referenced Feb 23, 2022
Closed
Closed
@gopherbot
Copy link
Contributor

Backport issue(s) opened: #51331 (for 1.16), #51332 (for 1.17).

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

@bcmills bcmills mentioned this issue Feb 23, 2022
Open
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/387675 mentions this issue: cmd/go: avoid +incompatible major versions if a go.mod file exists in a subdirectory for that version

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/387917 mentions this issue: cmd/go/internal/modfetch: simplify handling of weird version tags

@bcmills
Copy link
Contributor Author

bcmills commented Feb 24, 2022

Reopening to track 1.18 cherry-pick.

@bcmills bcmills reopened this Feb 24, 2022
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/387921 mentions this issue: [release-branch.go1.18] cmd/go: avoid +incompatible major versions if a go.mod file exists in a subdirectory for that version

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/387922 mentions this issue: [release-branch.go1.17] cmd/go: avoid +incompatible major versions if a go.mod file exists in a subdirectory for that version

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/387923 mentions this issue: [release-branch.go1.16] cmd/go: avoid +incompatible major versions if a go.mod file exists in a subdirectory for that version

@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 Mar 3, 2022
gopherbot pushed a commit that referenced this issue Mar 3, 2022
@cagedmantis
[release-branch.go1.17] cmd/go: avoid +incompatible major versions if…
7f04645 
… a go.mod file exists in a subdirectory for that version

Previous versions of the 'go' command would reject a pseudo-version
passed to 'go get' if that pseudo-version had a mismatched major
version and lacked a "+incompatible" suffix. However, they would
erroneously accept a version *with* a "+incompatible" suffix even if
the repo contained a vN/go.mod file for the same major version, and
would generate a "+incompatible" pseudo-version or version if the user
requested a tag, branch, or commit hash.

This change uniformly rejects "vN.…" without "+incompatible", and also
avoids resolving to "vN.…+incompatible", when vN/go.mod exists.
To maintain compatibility with existing go.mod files, it still accepts
"vN.…+incompatible" if the version is requested explicitly as such
and the repo root lacks a go.mod file.

Fixes #51332
Updates #51324
Updates #36438

Change-Id: I2b16150c73fc2abe4d0a1cd34cb1600635db7139
Reviewed-on: https://go-review.googlesource.com/c/go/+/387675
Trust: Bryan Mills <bcmills@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
(cherry picked from commit 5a9fc94)
Reviewed-on: https://go-review.googlesource.com/c/go/+/387922
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
gopherbot pushed a commit that referenced this issue Mar 3, 2022
@cagedmantis
[release-branch.go1.16] cmd/go: avoid +incompatible major versions if…
02e5505 
… a go.mod file exists in a subdirectory for that version

Previous versions of the 'go' command would reject a pseudo-version
passed to 'go get' if that pseudo-version had a mismatched major
version and lacked a "+incompatible" suffix. However, they would
erroneously accept a version *with* a "+incompatible" suffix even if
the repo contained a vN/go.mod file for the same major version, and
would generate a "+incompatible" pseudo-version or version if the user
requested a tag, branch, or commit hash.

This change uniformly rejects "vN.…" without "+incompatible", and also
avoids resolving to "vN.…+incompatible", when vN/go.mod exists.
To maintain compatibility with existing go.mod files, it still accepts
"vN.…+incompatible" if the version is requested explicitly as such
and the repo root lacks a go.mod file.

Fixes #51331
Updates #51324
Updates #36438

Change-Id: I2b16150c73fc2abe4d0a1cd34cb1600635db7139
Reviewed-on: https://go-review.googlesource.com/c/go/+/387675
Trust: Bryan Mills <bcmills@google.com>
Reviewed-by: Michael Matloob <matloob@golang.org>
(cherry picked from commit 5a9fc94)
Reviewed-on: https://go-review.googlesource.com/c/go/+/387923
Run-TryBot: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Dmitri Shuralyov <dmitshur@golang.org>
@dmitshur dmitshur mentioned this issue Mar 3, 2022
Closed
33 tasks
@dmitshur
Copy link
Contributor

dmitshur commented Mar 3, 2022

Merged to Go 1.18 release branch via https://go.dev/cl/389554.

@dmitshur dmitshur closed this as completed Mar 3, 2022
gopherbot pushed a commit that referenced this issue May 10, 2022
@gopherbot
cmd/go/internal/modfetch: simplify handling of weird version tags
4d716e4 
This fixes an obscure bug in 'go list -versions' if the repo contains
a tag with an explicit "+incompatible" suffix. However, I've never
seen such a repo in the wild; mostly it's an attempt to wrap my brain
around the code and simplify things a bit for the future.

Updates #51324
Updates #51312

Change-Id: I1b078b5db36470cf61aaa85b5244c99b5ee2c842
Reviewed-on: https://go-review.googlesource.com/c/go/+/387917
Run-TryBot: Bryan Mills <bcmills@google.com>
Auto-Submit: Bryan Mills <bcmills@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Michael Matloob <matloob@golang.org>
@rsc rsc unassigned bcmills Jun 22, 2022
@golang golang locked and limited conversation to collaborators Jun 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge modules NeedsFix The path to resolution is known, but the work has not been done. release-blocker
Projects
None yet
Milestone
Go1.18
Development

No branches or pull requests
3 participants
@dmitshur @bcmills @gopherbot