-
Notifications
You must be signed in to change notification settings - Fork 629
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
Fix `.goreleaser.yaml and Makefile targets #8869
Conversation
WalkthroughThe 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
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: 0
🧹 Outside diff range and nitpick comments (1)
scripts/makefiles/release.mk (1)
40-40
: LGTM! Consider documenting the skip options.The consolidation of skip flags into
--skip=publish,validate
is correct and follows modern goreleaser syntax.Consider adding a comment above the release targets to document the available skip options for better maintainability:
+# Available skip options: +# - publish: Skip publishing artifacts +# - validate: Skip validation steps release-dry-run:
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
.goreleaser.yaml
is excluded by!**/*.yaml
📒 Files selected for processing (1)
scripts/makefiles/release.mk
(2 hunks)
🔇 Additional comments (2)
scripts/makefiles/release.mk (2)
Line range hint 1-40
: Verify related configuration changes.
The PR objectives mention removing Darwin builds and updating libwasmvm paths, but these changes aren't visible in the Makefile.
Let's verify the related changes:
✅ Verification successful
Let me gather more context about the PR changes by examining the .goreleaser.yaml file content:
Darwin builds have been removed and libwasmvm paths are correctly configured
The .goreleaser.yaml file confirms:
- Darwin/macOS builds are completely removed - only Linux builds for amd64 and arm64 are configured
- libwasmvm paths are correctly set up with architecture-specific paths:
- x86_64:
/usr/lib/libwasmvm_muslc.x86_64.a
- aarch64:
/usr/lib/libwasmvm_muslc.aarch64.a
- x86_64:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for the mentioned changes in .goreleaser.yaml
# Check for Darwin/macOS related configurations
echo "Checking for Darwin/macOS configurations:"
rg -i "darwin|macos" .goreleaser.yaml || echo "No Darwin/macOS configurations found"
# Check for libwasmvm path configurations
echo -e "\nChecking for libwasmvm configurations:"
rg -i "libwasmvm" .goreleaser.yaml || echo "No libwasmvm configurations found"
Length of output: 661
Script:
#!/bin/bash
# Get the full content of .goreleaser.yaml to understand the build configurations
cat .goreleaser.yaml
Length of output: 4726
27-27
: LGTM! Verify goreleaser flag compatibility.
The updated syntax for skipping publish is correct for newer versions of goreleaser.
Let's verify the flag compatibility with the used goreleaser version:
✅ Verification successful
The updated skip flag syntax is correct and consistent
The changes to use --skip=publish
and --skip=publish,validate
are correct and consistent throughout the Makefile. The new syntax:
- Is used consistently in both release targets (dry-run and snapshot)
- No instances of the old
--skip-publish
or--skip-validate
syntax remain - Follows the comma-separated format for multiple skip values
The goreleaser image version is dynamically set using the Go version (v$(GO_VERSION)
), which is a common practice, and the new skip flag syntax is compatible with recent goreleaser versions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check goreleaser documentation and version compatibility
# Extract goreleaser version from the image tag
version=$(rg -o 'GORELEASER_IMAGE := ghcr.io/goreleaser/goreleaser-cross:v([0-9.]+)' -r '$1' scripts/makefiles/release.mk)
echo "Goreleaser version: $version"
# Check if there are any other instances of old flag syntax that need updating
rg --type make "(-{2}skip-publish|-{2}skip-validate)"
Length of output: 232
Script:
#!/bin/bash
# Let's try a different approach to find the goreleaser image and version
rg "GORELEASER" scripts/makefiles/release.mk -A 2
# Also check for any remaining old-style skip flags in the entire codebase
rg "skip-[a-z]+" --type make
# Let's also see the content of the release.mk to understand the context better
cat scripts/makefiles/release.mk
Length of output: 1572
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.
LGTM!
(cherry picked from commit 1a181bf)
(cherry picked from commit 1a181bf) Co-authored-by: Niccolo Raspa <[email protected]>
What is the purpose of the change
This PR:
.goreleaser.yaml
as we are no longer publishing themlinux-amd64
andlinux-arm64
builds to fetchlibwasmvm
in specific paths--skip-publish
and--skip-validate
Testing and Verifying
I have tested the build locally on an Ubuntu machine, and it is working. However, it is not working on my ARM MacBook.
I encounter the following warnings during the build process:
To address these warnings, we would need to use
musl
libraries instead ofglib
. However, themusl-gcc
compiler is not available in the goreleaser-cross container.Documentation and Release Note
Unreleased
section ofCHANGELOG.md
?Where is the change documented?
x/{module}/README.md
)