Skip to content

Fix Windows installer torch index override - #6972

Merged
danielhanchen merged 6 commits into
unslothai:mainfrom
alkinun:fix-windows-installer-uv-default-index
Jul 9, 2026
Merged

danielhanchen merged 6 commits into
unslothai:mainfrom
alkinun:fix-windows-installer-uv-default-index

Conversation

@alkinun

@alkinun alkinun commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • switch Windows installer-managed PyTorch uv installs from deprecated --index-url to --default-index
  • ensure inherited UV_DEFAULT_INDEX does not override the CUDA/ROCm torch index selected by the installer
  • add a source-level regression test for the Windows installer torch index flags

Fixes #6898.

Tests

  • pytest tests/python/test_tokenizers_and_torch_constraint.py::TestStructuralInstallPs1Unchanged tests/python/test_tokenizers_and_torch_constraint.py::TestInstallPs1UvDefaultIndex -q
  • pytest tests/python/test_tokenizers_and_torch_constraint.py -q -m "not e2e"
  • git diff --check

@alkinun
alkinun requested a review from danielhanchen as a code owner July 8, 2026 10:20
@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

Gemini encountered an error creating the review. You can try again by commenting /gemini review.

@alkinun

alkinun commented Jul 8, 2026

Copy link
Copy Markdown
Collaborator Author

/gemini review

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code Review

This pull request updates the install.ps1 script to use the --default-index flag instead of --index-url when running uv pip install commands for PyTorch. This ensures that installer-managed PyTorch indexes correctly override inherited uv defaults. Additionally, a new test class TestInstallPs1UvDefaultIndex has been added to tests/python/test_tokenizers_and_torch_constraint.py to verify this behavior and prevent regressions. There are no review comments, so I have no feedback to provide.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

@danielhanchen

Copy link
Copy Markdown
Member

@codex review

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Breezy!

Reviewed commit: 53dc06240a

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@danielhanchen

Copy link
Copy Markdown
Member

Pushed a follow-up commit (d57e119) extending this to studio/setup.ps1, which still carried the same bug.

setup.ps1 installs torch through Fast-Install, which runs uv pip install ... --index-url <url> for the CUDA, CPU, and ROCm builds. With an inherited UV_DEFAULT_INDEX (or UV_INDEX_URL) set, uv overrides that --index-url and resolves CPU torch, the same failure #6898 hits in install.ps1.

I did not reuse --default-index here because:

  • Fast-Install falls back to python -m pip install, and pip has no --default-index flag.
  • setup.ps1 has no minimum uv version gate (unlike install.ps1, which requires uv >= 0.8.16), so an older uv without --default-index would fail both the uv call and the pip fallback.

Instead, Fast-Install now clears UV_DEFAULT_INDEX/UV_INDEX_URL only for index-pinned installs so the passed --index-url wins. Unpinned installs keep the user's mirror, and the pip fallback stays unaffected.

Verified locally:

  • PowerShell AST parse of setup.ps1 is clean.
  • Driving the real Fast-Install with a stubbed uv: the index vars are cleared during pinned installs, restored afterwards, and preserved for unpinned installs.
  • pytest tests/python/test_tokenizers_and_torch_constraint.py::TestInstallPs1UvDefaultIndex ::TestSetupPs1FastInstallIndex passes.

@danielhanchen

Copy link
Copy Markdown
Member

Pushed a1b521f hardening the setup.ps1 change after simulating it end to end.

While driving the real Fast-Install on PowerShell 7.6 I found that [Environment]::SetEnvironmentVariable($n, $null) does not remove the variable, it sets it to an empty string (a child process then sees UV_DEFAULT_INDEX="" rather than an absent var). The fix happened to still work because uv ignores an empty value, but that is fragile. Switched to Remove-Item "Env:$n" for a true removal, and made the restore skip vars that were originally unset so we never leave an empty UV_DEFAULT_INDEX behind.

Validated on Linux against real uv (metadata-only, uv pip compile / --dry-run):

  • Bug reproduced: uv pip install ... --index-url .../cu130 with UV_DEFAULT_INDEX set resolves torch==2.10.0 (CPU). The --default-index form in install.ps1, and the setup.ps1 env-clear, both resolve torch==2.10.0+cu130.
  • No regression for users without a global index: --index-url and --default-index resolve to the identical version for every index the installer emits (cu130, cu128, cu126, cu124, cu118, cpu).
  • Older CUDA pathways are among those fixed: cu124 goes from CPU to 2.6.0+cu124, cu118 to 2.7.1+cu118 under an inherited UV_DEFAULT_INDEX.
  • pip fallback stays intact: pip accepts --index-url and has no --default-index, which is why setup.ps1 keeps --index-url plus the env-clear instead of switching flags like install.ps1.

@danielhanchen

Copy link
Copy Markdown
Member

Extended the same fix to the Linux/Mac installer in 9936871.

install.sh had the identical exposure: it installs torch with uv pip install ... --index-url "$TORCH_INDEX_URL" (7 sites: fresh CUDA/CPU install, ROCm, Radeon-repo fallbacks, and the flavor-repair reinstall) with no UV_DEFAULT_INDEX handling. It is structurally like install.ps1 (enforces UV_MIN_VERSION=0.8.16, uv-only with no pip fallback for torch), so I used the same --index-url to --default-index swap rather than the env-clear approach.

Scope checks for the rest of the Linux/Mac surface:

  • studio/setup.sh needs no change: it does GPU detection but installs no torch by index (torch is handled by install.sh on Linux/Mac), so there is no vulnerable call.
  • scripts/install_rocm_wsl_strixhalo.sh uses pip install --index-url directly, not uv. pip reads PIP_INDEX_URL, never UV_DEFAULT_INDEX, so it is unaffected.

Validated:

  • Linux target resolves correctly: uv pip compile for --python-platform linux with UV_DEFAULT_INDEX set gives torch==2.10.0 (CPU) under --index-url and torch==2.10.0+cu130 under --default-index.
  • bash -n install.sh clean; tests/sh/test_mac_intel_compat.sh 56 passed; added a source-level install.sh regression test; full non-e2e Python suite 35 passed.

@danielhanchen

Copy link
Copy Markdown
Member

Hardened all three installers in 44f360a to neutralize every uv index env var for pinned torch installs, not just UV_DEFAULT_INDEX/UV_INDEX_URL.

Reason: UV_INDEX and UV_EXTRA_INDEX_URL are uv's extra-index list, which outranks the default index, so --default-index alone does not beat them. A user whose global uv mirror is configured via UV_INDEX would still get CPU torch. This was pre-existing (the old --index-url lost to UV_INDEX too), but worth closing now that we are here.

What changed:

  • install.ps1 Invoke-InstallCommand and install.sh run_install_cmd: when the command pins an index (--default-index), run it with all four uv index env vars removed (PowerShell restores in finally; the shell uses env -u, which is child-scoped). Gated on --default-index, so unsloth and dependency installs keep the user's mirror.
  • studio/setup.ps1 Fast-Install: extended its existing clear set to the same four vars.

Validated on Linux (real uv, metadata-only) and by driving the real helper functions:

  • Torch install: uv sees UV_DEFAULT_INDEX/UV_INDEX_URL/UV_INDEX/UV_EXTRA_INDEX_URL all unset; UV_INDEX=mirror + --default-index cu130 now resolves torch==2.10.0+cu130 (was CPU). Same for UV_EXTRA_INDEX_URL.
  • Non-torch install (unsloth): all four vars preserved, so a corporate mirror still applies.
  • Parent environment restored after each call.
  • install.ps1/setup.ps1 AST parse clean, bash -n install.sh clean, pytest -m "not e2e" 37 passed.

@danielhanchen

Copy link
Copy Markdown
Member

@codex review

@danielhanchen

Copy link
Copy Markdown
Member

/gemini review

@gemini-code-assist

Copy link
Copy Markdown
Contributor

Warning

Gemini encountered an error creating the review. You can try again by commenting /gemini review.

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. Bravo.

Reviewed commit: 06c5cf19db

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

@danielhanchen
danielhanchen merged commit 216a1fa into unslothai:main Jul 9, 2026
52 checks passed
@danielhanchen danielhanchen mentioned this pull request Jul 14, 2026
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.

[Bug] unsloth studio setup failed

2 participants