Unified action for installing macos deps#7379
Conversation
📝 WalkthroughWalkthroughIntroduces a new reusable GitHub Action to install macOS build dependencies and refactors the CI workflow to use this action instead of inline brew commands, consolidating dependency setup logic in two workflow jobs. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/actions/install-macos-deps/action.yml:
- Around line 39-47: The "Install macOS dependencies" step currently runs "brew
install" even when no formulae are selected, causing a failure; change the step
to compute the list of chosen inputs (inputs.autoconf, inputs.automake,
inputs.libtool, inputs.openssl) into a variable and skip running "brew install"
when that computed list is empty (i.e., no-op or echo a message), otherwise run
"brew install <packages>"; update the step's run script to perform this
conditional check so the action does nothing if all inputs are false.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
Run ID: d572a819-438a-405a-b51f-df21ad724e9e
📒 Files selected for processing (2)
.github/actions/install-macos-deps/action.yml.github/workflows/ci.yaml
| - name: Install macOS dependencies | ||
| shell: bash | ||
| if: ${{ runner.os == 'macOS' }} | ||
| run: > | ||
| brew install | ||
| ${{ fromJSON(inputs.autoconf) && 'autoconf' || '' }} | ||
| ${{ fromJSON(inputs.automake) && 'automake' || '' }} | ||
| ${{ fromJSON(inputs.libtool) && 'libtool' || '' }} | ||
| ${{ fromJSON(inputs.openssl) && 'openssl@3' || '' }} |
There was a problem hiding this comment.
Skip brew install when no packages were selected.
If a caller sets every input to false, this step still resolves to brew install with no formulae and fails. Since the action is meant to be reusable/configurable, it should no-op when the computed package list is empty.
Suggested fix
- run: >
- brew install
- ${{ fromJSON(inputs.autoconf) && 'autoconf' || '' }}
- ${{ fromJSON(inputs.automake) && 'automake' || '' }}
- ${{ fromJSON(inputs.libtool) && 'libtool' || '' }}
- ${{ fromJSON(inputs.openssl) && 'openssl@3' || '' }}
+ run: |
+ packages=()
+ ${{ fromJSON(inputs.autoconf) && 'packages+=("autoconf")' || '' }}
+ ${{ fromJSON(inputs.automake) && 'packages+=("automake")' || '' }}
+ ${{ fromJSON(inputs.libtool) && 'packages+=("libtool")' || '' }}
+ ${{ fromJSON(inputs.openssl) && 'packages+=("openssl@3")' || '' }}
+
+ if [ "${`#packages`[@]}" -gt 0 ]; then
+ brew install "${packages[@]}"
+ fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Install macOS dependencies | |
| shell: bash | |
| if: ${{ runner.os == 'macOS' }} | |
| run: > | |
| brew install | |
| ${{ fromJSON(inputs.autoconf) && 'autoconf' || '' }} | |
| ${{ fromJSON(inputs.automake) && 'automake' || '' }} | |
| ${{ fromJSON(inputs.libtool) && 'libtool' || '' }} | |
| ${{ fromJSON(inputs.openssl) && 'openssl@3' || '' }} | |
| - name: Install macOS dependencies | |
| shell: bash | |
| if: ${{ runner.os == 'macOS' }} | |
| run: | | |
| packages=() | |
| ${{ fromJSON(inputs.autoconf) && 'packages+=("autoconf")' || '' }} | |
| ${{ fromJSON(inputs.automake) && 'packages+=("automake")' || '' }} | |
| ${{ fromJSON(inputs.libtool) && 'packages+=("libtool")' || '' }} | |
| ${{ fromJSON(inputs.openssl) && 'packages+=("openssl@3")' || '' }} | |
| if [ "${`#packages`[@]}" -gt 0 ]; then | |
| brew install "${packages[@]}" | |
| fi |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/actions/install-macos-deps/action.yml around lines 39 - 47, The
"Install macOS dependencies" step currently runs "brew install" even when no
formulae are selected, causing a failure; change the step to compute the list of
chosen inputs (inputs.autoconf, inputs.automake, inputs.libtool, inputs.openssl)
into a variable and skip running "brew install" when that computed list is empty
(i.e., no-op or echo a message), otherwise run "brew install <packages>"; update
the step's run script to perform this conditional check so the action does
nothing if all inputs are false.
There was a problem hiding this comment.
Good point, but I think it's a bug if this gets called with everything being disabled
There was a problem hiding this comment.
@ShaharNaveh, that's a fair point — failing loudly on a misconfigured call (all packages disabled) is actually better behavior than silently no-oping, since it surfaces the bug immediately. Happy to defer to your judgment here.
✏️ Learnings added
Learnt from: ShaharNaveh
Repo: RustPython/RustPython PR: 7379
File: .github/actions/install-macos-deps/action.yml:39-47
Timestamp: 2026-03-07T11:52:42.756Z
Learning: In the RustPython project, ShaharNaveh prefers that the `.github/actions/install-macos-deps/action.yml` composite action fails (rather than silently no-ops) when called with all package inputs set to `false`. This is intentional: such a call is considered a bug/misconfiguration and should surface as a failure rather than being handled gracefully.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
🧠 Learnings used
Learnt from: ShaharNaveh
Repo: RustPython/RustPython PR: 5932
File: .github/workflows/comment-commands.yml:18-24
Timestamp: 2025-07-10T10:08:43.330Z
Learning: In GitHub Actions workflows for the RustPython project, the maintainer ShaharNaveh prefers to keep workflows simple and doesn't mind if steps fail when the desired state is already achieved (e.g., user already assigned to an issue). Avoid suggesting complex error handling for edge cases they don't consider problematic.
* Add `install-macos-deps` action * Use new action
Taken and modified from https://github.com/bevyengine/bevy/blob/162a7080911dc8b782475553fef8888c8d103655/.github/actions/install-linux-deps/action.yml
Summary by CodeRabbit
Chores