Unified action for installing macos deps by ShaharNaveh · Pull Request #7379 · RustPython/RustPython · GitHub

Taken and modified from https://github.com/bevyengine/bevy/blob/162a7080911dc8b782475553fef8888c8d103655/.github/actions/install-linux-deps/action.yml

Summary by CodeRabbit

Chores

Refactored macOS build environment setup for improved consistency and maintainability across CI/CD workflows.

📝 Walkthrough

Walkthrough

Introduces 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

Cohort / File(s)	Summary
New macOS Dependency Action .github/actions/install-macos-deps/action.yml	Defines a composite GitHub Action with inputs for autoconf, automake, libtool, and openssl. Conditionally installs selected packages via brew based on input flags.
CI Workflow Updates .github/workflows/ci.yaml	Replaces inline brew install commands in rust_tests and snippets_cpython jobs with invocations of the new install-macos-deps action; adds openssl: true parameter in snippets_cpython job.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

Restructure CI with matrix approach and multi-feature support #7376: Both PRs modify macOS setup in .github/workflows/ci.yaml, with this PR introducing a reusable action to replace inline brew dependency installation commands.

Suggested reviewers

youknowone

Poem

🐰 A rabbit hops through workflows, neat,
Bundling brew commands—oh what a treat!
Dependencies dance on macOS so bright,
Reusable actions make CI just right! ✨

🚥 Pre-merge checks | ✅ 3 ✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title 'Unified action for installing macos deps' accurately describes the main change: introducing a composite GitHub Action to standardize macOS dependency installation across workflows.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches 🧪 Generate unit tests (beta)

Create PR with unit tests
Post copyable unit tests in a comment

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

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

📥 Commits

Reviewing files that changed from the base of the PR and between abfb51e and 37b82ca.

📒 Files selected for processing (2)

.github/actions/install-macos-deps/action.yml
.github/workflows/ci.yaml

⚠️ Potential issue | 🟡 Minor

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.

Suggested change

      
                - 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.

Good point, but I think it's a bug if this gets called with everything being disabled

@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

ShaharNaveh added 2 commits March 7, 2026 12:43

Add install-macos-deps action

03f3d5e

Use new action

37b82ca

coderabbitai Bot reviewed Mar 7, 2026

View reviewed changes

youknowone approved these changes Mar 7, 2026

View reviewed changes

youknowone merged commit f680f8a into RustPython:main Mar 7, 2026
12 of 13 checks passed

ShaharNaveh mentioned this pull request Mar 7, 2026

Unified action for installing Linux deps #7381

Merged

youknowone pushed a commit to youknowone/RustPython that referenced this pull request Mar 8, 2026

Unified action for installing macos deps (RustPython#7379 )

fa63408

* Add `install-macos-deps` action * Use new action

coderabbitai Bot mentioned this pull request Mar 9, 2026

Use matrix for cargo check #7388

Merged

youknowone pushed a commit to youknowone/RustPython that referenced this pull request Mar 22, 2026

Unified action for installing macos deps (RustPython#7379 )

a547e83

* Add `install-macos-deps` action * Use new action

coderabbitai Bot mentioned this pull request Mar 24, 2026

Cleanup release.yml a bit #7499

Merged

Unified action for installing macos deps by ShaharNaveh · Pull Request #7379 · RustPython/RustPython · GitHub

Uh oh!

Unified action for installing macos deps - #7379

Unified action for installing macos deps#7379
youknowone merged 2 commits into
RustPython:mainfrom
ShaharNaveh:ci-install-macos-deps

ShaharNaveh commented Mar 7, 2026 •
edited by coderabbitai Bot
Loading

Uh oh!

Uh oh!

coderabbitai Bot commented Mar 7, 2026 •
edited
Loading

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

coderabbitai Bot Mar 7, 2026 •
edited
Loading

Uh oh!

Uh oh!

ShaharNaveh Mar 7, 2026

Uh oh!

coderabbitai Bot Mar 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Unified action for installing macos deps by ShaharNaveh · Pull Request #7379 · RustPython/RustPython · GitHub

Uh oh!

Unified action for installing macos deps - #7379

Unified action for installing macos deps#7379youknowone merged 2 commits intoRustPython:mainRustPython/RustPython:mainfrom ShaharNaveh:ci-install-macos-depsShaharNaveh/RustPython:ci-install-macos-depsCopy head branch name to clipboard

Conversation

ShaharNaveh commented Mar 7, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Chores

Uh oh!

coderabbitai Bot commented Mar 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Possibly related PRs

Suggested reviewers

Poem

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Mar 7, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ShaharNaveh Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Unified action for installing macos deps#7379
youknowone merged 2 commits into
RustPython:mainfrom
ShaharNaveh:ci-install-macos-deps

ShaharNaveh commented Mar 7, 2026 •
edited by coderabbitai Bot
Loading

coderabbitai Bot commented Mar 7, 2026 •
edited
Loading

coderabbitai Bot Mar 7, 2026 •
edited
Loading