-
Notifications
You must be signed in to change notification settings - Fork 71
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
ci: Switch GitHub macOS build workflow to use macos-13 (x86) and macos-14 (ARM) runners. #566
Conversation
WalkthroughThe pull request introduces updates to the GitHub Actions workflows and documentation related to macOS builds. The workflow for building on macOS has been modified to support a wider range of macOS versions by implementing a matrix strategy and updating installation paths. Additionally, documentation has been revised to generalize macOS setup instructions, removing specific references to macOS 12. Changes include updates to exclusion patterns in workflow triggers and adjustments in the installation script paths across various documentation files. Changes
Possibly related PRs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
✅ Files skipped from review due to trivial changes (2)
🔇 Additional comments (6).github/workflows/clp-core-build-macos.yaml (4)
The generalized path pattern for macOS installation scripts aligns well with the goal of supporting multiple macOS versions.
The generalized path aligns with the version-agnostic approach. However, we should verify that the installation script exists at the new location and maintains compatibility across macOS versions. #!/bin/bash
# Description: Verify installation script existence and contents
# Check if the script exists
echo "Checking for installation script..."
if [ -f "./components/core/tools/scripts/lib_install/macos/install-all.sh" ]; then
echo "Script found. Checking for version-specific commands..."
# Look for any remaining version-specific references
rg -i "macos-1[234]" "./components/core/tools/scripts/lib_install/macos/install-all.sh"
else
echo "Warning: Installation script not found at expected location"
fi
The matrix configuration with macos-13 and macos-14 runners is well-structured. However, we should verify that this covers both x86 and ARM architectures as intended. ✅ Verification successfulArchitecture coverage is adequately handled by the matrix configuration Based on the search results, we can confirm that:
The matrix strategy using both macos-13 and macos-14 will effectively cover both x86 and ARM architectures without requiring additional configuration. 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Check if there are any architecture-specific build configurations or tests
# that might need adjustment for the new runners
# Look for architecture-specific configurations
echo "Checking for architecture-specific configurations..."
rg -i "arch|x86|arm|aarch64" ./components/core/cmake/
rg -i "arch|x86|arm|aarch64" ./components/core/CMakeLists.txt
# Check for potential architecture-dependent code
echo "Checking for architecture-dependent code..."
rg -i "arch|x86|arm|aarch64" ./components/core/src/
Length of output: 438 Script: #!/bin/bash
# Let's try a different approach to check for architecture-specific configurations
# Check workflow files for architecture-specific settings
echo "Checking workflow files for architecture configurations..."
rg -i "arch|x86|arm|aarch64" .github/workflows/
# Check build scripts and installation files
echo "Checking build and installation scripts..."
fd -e sh -e bash . ./components/core/tools/scripts/ -x rg -i "arch|x86|arm|aarch64" {}
# Check CMake files with different path approach
echo "Checking CMake files..."
fd CMakeLists.txt . -x rg -i "arch|x86|arm|aarch64" {}
fd -e cmake . -x rg -i "arch|x86|arm|aarch64" {}
Length of output: 15371 docs/src/dev-guide/components-core/index.md (2)
The removal of the specific macOS version from the documentation link text aligns well with the PR's objective to support multiple macOS versions.
The toctree entry has been appropriately updated to match the generalized macOS documentation structure. 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.
The workflows succeed.
But I found that in /home/xiaochong/clp/components/core/submodules/json/.github/workflows/macos.yml, there is a:
xcode_2:
runs-on: macos-12
Also in /home/xiaochong/clp/components/core/submodules/outcome/boostify/.github/workflows/ci.yml, there is a:
- { name: "AppleClang 13", os: macos-12, cxxstd: 17 }
If these two are unrelated to the changes then the PR should be good to be merged.
Thanks. Yeah, those are from submodules (external libraries). |
* ffi: Add support for serializing/deserializing auto-generated and user-generated schema tree node IDs. (y-scope#557) Co-authored-by: kirkrodrigues <[email protected]> * clp: Add missing C++ standard library includes in IR parsing files. (y-scope#561) Co-authored-by: kirkrodrigues <[email protected]> * log-viewer-webui: Update `yscope-log-viewer` to the latest version (which uses `clp-ffi-js`). (y-scope#562) * package: Upgrade dependencies to resolve security issues. (y-scope#536) * clp-s: Implement table packing (y-scope#466) Co-authored-by: wraymo <[email protected]> Co-authored-by: Kirk Rodrigues <[email protected]> Co-authored-by: wraymo <[email protected]> * log-viewer-webui: Update `yscope-log-viewer` to the latest version. (y-scope#565) * ci: Switch GitHub macOS build workflow to use macos-13 (x86) and macos-14 (ARM) runners. (y-scope#566) * core: Add support for user-defined HTTP headers in `NetworkReader`. (y-scope#568) Co-authored-by: Lin Zhihao <[email protected]> Co-authored-by: Xiaochong Wei <[email protected]> * chore: Update to the latest version of yscope-dev-utils. (y-scope#574) * build(core): Upgrade msgpack to v7.0.0. (y-scope#575) * feat(ffi): Update IR stream protocol version handling in preparation for releasing the kv-pair IR stream format: (y-scope#573) - Bump the IR stream protocol version to 0.1.0 for the kv-pair IR stream format. - Treat the previous IR stream format's versions as backwards compatible. - Differentiate between backwards-compatible and supported versions during validation. Co-authored-by: kirkrodrigues <[email protected]> * fix(taskfiles): Trim trailing slash from URL prefix in `download-and-extract-tar` (fixes y-scope#577). (y-scope#578) * fix(ffi): Correct `clp::ffi::ir_stream::Deserializer::deserialize_next_ir_unit`'s return value when failing to read the next IR unit's type tag. (y-scope#579) * fix(taskfiles): Update `yscope-log-viewer` sources in `log-viewer-webui-clients` sources list (fixes y-scope#576). (y-scope#580) * fix(cmake): Add Homebrew path detection for `mariadb-connector-c` to fix macOS build failure. (y-scope#582) Co-authored-by: kirkrodrigues <[email protected]> * refactor(ffi): Make `get_schema_subtree_bitmap` a public method of `KeyValuePairLogEvent`. (y-scope#581) * ci: Schedule GitHub workflows to daily run to detect failures due to upgraded dependencies or environments. (y-scope#583) * docs: Update the required version of task. (y-scope#567) * Add pr check workflow --------- Co-authored-by: kirkrodrigues <[email protected]> Co-authored-by: Junhao Liao <[email protected]> Co-authored-by: Henry8192 <[email protected]> Co-authored-by: Devin Gibson <[email protected]> Co-authored-by: wraymo <[email protected]> Co-authored-by: wraymo <[email protected]> Co-authored-by: Xiaochong(Eddy) Wei <[email protected]> Co-authored-by: Xiaochong Wei <[email protected]> Co-authored-by: haiqi96 <[email protected]>
…s-14 (ARM) runners. (y-scope#566)
Description
The GH macos-12 hosted runner image has begun to be deprecated leading to much longer build times for our macOS workflow.
This PR addresses the issue by:
core
.Validation performed
task docs:serve
and validated the docs about installing on macOS were updated.Summary by CodeRabbit
New Features
Documentation