Skip to content

Force-reinstall maturin in portable top_of_tree dynamo source build#183

Merged
ishandhanani merged 1 commit into
NVIDIA:mainfrom
Ankur-singh:fix/maturin-portable-install
May 29, 2026
Merged

Force-reinstall maturin in portable top_of_tree dynamo source build#183
ishandhanani merged 1 commit into
NVIDIA:mainfrom
Ankur-singh:fix/maturin-portable-install

Conversation

@Ankur-singh
Copy link
Copy Markdown
Collaborator

Problem

The portable (non-SGLang) branch of the top_of_tree dynamo source install guarded maturin behind command -v maturin and ran a plain pip install:

if ! command -v maturin &> /dev/null; then pip install --break-system-packages maturin; fi

On images that ship the maturin Python module without a console script, the guard trips (command -v maturin fails) but pip reports "already satisfied" — so no binary is created and the later maturin build dies with maturin: command not found. The hash and SGLang paths already avoid this with --force-reinstall; the portable branch was the only one missing it.

Changes

  • schema.py — portable branch now force-reinstalls maturin unconditionally, matching the other two source-install paths:
    pip install --break-system-packages --force-reinstall --quiet maturin
  • test_configs.py — branch-pinned assertions so the same regression is caught on all three paths (hash, top_of_tree sglang, top_of_tree portable).

Before / After

Generated top_of_tree portable branch:

- if ! command -v maturin &> /dev/null; then pip install --break-system-packages maturin; fi; fi &&
+ ...; fi &&
+ pip install --break-system-packages --force-reinstall --quiet maturin &&

bash -n confirms the generated command still parses cleanly.

Tests

The new portable assertion fails on the old code (proving it catches the bug) and passes after the fix:

# fix reverted:
tests/test_configs.py:255: assert "--force-reinstall --quiet maturin" in portable_branch  -> FAILED

Full suite with this PR — no tests broken:

705 passed, 2 skipped in make check

Note: one unrelated, pre-existing failure (test_mcp_spec.py::test_explain_field_resolves_nested_reporting_endpoint, a Union vs UnionType string-repr quirk under local Python 3.14) is present on main independent of this change.

The portable (non-SGLang) branch of the top_of_tree dynamo source install
guarded maturin behind `command -v maturin` and ran a plain `pip install`.
On images that ship the maturin module without a console script, the guard
trips but pip reports 'already satisfied', so no binary is created and the
later `maturin build` fails with 'maturin: command not found'.

Match the hash and SGLang paths: drop the guard and force-reinstall maturin
unconditionally. Add branch-pinned test assertions so the same regression is
caught on all three source-install paths.
@codecov-commenter
Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
⚠️ Please upload report for BASE (main@3cdb250). Learn more about missing BASE report.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #183   +/-   ##
=======================================
  Coverage        ?   65.31%           
=======================================
  Files           ?       67           
  Lines           ?     8251           
  Branches        ?        0           
=======================================
  Hits            ?     5389           
  Misses          ?     2862           
  Partials        ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ishandhanani ishandhanani merged commit 8386e52 into NVIDIA:main May 29, 2026
6 checks passed
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.

3 participants