[go] cmd/go: update default go directive in mod init
Ian Alexander (Gerrit)
Ian Alexander voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
- Dmitri Shuralyov
- Michael Matloob
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Matloob (Gerrit)
Michael Matloob added 8 comments![]( "Open in Gerrit")
func LocalInit() string {We should add this function to the modload package since it's only relevant to `go mod init` and `go work init`.
(this package is a "lower level" package that should have more generally applicable functions)
We should probably choose a name that more clearly indicates that it's the default go version in a new module. (init is an overloaded term, for example modload.Init or the init function in a package, so it can be unclear which meaning it refers to) Don't have a super good one off the top of my head but maybe `DefaultModInitGoVersion` ?
if strings.Count(v, ".") < 2 {Could we parse the version to figure out if it's a language or release version? Ideally we shouldn't be doing manual string operations on it. If not, maybe we can add a comment about why we can't parse it?
env TESTGO_VERSION=go1.26.0add a comment saying we need this to be able to check produced mod files for equality?
cmpenv go.mod $WORK/go.mod.initthese can be cmp now that we don't use `$goversion`
cmp go.mod go.mod.want1maybe we can name the want files based on the version they contain so it's easier to see what the test is looking for by reading the test case without looking down to see the contents of the files?
env TESTGO_VERSION=go1.201.18.0 never existed (and would never be returned by `gover.LocalInit` in practice), so it would probably be better to bump all the version up so that it's greater than 1.23 (so that the version returned by LocalInit is > 1.21.0)
cmp go.work go.work.want1see comment on the mod_init_version test case: could we name these files semantically like `go.work.want_1_24_0`?
env TESTGO_VERSION=go1.21.0why did we need to make this change?
Related details
- Dmitri Shuralyov
- Ian Alexander
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Alexander (Gerrit)
Ian Alexander uploaded new patchset![]( "Open in Gerrit")
- TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
Related details
- Dmitri Shuralyov
- Ian Alexander
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Alexander (Gerrit)
Ian Alexander uploaded new patchset![]( "Open in Gerrit")
- TryBots-Pass: LUCI-TryBot-Result-1 by Go LUCI
Related details
- Dmitri Shuralyov
- Ian Alexander
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Alexander (Gerrit)
Ian Alexander uploaded new patchset![]( "Open in Gerrit")
Related details
- Dmitri Shuralyov
- Ian Alexander
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Matloob (Gerrit)
Michael Matloob voted and added 1 comment![]( "Open in Gerrit")
Votes added by Michael Matloob
| Code-Review | +2 |
1 comment
Related details
- Dmitri Shuralyov
- Ian Alexander
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Alexander (Gerrit)
Ian Alexander uploaded new patchset![]( "Open in Gerrit")
Related details
- Dmitri Shuralyov
- Ian Alexander
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Matloob (Gerrit)
Michael Matloob voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
- Dmitri Shuralyov
- Ian Alexander
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Alexander (Gerrit)
Ian Alexander added 9 comments![]( "Open in Gerrit")
We should add this function to the modload package since it's only relevant to `go mod init` and `go work init`.
(this package is a "lower level" package that should have more generally applicable functions)
We should probably choose a name that more clearly indicates that it's the default go version in a new module. (init is an overloaded term, for example modload.Init or the init function in a package, so it can be unclear which meaning it refers to) Don't have a super good one off the top of my head but maybe `DefaultModInitGoVersion` ?
Done
Could we parse the version to figure out if it's a language or release version? Ideally we shouldn't be doing manual string operations on it. If not, maybe we can add a comment about why we can't parse it?
Done
add a comment saying we need this to be able to check produced mod files for equality?
Done
these can be cmp now that we don't use `$goversion`
Done
maybe we can name the want files based on the version they contain so it's easier to see what the test is looking for by reading the test case without looking down to see the contents of the files?
Done
1.18.0 never existed (and would never be returned by `gover.LocalInit` in practice), so it would probably be better to bump all the version up so that it's greater than 1.23 (so that the version returned by LocalInit is > 1.21.0)
Done
i think we can revert this?
Done
see comment on the mod_init_version test case: could we name these files semantically like `go.work.want_1_24_0`?
Done
why did we need to make this change?
With the removal of the TESTGO_VERSION_INIT (in a previous patchset), this is no longer necessary and I've removed it.
Related details
- Dmitri Shuralyov
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dmitri Shuralyov (Gerrit)
Dmitri Shuralyov voted and added 2 comments![]( "Open in Gerrit")
Votes added by Dmitri Shuralyov
| Code-Review | +1 |
2 comments
I left a trivial nit, but overall this looks good and I'm not seeing any issues. Thanks for implementing this. Leaving a +1 since I mostly defer to Michael on this review, and he already +2'ed.
if isPrereleaseVersion(v) {This is a tiny nit. Consider naming this `isPrereleaseOrDevelVersion`, since it is in fact checking for development versions too, but its current name doesn't make that very visible.
Related details
- Ian Alexander
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Alexander (Gerrit)
Ian Alexander uploaded new patchset![]( "Open in Gerrit")
- TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI
Related details
- Ian Alexander
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Alexander (Gerrit)
Ian Alexander voted and added 1 comment![]( "Open in Gerrit")
Votes added by Ian Alexander
| Commit-Queue | +1 |
1 comment
This is a tiny nit. Consider naming this `isPrereleaseOrDevelVersion`, since it is in fact checking for development versions too, but its current name doesn't make that very visible.
- Dmitri Shuralyov
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Alexander (Gerrit)
Ian Alexander voted Commit-Queue+1![]( "Open in Gerrit")
- Dmitri Shuralyov
- Ian Alexander
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Alexander (Gerrit)
Ian Alexander voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
- Dmitri Shuralyov
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Dmitri Shuralyov (Gerrit)
Dmitri Shuralyov voted and added 1 comment![]( "Open in Gerrit")
Votes added by Dmitri Shuralyov
| Code-Review | +1 |
| TryBot-Bypass | +1 |
1 comment
The trybot failures seem unrelated to this change, and each trybot passed just not all at the same time, so it should be safe to bypass in this case.
Related details
- Ian Alexander
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Alexander (Gerrit)
Ian Alexander voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
- Ian Alexander
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Alexander (Gerrit)
Ian Alexander voted Commit-Queue+1![]( "Open in Gerrit")
| Commit-Queue | +1 |
Related details
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Ian Alexander (Gerrit)
Ian Alexander submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
10 is the latest approved patch-set.
The change was submitted with unreviewed changes in the following files:
```
The name of the file: src/cmd/go/internal/modload/init.go
Insertions: 2, Deletions: 2.
@@ -2262,7 +2262,7 @@
// Candidate M) or a development version of Go 1.N, default to go 1.(N-2).0
func DefaultModInitGoVersion() string {
v := gover.Local()
- if isPrereleaseVersion(v) {
+ if isPrereleaseOrDevelVersion(v) {
v = gover.Prev(gover.Prev(v))
} else {
v = gover.Prev(v)
@@ -2273,7 +2273,7 @@
return v
}
-func isPrereleaseVersion(s string) bool {
+func isPrereleaseOrDevelVersion(s string) bool {
v := igover.Parse(s)
return v.Kind != "" || v.Patch == ""
}
```
```
The name of the file: src/cmd/go/testdata/script/work_init_path.txt
Insertions: 1, Deletions: 1.
@@ -16,7 +16,7 @@
-- go.mod --
module example
-go 1.18.0
+go 1.18
-- dir/go.mod --
module example
go 1.21.0
```
Change information
cmd/go: update default go directive in mod or work init
This commit updates the default go directive when initializing a new
module.
The current logic is to use the latest version supported by the
toolchain. This behavior is simple, predictable, and importantly, it
can work while completely offline (i.e., no internet connection
required).
This commit changes the default version to the following behavior:
* If the current toolchain version is a stable version of Go 1.N.M,
default to go 1.(N-1).0
* If the current toolchain version is a pre-release version of Go
1.N (Release Candidate M) or a development version of Go 1.N, default
to go 1.(N-2).0
This behavior maintains the property of being able to work offline.
Fixes #74748.
- M src/cmd/go/internal/modload/init.go
- M src/cmd/go/internal/workcmd/init.go
- M src/cmd/go/testdata/script/mod_edit.txt
- A src/cmd/go/testdata/script/mod_init_version.txt
- M src/cmd/go/testdata/script/work.txt
- M src/cmd/go/testdata/script/work_edit.txt
- M src/cmd/go/testdata/script/work_init_path.txt
- M src/cmd/go/testdata/script/work_init_toolchain.txt
- A src/cmd/go/testdata/script/work_init_version.txt
- M src/cmd/go/testdata/script/work_sync_toolchain.txt
- M src/cmd/go/testdata/script/work_use_toolchain.txt
Code-Review: +1 by Dmitri Shuralyov, +1 by Michael Matloob, +2 by Michael Matloob
TryBots-Pass: LUCI-TryBot-Result-1 by Go LUCI, TryBot-Bypass+1 by Dmitri Shuralyov
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Michael Matloob (Gerrit)
Michael Matloob has created a revert of this change![]( "Open in Gerrit")
Related details
Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |