Skip to content

go.mod: bump golang.org/x/net to 0.36.0 #1729

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

go.mod: bump golang.org/x/net to 0.36.0 #1729

wants to merge 1 commit into from

Conversation

tklauser
Copy link
Member

This was generated by running:

go get golang.org/x/net@v0.36.0
go mod tidy
go mod edit -toolchain=none

Replaces #1724

@tklauser tklauser requested a review from a team as a code owner March 21, 2025 13:38
@tklauser tklauser mentioned this pull request Mar 21, 2025
Merged
lmb
lmb reviewed Mar 24, 2025
View reviewed changes
go.mod Outdated
@@ -1,6 +1,6 @@
module github.com/cilium/ebpf

go 1.23
go 1.23.0
Copy link
Collaborator

Choose a reason for hiding this comment

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

This runs into eaef6a7 again. I'm really quite sick of the the whole add a .0 or don't add a .0 to the go line saga xD

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd be grateful for some semi-official answer what the thing to do is for depending on a language version. I don't want to pin toolchain because I don't really understand why its necessary and it's another thing to manage.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm also equally confused about the meaning of the toolchain directive on projects like these. I guess it's the toolchain version used when running go test, etc. Maybe we can let this be managed by dependabot, because I don't see a way of configuring this in dependabot.yml. There's no ecosystem-specific configuration whatsoever, and we don't even have a gomod block in there, it's enabled through the GitHub UI.

@tklauser @ti-mo
go.mod: bump golang.org/x/net to 0.36.0
fee0f39 
This was generated by running:

    go get golang.org/x/net@v0.36.0
    go mod tidy
    go mod edit -toolchain=none

Replaces cilium#1724

Signed-off-by: Tobias Klauser <tobias@cilium.io>
@ti-mo
Copy link
Collaborator

ti-mo commented Mar 25, 2025

I force-pushed this branch to read go 1.23, but this somehow makes golangci-lint unable to run: https://github.com/cilium/ebpf/actions/runs/14054937043/job/39352226944?pr=1729

Reading eaef6a7 again:

On b3432fb, the minimum supported Go version was updated to 1.21.0. This changes the version to 1.21 so that we are using a language version. If not using toolchain, this seems to be the preferred convention in the ecosystem and golang/go.

Seems to me like go mod tidy behaviour changes depending on the used toolchain, so putting a toolchain directive in go.mod seems like the best way to keep this behaviour stable in the same sense that e.g. an LLVM builder image does.

Bottom line is eaef6a7 may no longer be an issue if both go 1.23.0 and toolchain go1.24.1 are specified. I've run go mod tidy on the dependabot PR and force-pushed it; looks like it's working. Let's go with the initial PR.

@ti-mo ti-mo closed this Mar 25, 2025
@tklauser tklauser deleted the x-net-update branch March 25, 2025 09:49
@tklauser
Copy link
Member Author

tklauser commented Mar 25, 2025 •
edited
Loading

I force-pushed this branch to read go 1.23, but this somehow makes golangci-lint unable to run: https://github.com/cilium/ebpf/actions/runs/14054937043/job/39352226944?pr=1729

Reading eaef6a7 again:

On b3432fb, the minimum supported Go version was updated to 1.21.0. This changes the version to 1.21 so that we are using a language version. If not using toolchain, this seems to be the preferred convention in the ecosystem and golang/go.

Seems to me like go mod tidy behaviour changes depending on the used toolchain, so putting a toolchain directive in go.mod seems like the best way to keep this behaviour stable in the same sense that e.g. an LLVM builder image does.

Bottom line is eaef6a7 may no longer be an issue if both go 1.23.0 and toolchain go1.24.1 are specified. I've run go mod tidy on the dependabot PR and force-pushed it; looks like it's working. Let's go with the initial PR.

AFAIK, if the go directive includes a patch version, a toolchain directive shouldn't be necessary and won't be added by go mod tidy. Given that go specifies the minimum language version and there are no language changes to be expected between patch versions, the safest way IMO would be to use go 1.x.0 and no toolchain directive. That way developers won't be forced to update their toolchain (as is currently the case with the toolchain go1.24.1 directive which will force developers using go1.24.0 to update). This is the scheme that's currently used in cilium/cilium and has worked quite well so far.

If you want I can send a follow-up patch changing to the above.

Nevermind, I missed that go and toolchain are different 🤦 the current approach seems fine to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lmb lmb lmb left review comments

@ti-mo ti-mo ti-mo left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@tklauser @ti-mo @lmb