Upgrade to Go 1.18 #6709
Upgrade to Go 1.18 #6709
Conversation
By taking advantage of smarter traversal of dependencies, a `go mod tidy` using Go 1.18 remove some items from go.sum. Signed-off-by: Tom Godkin <[email protected]>
|
Hi @BooleanCat. Thanks for your PR. I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
Build succeeded.
|
|
We still support Go 1.17, so probably it's better to keep go.mod version to be 1.17 too |
The Go directive does not impose a minimum Go version. If containerd is compiled with an earlier version of Go, it will point out the mismatch only if a compile error occurs. Compatibility with Go 1.17 is still supported and can be guaranteed with Ci. |
| module github.com/containerd/containerd | ||
|
|
||
| go 1.17 | ||
| go 1.18 |
There was a problem hiding this comment.
This is wrong. In here there should be a minimally supported version, so unless you do want to drop go 1.17 support right now (which is probably not the right thing to do, since go 1.17 is fully supported and is probably most used version at the moment), I would advise against this change.
There was a problem hiding this comment.
A go directive indicates that a module was written assuming the semantics of a given version of Go
I am not sure exactly what this means, but I guess that unless the code starts using Go 1.18 features, it should not say "go 1.18" in go.mod.
There was a problem hiding this comment.
The Go team has been very ambiguous wishy-washy on this; the version does indicate the minimum version that's expected to be compatible, but they were not enforcing that (yet??), and started doing things like this in their own repositories (e.g. golang/sys@0981d60) - effectively disregarding their own recommendations to at least support latest+previous (even if the version in go mod is not enforced yet).
Changing the version does affect what versions can be used to manage modules though; i.e., running go mod tidy with go 1.17 will no longer produce the same result (which means that anyone working on this project must use go1.18).
For this one, I'm a bit on the fence; is there anything we gain from the removed lines in go.sum? (the change from <1.17 to 1.17 did bring the improved "indirect" dependencies, so was clearer on benefits)
If we do decide to change, we should also update the go.mod in the integration client (which still looks to be on 1.15);
containerd/integration/client/go.mod
Line 3 in 388ee88
There was a problem hiding this comment.
One small benefit I can see (which is what led me to to make this change) is that it results in a less complicated go mod graph which brings two benefits:
- Fewer false positives on security scanning tools
- For downstream tools using the containerd library, a potentially smaller
go mod vendoroutput
There was a problem hiding this comment.
I think we can change this to go 1.18 as long as we build containerd against 1.17 on our CI. This will guaranty compatibility.
|
@AkihiroSuda @mxpv Please take a look. The directive doesn't block Go 1.17. |
8 participants
By taking advantage of smarter traversal of dependencies, a
go mod tidyusing Go 1.18 remove some items from go.sum.Signed-off-by: Tom Godkin [email protected]