Restructure CI with matrix approach and multi-feature support by ShaharNaveh · Pull Request #7376 · RustPython/RustPython · GitHub

Summary by CodeRabbit

Chores
- Simplified and unified CI workflows to use a platform matrix, reducing duplicated steps and consolidating build and check steps across macOS, Linux, and Windows.
- Improved local build script argument handling to allow multiple feature flags (as a list), passing them cleanly to build/run commands for more flexible feature combinations.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info ⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 90f95a49-33ee-4145-9886-e9fd8f001f47

📥 Commits

Reviewing files that changed from the base of the PR and between 89d4b84 and a2a80ab.

📒 Files selected for processing (2)

.github/workflows/ci.yaml
scripts/whats_left.py

📝 Walkthrough

Walkthrough

This PR refactors the CI workflow to a matrix-driven, OS-explicit configuration with unified cargo build steps (including jit/threading) and adds a dedicated Mac setup step. It also changes scripts/whats_left.py to accept multiple --features entries (append) and pass them as a comma-separated --features value to cargo commands.

Changes

Cohort / File(s)	Summary
CI workflow (matrix + unified builds) .github/workflows/ci.yaml	Replaced flat OS list with an include-based matrix enumerating macos-latest, ubuntu-latest, and windows-2025; added a “Set up the Mac environment” step; consolidated Rust build invocations into single OS-agnostic cargo commands that always enable jit (and threading where applicable); removed OS-specific build branches and unified the "whats_left" check to use matrix-provided CARGO_ARGS. Also applied the same consolidation to snippets_cpython job.
Feature flag handling in script scripts/whats_left.py	Changed --features CLI flag from action="store", default="ssl" to action="append", default=[]; join provided feature values into a comma-separated joined_features and only add --features joined_features to cargo commands when features are present. No change to --no-default-features behavior.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Optimize CI cache usage #6707: Related changes to how whats_left is invoked in CI and feature/jit handling across platforms.
relocate scripts under ./scripts #6753: Touches the same script and CI step (scripts/whats_left.py and its CI invocation).

Suggested reviewers

youknowone

Poem

🐰 I hopped through matrices, tidy and bright,
I gathered features by moonlit night,
One call now sings them, comma in tune,
CI hums softly beneath the moon,
Builds leap together — a rabbit’s delight!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check	✅ Passed	Title check skipped as CodeRabbit has written the PR title.

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

jit seems to be fixed on macos now

I'll try

Actionable comments posted: 2

🧹 Nitpick comments (1)

scripts/whats_left.py (1)
452-455: Logic issue: joined_features computed before conditional check.

The joined_features variable is computed unconditionally on line 453, but the check if args.features: on line 454 implies features could be empty/falsy. If args.features is an empty list, joined_features would be an empty string "", and the if args.features: check would be False, so this works correctly. However, with the current default=["ssl"], args.features is never empty, making the condition always true.

Once the default handling is fixed (per the comment above), ensure consistent behavior when no features are requested.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@scripts/whats_left.py` around lines 452 - 455, The code computes
joined_features before checking args.features; move the join into the
conditional so you only compute and append features when args.features is truthy
(i.e., when features were actually requested). Specifically, inside the block
that tests args.features, call ",".join(args.features) and then extend
cargo_build_command with ["--features", joined_features] (referencing
joined_features, args.features, and cargo_build_command) to ensure no empty
--features is added when no features are provided and to align with the
corrected default handling.

🤖 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/workflows/ci.yaml:
- Around line 451-453: The argparse setup in whats_left.py is incorrectly
pre-populating args.features with default=["ssl"], so when multiple --features
are passed (from CARGO_ARGS and matrix.cargo_args) the script erroneously
includes "ssl"; update the argument definition that uses action="append" for
features to use default=None instead of default=["ssl"] (so args.features will
be None unless flags provided), and ensure any downstream logic that joins
args.features handles None appropriately (e.g., treat None as empty list) to
avoid injecting the "ssl" feature.

In `@scripts/whats_left.py`:
- Around line 70-72: The current add_argument call for the --features flag uses
action="append" with default=["ssl"], which causes the default to persist and
prevents splitting comma-separated inputs; change the add_argument for
--features to use default=None (keep action="append" or consider action="store"
depending on desired behavior), and after parsing args in the main routine
(where args is available) set args.features = ["ssl"] if args.features is None;
additionally, if you want to support comma-separated lists like "ssl,sqlite",
normalize by splitting each entry on commas and flattening/stripping them (apply
this normalization to the values collected for args.features) so --features
"ssl,sqlite" yields ["ssl","sqlite"] and providing --features overrides the
default instead of appending to it.

---

Nitpick comments:
In `@scripts/whats_left.py`:
- Around line 452-455: The code computes joined_features before checking
args.features; move the join into the conditional so you only compute and append
features when args.features is truthy (i.e., when features were actually
requested). Specifically, inside the block that tests args.features, call
",".join(args.features) and then extend cargo_build_command with ["--features",
joined_features] (referencing joined_features, args.features, and
cargo_build_command) to ensure no empty --features is added when no features are
provided and to align with the corrected default handling.

ℹ️ Review info ⚙️ Run configuration

Configuration used: Path: .coderabbit.yml

Review profile: CHILL

Plan: Pro

Run ID: 2a545921-11ab-482f-960e-90124e0a4b05

📥 Commits

Reviewing files that changed from the base of the PR and between ed5bffe and 89d4b84.

📒 Files selected for processing (2)

.github/workflows/ci.yaml
scripts/whats_left.py

⚠️ Potential issue | 🔴 Critical

Bug: action="append" with non-None default causes incorrect feature handling.

Using action="append" with default=["ssl"] has two issues:

The default list is not cleared when --features is provided—values append to it, causing ["ssl", ...] always to include ssl.
Comma-separated values like --features "ssl,sqlite" are treated as a single string element, not split into separate features.

Per the context snippet from .github/workflows/cron-ci.yaml:95-100, the workflow calls --features "ssl,sqlite", which would result in ["ssl", "ssl,sqlite"] and joined_features = "ssl,ssl,sqlite".

🐛 Proposed fix: Use `default=None` and set default after parsing

     parser.add_argument(
         "--features",
         action="append",
-        help="which features to enable when building RustPython (default: [ssl])",
-        default=["ssl"],
+        help="which features to enable when building RustPython (default: ssl)",
+        default=None,
     )

     args = parser.parse_args()
+    if args.features is None:
+        args.features = ["ssl"]
     return args

Alternatively, if the intent is to accept comma-separated features in a single --features argument (matching cargo's behavior), the values should be split:

🔧 Alternative: Split comma-separated values

+    # Flatten comma-separated features into individual items
+    if args.features:
+        args.features = [f for feat in args.features for f in feat.split(",")]
+    else:
+        args.features = ["ssl"]
     return args

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@scripts/whats_left.py` around lines 70 - 72, The current add_argument call
for the --features flag uses action="append" with default=["ssl"], which causes
the default to persist and prevents splitting comma-separated inputs; change the
add_argument for --features to use default=None (keep action="append" or
consider action="store" depending on desired behavior), and after parsing args
in the main routine (where args is available) set args.features = ["ssl"] if
args.features is None; additionally, if you want to support comma-separated
lists like "ssl,sqlite", normalize by splitting each entry on commas and
flattening/stripping them (apply this normalization to the values collected for
args.features) so --features "ssl,sqlite" yields ["ssl","sqlite"] and providing
--features overrides the default instead of appending to it.

…thon#7376) * `--features` flag can take multiple values * Simplify CI job * Remove bad default * Enable jit on macOS

ShaharNaveh added 2 commits March 7, 2026 09:43

--features flag can take multiple values

87aa7ad

Simplify CI job

89d4b84

coderabbitai Bot changed the title @coderabbitai Restructure CI with matrix approach and multi-feature support Mar 7, 2026

youknowone reviewed Mar 7, 2026

View reviewed changes

coderabbitai Bot reviewed Mar 7, 2026

View reviewed changes

ShaharNaveh added 2 commits March 7, 2026 09:57

Remove bad default

9a84b59

Enable jit on macOS

a2a80ab

youknowone approved these changes Mar 7, 2026

View reviewed changes

youknowone merged commit ab32c78 into RustPython:main Mar 7, 2026
11 of 13 checks passed

coderabbitai Bot mentioned this pull request Mar 7, 2026

Unified action for installing macos deps #7379

Merged

This was referenced Mar 9, 2026

Use matrix for cargo check #7388

Merged

Enable jit on macos (CI) #7401

Merged

Cleanup ci.yaml matrix usage #7260

Merged

coderabbitai Bot mentioned this pull request Mar 19, 2026

Run CI tests with max processes #7254

Merged

This was referenced Mar 27, 2026

fix cron-ci #7483

Merged

Split cargo check matrix to individual targets. Avoid cache poisoning #7540

Merged

-                      os: [macos-latest, ubuntu-latest, windows-2025]
+                      include:
+                        - os: macos-latest
+                          cargo_args: "" # TODO: Fix jit on macOS

Restructure CI with matrix approach and multi-feature support by ShaharNaveh · Pull Request #7376 · RustPython/RustPython · GitHub

Uh oh!

Restructure CI with matrix approach and multi-feature support - #7376

Restructure CI with matrix approach and multi-feature support#7376
youknowone merged 4 commits into
RustPython:mainfrom
ShaharNaveh:whatsleft-append

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!

youknowone Mar 7, 2026

Uh oh!

ShaharNaveh Mar 7, 2026

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

coderabbitai Bot Mar 7, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Restructure CI with matrix approach and multi-feature support by ShaharNaveh · Pull Request #7376 · RustPython/RustPython · GitHub

Uh oh!

Restructure CI with matrix approach and multi-feature support - #7376

Restructure CI with matrix approach and multi-feature support#7376youknowone merged 4 commits intoRustPython:mainRustPython/RustPython:mainfrom ShaharNaveh:whatsleft-appendShaharNaveh/RustPython:whatsleft-appendCopy 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

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

❌ Failed checks (1 warning)

Uh oh!

youknowone Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

ShaharNaveh Mar 7, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

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

Restructure CI with matrix approach and multi-feature support#7376
youknowone merged 4 commits into
RustPython:mainfrom
ShaharNaveh:whatsleft-append

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

coderabbitai Bot commented Mar 7, 2026 •
edited
Loading