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: upgrade to Go 1.23.1 #3848

Merged
merged 15 commits into from
Oct 2, 2024
Merged

chore: upgrade to Go 1.23.1 #3848

merged 15 commits into from
Oct 2, 2024

Conversation

rootulp
Copy link
Collaborator

@rootulp rootulp commented Sep 6, 2024

Description

Upgrade to Go 1.23.1 motivated by #3847. I had to modify GoReleaser config because goreleaser/goreleaser#5127.

Maintainers need to update their local golangic-lint binary to v1.61.0. See golangci-lint install.

@rootulp rootulp marked this pull request as ready for review September 9, 2024 11:25
Copy link
Contributor

coderabbitai bot commented Sep 9, 2024

📝 Walkthrough
📝 Walkthrough

Walkthrough

This pull request includes updates to various configuration files and dependencies in the project. Key changes involve upgrading the versions of tools such as golangci-lint, Go itself, and Docker images, as well as modifications to linting configurations and documentation files to ensure up-to-date information regarding dependencies.

Changes

File Change Summary
.github/workflows/lint.yml Updated golangci-lint-action version from v1.59.1 to v1.61.0, adjusted PATTERNS, and removed skip-pkg-cache.
Makefile Changed GOLANG_CROSS_VERSION from v1.22.6 to v1.23.1 and added a comment about verifying Docker images.
README.md Updated Go version from 1.22.6 to 1.23.1 and golangci-lint version from 1.59.1 to 1.61.0, clarified installation instructions, and refined contributing guidelines.
docker/Dockerfile Updated BUILDER_IMAGE from docker.io/golang:1.22.6-alpine3.19 to docker.io/golang:1.23.1-alpine3.20.
docker/txsim/Dockerfile Updated base image from golang:1.22.6-alpine3.19 to golang:1.23.1-alpine3.20.
go.mod Updated Go version to 1.23.1, modified several dependencies, and added replace directives.

Possibly related PRs

Suggested labels

backport:v1.x, backport:v2.x, external


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

‼️ IMPORTANT
Auto-reply has been disabled for this repository in the CodeRabbit settings. The CodeRabbit bot will not respond to your replies unless it is explicitly tagged.

  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@rootulp rootulp requested review from cmwaters and ninabarbakadze and removed request for liamsi September 9, 2024 11:28
evan-forbes
evan-forbes previously approved these changes Sep 9, 2024
rach-id
rach-id previously approved these changes Sep 10, 2024
@rootulp rootulp dismissed stale reviews from rach-id and evan-forbes via 3ab7627 September 10, 2024 12:37
@celestia-bot celestia-bot requested a review from a team September 10, 2024 12:37
@rootulp rootulp enabled auto-merge (squash) September 10, 2024 12:40
evan-forbes
evan-forbes previously approved these changes Sep 10, 2024
@celestia-bot celestia-bot requested a review from a team September 16, 2024 14:38
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Outside diff range and nitpick comments (1)
tools/blocketa/README.md (1)

18-18: Add a newline at the end of the file.

The static analysis tool suggests adding a single newline character at the end of the file. This is a good practice for file formatting.

 > The block time is currently hard-coded. If you're running this for a network with a different block time, you'll need to update the `blockTime` constant in the main.go file. You can use [https://www.mintscan.io/celestia/block](https://www.mintscan.io/celestia/block/) or the blocktime tool.
+
Tools
Markdownlint

18-18: null
Files should end with a single newline character

(MD047, single-trailing-newline)

Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 3ab7627 and 9b6980b.

Files selected for processing (1)
  • tools/blocketa/README.md (1 hunks)
Additional context used
Markdownlint
tools/blocketa/README.md

18-18: null
Files should end with a single newline character

(MD047, single-trailing-newline)

Additional comments not posted (1)
tools/blocketa/README.md (1)

18-18: LGTM!

The change to the URL formatting improves the readability of the documentation.

Tools
Markdownlint

18-18: null
Files should end with a single newline character

(MD047, single-trailing-newline)

ninabarbakadze
ninabarbakadze previously approved these changes Sep 18, 2024
Copy link
Member

@ninabarbakadze ninabarbakadze left a comment

Choose a reason for hiding this comment

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

utAck

@rootulp rootulp marked this pull request as draft September 20, 2024 13:26
auto-merge was automatically disabled September 20, 2024 13:26

Pull request was converted to draft

@rootulp
Copy link
Collaborator Author

rootulp commented Sep 20, 2024

There's a bug in Go 1.23 golang/go#69312 that is impacting celestia-node so let's hold off on merging this until we identify if the same issue may hit us.

Copy link
Member

@rach-id rach-id left a comment

Choose a reason for hiding this comment

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

Added some more bumps in case we want to bump everything that is bumpable that's related to go version.

Btw, I already bumped core to 1.23.1. If the bug is infecting that too, we can revert ASAP

.github/workflows/lint.yml Outdated Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
docker/Dockerfile_txsim Outdated Show resolved Hide resolved
Co-authored-by: CHAMI Rachid <chamirachid1@gmail.com>
rootulp and others added 4 commits September 20, 2024 10:33
Co-authored-by: CHAMI Rachid <chamirachid1@gmail.com>
Co-authored-by: CHAMI Rachid <chamirachid1@gmail.com>
Co-authored-by: CHAMI Rachid <chamirachid1@gmail.com>
Co-authored-by: CHAMI Rachid <chamirachid1@gmail.com>
@rootulp
Copy link
Collaborator Author

rootulp commented Oct 1, 2024

Looks like golang/go#69312 was fixed in Go 1.23.2. celestia-app doesn't invoke timer.Stop so I think we can proceed with this Go bump.

@rootulp rootulp marked this pull request as ready for review October 1, 2024 18:58
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (4)
README.md (1)

Line range hint 77-95: BBR congestion control instructions are a great addition.

The new instructions for enabling the BBR congestion control algorithm are clear and helpful. This addition will improve network performance for users.

Consider adding a note about system restart requirements (if any) after applying these changes.

go.mod (3)

3-3: Approve Go version update, but consider 1.23.2

The update to Go 1.23.1 is good, but since Go 1.23.2 is available and resolves the bug mentioned in issue #69312, consider upgrading to 1.23.2 for additional bug fixes and improvements.

Would you like to update to Go 1.23.2 in this PR?


Line range hint 5-184: Approve dependency updates, suggest documenting pinned version

The dependency updates look good and seem to be part of routine maintenance. However, it's worth noting that github.com/cosmos/cosmos-sdk has been pinned to v0.46.16.

Consider adding a comment in the go.mod file or in the project documentation explaining why this specific version of cosmos-sdk is pinned. This will help future maintainers understand the reasoning behind this decision.


Line range hint 186-194: Approve replace directives, suggest documenting fork usage

The replace directives look good and seem to address specific project needs. The comment explaining the pin for ledger-cosmos-go is helpful.

Consider adding similar comments for the other replace directives, especially for the forks of cosmos-sdk and tendermint. This will help future maintainers understand why these forks are being used instead of the original repositories.

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 9b6980b and 9bcfbef.

📒 Files selected for processing (6)
  • .github/workflows/lint.yml (1 hunks)
  • Makefile (2 hunks)
  • README.md (2 hunks)
  • docker/Dockerfile (1 hunks)
  • docker/txsim/Dockerfile (1 hunks)
  • go.mod (1 hunks)
🔇 Additional comments (9)
.github/workflows/lint.yml (1)

25-25: Approved: golangci-lint-action version update

The update to golangci-lint-action version v1.61.0 is appropriate and aligns with the PR objectives. This keeps the CI tools up-to-date, which is a good practice.

Reminder: As mentioned in the PR description, maintainers should update their local golangci-lint binary to version 1.61.0. You can verify the installed version locally using:

If an update is needed, follow the installation instructions provided in the PR description.

docker/txsim/Dockerfile (3)

Line range hint 28-28: Approve Alpine Linux version update in the second stage.

Updating to alpine:3.20 in the second stage maintains consistency with the builder stage and keeps the final image up-to-date. This is a good practice for security and performance reasons.


Line range hint 1-64: Summary of Dockerfile changes and their implications.

The Dockerfile has been successfully updated to use Go 1.23.1 and Alpine 3.20 in both stages. These changes align with the PR objective and represent good practices in keeping the build environment up-to-date.

Key points:

  1. The updates are consistent across both stages of the build.
  2. No other changes were required, suggesting good compatibility with the new versions.
  3. The minimal nature of the changes reduces the risk of introducing new issues.

Recommendations:

  1. Ensure thorough testing of the built image to verify compatibility with Go 1.23.1.
  2. Update any relevant documentation or CI/CD pipelines to reflect the new Go version requirement.
  3. Monitor for any performance changes or new warnings/errors during the build process.

To ensure the update doesn't introduce any regressions, please run your full test suite with the new image. Additionally, consider comparing the size and performance of the final image:

#!/bin/bash
# Build both old and new images
docker build -t txsim-old -f docker/txsim/Dockerfile.old .
docker build -t txsim-new -f docker/txsim/Dockerfile .

# Compare image sizes
echo "Old image size:"
docker images txsim-old --format "{{.Size}}"
echo "New image size:"
docker images txsim-new --format "{{.Size}}"

# Compare startup times (if applicable)
time docker run --rm txsim-old txsim version
time docker run --rm txsim-new txsim version

2-2: Approve Go and Alpine Linux version update.

The update to golang:1.23.1-alpine3.20 aligns with the PR objective of upgrading to Go 1.23.1. The Alpine Linux update to 3.20 is also a good practice for keeping the base image current.

To ensure compatibility, please verify that the application builds and runs correctly with these updated versions. Consider running the following commands in your CI pipeline or locally:

docker/Dockerfile (1)

7-7: Approved: Go version upgrade looks good.

The update from golang:1.22.6-alpine3.19 to golang:1.23.1-alpine3.20 aligns with the PR objective of upgrading to Go 1.23.1. This change should bring the latest improvements and bug fixes from Go 1.23.1.

Consider the following points:

  1. Ensure all project dependencies are compatible with Go 1.23.1.
  2. The Alpine version has also been updated from 3.19 to 3.20. Verify that this doesn't introduce any unexpected changes in the build environment.
  3. Confirm that the build process succeeds with this new image.
  4. Update any relevant documentation to reflect this Go version change.
  5. Ensure that CI/CD pipelines are updated to use Go 1.23.1.

To verify the Go version and Alpine version, run:

README.md (3)

38-38: Go version update looks good.

The Go version has been correctly updated to 1.23.1, which aligns with the PR objective.


Line range hint 127-131: Improved Contributing guidelines.

The added emphasis on conventional commits and the instructions for using go.work are valuable additions. These changes will help maintain consistent commit messages and improve the development workflow for contributors working with multiple Go modules.


Line range hint 168-172: Audit information updated appropriately.

The addition of the new audit entry for Informal Systems (dated 2024/7/1) keeps the security information current and demonstrates ongoing commitment to the project's security. The format is consistent with previous entries.

Makefile (1)

10-12: LGTM! Verify impact on other parts of the project.

The upgrade to Go 1.23.1 is implemented as intended. The added comment about verifying the Docker image existence is a helpful reminder for maintainers.

To ensure this change doesn't introduce any incompatibilities, please run the following script to check for any hardcoded Go version references in the project:

✅ Verification successful

LGTM! The Go version has been successfully upgraded to 1.23.1 without any lingering references to the old version.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for hardcoded Go version references that might need updating

# Search for Go version references in all files
echo "Searching for Go version references:"
rg --type-add 'go:*.{go,mod}' --type-add 'yaml:*.{yml,yaml}' --type go --type yaml --type make '1\.22|go1\.22' .

# Check go.mod file for Go version
echo -e "\nChecking go.mod file:"
if [ -f go.mod ]; then
    grep "^go " go.mod
else
    echo "go.mod file not found"
fi

# Check GitHub Actions workflows for Go setup
echo -e "\nChecking GitHub Actions workflows:"
rg --type yaml 'uses: actions/setup-go@v[0-9]+' .github/workflows

Length of output: 1151

@rootulp rootulp merged commit f0e7fba into celestiaorg:main Oct 2, 2024
33 checks passed
@rootulp rootulp deleted the rp/go-1.23 branch October 2, 2024 13:33
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