-
Notifications
You must be signed in to change notification settings - Fork 5.7k
Drop Go min patch version #13418
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
Drop Go min patch version #13418
Conversation
| module github.com/docker/compose/v5 | ||
|
|
||
| go 1.24.11 | ||
| go 1.24.3 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:sadpanda: looks like some other dependency also set a patch version. Well 🤷♂️
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FWIW 1.24.3 is pretty common due to some breaking behaviors in 1.24.0-1.24.2.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't get the point here. What's wrong with 1.24.11? Why shall we use an older ref?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Go version defined in go.mod is currently used to determine which Go version our CI installs.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The version in go.mod should follow MVS (Minimal Version Selection) and reflect the minimum required version of Go; similar to other dependencies.
For features introduced in a Go "minor" version, that would indeed require updating the version; i.e., if the project uses features that are only available in, say, go1.25, then the go.mod should be updated to go1.25.0, because it won't compile on older versions.
But for the patch version, that's not the case; patch versions should only contain bugfixes, not new features, so it's good to keep it at .0 ("any go1.24.x version can be used to compile this code"); for sure, users should normally use the latest patch release, but that's not up to compose to dictate that.
Currently, compose updates the patch version if compose itself choses to update to a newer version of go, but this caused issues with Azure (and others), who use a hardened version of Go; those versions usually go through some extra verification period before becoming available, but if compose bumps the minor version, it's not possible to build with any older patch version of Go.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
.. as PR description suggests :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
current versions of go no longer allow omitting the .<patch> entirely; usually it would be set to .0, but in this specific case, go1.24.0, go124.1 and go1.24.2 were broken, so there was a legit reason for some projects to not allow using them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
right.
I'm fine we change go.mod then, but CI workflow must be updated so we run with latest
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
current versions of go no longer allow omitting the
.<patch>entirely; usually it would be set to.0, but in this specific case, go1.24.0, go124.1 and go1.24.2 were broken, so there was a legit reason for some projects to not allow using them.
This is not true. The requirement you mention is for the toolchain directive. 1.24 is a valid for the go directive as it is a "language version". See https://go.dev/doc/toolchain#version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, you're right, I misremembered! I know I tried really hard to keep those patch versions out, but at some point was no longer able to; looking at the moment I had to add it on Moby, it was this commit; moby/moby@36549fb and it was indeed because a dependency forced our hands, not Go itself.
The commit in golang.org/x/mod that added it, is actually interesting; looks like they did as workaround for a bug in Go 1.21 which DID cause omitting the patch version to break golang/mod@3afcd4e
go.mod: set go version to 1.22.0
The go verison was set to 1.22 but on Go versions 1.21.0 up to 1.21.10,
the toolchain upgrade logic will try to download the release "1.22",
which doesn't exist. Go 1.21.11+ incorporates CL 580217 (cherry-picked
in CL 583797) and will download 1.22.0, so it should be fine, but set
1.22.0 to allow the upgrade for users with older local toolchains.
I just tried again if it's possible to omit, but it looks indeed that if any dependency specifies a version with a patch version, there's no way to undo that;
cat go.mod
module example.com/foo
go 1.22
require golang.org/x/mod v0.21.0
cat main.go
package main
import "golang.org/x/mod/modfile"
func main() {
var _ = modfile.GoVersionRE
}
go run main.go
go: updates to go.mod needed; to update it:
go mod tidy
go mod tidy
cat go.mod
module example.com/foo
go 1.22.0
require golang.org/x/mod v0.21.0Relevant portion there (https://go.dev/doc/toolchain#version) is this (so "1.21" includes pre-releases, whereas "1.21.0" excludes them (1.21.0 > 1.21));
Any two Go versions can be compared to decide whether one is less than, greater than, or equal to the other. If the language versions are different, that decides the comparison: 1.21.9 < 1.22. Within a language version, the ordering from least to greatest is: the language version itself, then release candidates ordered by R, then releases ordered by P.
For example, 1.21 < 1.21rc1 < 1.21rc2 < 1.21.0 < 1.21.1 < 1.21.2.
Before Go 1.21, the initial release of a Go toolchain was version 1.N, not 1.N.0, so for N < 21, the ordering is adjusted to place 1.N after the release candidates.
For example, 1.20rc1 < 1.20rc2 < 1.20rc3 < 1.20 < 1.20.1.
thaJeztah
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM (but not a maintainer)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Blocking this operation until we are certain that it will have no potential impact on continuous integration.
|
Our CI set go version used based on go.mod (https://github.com/docker/compose/blob/main/.github/workflows/ci.yml#L201-L204) so this would mean we run CI with an older go runtime, with some potential security concerns |
0c562f4 to
ed0d3af
Compare
|
I took another look into this. actions/setup-go As a workaround, I have updated actions/setup-go to v6 which added the ability to specify the toolchain version through WDYT? @ndeloof, @glours, @thaJeztah |
ndeloof
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
I wonder we could get GO_VERSION build arg set from this .go-version file to avoid value being hard-coded in Dockerfile
Signed-off-by: Austin Vazquez <[email protected]>
Signed-off-by: Austin Vazquez <[email protected]>
Signed-off-by: Austin Vazquez <[email protected]>
ed0d3af to
4a44179
Compare
This MR contains the following updates: | Package | Update | Change | |---|---|---| | [docker/compose](https://github.com/docker/compose) | patch | `v5.0.0` -> `v5.0.1` | MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot). **Proposed changes to behavior should be submitted there as MRs.** --- ### Release Notes <details> <summary>docker/compose (docker/compose)</summary> ### [`v5.0.1`](https://github.com/docker/compose/releases/tag/v5.0.1) [Compare Source](docker/compose@v5.0.0...v5.0.1) #### What's Changed ##### 🐛 Fixes - Restored support for `COMPOSE_COMPATIBILITY` by [@​ndeloof](https://github.com/ndeloof) in [#​13424](docker/compose#13424) - Fixed grammatical errors and improve clarity in code. by [@​xiaolinny](https://github.com/xiaolinny) in [#​13429](docker/compose#13429) - Fixed broken `run --quiet`. by [@​ndeloof](https://github.com/ndeloof) in [#​13430](docker/compose#13430) - Fixed SDK example by [@​ndeloof](https://github.com/ndeloof) in [#​13416](docker/compose#13416) - Added a check buildx version is set before comparing it. by [@​yangfeiyu20102011](https://github.com/yangfeiyu20102011) in [#​13415](docker/compose#13415) - Fixed grammar: pluralize 'service' and remove apostrophes in lets. by [@​rashmivagha](https://github.com/rashmivagha) in [#​13423](docker/compose#13423) - Fixed progress UI not restoring terminal once operation completes. by [@​ndeloof](https://github.com/ndeloof) in [#​13439](docker/compose#13439) - Fixed status alignment in progress UI. by [@​ndeloof](https://github.com/ndeloof) in [#​13438](docker/compose#13438) - Restored image layer download progress details on pull. by [@​ndeloof](https://github.com/ndeloof) in [#​13445](docker/compose#13445) - Added 'configured' event at the end of model configuration phase. by [@​glours](https://github.com/glours) in [#​13446](docker/compose#13446) - Introduced a build tag to select watcher implementation. by [@​ndeloof](https://github.com/ndeloof) in [#​13452](docker/compose#13452) - Removed mention for `v2` on README. by [@​alexislefebvre](https://github.com/alexislefebvre) in [#​13451](docker/compose#13451) - Fixed missing error handling in `setEnvWithDotEnv`. by [@​htoyoda18](https://github.com/htoyoda18) in [#​13450](docker/compose#13450) - Adopted morikuni/aec library over raw ANSI sequences. by [@​ndeloof](https://github.com/ndeloof) in [#​13440](docker/compose#13440) - Prevented incorrect progress metrics to break compose display. by [@​ndeloof](https://github.com/ndeloof) in [#​13457](docker/compose#13457) - Restored support for BUILDKIT\_PROGRESS. by [@​ndeloof](https://github.com/ndeloof) in [#​13455](docker/compose#13455) - Added check model plugin is successfully loaded. by [@​ndeloof](https://github.com/ndeloof) in [#​13464](docker/compose#13464) - Added a warning when no service has been selected to build. by [@​ndeloof](https://github.com/ndeloof) in [#​13467](docker/compose#13467) ##### ⚙️ Dependencies - Drop Go min patch version by [@​austinvazquez](https://github.com/austinvazquez) in [#​13418](docker/compose#13418) - bump golang 1.24.11 by [@​austinvazquez](https://github.com/austinvazquez) in [#​13417](docker/compose#13417) - bump osxcross by [@​ndeloof](https://github.com/ndeloof) in [#​13425](docker/compose#13425) - bump golang.org/x/sys from 0.38.0 to 0.39.0 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​13433](docker/compose#13433) - bump github.com/docker/cli-docs-tool from 0.10.0 to 0.11.0 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​13437](docker/compose#13437) - bump golang.org/x/sync from 0.18.0 to 0.19.0 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​13434](docker/compose#13434) - bump tags.cncf.io/container-device-interface from 1.0.1 to 1.1.0 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​13441](docker/compose#13441) - bump github.com/moby/buildkit from 0.26.2 to 0.26.3 by [@​dependabot](https://github.com/dependabot)\[bot] in [#​13462](docker/compose#13462) #### New Contributors - [@​yangfeiyu20102011](https://github.com/yangfeiyu20102011) made their first contribution in [#​13415](docker/compose#13415) - [@​xiaolinny](https://github.com/xiaolinny) made their first contribution in [#​13429](docker/compose#13429) - [@​alexislefebvre](https://github.com/alexislefebvre) made their first contribution in [#​13451](docker/compose#13451) - [@​htoyoda18](https://github.com/htoyoda18) made their first contribution in [#​13450](docker/compose#13450) **Full Changelog**: <docker/compose@v5.0.0...v5.0.1> </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever MR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this MR and you won't be reminded about this update again. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box --- This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0Mi41OS4xIiwidXBkYXRlZEluVmVyIjoiNDIuNTkuMSIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiUmVub3ZhdGUgQm90Il19-->
What I did
This PR addresses #13417 (comment)
Related issue
(not mandatory) A picture of a cute animal, if possible in relation to what you did