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

build: upgrade -dlgo version to Go 1.24.0 #31159

Merged
merged 7 commits into from
Feb 19, 2025
Merged

Conversation

levisyin
Copy link
Contributor

@levisyin levisyin commented Feb 12, 2025

I have checked it via go run build/ci.go sanitycheck

Copy link
Member

@MariusVanDerWijden MariusVanDerWijden left a comment

Choose a reason for hiding this comment

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

Still waiting on BLST to cut a release before we can merge this

@holiman
Copy link
Contributor

holiman commented Feb 12, 2025

When doing the golang upgrade, it's probably better to keep all the changes together.
Go-ethereum retains compatibility with two golang versions, so updating to 1.24 means dropping support for 1.22.

See #30323 for an idea about what needs to be changed, in addition to the checksums (mostly CI workflows and readme).

@levisyin
Copy link
Contributor Author

When doing the golang upgrade, it's probably better to keep all the changes together. Go-ethereum retains compatibility with two golang versions, so updating to 1.24 means dropping support for 1.22.

See #30323 for an idea about what needs to be changed, in addition to the checksums (mostly CI workflows and readme).

Okay, I will try to change all related things.

@levisyin levisyin marked this pull request as draft February 13, 2025 01:41
@levisyin levisyin marked this pull request as ready for review February 13, 2025 02:21
@MariusVanDerWijden
Copy link
Member

We should also update golangci-lint to the newest version which supports 1.24: https://github.com/golangci/golangci-lint/releases/tag/v1.64.4

@levisyin
Copy link
Contributor Author

We should also update golangci-lint to the newest version which supports 1.24: https://github.com/golangci/golangci-lint/releases/tag/v1.64.4

Okay, I will do this now

README.md Outdated
@@ -16,7 +16,7 @@ archives are published at https://geth.ethereum.org/downloads/.

For prerequisites and detailed build instructions please read the [Installation Instructions](https://geth.ethereum.org/docs/getting-started/installing-geth).

Building `geth` requires both a Go (version 1.22 or later) and a C compiler. You can install
Building `geth` requires both a Go (version 1.23 or later) and a C compiler. You can install
Copy link
Member

Choose a reason for hiding this comment

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

Any particular reason to bump the minimal version requirement?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

See the comment by @holiman

Go-ethereum retains compatibility with two golang versions, so updating to 1.24 means dropping support for 1.22

So I updated the minimal version requirement.

@levisyin
Copy link
Contributor Author

WARN The linter 'tenv' is deprecated (since v1.64.0) due to: Duplicate feature another linter. Replaced by usetesting.

@MariusVanDerWijden Hi sir, shall we open usetesting linter? As this linter will report issues:

cmd/clef/run_test.go:60:15: os.MkdirTemp() could be replaced by t.TempDir() in runClef (usetesting)
        ddir, err := os.MkdirTemp("", "cleftest-*")
                     ^
cmd/utils/history_test.go:90:14: os.MkdirTemp() could be replaced by t.TempDir() in TestHistoryImportAndExport (usetesting)
        dir, err := os.MkdirTemp("", "history-export-test")
                    ^
core/txpool/blobpool/blobpool_test.go:455:16: os.MkdirTemp() could be replaced by t.TempDir() in TestOpenDrops (usetesting)
        storage, _ := os.MkdirTemp("", "blobpool-")
                      ^
core/txpool/blobpool/blobpool_test.go:778:16: os.MkdirTemp() could be replaced by t.TempDir() in TestOpenIndex (usetesting)
        storage, _ := os.MkdirTemp("", "blobpool-")
                      ^
core/txpool/blobpool/blobpool_test.go:867:16: os.MkdirTemp() could be replaced by t.TempDir() in TestOpenHeap (usetesting)
        storage, _ := os.MkdirTemp("", "blobpool-")
                      ^
core/txpool/blobpool/blobpool_test.go:954:16: os.MkdirTemp() could be replaced by t.TempDir() in TestOpenCap (usetesting)
        storage, _ := os.MkdirTemp("", "blobpool-")
                      ^
core/txpool/blobpool/blobpool_test.go:1044:16: os.MkdirTemp() could be replaced by t.TempDir() in TestChangingSlotterSize (usetesting)
        storage, _ := os.MkdirTemp("", "blobpool-")
                      ^
core/txpool/blobpool/blobpool_test.go:1511:17: os.MkdirTemp() could be replaced by t.TempDir() in TestAdd (usetesting)
                storage, _ := os.MkdirTemp("", "blobpool-")
                              ^
crypto/crypto_test.go:184:13: os.CreateTemp("", ...) could be replaced by os.CreateTemp(t.TempDir(), ...) in TestLoadECDSA (usetesting)
                f, err := os.CreateTemp("", "loadecdsa_test.*.txt")
                          ^
crypto/crypto_test.go:205:12: os.CreateTemp("", ...) could be replaced by os.CreateTemp(t.TempDir(), ...) in TestSaveECDSA (usetesting)
        f, err := os.CreateTemp("", "saveecdsa_test.*.txt")
                  ^
crypto/signify/signify_test.go:36:18: os.CreateTemp("", ...) could be replaced by os.CreateTemp(t.TempDir(), ...) in TestSignify (usetesting)
        tmpFile, err := os.CreateTemp("", "")
                        ^
crypto/signify/signify_test.go:78:18: os.CreateTemp("", ...) could be replaced by os.CreateTemp(t.TempDir(), ...) in TestSignifyTrustedCommentTooManyLines (usetesting)
        tmpFile, err := os.CreateTemp("", "")
                        ^
crypto/signify/signify_test.go:101:18: os.CreateTemp("", ...) could be replaced by os.CreateTemp(t.TempDir(), ...) in TestSignifyTrustedCommentTooManyLinesLF (usetesting)
        tmpFile, err := os.CreateTemp("", "")
                        ^
crypto/signify/signify_test.go:124:18: os.CreateTemp("", ...) could be replaced by os.CreateTemp(t.TempDir(), ...) in TestSignifyTrustedCommentEmpty (usetesting)
        tmpFile, err := os.CreateTemp("", "")
                        ^
internal/era/era_test.go:40:12: os.CreateTemp("", ...) could be replaced by os.CreateTemp(t.TempDir(), ...) in TestEra1Builder (usetesting)
        f, err := os.CreateTemp("", "era1-test")
                  ^
node/config_test.go:56:15: os.CreateTemp("", ...) could be replaced by os.CreateTemp(t.TempDir(), ...) in TestDatadirCreation (usetesting)
        file, err := os.CreateTemp("", "")

@MariusVanDerWijden
Copy link
Member

I think we should do that in a follow-up pr, same with bumping the min-version imo

@levisyin
Copy link
Contributor Author

I think we should do that in a follow-up pr, same with bumping the min-version imo

@MariusVanDerWijden Done.

I have reverted the min go version requirement in README.md, and brought tenv linter back.

@holiman
Copy link
Contributor

holiman commented Feb 13, 2025

same with bumping the min-version imo

I figure that the text in the readme is an instruction / promise to the uesrs that geth work with the version posted there. Since this PR removes the CI-workflows which detects when we break that compatibility, it's overall a safer bet to change it to 1.23.

I mean, if we do support 1.22 a while longer, despite not explicitly promising to do so, nobody will complain. Whereas if some PR breaks it without CI detecting it, somebody is bound to complain.

But yeah, it's up to you (Marius et al)!

@levisyin
Copy link
Contributor Author

@MariusVanDerWijden Hi sir, Is there anything else I need to update here?

@fjl
Copy link
Contributor

fjl commented Feb 17, 2025

Not really. We will merge this when we feel ready for the upgrade to Go 1.24.

.travis.yml Outdated
script:
- travis_wait 45 go run build/ci.go test $TEST_PACKAGES

- stage: build
if: type = push
os: linux
dist: focal
go: 1.22.x
go: 1.24.x
Copy link
Contributor

Choose a reason for hiding this comment

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

This is not correct. We always want to keep a builder around for the previous Go releases, to catch regressions where we accidentally start using Go 1.24 features.

@fjl fjl merged commit 07d7fe2 into ethereum:master Feb 19, 2025
1 of 2 checks passed
@fjl fjl added this to the 1.15.3 milestone Feb 19, 2025
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.

6 participants