-
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
fix(gh-macos-build): Check if pkg-config is installed before trying to install it (fixes #610). #611
Conversation
WalkthroughThe pull request modifies the Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🔇 Additional comments (2)components/core/tools/scripts/lib_install/macos/install-all.sh (2)Line range hint The removal of pkg-config from the main installation list is consistent with the new conditional installation approach.
The implementation effectively addresses the pkg-config installation conflicts while maintaining proper error handling with Let's verify the error handling behaviour: ✅ Verification successfulLet me verify the error handling behavior further with additional context. Verified: The conditional pkg-config installation is safe with set -e The implementation is correct and safe. The script has proper error handling with 🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Description: Verify that the command check works correctly with set -e
# Test: Check if the script handles the command check properly
# Expected: Should show that command -v doesn't affect set -e
rg -A 2 -B 2 'set -e' components/core/tools/scripts/lib_install/macos/install-all.sh
# Test: Look for similar patterns in other installation scripts
# Expected: May find other opportunities for similar improvements
rg -l 'brew install.*pkg-config' 'components/core/tools/scripts'
Length of output: 273 Script: #!/bin/bash
# Check the full context around set -e and command check
rg -A 10 -B 2 'set -e' components/core/tools/scripts/lib_install/macos/install-all.sh
# Check if there's any error handling or set +e
rg 'set \+e|set -e' components/core/tools/scripts/lib_install/macos/install-all.sh
Length of output: 315 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 (
|
…o install it (fixes y-scope#610). (y-scope#611)
Description
To fix #610, this PR only installs pkg-config on macOS if it isn't already installed.
Validation performed
clp-core-macos-build workflow now succeeds.
Summary by CodeRabbit
pkg-config
, preventing conflicts in various environments.