-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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:release wrong #2979
fix:release wrong #2979
Conversation
WalkthroughThe pull request updates the GitHub Actions release workflow configuration, specifically changing the macOS build environment from version 12 to version 13. This modification updates the operating system used for building the macOS binary in the release process, ensuring compatibility with the latest macOS runner provided by GitHub Actions. Changes
Poem
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
🧹 Nitpick comments (1)
.github/workflows/release.yml (1)
Line range hint
1-106
: Consider these workflow improvementsWhile the workflow is functional, here are some suggested improvements:
Pin the GitHub Actions versions more specifically for better reproducibility:
softprops/action-gh-release@v1
→softprops/action-gh-release@v1.1.0
Add artifact retention period to manage storage:
- Add
retention-days: 5
toupload-artifact
stepsEnhance error handling:
- Add
if: failure()
steps for error notifications- Add timeout-minutes to long-running jobs
Would you like me to provide the specific changes for any of these improvements?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
.github/workflows/release.yml
(1 hunks)
🔇 Additional comments (2)
.github/workflows/release.yml (2)
Line range hint 13-20
: Verify artifact naming consistency
The naming convention for release artifacts appears consistent, but let's verify the implementation in the build script.
✅ Verification successful
Artifact naming convention is correctly implemented
The verification confirms that the artifact naming is consistent and properly implemented:
- The workflow correctly defines artifact names using the pattern
{repo}-{tag}-{platform}-amd64.tar.gz
for all platforms:- Ubuntu:
${{ github.event.repository.name }}-${{ github.ref_name }}-ubuntu-amd64.tar.gz
- macOS:
${{ github.event.repository.name }}-${{ github.ref_name }}-macos-amd64.tar.gz
- Rocky:
${{ github.event.repository.name }}-${{ github.ref_name }}-rocky-amd64.tar.gz
- Ubuntu:
- The build script (
ci/release-build.sh
) correctly handles the package name passed from the workflow - The release job properly collects and publishes all artifacts using the pattern
**/${{ github.event.repository.name }}-*
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify artifact naming consistency across the build script
# Expected results: Consistent naming pattern usage
# Check for artifact naming patterns in the build script
rg -A 2 "tar\.gz|\.sha256sum" "ci/release-build.sh"
# Verify no hardcoded platform names that might conflict
rg "macos-|ubuntu-|rocky-" "ci/release-build.sh"
Length of output: 184
Script:
#!/bin/bash
# Let's check the release workflow and build script more thoroughly
# First, let's see the complete release workflow
cat .github/workflows/release.yml
# Then, let's examine the build script
cat ci/release-build.sh
# Also check for any other related workflow files
fd workflow .github/workflows --exec cat {}
Length of output: 5055
19-19
: Verify macOS binary compatibility
The change from macos-12
to macos-13
is aligned with GitHub Actions' supported runners. However, we should ensure that the built binary maintains compatibility with older macOS versions that your users might be running.
Let's verify the current GitHub Actions runner recommendations and check if there are any known issues:
Consider documenting the minimum supported macOS version in your project's documentation to set clear expectations for users.
✅ Verification successful
Let me gather more information about the build configuration and minimum macOS version requirements.
The previous results showed that:
- The build script uses gcc@10 specifically for macOS builds
- There's a reported issue about runner compatibility with macOS 10.13, but it's related to the runner's Node.js binary, not our build output
- The build process uses standard tools (autoconf, protobuf, llvm) installed via brew
Let's check for any explicit macOS version requirements or deployment targets in the codebase before making the final decision.
Upgrading to macos-13 runner is safe
The change from macos-12 to macos-13 is safe because:
- The build script uses standard tools installed via Homebrew and gcc@10 as the compiler
- No explicit macOS version constraints or deployment targets are specified in the codebase
- The build process doesn't employ any version-specific features or configurations
The binary compatibility is maintained through the use of gcc@10 and standard build tools, which handle backward compatibility well on macOS systems.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check GitHub's documentation for macOS runner versions and any related discussions
# Expected results: Confirmation that macos-13 is a stable, recommended runner
# Check for any existing issues or discussions about macOS runner compatibility
gh search issues "macos-13 runner compatibility" --repo "$GITHUB_REPOSITORY" --json title,url
# Check the build script for any macOS-specific configurations
rg -A 5 "macos|darwin" "ci/release-build.sh"
Length of output: 959
Script:
#!/bin/bash
# Check for any macOS version specifications in CMake files and documentation
rg -i "macos.*version|darwin.*version|osx.*version" --type md --type cmake
# Check for any deployment target or minimum version settings
rg "CMAKE_OSX|MACOSX_DEPLOYMENT_TARGET"
# Look for any macOS compatibility notes in documentation
fd -e md -e txt -X grep -i -A 3 "macos.*requirement|system.*requirement"
Length of output: 193
Co-authored-by: chejinge <chejinge@360.cn>
Summary by CodeRabbit