-
Notifications
You must be signed in to change notification settings - Fork 2.6k
fix(manager/gomod): perfer to use go version defined as toolchain to update artifacts #34564
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
fix(manager/gomod): perfer to use go version defined as toolchain to update artifacts #34564
Conversation
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.
Need to consider the edge case where the toolchain version is being upgraded in the same PR
So I think in that case we do not parse GoConstraints but use updated toolchain version from |
Yes first check would be if Go is being updated in updatedDeps. There could also be some weird things like go mod directive updated but go toolchsin not. |
Is it possible to update go.mod first then get constraints? this should avoid this problem naturally |
By default renovate doesn't bump the go.mod directive. This was the recommendation/request of the golang team. So 99% of the time it's probably only the tool chain getting bumped, or none. I agree with you that we shouldn't "accidentally" bump the tool chain by running a latest go |
I think we simply need to use new file content to extract this information instead of reading old file from disk here.
|
Yes, this is what I suggest |
I don't mean go directive, I mean we update go.mod first then parse toolchain directive from go.mod ( like this PR already did). In that case if we need to update I don't know if it's possible. |
it should be something like this? const goConstraints =
config.constraints?.go ?? getGoConstraints(massagedGoMod); |
Yes. Any updates to go.mod would have been written out already |
… into fix/go/prefer-toolchain-version
Co-authored-by: Rhys Arkins <[email protected]>
I see, seemingly there is a typo in your observations. Lets clarify: For go.mod: go 1.23.5 renovate now pick That will satisfy my ask to pin the toolchain at a specific version. In this case its implicitly specified by the go directive. @trim21 I had observed that if renovate makes upgrades because of known CVEs in dependencies, it would ignore the go directive at all, and even upgrade. e.g. go1.22.0 to go1.22.13 and toolchain to go1.24.1 with this config:
Might be unrelated to this PR, just giving a heads-up. |
thanks, it's indeed a typo |
I don't know how go pick toolchain in this case... |
I think I'd be happy not to add the toolchain directive in the last case |
In that case we need to know which go version we will pick finally, it need Need some kind of constraints on command I think, for example Another method is based on the go directive, if it's >1.21 then we add such argument, that's also possible, I think. |
I think this is out of scope of this PR, this PR just want to resolve first 2 case. It should be done with the removing of -d argument, they need same mechanism |
👀 need anything else before merge? |
🎉 This PR is included in version 39.200.1 🎉 The release is available on: Your semantic-release bot 📦🚀 |
…update artifacts (renovatebot#34564) Co-authored-by: Rhys Arkins <[email protected]>
Changes
prefer use version from toolchain directive instead of go diective in go.mod, this will avoid change current toolchain directive.
Context
As explained in #34563, when running
go get ./...
, go will update toolchain directive in go.mod, which is adepType=toolchain
change instead of expecteddepType=require
change.And there are 2 ways to avoid change of toolchain directive.
I don't think old version of go support
go get ./... toolchain@${current toolchain}
(just like we still didn't remove-d
flag here), so it would be more backward compatible to just use version of current toolchain to avoid this.If go.mod doesn't have such directive, we still use version frmo go directive.
Documentation (please check one with an [x])
How I've tested my work (please select one)
I have verified these changes via: