Skip to content

fix: evaluate extra == markers per-extra with OR semantics#1099

Open
xangcastle wants to merge 1 commit into
1.xfrom
fix/extra-marker-evaluation
Open

fix: evaluate extra == markers per-extra with OR semantics#1099
xangcastle wants to merge 1 commit into
1.xfrom
fix/extra-marker-evaluation

Conversation

@xangcastle

Copy link
Copy Markdown
Member

_decide_marker_impl was joining multiple extras into a single comma-separated string before passing it to the PEP-508 evaluator, so extras = ["dev", "other"] became extra = "dev,other" which never matched a marker like extra == 'dev', causing optional dependencies gated by extras to be silently excluded.
This PR fixes that by evaluating the marker once per extra and ORing the results, so a dependency is included if any of the active extras matches. It also adds venv-derived extras inference: since generated decide_marker calls never pass extras explicitly, the fix now reads the active venv name from the _venv flag and extracts matching extra values from the marker string, which handles conflict group markers of the form extra == 'group-N-project-venvname'.
Tests added to pep508_evaluate_test.bzl cover positive/negative extra == cases including the comma-join regression, and new decide_marker targets in BUILD.bazel exercise the OR evaluation path directly.


Changes are visible to end-users: no

Test plan

  • New test cases added

@xangcastle xangcastle changed the title evaluate extra == markers per-extra with OR semantics fix: evaluate extra == markers per-extra with OR semantics Jun 11, 2026
@aspect-workflows

aspect-workflows Bot commented Jun 11, 2026

Copy link
Copy Markdown

✨ Aspect Workflows Tasks

📅 Thu Jun 11 00:14:08 UTC 2026

✅ 7 successful tasks

  • ✅ buildifier · ⏱ 23.3s · 🐙 GitHub Actions · ☑️ Check
    💬 Format complete (clean)
  • ✅ gazelle · ⏱ 19.4s · 🐙 GitHub Actions · ☑️ Check
    💬 Gazelle complete (clean)
  • ✅ test (test-e2e-bazel-8) · ⏱ 32m 44s · 🐙 GitHub Actions · ☑️ Check
    💬 Bazel test complete (61/61 passed)
  • ✅ test (test-e2e-bazel-9) · ⏱ 34m 14s · 🐙 GitHub Actions · ☑️ Check
    💬 Bazel test complete (61/61 passed)
  • ✅ test (test-examples-uv_pip_compile-bazel-8) · ⏱ 21.7s · 🐙 GitHub Actions · ☑️ Check
    💬 Bazel test complete (1/1 passed · 1 cached)
  • ✅ test (test-root-bazel-8) · ⏱ 39m 24s · 🐙 GitHub Actions · ☑️ Check
    💬 Bazel test complete (123/123 passed)
  • ✅ test (test-root-bazel-9) · ⏱ 37m 28s · 🐙 GitHub Actions · ☑️ Check
    💬 Bazel test complete (122/122 passed)

⏱ Last updated Thu Jun 11 00:53:41 UTC 2026 · 📊 GitHub API quota 490/15,000 (3% used, resets in 17m, throttle 3×)
🚀 Powered by Aspect CLI (v2026.22.44)  |  Aspect Build · X · LinkedIn · YouTube

@xangcastle xangcastle requested a review from jbedard June 11, 2026 00:11
@xangcastle xangcastle force-pushed the fix/extra-marker-evaluation branch from f915ae6 to 5209926 Compare June 11, 2026 00:13
@xangcastle xangcastle marked this pull request as ready for review June 11, 2026 00:58
@jbedard

jbedard commented Jun 11, 2026

Copy link
Copy Markdown
Member

Please add the same tests (and possibly fixes) to main if applicable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants