-
Notifications
You must be signed in to change notification settings - Fork 109
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
feat: improve build reproducability #3091
Conversation
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 Walkthrough📝 WalkthroughWalkthroughThe changes in this pull request involve modifications to the Changes
Possibly related PRs
Suggested labels
Suggested reviewers
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? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
Documentation and Community
|
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
🧹 Outside diff range and nitpick comments (2)
.github/workflows/publish-release.yml (2)
Line range hint
82-92
: Fix case sensitivity in changelog filename.The script uses
changelog.md
but the error message refers toCHANGELOG.md
. This inconsistency could cause issues on case-sensitive filesystems.Apply this fix:
- CHANGELOG_DIFF=$(git diff ${DEVELOP_SHA}..${CURRENT_BRANCH_SHA} -- changelog.md) + CHANGELOG_DIFF=$(git diff ${DEVELOP_SHA}..${CURRENT_BRANCH_SHA} -- CHANGELOG.md)🧰 Tools
🪛 yamllint
[error] 54-54: trailing spaces
(trailing-spaces)
Line range hint
164-177
: Improve release tag handling robustness.The current tag handling implementation has several potential issues:
- Using
sleep
for API synchronization is fragile- No error handling for tag deletion failures
- No verification of successful tag recreation
Consider this more robust implementation:
- create_tag=$(git push --tags || echo "tag exists") - if [[ $create_tag == "tag exists" ]]; then - echo "Delete existing tag to re-create" - git tag -d ${GITHUB_TAG_MAJOR_VERSION} - git push --delete origin ${GITHUB_TAG_MAJOR_VERSION} - echo "sleep for 5 seconds to let github catch up." - sleep 5 - echo "Re-Create Tag." - git tag ${GITHUB_TAG_MAJOR_VERSION} - git push --tags + if ! git push --tags 2>/dev/null; then + echo "Attempting to recreate tag..." + if ! git tag -d ${GITHUB_TAG_MAJOR_VERSION} && \ + ! git push --delete origin ${GITHUB_TAG_MAJOR_VERSION}; then + echo "Failed to delete existing tag" + exit 1 + fi + # Verify tag is gone + if git ls-remote --tags origin | grep -q ${GITHUB_TAG_MAJOR_VERSION}; then + echo "Failed to delete remote tag" + exit 1 + fi + if ! git tag ${GITHUB_TAG_MAJOR_VERSION} && \ + ! git push --tags; then + echo "Failed to recreate tag" + exit 1 + fi + fi🧰 Tools
🪛 yamllint
[error] 54-54: trailing spaces
(trailing-spaces)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
.github/workflows/publish-release.yml
(1 hunks).goreleaser.yaml
(1 hunks)Makefile
(2 hunks)
🔇 Additional comments (7)
.goreleaser.yaml (2)
61-66
: Excellent reproducibility improvements!
The changes effectively address build reproducibility:
- Using
CommitDate
ensures stable timestamps - Empty
buildid
removes non-deterministic build IDs - Strip flags (
-s -w
) remove debug symbols
These modifications align perfectly with the goal of achieving bit-perfect builds across different machines.
Line range hint 78-80
: Please clarify the platform support reduction.
The Darwin and Windows builds for zetaclientd
have been commented out. This significantly reduces platform support:
goos:
- linux
# - darwin
# - windows
Was this intentional? If so, please:
- Document the rationale for this change
- Update relevant documentation to reflect the reduced platform support
- Consider adding a migration guide for affected users
.github/workflows/publish-release.yml (2)
Line range hint 196-211
: LGTM: Excellent addition of build provenance attestation.
The implementation of build provenance attestation is a valuable addition that:
- Enhances supply chain security
- Provides verifiable build artifacts
- Complements the reproducible build objectives
🧰 Tools
🪛 yamllint
[error] 54-54: trailing spaces
(trailing-spaces)
51-53
: LGTM: Build snapshot step aligns with reproducibility goals.
The change from release-dry-run
to release-snapshot
better reflects the actual purpose of this step and aligns with the PR's objective of ensuring reproducible builds.
Let's verify the make target exists:
✅ Verification successful
Verified: The release-snapshot
make target is correctly defined and configured
The release-snapshot
make target exists in the Makefile and is properly configured to run goreleaser in snapshot mode with appropriate flags:
- Uses
--clean
for clean builds - Skips validation and publishing with
--skip=validate --skip=publish
- Enables snapshot mode with
--snapshot
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify the release-snapshot target exists in Makefile
rg -A 3 '^release-snapshot:' Makefile
Length of output: 143
Makefile (3)
443-454
: LGTM! Release commands are well-structured.
The changes improve the release process by:
- Providing clear command separation (snapshot vs. build-only)
- Using the standardized GORELEASER variable
- Adding proper validation for release environment
32-37
: Build flags changes enhance reproducibility.
The modifications effectively address build reproducibility:
-buildid=
ensures consistent build IDs- Removal of BUILDTIME prevents timestamp variations
Let's verify the impact of build flags:
✅ Verification successful
Build flags are consistently applied across all build targets
The verification confirms that:
- The build flags (including
-buildid=
) are uniformly applied viaBUILD_FLAGS
variable across all build targets - No remaining timestamp-related flags exist
- The flags are properly used in all
go install
commands throughout the Makefile - Different build configurations (standard, test, HSM) maintain consistent base flags
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if build flags are consistently applied across build targets
# Expected: Flags should be present in all relevant build commands
# Check build flags in go install commands
rg -A 2 "go install.*BUILD_FLAGS"
# Check if any timestamp or build ID related flags remain
rg "buildtime|buildid"
Length of output: 353
Script:
#!/bin/bash
# Let's verify the build flags more comprehensively
# Check all go build/install commands in Makefile
rg -A 5 "go (build|install)" Makefile
# Check for any other build flag definitions or usages
rg "BUILD_FLAGS|LDFLAGS" Makefile
# Check if there are any other files containing build configurations
fd Makefile
fd "\.mk$"
Length of output: 2356
15-26
: LGTM! Build configuration ensures reproducibility.
The changes effectively standardize the build environment by:
- Pinning the Go version with SHA256 verification
- Using Docker to provide a consistent build environment
- Setting required environment variables
Let's verify the Docker image hash:
✅ Verification successful
✓ Verified: Docker image configuration is correct and secure
The verification confirms:
- The specified goreleaser-cross v1.22.7 tag exists in the official repository
- The SHA256 hash
24b2d75007f0ec8e35d01f3a8efa40c197235b200a1a91422d78b851f67ecce4
matches the official container image - The image was published on 2024-09-09 and is properly tagged
The Makefile's build configuration ensures reproducible builds by correctly pinning to a verified, immutable container image.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the goreleaser Docker image exists and matches the expected hash
# Expected: Image exists and hash matches
docker pull ghcr.io/goreleaser/goreleaser-cross:v1.22.7@sha256:24b2d75007f0ec8e35d01f3a8efa40c197235b200a1a91422d78b851f67ecce4
Length of output: 353
Script:
#!/bin/bash
# Let's verify the image existence and tag using GitHub API
gh api -H "Accept: application/vnd.oci.image.index.v1+json" /v2/goreleaser/goreleaser-cross/manifests/v1.22.7 || \
gh api -H "Accept: application/vnd.docker.distribution.manifest.v2+json" /v2/goreleaser/goreleaser-cross/manifests/v1.22.7
# Also check if this version exists in their releases
gh api repos/goreleaser/goreleaser-cross/releases | jq '.[] | select(.tag_name=="v1.22.7")'
# Let's also verify the Makefile configuration
rg -A 2 'GOLANG_CROSS_VERSION' Makefile
Length of output: 1281
Script:
#!/bin/bash
# Let's verify the image using a different approach
# Check if the version exists in the goreleaser-cross repository
gh api repos/goreleaser/goreleaser-cross/tags | jq '.[] | select(.name=="v1.22.7")'
# Check the container registry directly
gh api \
-H "Accept: application/vnd.github+json" \
-H "X-GitHub-Api-Version: 2022-11-28" \
/orgs/goreleaser/packages/container/goreleaser-cross/versions | \
jq '.[] | select(.metadata.container.tags[] | contains("v1.22.7"))'
# Let's also check the repository's workflow files for this version
rg -l "v1.22.7" .github/workflows/
Length of output: 7631
Something must have changed since you posted your checksums.
|
Sorry yeah it's |
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.
Looks good.
My updated checksums after using
|
af1badd
to
91a4510
Compare
Description
Attempt to make builds bit perfect reproducable. If this is successful, two machines performing a build with the same code should build a binary with the exact same hash.
Steps taken:
buildid
as this is usually randomMake targets are now:
make release-snapshot
: build a release without a tagmake release-build-only
: build a release with a tag but without uploading the artifactsmake release
: build a release with a tag and upload the artifactsgoreleaser reference
goreleaser output
make install-zetacore output
ci-testing-node workflow runs
first run: https://github.com/zeta-chain/ci-testing-node/actions/runs/11674433708
second run: https://github.com/zeta-chain/ci-testing-node/actions/runs/11674641533
Summary by CodeRabbit
New Features
Bug Fixes
Refactor