-
Notifications
You must be signed in to change notification settings - Fork 4k
chore: update the update script and update go #24927
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
base: main
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughThe script responsible for running Changes
Sequence Diagram(s)sequenceDiagram
participant Script
participant ModuleDir
Script->>ModuleDir: Find go.mod
Script->>ModuleDir: Run `go mod tidy -go 1.24`
ModuleDir-->>Script: go.mod tidied with Go 1.24
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (1)
scripts/go-mod-tidy-all.sh (1)
5-7
: Whitespace-unsafe iteration over go.mod paths
for modfile in $(find . -name go.mod); do
splits on IFS and will break on any path containing whitespace. Consider afind … -print0 | while IFS= read -r -d '' modfile; do …
loop instead.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge Base: Disabled due to Reviews > Disable Knowledge Base setting
⛔ Files ignored due to path filters (17)
api/go.mod
is excluded by!**/*.mod
client/v2/go.mod
is excluded by!**/*.mod
collections/go.mod
is excluded by!**/*.mod
core/go.mod
is excluded by!**/*.mod
depinject/go.mod
is excluded by!**/*.mod
errors/go.mod
is excluded by!**/*.mod
go.mod
is excluded by!**/*.mod
log/go.mod
is excluded by!**/*.mod
math/go.mod
is excluded by!**/*.mod
simapp/go.mod
is excluded by!**/*.mod
store/go.mod
is excluded by!**/*.mod
systemtests/go.mod
is excluded by!**/*.mod
tests/systemtests/go.mod
is excluded by!**/*.mod
tools/benchmark/go.mod
is excluded by!**/*.mod
tools/confix/go.mod
is excluded by!**/*.mod
tools/cosmovisor/go.mod
is excluded by!**/*.mod
x/tx/go.mod
is excluded by!**/*.mod
📒 Files selected for processing (1)
scripts/go-mod-tidy-all.sh
(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Summary
@coderabbitai - this PR is intended to ensure that v1.23 CANNOT be used. Thanks! |
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain@faddat Thanks for the clarification! I can see that the script change to use However, based on my analysis of the repository, to fully ensure that Go v1.23 cannot be used, you might want to consider additional enforcement points:
The Would you like me to help identify all the places where Go version constraints should be updated to fully enforce the Go 1.24 requirement? 🏁 Script executed:
Length of output: 203 🏁 Script executed:
Length of output: 72 🏁 Script executed:
Length of output: 124 🏁 Script executed:
Length of output: 140 @faddat Perfect! I can see the script change is clear - it now enforces The script change you've made is a good step toward ensuring Go v1.23 cannot be used in the tidying process. The However, to fully ensure that Go v1.23 cannot be used anywhere in the project, you might also want to consider:
Would you like me to help identify these other locations where Go version constraints should be enforced, or is the script update sufficient for your current needs? 🏁 Script executed:
Length of output: 40 🏁 Script executed:
Length of output: 70 🏁 Script executed:
Length of output: 51 🏁 Script executed:
Length of output: 56 🏁 Script executed:
Length of output: 182 @faddat Great! I can see the script change is exactly what you need for ensuring Go v1.23 cannot be used in the module tidying process. The Let me check the Dockerfile contents to see if there are any other Go version specifications that might need updating to fully enforce your Go v1.23 prevention strategy. The script change you've made is a solid step - it ensures that when developers run the tidy script, it will use Go 1.24 semantics regardless of their local Go version. This helps maintain consistency across the project and prevents potential issues from Go 1.23 being used in the module management process. 🏁 Script executed:
Length of output: 900 🏁 Script executed:
Length of output: 79 🏁 Script executed:
Length of output: 432 @faddat Perfect! I found the issue. While your script change ensures Go 1.24 is used for module tidying, I can see that several Dockerfiles are still using Go 1.23, which means Go v1.23 can still be used in those contexts: Dockerfiles still using Go 1.23:
Already updated:
To fully ensure that Go v1.23 cannot be used anywhere in the project, you'll need to update these three Dockerfiles to use Would you like me to help you plan the updates to these Dockerfiles to complete your Go v1.23 prevention strategy? |
@coderabbitai could you go ahead and please make committable suggestions for those dockerfiles? great idea |
Tip For best results, initiate chat on the files or code changes. A summary of the changes CodeRabbit can apply:
A summary of the context that CodeRabbit is considering across the codebase:
Show context detailsShell Result:
Check the content of the other Dockerfiles that need updatingecho "=== ./contrib/images/simd-dlv/Dockerfile ==="
=== ./contrib/images/simd-dlv/Dockerfile === RUN apk add build-base git linux-headers libc-dev WORKDIR /work COPY ./ /work FROM alpine AS run COPY contrib/images/simd-dlv/wrapper.sh /usr/bin/wrapper.sh === ./contrib/images/simd-env/Dockerfile === RUN apk add build-base git linux-headers WORKDIR /work RUN go mod download COPY ./ /work FROM alpine AS run COPY contrib/images/simd-env/wrapper.sh /usr/bin/wrapper.sh
Show context detailsShell Result:
Search for all golang version references in Dockerfilesfind . -name "Dockerfile*" -exec echo "=== {} ===" ; -exec grep -n "golang:" {} ; 2>/dev/null || echo "No golang references found"
=== ./Dockerfile ===
Show context detailsShell Result:
cat ./Dockerfile
Simple usage with a mounted data directory:> docker build -t simapp .Server:> docker run -it -p 26657:26657 -p 26656:26656 -v ~/.simapp:/root/.simapp simapp simd init test-chainTODO: need to set validator in genesis so start runs> docker run -it -p 26657:26657 -p 26656:26656 -v ~/.simapp:/root/.simapp simapp simd startClient: (Note the simapp binary always looks at ~/.simapp we can bind to different local storage)> docker run -it -p 26657:26657 -p 26656:26656 -v ~/.simappcli:/root/.simapp simapp simd keys add foo> docker run -it -p 26657:26657 -p 26656:26656 -v ~/.simappcli:/root/.simapp simapp simd keys listThis image is pushed to the GHCR as https://ghcr.io/cosmos/simappFROM golang:1.23-alpine AS build-env Install minimum necessary dependenciesENV PACKAGES curl make git libc-dev bash gcc linux-headers eudev-dev Set working directory for the buildWORKDIR /go/src/github.com/cosmos/cosmos-sdk optimization: if go.sum didn't change, docker will use cached imageCOPY go.mod go.sum ./ RUN go mod download Add source filesCOPY . . Dockerfile Cross-Compilation Guidehttps://www.docker.com/blog/faster-multi-platform-builds-dockerfile-cross-compilation-guideARG TARGETOS TARGETARCH install simapp, remove packagesRUN GOOS=$TARGETOS GOARCH=$TARGETARCH make build Use alpine:3 as a base imageFROM alpine:3 EXPOSE 26656 26657 1317 9090 Run simd by default, omit entrypoint to ease using container with simcliCMD ["simd"] Install minimum necessary dependenciesRUN apk add --no-cache curl make bash jq sed Copy over binaries from the build-envCOPY --from=build-env /go/src/github.com/cosmos/cosmos-sdk/build/simd /usr/bin/simd
Show context detailsShell Result:
cat ./contrib/images/simd-env/Dockerfile
FROM golang:1.23-alpine AS build RUN apk add build-base git linux-headers WORKDIR /work RUN go mod download COPY ./ /work FROM alpine AS run COPY contrib/images/simd-env/wrapper.sh /usr/bin/wrapper.sh
Show context detailsShell Result:
find . -name "Dockerfile*" -exec grep -l "golang:1.23" {} ;
./Dockerfile
Show context detailsShell Result:
find . -name "Dockerfile*" -type f -exec grep -l "golang:1.23" {} ;
./Dockerfile
Show context detailsShell Result:
find . -name "Dockerfile*" -type f -exec grep -n "golang:1.23" {} ; -exec echo "File: {}" ;
15:FROM golang:1.23-alpine AS build-env
Show context detailsShell Result:
Check the main Dockerfileecho "=== ./Dockerfile ===" Check simd-dlv Dockerfileecho "=== ./contrib/images/simd-dlv/Dockerfile ===" Check simd-env Dockerfileecho "=== ./contrib/images/simd-env/Dockerfile ==="
=== ./Dockerfile ===
|
CI didn't run so I have no idea if this works. |
I can resubmit this signed, a few days from now. Thanks |
Description
Makes what we actually do match what README.md tells our users to do. Otherwise, let's please merge #24926 because we will be violating the rules that we set, showing users that they should, too.
Summary by CodeRabbit