Skip to content
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

Update dependencies to fix panic in go generate #326

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mat007
Copy link

@mat007 mat007 commented Jan 30, 2025

I picked Go 1.22 because I’ve seen this in another recent PR run:

Setup go version spec oldstable
oldstable version resolved as 1.22.10

Also updating golang.org/x/tools/cmd/stringer requires Go 1.22:

$ GOTOOLCHAIN=go1.21.0 go get golang.org/x/tools/cmd/stringer
go: golang.org/x/tools/cmd/stringer: golang.org/x/[email protected] requires go >= 1.22.0 (running go 1.21.0; GOTOOLCHAIN=go1.21.0)

Here is what I did:

GOTOOLCHAIN=go1.22.0 go get golang.org/x/tools/cmd/stringer
GOTOOLCHAIN=go1.22.0 go mod tidy

then manually edited go.mod to remove the added toolchain, and re-ran go mod tidy to make sure it’s stable.

@mat007 mat007 requested a review from a team as a code owner January 30, 2025 08:14
This fixes the crash when go generating

Signed-off-by: Mathieu Champlon <[email protected]>
@mat007 mat007 changed the title Update dependencies to fix go generate crash Update dependencies to fix panic in go generate Jan 30, 2025
@mat007 mat007 mentioned this pull request Jan 30, 2025
Copy link

@kiashok kiashok left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

go.mod Outdated
go 1.21
go 1.22.0

toolchain go1.22.10
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can probably remove this one if it automatically added it (go modules tends to randomly add these, but adding also means that users get either down- or upgraded when running commands, so I try to avoid adding these.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I’ll remove the toolchain.

@@ -1,11 +1,16 @@
module github.com/Microsoft/go-winio

go 1.21
go 1.22.0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did the updated modules enforce updating to go1.22.0, or does it accept staying on go1.21 ?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it needs go 1.22:

$ GOTOOLCHAIN=go1.21.0 go get golang.org/x/tools/cmd/stringer
go: golang.org/x/tools/cmd/stringer: golang.org/x/[email protected] requires go >= 1.22.0 (running go 1.21.0; GOTOOLCHAIN=go1.21.0)

Signed-off-by: Mathieu Champlon <[email protected]>
@mat007
Copy link
Author

mat007 commented Feb 11, 2025

@kiashok can you trigger the CI again?

@mat007
Copy link
Author

mat007 commented Feb 11, 2025

All right, so this effectively fixes the «CI / Go Generate» workflow.
Do you folks want to merge this PR as is, or should we cherry-pick this to #325 to fix all workflows in a single PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants