X Tutup
Skip to content

Unified action for installing macos deps #7379

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.

Sign up for GitHub

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

Merged
youknowone merged 2 commits into from
Mar 7, 2026
+53 −6
Merged

Unified action for installing macos deps #7379

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
03f3d5e
Add `install-macos-deps` action
ShaharNaveh Mar 7, 2026
37b82ca
Use new action
ShaharNaveh Mar 7, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions .github/actions/install-macos-deps/action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
# This action installs a few dependencies necessary to build RustPython on macOS. By default it installs
# autoconf, automake and libtool, but can be configured depending on which libraries are needed:
#
# ```
# - uses: ./.github/actions/install-macos-deps
# with:
# openssl: true
# libtool: false
# ```
#
# See the `inputs` section for all options and their defaults. Note that you must checkout the
# repository before you can use this action.
#
# This action will only install dependencies when the current operating system is macOS. It will do
# nothing on any other OS (Linux, Windows).

name: Install macOS dependencies
description: Installs the dependencies necessary to build RustPython on macOS.
inputs:
autoconf:
description: Install autoconf (autoconf)
required: false
default: "true"
automake:
description: Install automake (automake)
required: false
default: "true"
libtool:
description: Install libtool (libtool)
required: false
default: "true"
openssl:
description: Install openssl (openssl@3)
required: false
default: "false"
runs:
using: composite
steps:
- 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' || '' }}
Comment on lines +39 to +47
Copy link
Contributor

@coderabbitai coderabbitai bot Mar 7, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

@ShaharNaveh ShaharNaveh Mar 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor

@coderabbitai coderabbitai bot Mar 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

12 changes: 6 additions & 6 deletions .github/workflows/ci.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,9 +135,8 @@ jobs:
components: clippy
- uses: Swatinem/rust-cache@v2

- name: Set up the Mac environment
run: brew install autoconf automake libtool
if: runner.os == 'macOS'
- name: Install macOS dependencies
uses: ./.github/actions/install-macos-deps

- name: run clippy
run: cargo clippy ${{ env.CARGO_ARGS }} --workspace --all-targets ${{ env.WORKSPACE_EXCLUDES }} -- -Dwarnings
Expand Down Expand Up @@ -299,9 +298,10 @@ jobs:
with:
python-version: ${{ env.PYTHON_VERSION }}

- name: Set up the Mac environment
run: brew install autoconf automake libtool openssl@3
if: runner.os == 'macOS'
- name: Install macOS dependencies
uses: ./.github/actions/install-macos-deps
with:
openssl: true

- name: build rustpython
run: cargo build --release --verbose --features=threading,jit ${{ env.CARGO_ARGS }}
Expand Down
Loading
X Tutup