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

chore: update go version in module #1804

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mmorel-35
Copy link
Contributor

@mmorel-35 mmorel-35 commented Mar 3, 2025

Description

Go 1.24.0 has been released on since 11 February 2025

This updates Go version from 1.18 to 1.23

Notice that loong64 arch is only available since Go1.19 so it's surprising to see that gopsutil supports it (at least partially) with a Go1.18 module.

@mmorel-35 mmorel-35 force-pushed the go1.23 branch 3 times, most recently from 7c8a2dd to 84c9b6e Compare March 4, 2025 06:27
@shirou
Copy link
Owner

shirou commented Mar 4, 2025

Thank you for the suggestion!

However, gopsutil is a library that supports a wide range of platforms. Not all platforms provide the latest version of Go in their package managers, and some users might not be able to upgrade their Go version.

gopsutil depends on golang.org/x/sys, and x/sys's golang dependency is regularly updated. So, in practice, the version from golang.org/x/sys might be used, but I still believe that the library itself should aim to support the lowest possible Go version.

Of course, if there’s a need to use new language features in Go, I’m not against it. However, I think we should discuss whether it’s truly necessary at that time.

@mmorel-35
Copy link
Contributor Author

mmorel-35 commented Mar 4, 2025

Would you consider at least 1.19 ? That would then allow to extend golangci-lint matrix for loong64

@mmorel-35
Copy link
Contributor Author

github.com/google/go-cmp require go1.21 and there are no workflows that are still using go1.19

@mmorel-35 mmorel-35 force-pushed the go1.23 branch 2 times, most recently from af79113 to 36b7f45 Compare March 4, 2025 17:47
@shirou
Copy link
Owner

shirou commented Mar 6, 2025

I just noticed golang/go#69095. This proposal suggests automatically updating the go.mod version to 1.(N-1).0, including x/sys, and already accepted.

In the discussion, there are comments suggesting that the required version should be set to the minimum, but it seems that people like rsc believe it's important to encourage the use of the latest version.

Personally, I also agree with the idea of keeping the version at the minimum. However, the overall direction of Go team seems to be to promote the use of the latest version. Therefore, I've changed my mind and now think it might be better to change 1.23.0 in go.mod.

@mmorel-35
Copy link
Contributor Author

mmorel-35 commented Mar 6, 2025

Alright, that ready to Go :) !

Golangci-lint timeout it extended because of Windows exeeding it.

@mmorel-35 mmorel-35 force-pushed the go1.23 branch 2 times, most recently from ff4a1cb to 4248431 Compare March 8, 2025 09:56
@shirou
Copy link
Owner

shirou commented Mar 9, 2025

Thank you for the update. However, when I looked at the diff, I wondered, "Why is the toolchain directive included?" So I did some research and found golang/go#65847. I haven't read through the entire issue comments, but it seems that the toolchain directive might be automatically added and could potentially cause issues.

It doesn't seems to be resolved yet, so how about removing the toolchain directive in this PR for now?

@mmorel-35 mmorel-35 force-pushed the go1.23 branch 3 times, most recently from 01d6f63 to 194d27d Compare March 9, 2025 08:02
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.

2 participants