Closed crazy-max closed 4 months ago
Hello,
technically speaking, the patch part of the version is now a kind of requirement, there is some tolerance for compatibility reasons.
I don't remember where I saw the explanation, I will find it.
:thinking: I didn't find where I saw that, maybe I made a confusion with toolchain
.
Why is this important for you?
the go
version is the minimal Go version so go1.21
and go1.21.0
are the same thing.
Why is this important for you?
the
go
version is the minimal Go version sogo1.21
andgo1.21.0
are the same thing.
It means downstream projects need to set same go version 1.21.0
in go.mod
even if 1.21
is still correct. My change is just to avoid this kind of noise if that makes sense.
technically speaking, the patch part of the version is now a kind of requirement,
I guess since 1.22 correct?
I dont see anything in the documentation that implies youre forced to use 1.21.0 and cant go to 1.21.1. Everything in the go mod docs indicate that this is the minimum go version required for this package and consumers are free to go to a later version as needed.
Is there an error youre observing?
I understand what you called "noise" (the useless patch version), I had the same reaction at the beginning.
I guess since 1.22 correct?
As I didn't find a reference, I will not be able to really answer this question. From my memories, yes and no, as go1.21 introduced the behavior.
But without an official reference, I will not fight against this change.
Is there an error youre observing?
Now I'm forced to set 1.21.0
as version in go.mod
when I vendor: https://github.com/moby/buildkit/actions/runs/9779428021/job/26998669203?pr=5118#step:5:539
Nothing functional that's just a nit but still...
I guess I will have the same issue in the future if someone is doing the same so not directly directed to you but go toolchain shenanigans :sweat_smile:
I guess I will have the same issue in the future if someone is doing the same so not directly directed to you but go toolchain shenanigans ๐
I don't take it badly: I did it because I was thinking it was a good practice.
I'm annoyed not to find the reference.
I think I have a kind of reference:
Go 1.21 introduces a small change to the numbering of releases. In the past, we used Go 1.N to refer to both the overall Go language version and release family as well as the first release in that family. https://tip.golang.org/doc/go1.21
This means that go1.21
is now a "release family" and go1.21.0
is a version.
At go 1.21 or higher:
- The go line declares a required minimum version of Go to use with this module.
- ...
The go
directive should theoretically declare a version (the minimum version).
I know it's not really explicit, and I'm sure I read an explicit message about that, I'm annoyed to not find it.
I think I have a kind of reference:
Go 1.21 introduces a small change to the numbering of releases. In the past, we used Go 1.N to refer to both the overall Go language version and release family as well as the first release in that family. https://tip.golang.org/doc/go1.21
This means that
go1.21
is now a "release family" andgo1.21.0
is a version.At go 1.21 or higher:
- The go line declares a required minimum version of Go to use with this module.
- ...
The
go
directive should theoretically declare a version (the minimum version).I know it's not really explicit, and I'm sure I read an explicit message about that, I'm annoyed to not find it.
Ah thanks for this feedback, ok I get it better now.
The PR https://github.com/moby/buildkit/pull/5118 has been merged, is your PR still required?
Not required, I guess we can't do anything about it if we have other dep doing the same and related to your previous comment I think this is fine now. Thanks for your time!
@ldez I can add a bit of context on the why here because it's been a thorn for us (at work) for a while.
The new automatic toolchain upgrades introduced in Go 1.21 mean that the go
directive in each of your dependencies' go.mod
files gets fed into an MVS-style selection algorithm, which will update your go.mod
file if necessary. It also means that, by default at least, go build
, go test
, etc. will check that directive and automatically (and silently) download and use that specific version of Go.
As a library, though, the practical effect of this is that updating the Go version in our go.mod
will force all downstream consumers to also use that version of Go (or a higher one). I don't think that's a decision that library authors should be making for their consumers, so our go.mod
should have the lowest version of Go that includes the features actually in use by the library's code and NOT whatever version any of us happen to be using locally for development.
our go.mod should have the lowest version of Go that includes the features actually in use by the library's code and NOT whatever version any of us happen to be using locally for development
Agree. How would we feel about using 1.15 since that was the latest version that the travisci build supported?
I think 1.19, or maybe 1.20, would be good since that shouldn't trigger any inadvertent/unwanted Go version changes downstream.
I talk about that inside Gofrs Slack channel.
The new MVS clarification of the meaning of the go
directive creates a hard requirement since go1.21.
As a library, like you said, the version of go
or dependencies should be handled very carefully.
I updated the Go version to the minimally supported Go version by the Go team, I did it with the knowledge of the impact, and I think that is not a problem to require a Go version supported by the Go team.
The problem exposed through the PR is the difference between "release family" and "version".
I was thinking that setting it to 1.20 would be "safe" since everyone should be using at least 1.21 now, but that's definitely not a point I'm going to argue over.as long as we're not pushing consumers onto newer versions of Go.
Going forward, though, we shouldn't change the version unless we are using newer language features
I think 1.19, or maybe 1.20, would be good since that shouldn't trigger any inadvertent/unwanted Go version changes downstream.
I don't share this opinion because using unmaintained go1.19 or go1.20 is a security issue.
I think "forcing" the usage of a Go team-supported version (at least to the .0
patch) is not a bad practice.
But I'm against, for a lib, using the latest Go version (currently go1.22), or the latest patch version, unless it's really needed. (I think we agree on that)
I will not update the go
directive after each new minor/major release of Go, but if I need to update Go I will update to the "old stable" version (currently go1.21).
(I think we also agree on that)
The whole versioning has become so complex; fun fact: go now can downgrade language versions per package / file; despite the go1.0 compatibility promise, it may disable features.
Another fun example; backward compatibility code (i.e. files with stubs for old go versions) may no longer always work; see https://github.com/opencontainers/runc/pull/4277#issuecomment-2114392288
using unmaintained go1.19 or go1.20
I guess my point is that the version of Go being used won't be what's in this library's go.mod
, it will be whatever is specified by the consuming application.
FWIW, I've personally set GOTOOLCHAIN=local
on my development machines so that it will always use what I have installed (1.22.5 right now).
Philosophically, I share @dylan-bourque's opinion. I'd prefer that we avoid compelling users to upgrade go versions unless we are leveraging a specific feature of the go version in our mod file. I definitely get that people should be upgrading, but the reality is there are some cases where they don't or can't for a number of reasons and using our package shouldn't get in the way of that. Compelling people to upgrade (as we see in this very PR ;)) does have a side effect of increasing friction to using our package and we want to make the use of our package a smooth experience.
That doesn't mean that we need to support old go versions. We can continue testing against stable and old stable, and guarantee that we fix bugs for latest go and the latest version of our package. I think though that we should not be the pressure that forces people to upgrade.
Definitely ๐ ๐๐๐ on GOTOOLCHAIN=local
... I'm not a big fan of --help
meaning; "download a binary and install" ๐๐ https://github.com/golang/go/issues/60956
I think we all agree on the background subject: don't update the Go version unless you needed.
I just have no problem defining the old stable as a requirement (not day one, and not the latest patch).
not a big fan of
--help
meaning; "download a binary and install"
@thaJeztah we had someone run into that with go list
the other day
I found the reference I was thinking of (yes I never give up :smile:)
Code QL, when using at least go1.21, reports a warning:
This report is about this file.
module github.com/golangci/golangci-lint/scripts/gen_github_action_config
go 1.21
require (
The documentation referenced inside the screenshot is https://go.dev/doc/toolchain#version
If you add my previous comment with this warning and this doc, we have the full overview of the topic.
relates to https://github.com/gofrs/flock/pull/57#discussion_r1664265710