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

Cherry pick fix for go1.23 from go-ethereum #355

Merged
merged 1 commit into from
Aug 13, 2024

Conversation

anacrolix
Copy link
Contributor

I cherry picked a fix in go-ethereum for Go 1.23 since I just switched to that toolchain.

I assume this fix will come down through the normal merge process eventually anyway but it doesn't hurt to get unstuck early for go1.23.

I expect go-ethereum will evolve its approach as it determines the best way forward.

@anacrolix anacrolix requested a review from a team as a code owner August 5, 2024 03:29
Copy link
Member

@sebastianst sebastianst left a comment

Choose a reason for hiding this comment

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

It would be nice to credit original authorship by using git cherry-pick e467577 after adding upstream geth as a remote in git. Then the commit will be authored by fjl, and only committed by you, see 11c13e8 where I did this.

@axelKingsley
Copy link
Contributor

We should also note that as we merge in an upstream which includes this commit, we should first revert the cherry-pick in order to maintain a clean merge.

@danyalprout is working on a merge of upstream geth, but I don't think that will take this commit.

@anacrolix
Copy link
Contributor Author

I redid the cherry pick and this time it kept the author. Possibly last time I did another fix on top then squashed it and it threw everything away, but now it's correct.

@sebastianst
Copy link
Member

We should also note that as we merge in an upstream which includes this commit, we should first revert the cherry-pick in order to maintain a clean merge.

@danyalprout is working on a merge of upstream geth, but I don't think that will take this commit.

In general, I think the upstream merge would just work and this change would be a noop when they merge it in (or they have to resolve a little merge conflict).

@anacrolix anacrolix enabled auto-merge (squash) August 8, 2024 12:54
@anacrolix
Copy link
Contributor Author

Updated for the geth merge.

internal/debug: remove memsize (#30253)

Removing because memsize will very likely be broken by Go 1.23. See
fjl/memsize#4

(cherry picked from commit e467577)
@anacrolix anacrolix merged commit dc04347 into optimism Aug 13, 2024
8 of 9 checks passed
@anacrolix anacrolix deleted the anacrolix/go1.23-memsize branch August 13, 2024 17:59
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.

4 participants