Commit graph

4 commits

Author SHA1 Message Date
Daniel Han
50b9deacb0
Trim comments in tests (#13844)
Comment and docstring changes only, in test files, split out of #12968 so the test-only part can merge first.
2026-10-15 05:30:27 -07:00
Daniel Han
e2104761c5
Windows installer: always build Studio on a uv-managed Python, never one already installed (#13399)
* Windows installer: always build Studio on a uv-managed Python, never one already installed

A fresh install on a machine with Python 3.12 took that interpreter, so the
cu130 torch 2.13 route (cp313 only) closed and the box got torch 2.11 with
no prebuilt flash-attn / causal-conv1d / mamba-ssm. uv only provided Python
when nothing 3.11-3.13 was installed, and Windows on ARM never used it.

The environment is now always built on a uv-managed CPython, requested by
minor and architecture (cpython-3.13-windows-x86_64-none, or aarch64 for the
native Windows on ARM CUDA stack and UNSLOTH_ALLOW_ARM64_PYTHON), resolved
after uv is installed and before the existing environment is touched. A
reinstall keeps the environment's minor so its cached wheels are reused;
UNSLOTH_TORCH_UPGRADE=1 moves it to 3.13. When uv cannot provide one, an
earlier uv-installed runtime is used, else the installer stops with the
mirror / pre-install fix; there is no system fallback. --python X.Y[.Z] now
works as the header always said, and an explicit request is exact. Legacy
layouts on a non-uv base are rebuilt instead of moved in.

* Tests: drop the system-Python bootstrap cases; winget now only installs uv

* Legacy environments are reused only on the Python this install resolved

A legacy .venv / ~/unsloth_studio on a uv-managed base was moved in and kept
even when --python, UNSLOTH_PYTHON or UNSLOTH_TORCH_UPGRADE asked for another
version. It is now reused only when its pyvenv.cfg version matches the
resolved interpreter's minor (its exact patch when one was requested).

* Trim comments

* Windows installer: legacy layouts keep their minor and arch, UTF-8 pyvenv.cfg, older cached patch past a skip

- A legacy .venv / ~/unsloth_studio reinstall keeps its Python minor, as the
  current layout does.
- A legacy environment is reused only on a uv build of the selected
  architecture.
- pyvenv.cfg is read as UTF-8 (uv writes it without a BOM; Windows
  PowerShell 5.1 read it as ANSI and rejected a non-ASCII store path).
- Past a skipped patch the resolver tries the next one, then an older one,
  so an offline machine with 3.13.7 cached still installs.

* Trim comments
2026-10-12 06:27:51 -07:00
Daniel Han
62923f2b15
Studio installers: add --no-rollback, and report a full disk as a full disk (#11364)
* Studio installers: let a reinstall opt out of the rollback copy, and say what it costs

A reinstall or Repair renames the live venv aside, builds a new one beside
it, and deletes the old one only on success. uv hardlinks a wheel only
within one filesystem, so when its cache is on another volume every file is
a real copy and the install needs room for two whole environments. Measured
on a 200 MB environment: same volume 0 MB extra, other volume 200 MB extra.
A full disk then surfaces as nothing but 'studio setup failed (exit code 1)'.

Add --no-rollback and UNSLOTH_INSTALL_NO_ROLLBACK=1 to both installers,
which delete the old environment right after renaming it aside. The rename
still happens first, because uv creates only into a path that is absent or
empty. The restore state is cleared before the delete, as the commit path
already does, so an interrupt is never handed a half-deleted backup; a
failed install then cannot be undone, which is the trade the flag makes.

Warn before the move when the volume has less free space than the existing
environment occupies, naming both figures and the opt-out, and warn once
after the cache is selected when that cache is on another volume. Both warn
and continue: the estimate is a guess and being wrong must not stop an
install that would have fitted. On failure, ask the volume directly and say
the disk is full rather than leaving it at an exit code.

The cache ranking itself is unchanged. It is pinned across install.sh,
install.ps1, studio/setup.sh and unsloth_cli/commands/studio.py by
tests/python/test_uv_cache_selector_agreement.py, and both installers lift
the selector out and run it alone in their own tests, so the new warnings
live at the call site rather than inside it.

Reported in #11313.

* Never let the disk diagnostics abort the install

Enumerating the new predicates over their full input space turned up two
gaps on the Windows side.

The cross-volume cache notice was the only one of the new diagnostics not
wrapped. It runs under the installer's "Stop", so anything escaping
Test-StudioSameVolume would abort the install at the point the cache is
selected, which is the same defect class this branch's sibling work fixes
elsewhere: a diagnostic must never cost the operation it describes.

The three helpers also threw ParameterBindingValidationException on an
empty path, because a mandatory [string] rejects one. No caller can reach
that today, since each guards its argument, but a helper whose whole job is
to answer "I cannot tell" should not have a degenerate input that throws
instead. They now accept an empty string and answer the quiet way: no size,
no free space, and same volume, so nothing is printed.

Rollback lifecycle suites still pass on both platforms, 63 shell checks and
the full PowerShell set.

* Do not let --no-rollback send the ARM64 migration after a deleted tree

The Windows on ARM migration decides what to do from the rollback state:
if a rollback is active the previous environment has already been moved
aside and only needs marking as kept, otherwise it calls
Start-StudioVenvRollback to move it now.

--no-rollback left that state merely inactive, which the second arm reads
as "not moved aside yet". It then tried to rename a directory this flag
had already deleted, Move-Item threw, and the whole migration went out
through Exit-InstallFailure -- after the environment was already gone. A
user on ARM64 running a reinstall with --no-rollback lost their environment
and got a failed install rather than the x64 rebuild.

Track discarded separately from never-started, and give the migration its
own arm for it: there is nothing to move and nothing to keep, so say that
the previous environment was discarded on request and build fresh, instead
of promising an unsloth_studio.arm64.* copy that no longer exists.

Found by walking the interruption and lifetime paths of the new flag rather
than by the flag's own tests, which only covered the reinstall path. The
regression check fails on the previous commit.

* Say what --no-rollback actually did, and stop advising a flag already given

* Only warn about rollback space where keeping it costs any, and name a remedy that works

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Document the rollback opt-out where the installer says the options live

* Answer for the volume that holds the path, not its drive letter

* Let the desktop install see the disk-full diagnosis too

* Do not prescribe the opt-out to a run that already used it

* Match the mount point itself, not only paths beneath it

* A cacheless install still needs room for two environments

* Make the volume-probe check true on Windows as well

* Build the volume-matching cases the way the code reads paths

* Measure the volume a link points at, not the one it sits on

* Opting out of the rollback copy must not cost XPU detection

* Record what the volume lookup does not cover

* Bound the volume query the way the installer already bounds WMI

* Never answer the free-space question from a snapshot

* Answer nothing rather than guess, and confirm a removal the link-aware way

* Do not promise an ARM64 copy the flag is about to delete

* Ask st_dev whether uv can hardlink, not the df label

* Vary every message that promises a copy the flag may delete

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* A caller who asked uv to copy still needs room for two

* A symlinked venv costs nothing, wherever the cache sits

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Measure the blocks a discard would actually free

* Let the measurement decide on POSIX instead of this run's cache mode

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Judge the tree by the cache it was built against

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Never let a pre-rename diagnostic abort the rename

* Record what the building run knew instead of inferring it later

* Keep the old tree's verdict out of this run's stamp

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Tighten the comments added by this change

* Confine the disk work to --no-rollback and the failure path

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Keep the stricter removal check to the path that asked for it

* Diagnose a full disk wherever the install failed, not only at studio setup

* Advise from what became of the old environment, not from the flag

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Restore the environment before diagnosing why the install failed

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Measure the disk before the restore empties it

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Remove comments left behind by the removed helpers

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
2026-09-21 18:00:19 -07:00
Daniel Han
37a66c8a78
Installers: require x64 Python on Windows on ARM, and respect active conda (#11024)
* Installers: require x64 Python on Windows on ARM, and respect active conda

Windows on ARM: pyarrow and hf-transfer publish no win_arm64 wheel, at any
version, so a native ARM64 interpreter source-builds both and fails on CMake
minutes into the install. install.ps1 already swapped a freshly selected ARM64
interpreter for x64, but it warned and continued when no x64 interpreter could
be installed, and it never re-checked the interpreter of an environment it
reused, so a venv migrated from an older install stayed on ARM64 CPython. Both
now stop or rebuild, with UNSLOTH_ALLOW_ARM64_PYTHON as the opt-out.

Conda: every persistent PATH write prepended, which writes an ordering that
outlives the conda activation it was made under, so from the next shell on our
directory sits ahead of the active environment's own entries. install.ps1,
studio/setup.ps1, install.sh and studio/setup.sh now register the same
directory at the back while a conda environment is active, and a system Python
installed from python.org gets AppendPath instead of PrependPath.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Honour the ARM64 opt-out before installing x64, and agree with the shell installers on conda

Three corrections.

UNSLOTH_ALLOW_ARM64_PYTHON was read only after Install-X64Python had already
failed, so on any Windows on ARM machine that can reach python.org, which is
most of them, setting it did nothing: an x64 interpreter the user had asked not
to have was downloaded, installed and then used. It is read first now, and the
warning says how to undo it.

Inside an active conda environment the python.org route runs before winget, and
the winget arm's own python.org fallback then ran it a second time when both had
failed. On an offline machine that is the same failing download twice, announced
as a fallback that had already been tried.

install.sh and studio/setup.sh decided "are we inside conda" from CONDA_PREFIX
alone while install.ps1 and studio/setup.ps1 read CONDA_PREFIX and
CONDA_DEFAULT_ENV, so a shell hook that exports only the second gave the same
user a prepend on Linux and an append on Windows. Both shell installers share one
_unsloth_conda_env_active predicate now, and a test asserts the two halves name
the same variables in either direction.

Cases added: the opt-out with x64 installable, the single python.org attempt,
CONDA_DEFAULT_ENV on its own for all three POSIX writers, and a zsh rc file.

* Append to PATH in fish for real, and honour the ARM64 opt-out on a reused environment

Two defects in this branch's own new code.

fish: -a alone is not an append to PATH. Without --path, fish_add_path edits
$fish_user_paths, and fish's own documentation says that variable 'is itself prepended to
PATH, so they still stay ahead of the system paths'. Its worked example for exactly this
case is 'fish_add_path --append --path /opt/fallback/bin'. So the conda arm wrote a line
that left the directory in front of the active conda environment, which is the defect
#5871 reports, arriving through fish instead of an rc file. Both installers now write
'fish_add_path -a -P'. The drop-in lives in conf.d, which is read alphabetically after
conda.fish and sourced by every shell, and fish_add_path leaves an already-included
directory in place, so re-sourcing and re-running the installer are both no-ops. The bare
-a spelling still counts as present, so an install from before this fix does not collect a
second line.

UNSLOTH_ALLOW_ARM64_PYTHON: Resolve-WindowsOnArmX64Python never runs when the ordinary
probe already found an x64 Python on the machine, so on an ARM64 box that has both
interpreters the variable was never read, and the migrated-environment re-check then moved
aside the very native ARM64 environment the user had asked to keep. One
Test-Arm64PythonOptOut helper is now read at both sites. The rebuild also names where the
old environment went and that its extra packages are not carried over, which a rebuild on
a different architecture cannot do.

Sources: https://fishshell.com/docs/current/cmds/fish_add_path.html

Nine pwsh cases and three shell cases added; they fail on the previous head of this branch.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Reposition an existing PATH entry when conda downgrades the prepend

A machine that ran the installer once outside conda has our directory at the
front of the User PATH. Rerunning inside an activated environment downgraded the
prepend to an append, but the append-idempotent early return then returned
without rewriting the registry, so the front entry survived and the run reported
that conda keeps priority while it did not.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Take the python.org route for the x64 bootstrap under conda, and keep the ARM64 environment

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Keep an active conda ahead of the User PATH in the refreshed session

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Preserve the session PATH for a name-only conda activation

* Remove conflict markers left in the installer PATH test

* Keep an active conda environment in front during setup PATH refreshes

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Reposition a persisted prepend, and read the venv architecture before the move

* Do not move the venv twice, keep what the opt-out promises, repoint before the guards

* Anchor the ARM64 swap test on the call, not the shape of the if

The guard is multi-line now that the native CUDA torch check joined it, and an anchor
on "$DetectedPython -and" matched an unrelated earlier block instead of failing.

* Let the ARM64 opt-out choose the interpreter, not only keep it

UNSLOTH_ALLOW_ARM64_PYTHON was read at the swap that follows the selection, but on a
host that already has an x64 interpreter the selection is x64 and that guard never
fires, so the variable did nothing: a fresh install built an x64 venv and a reinstall
replaced the native ARM64 one. Rank ARM64 first when the opt-out is set on an ARM64
host, leaving Install-X64Python's own -X64Only lookup alone.

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Commit the rc rewrite by rename, and enumerate every stacked conda prefix

cat staged > rc truncated the user's profile before writing a byte back, so an interrupt
or an I/O error left it half written and the failure branch then deleted the only complete
copy. Stage beside the real file, carrying its permission bits, and rename onto it; the
symlink chain is resolved by hand first, since renaming over a link managed by chezmoi or
stow would replace it with a regular file.

conda activate --stack records the outer environments as CONDA_PREFIX_1 upward and
CONDA_SHLVL counts them, so a fixed list ending at _3 dropped everything past the fourth
and left those prefixes sorting after Machine and User. Also gives the ARM64 resolver
harness the two real functions it calls, without which three of its four cases were
failing on an unknown command rather than on an answer.

* Hand the rewrite values to awk untouched, and repoint the $HOME spelling too

POSIX awk decodes backslash escapes in a -v assignment, so a line holding a backslash --
which the writers escape before building it -- matched the literal grep and then matched
nothing in awk: the helper renamed an unchanged file and reported the stale prepend as
moved with conda still in front of it. ENVIRON hands the value over as it is.

And the uv repoint pass was built only from the expanded path, so on every ordinary
machine, where the destination IS ~/.local/bin, it could not match the export PATH=
"$HOME/.local/bin:$PATH" line the shim block writes unexpanded. That pass already visits
every startup file astral wired, so both spellings go through it; the shim's own repoint
reaches only whichever profile is selected now.

* Repoint the $HOME spelling in the standalone setup path too

studio/setup.sh runs on its own during an update, after install.sh wrote the shim line
unexpanded. _SETUP_LOGIN_PATH holds the expanded directory, which is what selects the
repoint-only branch, while the line in the profile says $HOME, so the rewrite never fired
and the stale prepend stayed ahead of the active conda environment. Both spellings go
through the repoint, in that branch and in the already-present one.

* Run the repoint helper in the shell harness and fix the spelling it uncovered

* Move the fish entry rather than append one fish will ignore

* Let the ARM64 opt-out find a native interpreter, and only a native one

* Move only a line the installer put there

* Bring the repointer fixtures to the ownership marker

* Keep the rc file's permissions through the repoint

The rewrite renames a staged copy onto the user's profile, so the copy's mode
becomes the profile's mode for good, and a plain cp does not carry the source
bits across: POSIX creates the destination with the source's mode as the mode
argument, and a mode argument is always modified by the file creation mask. A
0644 .bashrc rewritten under umask 077 came back 0600. Both copies of the
helper stage with cp -p now, which is specified to duplicate the permission
bits, and fall back to the plain copy only where -p is refused outright.

* Keep the ARM64 opt-out's environment on an ARM64 interpreter

The selection prefers ARM64 under UNSLOTH_ALLOW_ARM64_PYTHON, but only among
the interpreters discovery can see. With the ARM64 CPython that built the venv
gone, there was nothing left to prefer: an x64 interpreter was selected, the
reinstall had already moved the native environment to its rollback path, and
the new environment really was built on x64 while the opt-out's own message
said it would not be. pyvenv.cfg still names that base interpreter, so it is
tried before the swap, and where it cannot be found the message now says
plainly that the new environment is built on x64 and the ARM64 one is kept.

* Keep the x64 bootstrap fixture parseable on the lint target

A backslash inside an f-string expression is a 3.12 syntax addition and the
repo's ruff target is 3.11, so the whole-repo check failed on the Windows paths
in the stub preamble. Bind them before the template.

* Cut the commentary down

Same code, fewer words. The product diff went from 446 comment lines to 290,
and the whole PR from 3381 added lines to 3099. Every reason survives; the
paragraphs explaining them do not.

---------

Co-authored-by: danielhanchen <unslothai@gmail.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
2026-09-17 03:43:44 -07:00