mirror of
https://github.com/unslothai/unsloth.git
synced 2026-08-16 20:33:56 +00:00
* Windows setup: install uv from a pinned release instead of running remote script text
studio/setup.ps1 piped astral's install.ps1 straight into Invoke-Expression. That
download-and-execute shape is the single construct AMSI providers and cloud ML
scanners score hardest, and install.ps1 already replaced it with a pinned-SHA-256
archive download. Port the same implementation across.
Progress goes to the pipeline rather than the console, so the quiet path swallows
it exactly as it swallowed astral's installer output and the printed lines around
the call site are unchanged.
* Windows: stop pairing a hidden window with a bypassed execution policy
The Studio shortcut launched launch-studio.ps1 with -WindowStyle Hidden and
-ExecutionPolicy Bypass on the same command line. That pair is what Microsoft's
own detections key on, and studio/src-tauri/src/install.rs already refuses it for
the app's own launch of install.ps1.
The installer writes launch-studio.ps1 itself, so the file carries no
mark-of-the-web and RemoteSigned loads it. The hidden window is unchanged, so the
shortcut behaves exactly as before. The generated launcher's own child launch
moves to RemoteSigned for the same reason: it runs an inline -Command against an
executable, where no script file is loaded and the two policies are equivalent.
Also refresh a stale comment in studio/setup.ps1 that attributed the PSModulePath
fix to astral's uv installer, which no longer runs in-process.
* Installers: keep download-and-run command lines out of the shipped script text
AMSI scans install.ps1 in full before a single line of it runs, and generic
script classifiers read install.sh the same way inside the Linux bundle. Both
headers rehearsed the piped web one-liner five times over, plus a scriptblock
form and an execution-policy bypass, none of which anything in the scripts reads
and all of which the README already documents.
Point at the README instead and reword the in-body comments that quoted the
one-liner as shorthand. Every printed line is untouched: the remediation text the
installers show users still spells out the command in full.
Same treatment for scripts/uninstall.ps1's header.
* Windows: resolve process image paths with one Win32_Process query
install.ps1's venv-holder probe opened a handle to every running PID through
inline C# compiled at runtime. Opening a handle per process is a shape AV
heuristics score hard, and it bought nothing: Win32_Process reports
ExecutablePath for exactly the processes those handles could be opened against,
and answers for all of them in a single query instead of once per PID.
The remaining file-canonicalisation imports stay -- handle-based resolution of
linked ancestors has no faithful Windows PowerShell 5.1 equivalent, and it runs
on security-relevant paths.
Falls back to the per-process .Path when the query is unavailable, so a degraded
WMI repository degrades exactly as the old code did on a process it could not
open.
* Desktop: say who blocked the install when AMSI stops the script
PowerShell hands the whole top-level script block to AMSI while compiling it, so
a security product's verdict arrives as a parse error over the entire file before
install.ps1 runs a statement: no [TAURI:ERROR] marker, no phase log, and a stderr
tail the user cannot act on. unsloth#8523 shows what that looks like in the UI --
"Installation failed: + FullyQualifiedErrorId : ScriptContainedMaliciousContent".
Recognise the two stable error ids on either stream and append what the user
actually needs: nothing was installed, nothing was changed, it is a false
positive, update definitions and retry, do not turn off endpoint protection. The
raw id stays in the message, because the diagnostics report and any vendor
submission both need it.
Matches the id, never the message text, which is localized, and tolerates the
cmdlet suffix the Invoke-Expression form carries.
* Desktop: ship each bundle only the installer it can run
resolve_install_script picks install.sh on unix and install.ps1 everywhere else,
but the shared Tauri config bundled both into every target. The Linux AppImage
therefore carried 280 KB of Windows PowerShell it can never execute -- and it is
the largest script body a generic classifier walking the squashfs reads, which is
where Microsoft's Trojan:Script/Wacatac.B!ml verdict on 0.1.701-beta landed.
Move the resource map into the per-platform configs. The clean-machine job
already fails when a Linux bundle ships no install.sh; it now also fails when one
ships install.ps1, so the split cannot silently regress in either direction.
The .deb scanned clean with the same payload, so this is surface reduction rather
than a proven fix for that verdict.
* POSIX installers: install uv from a pinned release before falling back
install.sh downloaded astral's install.sh to a temp file, ran it and deleted the
file; studio/setup.sh piped it straight into a shell. Both are, shape for shape,
what a dropper does, and generic ML script classifiers score them accordingly --
the 0.1.701-beta Linux AppImage came back Trojan:Script/Wacatac.B!ml while the
.deb carrying the same scripts came back clean.
Fetch the pinned release archive and verify a hardcoded SHA-256 instead, matching
what install.ps1 already does on Windows. Only the four mainstream targets are
pinned: musl, armv7 and any host without a digest tool keep the path they have
today, because guessing a target triple wrong would break the install outright
and that costs far more than the heuristic score of the fallback.
Destination, PATH handling and every printed line are unchanged, so a host that
takes either path ends up in the same state it did before.
* tests: pin the installer shapes antivirus heuristics score
One file collecting what was removed, so it cannot drift back: no remote script
run in-process, no encoded or base64 payload, no hidden window paired with a
bypassed execution policy, no handle opened against another process, and no new
runtime-compiled native import outside an allowlist that carries a reason for
each entry that stays.
The last test is the other half of the contract. Hardening must not change what a
user sees, so the remediation lines the installers print -- which still spell out
the web one-liner in full -- are asserted verbatim. Removing the one-liner from
comments is the point; removing it from what the user is told to run would be a
regression.
Runs on the existing discovery-based pytest step, no workflow list to update.
* release: emit a false-positive submission packet for whatever gets flagged
The build job assembles a Microsoft submission packet, but only for the Windows
-setup.exe. The detection that actually arrived on 0.1.701-beta was
Trojan:Script/Wacatac.B!ml on the Linux AppImage, so nothing was produced for the
one asset that needed it.
The VirusTotal job already knows which assets were flagged and by which engines,
so put the packet there: hash, size and both portals, for every flagged asset
whatever platform it came from, with a note that clearance is per hash and per
vendor. Engine names are not repeated -- they are third-party text and already
appear escaped under Flagging engines.
The gate stays advisory; this only makes acting on it take seconds.
* Revert "Windows: resolve process image paths with one Win32_Process query"
This reverts commit 7897865c9.
tests/python/test_windows_installer_concurrency_guard.py bans Get-CimInstance
and $process.Path from Get-RunningStudioVenvProcesses outright, and requires the
native image-path lookup. That contract came out of #7764, which closed a set of
races where the installer inferred "in use" from something other than a confirmed
executable identity and blocked installs that should have proceeded.
Win32_Process.ExecutablePath does answer the same question, but a wrongly blocked
install costs far more than the heuristic weight of three native imports. Record
the imports in the AV-shapes allowlist with that reasoning instead, and keep the
ban on the process-memory APIs, which the installer has no use for.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Tighten the comments added by this branch
Opening comment-reduction pass over the PR diff: same intent, fewer lines. Cut
hardest on the prose that restated the PR description rather than explaining the
code next to it. Comments and docstrings only, verified with comment_tools.py
check --strip-docstrings across every Python file in the diff.
* Drop an unused helper from the uv pinned-release test
* Fix three review findings on the installer hardening
Stray-resource check aborted the step it was meant to assert. grep exits 1 when
it selects nothing, and under this step's set -o pipefail plus the runner's
bash -e that kills the assignment outright, so every correctly split .deb failed
clean-machine CI before reaching the check. Both lookups take || true now: no
match is the passing case for the stray one, and for install.sh it was swallowing
the explicit annotation in favour of a bare exit 1.
studio/setup.sh skipped astral's XDG_DATA_HOME/../bin destination tier, which
install.sh, install.ps1 and studio/setup.ps1 all honour. A host that configured
an XDG location got uv under ~/.local/bin instead, where no later shell looks for
it. The session PATH prepend hid it at install time.
The AMSI guidance claimed nothing was changed even when the block landed on the
nested studio/setup.ps1, which install.ps1 launches through the same inherited
pipes after the venv, PyTorch and the packages are already on disk. Split the
wording on whether a [TAURI:STEP] marker has been seen: a pre-start block
produces none, so the reassurance is only given where it is true.
* Key the submission packet on the flagged count, not the engine list
stats and results are separate fields of the same VirusTotal response, so an
asset can carry a flagged count with no readable results map. The summary table
reports that asset and the packet skipped it, which is exactly the one that needs
a packet. Select on stats.flagged and keep the engine list for the Flagging
engines section, which is correctly keyed on having engines to name.
* Drop the bundle stray-resource assertion from clean-machine CI
That job downloads a published release, never a bundle built from the branch, so
asserting the new resource split there turns every run red until a release ships
with it. The split is a property of the Tauri config, and
tests/studio/test_tauri_installer_resource_contract.py already enforces it at the
right layer.
The || true on the install.sh lookup stays: it is what lets the explicit
annotation print instead of the step dying on grep's exit 1 under pipefail.
* Windows: stop depending on the generated unsloth.exe console script
Fixes #8490. On Windows the `unsloth` entry point is materialised as a
generated, unsigned launcher .exe. AppLocker, WDAC and Smart App Control
deny it, while the venv's python.exe, a copy of the signed CPython binary,
still runs. The installer died at "running unsloth studio setup" with
`Program 'unsloth.exe' failed to run: An Application Control policy has
blocked this file`, and because the launch throws rather than returning an
exit code, it escaped Install-UnslothStudio and printed a raw
NativeCommandFailed dump instead of a diagnostic.
The desktop updater already solved this in update.rs by reaching the CLI
through the interpreter. This applies the same idea everywhere else: the
setup handoff, autostart, the shortcut launcher, the Tauri backend, auth
provisioning, the install health probe, the preflight probes and the
`studio run` respawn. unsloth.exe is still generated, still hardlinked to
the shim, and still works. Nothing depends on it any more.
Also adds `python -m unsloth_cli` as a supported entry point, and a
bin\unsloth.cmd companion to the shim so `unsloth.cmd` is available where
the .exe is denied.
The trampoline is one string shared by install.ps1, process.rs and
studio.py:
import sys, os; sys.path[:1] = [x for x in sys.path[:1] if x not in ('', os.getcwd())]; sys.argv[0] = 'unsloth'; from unsloth_cli import app; app()
Both halves are load bearing. argv[0] is assigned before the import
because unsloth_cli decides at import time whether it is the console
script, which gates the UTF-8 stream setup and the -np<N> rewrite, and it
keeps typer's prog_name at `unsloth`. The sys.path[:1] filter drops the
working directory entry that `python -c` adds and a console script does
not, which is what lets the invocation stay off -I: -I would drop it too,
but also PYTHONPATH, PYTHONWARNINGS and user site-packages, which the
console script honours.
Behaviour on a machine with no policy is unchanged, and that is enforced
rather than asserted. tests/python/test_module_entry_point.py compares
stdout, stderr and exit code between the console script, `-m unsloth_cli`
and the trampoline over --version, --help, `studio --help` and two error
paths. The writes are idempotent: bin\unsloth.cmd, launch-studio.ps1 and
the .lnk files are content compared, so a second install changes no bytes
and no timestamps.
tests/studio/test_application_control_cli_fallback.ps1 pins the pieces
that are easy to get wrong: the failure is classified off the exception
(Win32 1260), never off $LASTEXITCODE, which no process was created to
set; Start-Process gets one pre-quoted command line, since -ArgumentList
joins an array with spaces and quotes nothing; and bin\unsloth.cmd only
counts as an ownership marker when its contents match the shim we write,
so an unrelated file of that name in a custom root cannot qualify it for
removal.
The new windows-application-control-ci.yml leg reproduces the report:
AppLocker denies only Scripts\unsloth.exe for a standard user, a negative
control proves the rule is actually enforced (the job fails loudly if the
stub runs), and the full installer then has to succeed.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Tighten the comments added for the Application Control fix
* Add the AGPL header to the module entry point test
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Drain the shim launch probe's pipes before waiting on it
* Harden the cmd shim ownership marker, updater env and launcher hints
* Run the Application Control CI leg without --tauri so the pinned root applies
* Stub the runtime gate so the Windows launcher tests run on Windows
* Isolate the advertised module route from the working directory
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Fix the contradictory updater env assertion and the user-site fallback
* Treat a quarantined stub as a managed install on Windows
* Tighten the duplicated trampoline rationale to one authoritative copy
* Windows: keep a quarantined launcher and a partial migration recoverable
Two follow-ups on the Application Control work.
An antivirus quarantine deletes the unsigned unsloth.exe rather than
denying it. The updater then found no launcher, no copy to restore, and
reported a broken update, rolling back a package that was in fact fine.
Absence is now excused the same way a policy denial is, but only after
every recovery copy has been tried, so a launcher that could be put back
still is.
find_unsloth_binary_in_studio_dir accepted a bare python.exe in layout
order, so an interrupted migration leaving a partial new environment
beside a working legacy .venv targeted the broken one. A launcher
anywhere now outranks an interpreter on its own.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Windows: let studio run start a venv whose console script was quarantined
The Windows respawn goes through the interpreter and never launches
Scripts\unsloth.exe, but the gate before it still required that file, so
an install whose stub antivirus had taken aborted with "Unsloth venv
missing 'unsloth' entry point" despite being able to run. The installed
package now answers for the deleted stub, one layer down and just as
cheap. POSIX still proves a CLI with the console script it execs.
* CI: apply the AppLocker policy before AppIDSvc reads it
The negative control watched the denied user start the stub. The job
started AppIDSvc and set the policy afterwards, and the service loads the
effective policy when it starts, so it was enforcing nothing; gpupdate
does not make it re-read a local policy. Restart the service once the
policy is in place, and retry the control while enforcement goes live,
which is asynchronous and unsignalled.
* Cover the uv host matrix and repeat application in the pinned-release test
The pinned path picks an archive per host triple, and a wrong pick installs a
binary that cannot execute, which is worse than not installing at all. Drive
_uv_pinned_asset over 20 host combinations and require each one to return its
own triple or decline to the fallback.
Also run the installer three times over one HOME and require an identical tree,
and require a stale uv at the destination to be replaced rather than joined by a
second copy: the installer is re-run on every upgrade and every repair.
* Windows: close the parity and old-install gaps found by the idempotency audit
Five independent audits of the before/after parity bar, plus local
simulations, turned up six things worth fixing.
Parity, on machines with no policy at all:
- Under PYTHONSAFEPATH or -P there is no implicit -c working-directory
entry to strip, so sys.path[0] is whatever PYTHONPATH put there and the
console script honours it. The filter removed it anyway; a PYTHONPATH
starting at the working directory was measured selecting a different
package through the trampoline than through the console script.
- The backend start log went from a joined argument string to Rust's
debug list on every platform. It is what users paste into issues.
Idempotency:
- The .cmd shim and launch-studio.ps1 compared decoded text, which drops
a BOM and ignores case, so a BOM-prefixed shim was called unchanged and
left with cmd.exe reading the BOM as part of @echo off. Both compare
bytes now, launcher preamble included.
- A run killed between the temp write and the rename left a temp file no
later run would collect, since each names its own after its PID. Swept,
skipping any whose owner is still alive.
- The Application Control probe cached its verdict in :, which
under irm | iex is the caller's session, so a second run in one console
answered from the first run's machine state.
Old installs:
- An installer older than the shim directory never created one, and
unsloth studio update is the only route those installs take back into
install.ps1, so they never gained the .cmd. Created there now.
- A migration interrupted by an open handle can split either layout. The
finder now prefers a launcher with its interpreter beside it in either
base, then an interpreter alone, then a launcher alone, so neither half
of a split tree wins by layout order.
* Pick the pinned uv archive off a positive libc check, not the absence of musl
An independent audit pass found the Linux selector accepts any host whose ldd
output does not say musl. That is not the same question astral's installer asks:
it checks a minimum glibc and drops to its musl-static archive below it, so
three hosts that worked before this branch now get a GNU binary that cannot exec,
and the helper reports success so the fallback never runs.
aarch64 with glibc below 2.28 (Ubuntu 18.04)
x86_64 with glibc below 2.17 (RHEL 6)
a musl image with no ldd at all, where the probe simply finds nothing
Read the version instead, from ldd or getconf, and require it to clear astral's
floor for the triple. Anything unreadable declines to the fallback. Also ask the
userland for its bitness rather than trusting uname on a 64-bit kernel running a
32-bit userland, and follow astral in reading hw.optional.arm64 so a translated
shell under Rosetta 2 still gets the native macOS build.
Three more from the same pass:
Report success only when the destination uv is executable. A copy onto a busy or
read-only destination could leave a file that is not, and reporting success there
skipped the fallback. Nothing is unwound on the failure path on purpose: the
fallback installs over whatever is at the destination, and deleting there would
take out a working uv the host already had.
Clear the mark of the web on the launcher we author. WriteAllText replaces the
unnamed data stream and leaves other NTFS streams alone, so a launch-studio.ps1
that somehow carried one would keep it across the rewrite, and RemoteSigned
refuses a marked unsigned script.
Store the security-block kind and resolve its wording in message(). stdout and
stderr are read by independent threads, so a [TAURI:STEP] written before a block
can be observed after it, and freezing the wording at observation time could tell
a user nothing was changed on a run that had already installed PyTorch. Also
require the error id to appear as the value of a FullyQualifiedErrorId field, so
a scanner log that merely names it cannot attach antivirus guidance to whatever
fails next.
The host matrix in the shell test grows to 28 rows covering every case above, and
removing the new gate fails 8 of them. install.rs gains two tests: 37 pass.
* Replace a symlinked uv destination instead of writing through it
Three from the review on the previous head.
cp onto a destination that is a symlink follows the link, so installing over
`~/.local/bin/uv -> /opt/homebrew/bin/uv` rewrote the Homebrew binary in place
and left the link pointing at a file another package manager owns. Stage next to
the destination and rename over it: rename replaces the link itself, and it is
atomic, so a concurrent reader never sees a half-written uv either. The staging
file is removed when the rename fails, so a failed run leaves no debris.
Verify the Windows copy the same way the shell scripts now do. Copy-Item is
non-terminating under the caller's ErrorActionPreference, so a locked or
ACL-denied destination let execution reach `$haveUv = $true` and the function
reported success over whatever was already there. Compare the destination against
the archive we just verified, so a stale uv.exe cannot pass for the one we meant
to install. install.ps1 carried the same shape and gets the same treatment.
Point the header links at the heading that exists. The README has no "Install
Unsloth Studio"; it is "Unsloth Studio (web UI)", whose anchor is
#unsloth-studio-web-ui.
Three test cases cover the symlink: the file behind the link is untouched, the
link itself is replaced, and no staging file survives. Reverting the fix fails
two of them.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* CI: keep one Application Control negative-control log per attempt
Enforcement went live on the fourth try on the hosted runner, and a single
overwritten log left the evidence artifact showing a pre-enforcement
attempt's output beside a passing step.
* Fail the build when the uv pin drifts from a version floor
Before the pin, astral's endpoint always delivered the newest uv, so raising
UV_MIN_VERSION was safe on its own. It is not any more: a floor above the pin
means a host with no uv gets 0.12.1 installed and then judged too old by the same
script that installed it, on the one path where the pin is what runs.
Two checks. All four installers must name the same uv, or which version a machine
ends up with depends on which script reached it first. And the pin must clear
every floor in the tree (UV_MIN_VERSION, UV_OFFLINE_MIN_VERSION, $UvMinVersion).
Raising a floor past the pin fails the first, bumping one installer's pin alone
fails the second.
* Fix two Windows-only test failures that predate this branch
test_path_identity_failure_is_reported_as_unknown failed on both shells,
on main as much as here. Test-StudioPathEqual reports an unresolvable
path identity through Write-StudioLine, the harness extracts the mutex
helpers but not that, and these scripts run under -ErrorActionPreference
Stop, so the catch path died with CommandNotFound before the test could
measure anything.
Extracted rather than stubbed: it is self-contained, and a stub would
keep passing if the real call ever went wrong. A new check asserts every
installer function the extracted helpers call is in the harness, and it
runs on every platform, so the next drift cannot hide where only a
Windows runner would see it.
Measured on a Windows runner: main fails 24 of these, this branch fails
2, and both of those 2 are in main's set. With this, 0.
* Write the shell profile entry the pinned uv path no longer gets for free
The P1 here is a real regression and it took a second look to see why.
install.sh decides whether to add ~/.local/bin to the user's shell profile with
`case ":$PATH:"`, near the end of the run. By then this process has prepended
that directory twice, once for the uv bootstrap and once for the venv, so the
guard answers yes for a login shell that would answer no and the profile line is
never written. That was survivable while astral's installer ran, because it wrote
its own profile line and its env file. The pinned path writes neither, so on a
fresh account whose login PATH lacks ~/.local/bin the install succeeds, the
current shell works, and the next terminal cannot find `unsloth` or `uv`.
Snapshot the inherited PATH before anything prepends to it and test the guard
against that.
Two more from the same review.
Honour a configured uv mirror exclusively. UV_INSTALLER_GHE_BASE_URL and
UV_INSTALLER_GITHUB_BASE_URL already win outright in both PowerShell installers
and in astral's own; the shell path ignored them and tried the public hosts
first. A restricted network sets one precisely because those hosts are
unreachable, and download() has no timeout, so it would hang rather than reach
the fallback.
Do not let the twin of an earlier clear erase a later AMSI verdict.
Clear-TauriInstallError writes one logical clear to BOTH streams
(install.ps1:198) and independent threads read them, so a block observed between
a clear and its own twin was discarded by the twin. Ignore a clear identical to
the one just processed; a genuine later recovery carries different text and still
clears. Two tests cover both directions, 39 install tests pass.
* Stage the uv copy under a per-process name
An audit pass reproduced a race I introduced with the symlink fix. Both POSIX
helpers staged through a fixed destination-side name, so two installers targeting
one directory shared it:
A finishes copying the staging file
B opens the same path with truncation
A renames that inode into place as uv
B keeps writing through its open descriptor, which is now the published uv
The published uv was observable at zero bytes until B resumed, which makes the
claim in the comment about a concurrent reader flatly wrong. install.ps1 is
covered by its named mutex, but nothing serialises the POSIX helpers, and
studio/setup.sh runs standalone on every studio update.
mktemp in the destination directory instead. Each rename then publishes a file no
other process can still be writing, which is what the atomicity argument needed
all along. The loser cleans up its own staging file and declines, so the caller
falls back rather than reporting a success it did not achieve.
* Keep a default install as quiet as it was when a uv mirror misbehaves
Two console regressions from the audit pass, both on paths the install still
recovers from.
download() runs curl -LsSf, and -S deliberately prints its own errors. The
fallback ran under run_maybe_quiet, so a failed download printed nothing before;
the pinned attempts run outside that wrapper, so an unreachable mirror now put
two curl: (N) lines on the console of a default install that then succeeded.
Redirect stderr on the speculative attempts only, leaving download() untouched
for every other caller.
[TAURI:WARN] is a marker level install.sh has never emitted, and the app forwards
unknown markers to its progress UI verbatim (install.rs:639), so a digest
mismatch would have surfaced as raw text in the desktop window. Make it a verbose
only stderr line: the next mirror or the fallback still runs, so a default
install has nothing to say here.
Printed-string diff against the merge base is back to additions inside $(...)
capture plus that one verbose-gated line, with nothing removed or changed.
* Ask the installed uv whether it runs before skipping the fallback
The libc gate reads a glibc version from ldd or getconf and treats that as proof
a GNU binary will execute. It is not. A stripped NixOS-derived image without
nix-ld reports a glibc version through getconf while its loader lives in the Nix
store, so the pinned x86_64 uv asks for /lib64/ld-linux-x86-64.so.2 and gets
nothing. Every static check passed, so the helper reported success, the astral
fallback was skipped, and the first real uv call failed with No such file or
directory. astral's installer fails its own glibc probe on that host and ships
the fully static musl archive, which runs. The user went from a working uv to
none.
The archive is digest-verified astral uv by the time it is placed, so ask it:
run --version and require it to succeed. One exec closes the whole class rather
than this one host, covering a wrong triple, a loader that is not where the
binary looks, and a destination we could not really write.
A test drives an archive whose uv cannot execute and requires the helper to
decline; removing the exec check fails it.
* Pair every clear with its twin, not just the previous one
install.ps1 clears after each recovered step, so a lagging reader can be several
clears behind when a block lands. With clears A then B on one stream and A's twin
arriving on the other after the verdict, asking only whether this is the message
just seen answers no, and the delayed twin discarded the verdict the guidance
exists to explain.
Each logical clear emits exactly two markers, so count unpaired ones by message:
the first sighting is the clear, the next pairs with it. A test drives the A, B,
verdict, A', B' ordering; 40 install tests pass.
* Close the exactness gaps found by ten adversarial audits
Ten independent audits, each asked to falsify the claim that this is pure
hardening. Six things were worth changing.
- The desktop updater is isolated again. It shipped with -I, it is the one
managed invocation nobody types by hand, and it decides which install
gets rewritten, so a user-site unsloth_cli must not answer
`from unsloth_cli import app` there. Every other call site inherits,
because the console script does.
- The trampoline ends in sys.exit(app()), like the generated console
script, so a returned value becomes the exit status. Typer raises
SystemExit itself today, but the two routes have to agree.
- A launcher that could not be restored keeps its recovery copies. Judged
healthy through the interpreter is not the same as repaired, and
deleting the copies threw away what a later run needed.
- `unsloth studio update` puts the shim directory on PATH. An installer
older than that directory put the venv Scripts dir there instead, so the
.cmd was written where nothing would look for it.
- The console script reconfigured its streams twice off Windows, once
through the import gate and once through the module-entry path.
- Replacing a bin\unsloth.cmd that carries neither our marker nor our
trampoline now says so.
Also states the scope plainly: this answers EXE-and-DLL enforcement of the
unsigned console script. A machine that also enforces AppLocker's Script
collection denies .cmd and .ps1 alike, and install.ps1 would not have run
there either.
The two test harnesses that extract functions out of install.ps1 now
assert they define everything those functions call; both had already
shipped a gap that made a check pass for the wrong reason.
* Ask the interpreter, not site-packages, whether the managed CLI is there
The quarantine fallback accepted an unsloth-*.dist-info or an
unsloth_cli/ directory as proof of a runnable CLI. Neither is: an
interrupted install, or an editable install whose checkout has moved,
leaves metadata with nothing to import. This gate sits in front of the
headless-public strip of .bootstrap_password, so a false yes lands the
exact lockout its placement exists to prevent -- a public Studio with no
login page and no plaintext recovery credential.
find_spec through the managed interpreter answers the question the
trampoline will actually ask, with the same sys.path[0] scrub so a
checkout in the caller's cwd cannot stand in for the venv. A probe that
produces no verdict at all falls back to the old on-disk layout, so a
half-quarantined install still starts.
* Hide the import probe's console window, as every other managed probe does
* Validate the staged uv before it replaces a working one
My own exec check was on the wrong side of the rename. The sequence that bites:
a host has a uv good enough for UV_OFFLINE_MIN_VERSION but below UV_MIN_VERSION,
so the block runs with _uv_present_before true; the pinned path renames over that
working binary; the --version check then fails because the loader is missing or
the destination is mounted noexec; the fallback download also fails. The
installer neither restores the old uv nor reports that none is available, and
every later command runs the broken one.
Test the staging file instead, before the rename. It sits on the destination
filesystem, so it answers the noexec question too, and a binary that cannot run
here never gets to replace one that could.
Two tests: a working incumbent uv survives an archive whose uv cannot execute,
and the rejected staging file is cleaned up. Moving the check back after the
rename fails the first.
* Make each managed CLI probe ask the question its launch will answer
Three findings from the latest review round, one theme: a probe that stands in
for a launch has to run under the same conditions as that launch, or it can pass
where the launch then fails.
* The quarantine gate in `studio run` asked find_spec whether unsloth_cli
resolves. It resolves for an emptied unsloth_cli/ directory (find_spec calls
that a namespace package), for a package whose __init__ raises, and for one
whose dependencies an interrupted install never fetched, and the trampoline's
`from unsloth_cli import app` fails on all three. Verified: an empty package
directory in a bare venv gives find_spec True and ImportError on the import.
This gate stands in front of the headless-public strip of .bootstrap_password,
so a false pass there is a public Studio with no login page and no plaintext
recovery credential. The probe now performs that exact import.
* The updater's interpreter health check ran without isolation while the launch
it predicts, build_update_command in studio/src-tauri/src/update.rs, runs under
Isolation::Isolated with PYTHONHOME/PYTHONPATH cleared. A foreign checkout on
PYTHONPATH could answer --version for a managed package the update had broken,
and validate_launcher would keep an update the next desktop launch cannot
start. _managed_cli_argv now takes the same isolated flag the Rust Isolation
enum carries; the health probe is the only caller that sets it, and a test
pins that it stays the only one. Every other invocation keeps PYTHON* parity
with the console script.
* Binary resolution, second pass. With an interrupted migration leaving an
interpreter in both layouts and a launcher in neither, layout order handed back
the new base even when its site-packages was empty and the legacy base still
held the package. A directory test rather than an import probe: this runs on
the launch path and from the capability checks, so it stays a stat.
Tests: the four unimportable package shapes, the isolated/inherited argv split
and its single caller, and both directions of the two-interpreter tie-break.
The old whole-file "no -I anywhere" assertion is now read off the ternary, since
one deliberate -I exists.
* Stop the AMSI guidance claiming more than it knows
Two of these are honesty defects in text a blocked user reads.
"nothing was changed on this machine" is false. Rust starts a diagnostics
attempt and its phase log before PowerShell is ever spawned, and spawn_script can
create ~/.unsloth first, so a pre-start block has already written to disk. The
honest claim is that no installation step ran.
"This is a false positive" is not something the classifier can know. It proves
the output carries a PowerShell error id and nothing about the script's
integrity, and install.ps1 can sit in a user-writable directory, so a locally
modified copy can earn a genuine verdict. Telling someone to report a correct
detection to their vendor is worse than telling them to reinstall from an
official package first and only escalate if an unmodified copy is still blocked.
Two smaller ones from the same pass. The matcher tested for the field name and
the id independently, so a line naming both in prose qualified; it now requires
the id to follow the colon and end at a comma or whitespace, which is what the
comment always claimed. And the clear-pairing map is bounded: legitimate
producers use a small fixed label set, and child output must not be able to grow
it without limit.
42 install tests pass.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Honour astral's download override, and stop Unblock-File asking
Three from the second audit round.
Unblock-File declares SupportsShouldProcess at the default Medium impact, so a
profile that sets $ConfirmPreference to Medium or Low gets a prompt from the line
I added, even for a launcher that never carried the stream. -ErrorAction does not
suppress a ShouldProcess prompt, and a noninteractive host turns it into an error
that skips shortcut setup entirely. -Confirm:$false.
UV_DOWNLOAD_URL and its older alias INSTALLER_DOWNLOAD_URL outrank the mirror
variables in astral's installer, and the merge-base path inherited that because it
ran astral's script. All four implementations now honour them first and
exclusively. My earlier comment argued they point at a version the pin would
reject, but that reasoning had it backwards: a host sets one because it cannot
reach the public endpoints, so ignoring it meant public egress first and, with no
timeout on the download, a hang instead of a fallback. The pin still applies, so a
source serving a different build fails the digest and the caller falls back to
astral's installer, which honours the same variable.
chmod 0755 on the staging file rather than +x. cp gives it the umask default and
+x then adds execute only where the umask allowed read, so a umask of 077 left uv
unusable for every other account on a shared machine. astral ships them 0755.
Four checks pin the override precedence across all four installers and the mode
across both shell ones, with the behaviour verified against a stubbed downloader.
* Validate uv before it replaces an incumbent on Windows, and bound the probe
install.ps1 and studio/setup.ps1 copied the extracted uv.exe straight over the
destination and only asked whether it ran afterwards. A host with a working older
uv and a policy (AppLocker, WDAC, endpoint protection) that refuses the new one
was left with neither. Run the extracted binary where it landed first, then keep
a copy of the incumbent across the publish and restore it if the published copy
will not run, since Windows has no atomic replace for a file that may be open.
The probe itself is bounded: Start-Process with a 20s WaitForExit and redirected
streams, and on POSIX no stdin plus a 20s ceiling where timeout exists. A binary
this installer just downloaded must not be able to hang an unattended install by
prompting or by never exiting.
install.sh and studio/setup.sh also published the pinned uvx after rejecting the
pinned uv, leaving a pairing that is never built or tested. A uv that fails to
stage, copy or run now abandons the whole placement.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Verify each uv mirror, and persist PATH when the account has no rc file
Both PowerShell installers checked the archive digest once, after the download
loop had already broken out. A captive portal or a proxy answering 200 with its
own body is a successful download by every measure Invoke-WebRequest has, so the
first mirror consumed the only attempt and the second, healthy one was never
tried. The digest now decides whether a mirror counts as served.
install.sh picked a shell profile from .zshrc, .bashrc or .profile and did
nothing when none existed. A fresh account has none: astral's installer used to
create its own PATH setup there, the pinned path does not, so the next terminal
resolved neither unsloth nor uv. Fall back to creating ~/.profile, which every
POSIX login shell reads. The existing content guard keeps it written once.
* Remove the install.sh a Windows upgrade would otherwise keep forever
Windows bundles now carry only install.ps1, but NSIS writes the current resource
manifest and deletes nothing, and the uninstaller deletes only what is in that
manifest. An in-place upgrade from a release that bundled both installers left
install.sh in $INSTDIR permanently, which also made the non-recursive
RMDir "$INSTDIR" fail at uninstall. The pre-install and pre-uninstall hooks now
delete it, so the population most likely to upgrade actually gets the split.
Also silence the speculative mktemp -d in the pinned uv path: its failure falls
back to astral's installer, so an unusable TMPDIR printed a line the user could
not act on and that the merge base did not print.
* Remove the pinned uv temporaries when an install is interrupted
The pinned path unpacks a 40 MB archive into a work directory and stages the
binary next to the destination, but only cleaned both up when the helper returned
normally. A Ctrl-C in between left the archive behind and left a staging file
inside a directory that is on PATH. Both paths are now published to the exit and
signal traps as they are created and cleared when the helper releases them.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Persist the PATH the way each shell actually reads it, and fail a half-published pair
Four follow-ups from review:
A uvx that the archive carried but that could not be staged or renamed left uv
published next to a stale or missing uvx and still reported success, skipping the
fallback that would have installed both. Either half failing now fails the
placement, in install.sh and studio/setup.sh.
studio/setup.sh had none of the interrupt cleanup install.sh gained: a Ctrl-C
left the unpacked archive behind and a staging file inside a directory on PATH.
It now owns HUP, INT, TERM and EXIT for the duration of the pinned install and
hands them back on the way out.
fish sources none of the POSIX rc files, so the ~/.profile fallback was a no-op
for a fish user. The persistence helper writes a conf.d drop-in with
fish_add_path there, and honours ZDOTDIR for zsh.
UV_INSTALL_DIR, UV_UNMANAGED_INSTALL, XDG_BIN_HOME and XDG_DATA_HOME can put uv
somewhere other than ~/.local/bin, and astral's installer wrote a PATH line for
whichever it picked. The pinned path now persists its own destination too, with
UV_NO_MODIFY_PATH honoured as astral honours it.
* Make the Windows uv publish a real transaction, and quote persisted paths
The companion copies ran bare: under install.ps1's Stop preference a locked or
ACL-denied destination threw past the rollback and left a mismatched set with the
backups still on disk, and under setup.ps1's Continue preference it kept a stale
companion and reported success. Both now copy under -ErrorAction Stop inside the
transaction, so any failure unwinds like the others.
A failed restore also used to delete the backup anyway, which is the one path in
this block that could leave the host with less than it started with: the two
things that make a restore fail, an open incumbent and a denied ACL, are the same
two that made the replace risky. The backup is now kept and named.
fish takes an unquoted path with a space as two directories, neither of which
exists, so the drop-in single-quotes it; and the rc line is written inside double
quotes, so a uv directory holding a dollar or a backtick is escaped. The second
test caught a doubled backslash in the escaper itself.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Tighten the comments added by this branch
Comments only, no code touched: 161 comment lines become 106 across install.sh,
install.ps1, studio/setup.sh, studio/setup.ps1 and the NSIS hooks. Each one keeps
the reason it was written for, said once.
Verified with the PowerShell AST parser, sh -n and bash -n, the 50-check uv
pinned release suite and 114 installer tests, and by confirming the diff contains
no non-comment line.
* Tighten the install.rs comments too
Comments only: 35 lines become 27, each keeping the reason it was written for.
42 install tests pass and the diff contains no non-comment line.
* Abort on a companion that cannot be backed up, and pair clears by stream
A uvx.exe that could not be copied aside, because it is locked or its ACL denies
reads, was skipped and the new uv.exe published anyway, so the function reported
success with a mismatched pair and the fallback never ran. Any backup failure now
fails the placement and runs the rollback, in install.ps1 and studio/setup.ps1.
The ERROR_CLEAR pairing keyed only on the message, so two real clears of one label
on one stream were taken for a clear and its twin. That happens:
_install_torch_default_index emits its recovery during the install and again
during the ROCm repair. A verdict landing between them was then erased by the
genuinely later clear arriving on the other stream. The map is keyed by stream as
well, so only the opposite stream's copy can consume a pending marker.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Do not fail an install because the uv probe could not get an answer
Three clean-machine CI legs that pass on main failed on this branch: arm64 and
two Windows containers, all three with winget unavailable, which is the only
condition under which the pinned fallback runs. Each downloaded the right asset,
passed the digest, and then failed the probe. Start-Process -NoNewWindow with
redirected streams does not behave in a container or on the arm64 image the way
it does in a desktop session, and a boolean probe reported that as a broken
binary and aborted the install.
The probe is now tri-state. Only the binary answering non-zero is a failure. A
launch that throws or a wait that times out is inconclusive, and since the digest
already proved the bytes are astral's pinned release, an inconclusive probe
publishes as the pre-pin code did. Every path prints why, with the captured
stderr and the exit code, so the next occurrence is not opaque.
Also from review: the POSIX path now stages both binaries and publishes them
together with the incumbents saved aside, so a failed uvx rename restores the
uv it replaced instead of leaving a new uv beside a stale uvx; the Windows
rollback records the destination before the copy that can truncate it; and
UV_UNMANAGED_INSTALL suppresses the profile write, as it does for astral.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Give setup.sh the same pair publish and PATH persistence as install.sh
studio/setup.sh published uv and then uvx one after the other, so a failed uvx
rename left a new uv beside the host's stale one, and the remote fallback can be
unavailable. It now stages both, validates uv, and publishes the two renames back
to back with the incumbents saved aside, restoring them if the second fails.
setup.sh is also run directly for local and Colab setup, where astral's installer
used to write the profile line for whichever destination it chose. Without one the
PATH export died with that shell and every later run reinstalled uv. It now
persists its own destination, with fish handled on its own terms and both of
astral's opt-outs honoured.
* Treat an empty uv exit code as no verdict, not as a failure
The arm64 clean-machine leg still failed on the tri-state probe, and the
diagnostic that came with it said why: "uv --version exited ." with no number.
WaitForExit(ms) can return before the exit code is cached, so ExitCode was empty
and an empty value is not 0, which read a working uv as broken.
The parameterless WaitForExit settles it and returns at once because the process
has already exited, and a code that is still missing is inconclusive rather than
a failure, which is the same rule the launch and timeout paths already follow.
Verified against pwsh that a real non-zero exit and a real launch failure still
classify as failed and unknown respectively.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Eight review fixes across the uv publish and PATH persistence
The fish escaper in studio/setup.sh reached sed as an invalid expression, so a
fish user running setup directly would have had setup killed under set -e right
after uv was published. It now matches the one in install.sh, and the test runs
both escapers rather than reading them.
An incumbent that cannot be hard-linked or copied cannot be restored either, so
publishing over it would be a one-way move. Both shells now decline. Writing that
test turned up that my own rollback deleted both incumbents when nothing had been
published, since the no-predecessor branch cannot tell the two cases apart; the
rollback is now reached only after a publish was attempted.
A rollback with no predecessor removes the binary it published, rather than
leaving half a pair the host never had.
A signal between the two renames left the undo copy as the only reference to the
incumbent, and the handler deleted it. It restores it now, in both shells.
setup.sh prepended ~/.local/bin unconditionally after a successful pinned
install, so a stale uv there could shadow a custom UV_INSTALL_DIR destination and
the rest of setup would run the wrong one. That prepend is now only for astral's
installer, which is what writes there.
PATH entries are compared literally rather than as case patterns, so a
destination holding *, ? or [ is not mistaken for an unrelated entry.
On Windows, a .unsloth-old left behind by a failed restore is the only copy of a
working uv, and the next run reused that exact name. It takes a distinct one.
* Keep the pinned uv first on PATH, and only count an active profile entry
install.sh prepends ~/.local/bin after the uv bootstrap, and astral's env file
does too, so a custom UV_INSTALL_DIR destination was pushed behind a stale uv
sitting in the home directory and every bare uv below picked the wrong one. The
pinned destination goes back in front. setup.sh had the same shape and was fixed
in defd2292a.
The profile check treated any occurrence of the destination text as proof the
PATH entry was already there, so a commented-out old export, or /opt/uv-old when
the destination is /opt/uv, suppressed the write and left the next shell without
uv. Comments are stripped and the directory has to appear as a whole entry.
* Close five review findings on the quarantine and stubless paths
* The installer still required Scripts\unsloth.exe to exist, and aborted the
whole install when it did not. That reasoning held for a policy, which denies
the file and leaves it on disk, but not for antivirus, which quarantines it out
of a venv that still runs, and nothing past that point executes it: the setup
handoff, the shortcuts and bin\unsloth.cmd all go through the interpreter. It
refused to install or repair Studio for exactly the machines this change is
for. Absence now asks the interpreter for --version through the trampoline, and
only a venv that cannot answer fails, with the same older-unsloth guidance.
* The import probe's no-verdict fallback is now split by cause. A timeout keeps
the on-disk layout, because slow is not broken: a cold venv under an antivirus
scan is exactly that, and the re-exec has no timeout of its own. A failure to
START the interpreter fails closed, because the re-exec runs that same
interpreter and will fail the same way, and the caller strips
.bootstrap_password before re-execing on a headless public launch.
* The updater's interpreter fallback used the launcher's 10s timeout for a call
that has to import the entire CLI package. That is the work the import probe's
60s ceiling is deliberately generous for, and under the antivirus scan this
path exists to survive the short one would call a healthy update broken and
roll it back, once per recovery candidate.
* Binary resolution now accepts an unsloth-*.dist-info alongside the package
directory when ranking stubless venvs, matching _managed_cli_site_packages_
layout. A PEP 660 editable install leaves a .pth and a dist-info and no
unsloth_cli/ at all, so the directory test alone ranked a working legacy venv
below an empty new one.
* managed_bin_fingerprint required fs::metadata on the launcher, which the
stubless layout deliberately reports as a path that does not exist, so the
capability cache could be neither read nor written and every preflight paid
both probe subprocesses again. It falls back to python.exe, which is what
starts the CLI there, while the cache key stays the launcher path.
Tests: the fail-closed/fallback split in both directions, the timeout contract
and that the two constants differ, the editable-install ranking with an
unrelated dist-info as the negative control, the stubless fingerprint and its
invalidation, and the installer gate through the extracted AST harness.
* Gate the NSIS tidy-up, and remove an orphan uv on signal
The pre-install hook runs before the user can still cancel, and $INSTDIR can be a
directory they picked in the GUI, so deleting install.sh there could take a file
that was never ours. Both hooks now only act where our own executable already is.
A signal between the two renames restored a predecessor but did nothing when
there was none, leaving a 0.12.1 uv beside whatever uvx the machine had. It now
removes what it published, which is what the ordinary rollback already does.
* Write the uv PATH entry to every startup file astral's installer wired
astral's uv installer wires ~/.profile, each of .bashrc, .bash_profile and
.bash_login that exists, .zshrc or .zshenv under ZDOTDIR, and a fish drop-in
under ~/.config. Replacing that installer with a pinned archive meant the PATH
entry only reached the one file for whichever shell happened to be running, so a
bash user whose .bash_profile does not source .bashrc, a /bin/sh login, or anyone
who later switched shells would have no uv on PATH where they used to.
Both POSIX installers now write the same set, once each, with the existing
whole-entry check keeping a re-run idempotent. Files that do not exist are not
created, apart from ~/.profile, which astral creates too.
* Cut the uv publish back to what the common case needs
The rollback machinery that grew over the review rounds covered cases a user is
very unlikely to meet: an incumbent that cannot be hard-linked, a signal landing
between two renames, a restore that itself fails, a second installer racing the
first. It was 281 net lines, and every finding in the last two rounds was in it
rather than in the hardening.
What stays is what the common case needs. POSIX stages both binaries, runs the
staged uv, and publishes the pair with two renames; a failure anywhere before
them leaves the destination untouched, and the caller falls back to astral's
installer exactly as before. Windows probes the extracted uv.exe before touching
the destination, then copies the three under -ErrorAction Stop and re-checks the
digest at the destination.
The staging files are still removed on a signal, since they live in a directory
that is on PATH. 64 shell checks and 114 installer tests cover the rest.
* Match the exact fish entry, and let a UNC launcher load
The fish drop-in is the only thing that puts uv on a fish user's PATH, since fish
reads none of the POSIX files, and its check treated any occurrence of the
directory as proof: /opt/uv-old suppressed /opt/uv. It now matches the exact
fish_add_path line it would write.
A launcher on a UNC share is a remote script to PowerShell, and RemoteSigned
refuses an unsigned one, so a roaming profile got a shortcut that exits without
starting Studio. That case, and only that case, uses Bypass, and drops
-WindowStyle Hidden with it so the pair the detections key on never appears.
* Wire every startup file on a DEFAULT install too, and give setup.ps1 a fallback
The all-profile PATH write was gated on the uv destination differing from
~/.local/bin, which is exactly where a normal install puts it, so every ordinary
machine still got the single-file write the shim path has always done. Three
independent audits found this. The gate is gone, and the idempotency check now
also matches the $HOME-relative spelling the shim block writes, so the default
case does not end up with two lines for one directory.
studio/setup.ps1 replaced astral's installer with the pinned archive and had
nothing to fall back to. A failed pinned install therefore left UseUv false and
silently ran torch, bitsandbytes, Triton and the rest through pip: a different
resolver, not just a different download. winget is the fallback, as install.ps1
already does, rather than the remote script this branch exists to remove.
* Make the quarantine case survive root inference, PATH and the reset hint
Three more from review, all the same shape: a Windows path that still treats
the generated unsloth.exe as the only evidence of an install.
* Root inference. _looks_like_installer_managed_studio_home accepted
share/studio.conf or bin\unsloth.exe, and only install.sh writes studio.conf,
so on a custom-root Windows install the quarantinable launcher was the only
sentinel there was. Once antivirus took it, STUDIO_HOME fell back to
~/.unsloth/studio and every studio subcommand read and wrote the wrong tree
while reporting success. bin\unsloth.cmd now counts, validated against the
same marker pair and 8 KB ceiling Test-UnslothCmdShimFile and the
uninstaller's recursive-delete guard use, because this decides which
installation the CLI manages and the directory is on PATH.
* The PATH gate took any leaf named unsloth.cmd as a usable launcher.
Write-UnslothCmdShim warns and leaves an unwritable file alone, so a foreign
shim in a custom root survives the run, and counting it put its directory on
PATH and advertised someone else's command as the policy-safe way in. It goes
through Test-UnslothCmdShimFile now.
* The reset-password hint always advertised `-I -m unsloth_cli` on Windows. -I
implies -s, so a pip install --user install was handed a command that cannot
find its own package, and the person reading it is by definition already
locked out. It now checks whether the package is inside the interpreter's
prefix and otherwise prints the bootstrap unsloth_cli/__main__.py documents
for exactly this case, which carries no double quote and so wraps identically
for cmd and PowerShell.
Tests: root inference through a validated .cmd with four rejected impostors, an
oversized shim, POSIX unchanged; the PATH gate as a source contract; and a new
studio/backend/tests/test_reset_password_command.py covering both interpreter
shapes, the spaced-path fallback, the prefix check, and drift between the
bootstrap here and _WINDOWS_CLI_ENTRYPOINT.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
---------
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: danielhanchen <danielhanchen@users.noreply.github.com>
Co-authored-by: danielhanchen <unslothshared@gmail.com>
629 lines
28 KiB
Python
629 lines
28 KiB
Python
# SPDX-License-Identifier: AGPL-3.0-only
|
|
# Copyright 2026-present the Unsloth AI Inc. team. All rights reserved.
|
|
|
|
"""setup.ps1 must report the AMD GPU on a host with exactly one AMD adapter.
|
|
|
|
`$wmiGpus = if (...) { $healthyGpus } else { $amdGpus }` unrolled a one-element branch into a bare
|
|
WMI object, which has no .Count in PS 5.1, so the guard after it never fired: setup printed "gpu
|
|
none (chat-only / GGUF)" while install.ps1 had just resolved the same GPU, then expected cpu torch
|
|
against the ROCm wheels the installer placed, called the venv stale and exited, the installer rolled
|
|
back, and the desktop app retried forever. Same expression one block down, `$gpuNames`, wraps each
|
|
BRANCH but not the if, so a lone adapter name unrolls to a String and `$gpuNames[$nameIdx]` yields
|
|
"A"; the `$nameArches[0]` rescue hides that unless a visible-device mask is set.
|
|
|
|
Why the assertions look the way they do: only PowerShell's optimized member-binding path carries the
|
|
PSv3 scalar Count fallback, and custom-adapter types (CimInstance, ManagementObject, COM,
|
|
PSCustomObject) take the other one, which returned null until PowerShell/PowerShell#5745 shipped in
|
|
6.1 -- never backported to 5.1. So under pwsh `.Count` answers 1 and the bug is INVISIBLE; asserting
|
|
on it would pass against the unfixed source. Every runtime case asserts the SHAPE of the value,
|
|
which is identical on both engines, and `ps51` re-runs the same block against stubs carrying an
|
|
explicit `Count = $null` to reproduce 5.1's consequence rather than only its cause.
|
|
"""
|
|
|
|
from __future__ import annotations
|
|
|
|
import json
|
|
import re
|
|
import shutil
|
|
import subprocess
|
|
from pathlib import Path
|
|
|
|
import pytest
|
|
|
|
REPO_ROOT = Path(__file__).resolve().parents[1]
|
|
INSTALL_PS1 = REPO_ROOT / "install.ps1"
|
|
SETUP_PS1 = REPO_ROOT / "studio" / "setup.ps1"
|
|
|
|
requires_pwsh = pytest.mark.skipif(shutil.which("pwsh") is None, reason = "PowerShell is unavailable")
|
|
|
|
_RADEON = "AMD Radeon(TM) 8060S Graphics" # Strix Halo iGPU -> gfx1151
|
|
_RX9070 = "AMD Radeon RX 9070 XT" # RDNA 4 discrete -> gfx1201
|
|
_R780M = "AMD Radeon 780M Graphics" # Phoenix iGPU -> gfx1103, a shadowing arch
|
|
_R9700 = "AMD Radeon AI PRO R9700" # RDNA 4 workstation -> gfx1201 (#7624, #7307)
|
|
_ARC = "Intel(R) Arc(TM) A770 Graphics"
|
|
|
|
HANDOFF = "_UNSLOTH_ROCM_GFX_ARCH_HANDOFF"
|
|
|
|
|
|
# ── extracting the shipped source, so these tests exercise it rather than a copy ──────────────
|
|
|
|
|
|
def _balanced(src: str, start: int, opener: str, closer: str) -> str:
|
|
"""Slice from `start` through the delimiter that closes the first `opener` after it."""
|
|
depth, i = 0, src.index(opener, start)
|
|
while True:
|
|
if src[i] == opener:
|
|
depth += 1
|
|
elif src[i] == closer:
|
|
depth -= 1
|
|
if depth == 0:
|
|
return src[start : i + 1]
|
|
i += 1
|
|
|
|
|
|
def _function(src: str, name: str) -> str:
|
|
return _balanced(src, src.index(f"function {name} {{"), "{", "}")
|
|
|
|
|
|
def _setup_source() -> str:
|
|
return SETUP_PS1.read_text(encoding = "utf-8")
|
|
|
|
|
|
# The two fixes, as (fixed, unfixed) pairs. Undoing them textually is what the red/green check
|
|
# below runs against, rather than an older commit: a revision that is "before the fix" is only
|
|
# before it until this merges, and reaching for it through git also fails in a shallow CI clone.
|
|
# Reverting the wraps in memory keeps the comparison immutable and isolates the one thing under
|
|
# test, since everything else about the two sources is identical by construction.
|
|
_ARRAY_WRAPS = (
|
|
(
|
|
"$wmiGpus = @(if ($healthyGpus.Count -gt 0) { $healthyGpus } else { $amdGpus })",
|
|
"$wmiGpus = if ($healthyGpus.Count -gt 0) { $healthyGpus } else { $amdGpus }",
|
|
),
|
|
(
|
|
"$gpuNames = @(if ($script:ROCmGpuLabels) { @($script:ROCmGpuLabels) } else { @($ROCmGpuLabel) })",
|
|
"$gpuNames = if ($script:ROCmGpuLabels) { @($script:ROCmGpuLabels) } else { @($ROCmGpuLabel) }",
|
|
),
|
|
)
|
|
|
|
|
|
def _without_the_array_wraps(src: str) -> str:
|
|
"""setup.ps1 with only the two `@()` wraps undone: the source as it behaved before the fix."""
|
|
for fixed, unfixed in _ARRAY_WRAPS:
|
|
assert src.count(fixed) == 1, f"expected exactly one occurrence of {fixed!r}"
|
|
src = src.replace(fixed, unfixed)
|
|
return src
|
|
|
|
|
|
def _amd_scan_block(src: str) -> str:
|
|
"""The `if (-not $HasROCm)` WMI fallback: the adapter list the label is read from.
|
|
The report-only peer scan added for #8529 is a SEPARATE block and is deliberately not
|
|
covered here: it feeds no label and no arch."""
|
|
m = re.search(
|
|
r"^ if \(-not \$HasROCm\) \{\n try \{\n.*?^ \}\n",
|
|
src,
|
|
re.DOTALL | re.MULTILINE,
|
|
)
|
|
assert m, "AMD adapter scan block not found in setup.ps1"
|
|
return m.group(0)
|
|
|
|
|
|
def _arch_resolution_block(src: str) -> str:
|
|
"""Everything from the arch-resolution header up to the ROCm version capture that follows."""
|
|
start = src.index(" # ── Arch resolution:")
|
|
end = src.index(" # Capture ROCm version early", start)
|
|
return src[start:end]
|
|
|
|
|
|
def _prelude(src: str) -> str:
|
|
"""The declarations and helpers the two blocks close over."""
|
|
shadowing = _balanced(src, src.index("$script:ShadowingIntegratedGfx = @("), "(", ")")
|
|
arch_family = _balanced(src, src.index("$archFamilyMap = @{"), "{", "}")
|
|
return "\n".join(
|
|
[
|
|
"$script:ShadowingIntegratedGfx = " + shadowing[shadowing.index("@(") :],
|
|
"$archFamilyMap = " + arch_family[arch_family.index("@{") :],
|
|
_function(src, "Test-VisibleDevicesPinned"),
|
|
_function(src, "Resolve-VisibleGpuIndex"),
|
|
_function(src, "Resolve-ShadowingGfxPick"),
|
|
]
|
|
)
|
|
|
|
|
|
# ── the driver: run the shipped blocks against a stubbed adapter list ─────────────────────────
|
|
|
|
|
|
def _driver(
|
|
src: str,
|
|
adapters: list[tuple[str, int]],
|
|
*,
|
|
ps51: bool = False,
|
|
strict: bool = False,
|
|
) -> str:
|
|
"""Wrap the shipped blocks in a Get-CimInstance stub and report the result as JSON.
|
|
|
|
ps51 gives every stub adapter an explicit `Count = $null`, which is what a bare CimInstance
|
|
answers on Windows PowerShell 5.1 and what pwsh would otherwise paper over with 1.
|
|
"""
|
|
count_member = "; Count = $null" if ps51 else ""
|
|
items = ", ".join(
|
|
f"[pscustomobject]@{{ Name = '{name}'; ConfigManagerErrorCode = {code}{count_member} }}"
|
|
for name, code in adapters
|
|
)
|
|
return "\n".join(
|
|
[
|
|
"$ErrorActionPreference = 'Stop'",
|
|
"Set-StrictMode -Version Latest" if strict else "Set-StrictMode -Off",
|
|
f"function Get-CimInstance {{ param([Parameter(ValueFromRemainingArguments = $true)]$Rest) @({items}) }}",
|
|
"function substep { param($a, $b) }",
|
|
"$HasROCm = $false",
|
|
"$ROCmGpuLabel = $null",
|
|
"$script:ROCmGpuLabels = @()",
|
|
"$script:ROCmGfxArch = $null",
|
|
"$script:GpuNamesProbe = $null",
|
|
"$wmiGpus = $null",
|
|
_prelude(src),
|
|
_amd_scan_block(src),
|
|
# Captured from inside the arch block's own scope: $gpuNames is the value the indexing
|
|
# bug corrupts, and its first element is what Get-GfxArchFromGpuName is actually handed.
|
|
_arch_resolution_block(src).replace(
|
|
"$nameIdx = Resolve-VisibleGpuIndex $gpuNames.Count",
|
|
"$script:GpuNamesProbe = $gpuNames\n $nameIdx = Resolve-VisibleGpuIndex $gpuNames.Count",
|
|
),
|
|
# ConvertTo-Json, not string interpolation: a null stays null instead of becoming "".
|
|
"@{",
|
|
" wmi_type = $(if ($null -ne $wmiGpus) { $wmiGpus.GetType().FullName } else { $null })",
|
|
" wmi_array = [bool]($wmiGpus -is [array])",
|
|
" label = $ROCmGpuLabel",
|
|
" labels = @($script:ROCmGpuLabels)",
|
|
" arch = $script:ROCmGfxArch",
|
|
" names_type = $(if ($null -ne $script:GpuNamesProbe) { $script:GpuNamesProbe.GetType().FullName } else { $null })",
|
|
" names_first = $(if ($null -ne $script:GpuNamesProbe) { $script:GpuNamesProbe[0] } else { $null })",
|
|
"} | ConvertTo-Json -Compress",
|
|
]
|
|
)
|
|
|
|
|
|
def _run(
|
|
tmp_path: Path,
|
|
adapters: list[tuple[str, int]],
|
|
*,
|
|
env: dict[str, str] | None = None,
|
|
source: str | None = None,
|
|
ps51: bool = False,
|
|
strict: bool = False,
|
|
) -> dict:
|
|
script = tmp_path / "scan.ps1"
|
|
script.write_text(
|
|
_driver(source or _setup_source(), adapters, ps51 = ps51, strict = strict), encoding = "utf-8"
|
|
)
|
|
# Only what each case names may reach the child: a developer's own exported
|
|
# UNSLOTH_ROCM_GFX_ARCH would otherwise silently win every inference assertion here.
|
|
child_env = {"PATH": "/usr/bin:/bin", "HOME": str(tmp_path)}
|
|
child_env.update(env or {})
|
|
proc = subprocess.run(
|
|
[shutil.which("pwsh") or "pwsh", "-NoProfile", "-NonInteractive", "-File", str(script)],
|
|
capture_output = True,
|
|
text = True,
|
|
timeout = 120,
|
|
env = child_env,
|
|
)
|
|
assert proc.returncode == 0, f"scan block failed:\n{proc.stdout}\n{proc.stderr}"
|
|
return json.loads(proc.stdout)
|
|
|
|
|
|
# ── source assertions ─────────────────────────────────────────────────────────────────────────
|
|
|
|
|
|
def test_scan_wraps_the_whole_if_in_an_array():
|
|
"""The unwrapped form is the bug, so keep it out of the source."""
|
|
block = _amd_scan_block(_setup_source())
|
|
assert "$wmiGpus = @(if (" in block
|
|
assert re.search(r"\$wmiGpus = if \(", block) is None
|
|
|
|
|
|
def test_gpu_name_list_wraps_the_whole_if_in_an_array():
|
|
"""Same expression, one block down: wrapping each branch is not enough."""
|
|
block = _arch_resolution_block(_setup_source())
|
|
assert "$gpuNames = @(if (" in block
|
|
assert re.search(r"\$gpuNames = if \(", block) is None
|
|
|
|
|
|
def test_installer_forwards_the_arch_through_a_private_handoff():
|
|
src = INSTALL_PS1.read_text(encoding = "utf-8")
|
|
forward = src.index(f"$env:{HANDOFF} = $ROCmGfxArch")
|
|
invoke = src.index("Invoke-ManagedUnslothCli -Python $VenvPython -Arguments $studioArgs")
|
|
assert forward < invoke, "the arch must be handed over before setup.ps1 is invoked"
|
|
|
|
|
|
def test_installer_never_exports_the_public_override():
|
|
"""install_llama_prebuilt.py reads UNSLOTH_ROCM_GFX_ARCH back as _manual to decide whether a
|
|
forwarded --rocm-gfx outranks its own probe, so publishing an auto-detected arch there disarms
|
|
that safeguard on exactly the multi-GPU hosts it exists for."""
|
|
src = INSTALL_PS1.read_text(encoding = "utf-8")
|
|
assert re.search(r"\$env:UNSLOTH_ROCM_GFX_ARCH\s*=", src) is None
|
|
|
|
|
|
def test_installer_restores_the_private_handoff_after_setup():
|
|
"""install.ps1 is documented as `irm ... | iex`, so it runs in the caller's own process and a
|
|
value left behind would be read as an override by the next install in that terminal."""
|
|
src = INSTALL_PS1.read_text(encoding = "utf-8")
|
|
assert f"$previousRocmGfxHandoff = $env:{HANDOFF}" in src
|
|
assert f"$env:{HANDOFF} = $previousRocmGfxHandoff" in src
|
|
assert f"Remove-Item Env:{HANDOFF} -ErrorAction SilentlyContinue" in src
|
|
# Saved after the last early return, so no path skips the restore.
|
|
assert src.index("$previousRocmGfxHandoff") > src.index(
|
|
"--with-llama-cpp-dir path does not exist"
|
|
)
|
|
|
|
|
|
def test_setup_consumes_the_handoff_only_after_its_own_inference():
|
|
"""Order is the whole point: the installer takes the first AMD adapter with no mask and no
|
|
shadowing repick, both of which setup applies, so the handoff must never pre-empt it."""
|
|
block = _arch_resolution_block(_setup_source())
|
|
assert block.index("gfx arch inferred from GPU name") < block.index(f"$env:{HANDOFF}")
|
|
|
|
|
|
# ── runtime: the adapter scan ─────────────────────────────────────────────────────────────────
|
|
|
|
|
|
@requires_pwsh
|
|
@pytest.mark.parametrize("ps51", [False, True], ids = ["pwsh", "ps51"])
|
|
@pytest.mark.parametrize("strict", [False, True], ids = ["lax", "strict"])
|
|
def test_single_amd_adapter_is_reported(tmp_path, ps51, strict):
|
|
out = _run(tmp_path, [(_RADEON, 0)], ps51 = ps51, strict = strict)
|
|
assert out["wmi_array"], f"one adapter must stay an array, got {out['wmi_type']}"
|
|
assert out["labels"] == [_RADEON]
|
|
# A label alone still lands on the "AMD ROCm" branch with no arch and installs cpu torch, so
|
|
# "reported" has to mean the name reached the inference.
|
|
assert out["arch"] == "gfx1151"
|
|
assert out["label"] == "AMD ROCm (gfx1151)"
|
|
|
|
|
|
@requires_pwsh
|
|
def test_every_amd_adapter_is_kept_for_shadowing_inference(tmp_path):
|
|
out = _run(tmp_path, [(_R780M, 0), (_RX9070, 0)])
|
|
assert out["labels"] == [_R780M, _RX9070]
|
|
|
|
|
|
@requires_pwsh
|
|
def test_a_parked_adapter_still_reports_when_it_is_the_only_one(tmp_path):
|
|
"""Error code 45 ("not connected") is routine on a muxless laptop, so do not drop the host."""
|
|
out = _run(tmp_path, [(_RX9070, 45)])
|
|
assert out["wmi_array"]
|
|
assert out["labels"] == [_RX9070]
|
|
|
|
|
|
@requires_pwsh
|
|
def test_a_healthy_adapter_wins_over_a_parked_one(tmp_path):
|
|
out = _run(tmp_path, [(_RX9070, 45), (_RADEON, 0)])
|
|
assert out["labels"] == [_RADEON]
|
|
|
|
|
|
@requires_pwsh
|
|
@pytest.mark.parametrize("adapters", [[], [(_ARC, 0)]], ids = ["no_adapters", "intel_only"])
|
|
def test_a_host_with_no_amd_adapter_is_not_read_as_amd(tmp_path, adapters):
|
|
out = _run(tmp_path, adapters)
|
|
assert out["labels"] == []
|
|
assert out["label"] is None
|
|
assert out["arch"] is None
|
|
|
|
|
|
# ── runtime: name inference, where the second unwrapped if bites ──────────────────────────────
|
|
|
|
|
|
@requires_pwsh
|
|
def test_the_adapter_name_reaches_inference_whole(tmp_path):
|
|
"""Unwrapped, $gpuNames is a String and $gpuNames[0] is the character "A"."""
|
|
out = _run(tmp_path, [(_RADEON, 0)])
|
|
assert out["names_first"] == _RADEON, "the name was indexed as a string, not as a list"
|
|
assert "Object[]" in (out["names_type"] or "")
|
|
|
|
|
|
@requires_pwsh
|
|
@pytest.mark.parametrize(
|
|
"name, expected",
|
|
[
|
|
(_RADEON, "gfx1151"),
|
|
(_RX9070, "gfx1201"),
|
|
(_R9700, "gfx1201"),
|
|
("ATI Radeon 9700 PRO", None),
|
|
("AMD Radeon RX 9060 XT", "gfx1200"),
|
|
("AMD Radeon 890M Graphics", "gfx1150"),
|
|
("AMD Radeon 860M Graphics", "gfx1152"),
|
|
("AMD Radeon RX 7900 XTX", "gfx1100"),
|
|
("AMD Radeon RX 7600", "gfx1102"),
|
|
(_R780M, "gfx1103"),
|
|
("AMD Radeon RX 6800 XT", "gfx1030"),
|
|
("AMD Radeon RX 6500 XT", "gfx1034"),
|
|
("AMD Radeon HD 8570", None),
|
|
],
|
|
)
|
|
def test_a_single_adapter_infers_its_arch(tmp_path, name, expected):
|
|
assert _run(tmp_path, [(name, 0)])["arch"] == expected
|
|
|
|
|
|
@requires_pwsh
|
|
@pytest.mark.parametrize("mask", ["0", "1", "", "-1", "not-a-number", "9", " 0 ", "0,1"])
|
|
@pytest.mark.parametrize(
|
|
"var", ["HIP_VISIBLE_DEVICES", "ROCR_VISIBLE_DEVICES", "CUDA_VISIBLE_DEVICES"]
|
|
)
|
|
def test_a_pinned_single_gpu_host_still_infers_its_arch(tmp_path, mask, var):
|
|
"""The mask disables the $nameArches[0] rescue, so the string-indexing bug surfaced here as a
|
|
host with a perfectly good Radeon reporting no arch and looping the installer."""
|
|
assert _run(tmp_path, [(_RADEON, 0)], env = {var: mask})["arch"] == "gfx1151"
|
|
|
|
|
|
@requires_pwsh
|
|
def test_a_discrete_card_is_preferred_over_a_shadowing_igpu(tmp_path):
|
|
out = _run(tmp_path, [(_R780M, 0), (_RX9070, 0)])
|
|
assert out["arch"] == "gfx1201", "the gfx1103 iGPU shadowed the discrete card"
|
|
|
|
|
|
@requires_pwsh
|
|
def test_a_pinned_mask_is_honoured_over_the_shadowing_preference(tmp_path):
|
|
out = _run(tmp_path, [(_R780M, 0), (_RX9070, 0)], env = {"HIP_VISIBLE_DEVICES": "0"})
|
|
assert out["arch"] == "gfx1103", "an explicit selection must never be repicked"
|
|
|
|
|
|
# ── runtime: handoff precedence ───────────────────────────────────────────────────────────────
|
|
|
|
|
|
@requires_pwsh
|
|
def test_the_handoff_fills_the_gap_when_nothing_else_resolves(tmp_path):
|
|
"""The case the handoff exists for: setup's own scan came up empty where the installer's did
|
|
not, and without this the two disagree and the install rolls back."""
|
|
out = _run(tmp_path, [], env = {HANDOFF: "gfx1151"})
|
|
assert out["arch"] == "gfx1151"
|
|
assert out["label"] == "AMD ROCm (gfx1151)"
|
|
|
|
|
|
@requires_pwsh
|
|
def test_the_handoff_never_deposes_setups_own_inference(tmp_path):
|
|
"""install.ps1 would forward the iGPU here: it takes the first AMD adapter, with no shadowing
|
|
repick. Setup's answer is the better one and has to win."""
|
|
out = _run(tmp_path, [(_R780M, 0), (_RX9070, 0)], env = {HANDOFF: "gfx1103"})
|
|
assert out["arch"] == "gfx1201"
|
|
|
|
|
|
@requires_pwsh
|
|
def test_a_user_override_still_wins_over_the_handoff(tmp_path):
|
|
out = _run(
|
|
tmp_path,
|
|
[(_R780M, 0)],
|
|
env = {"UNSLOTH_ROCM_GFX_ARCH": "gfx90a", HANDOFF: "gfx1103"},
|
|
)
|
|
assert out["arch"] == "gfx90a", "the documented operator override outranks an inferred value"
|
|
|
|
|
|
@requires_pwsh
|
|
@pytest.mark.parametrize("value", ["GFX1151", " gfx1151 "])
|
|
def test_the_handoff_is_normalized_like_the_override(tmp_path, value):
|
|
assert _run(tmp_path, [], env = {HANDOFF: value})["arch"] == "gfx1151"
|
|
|
|
|
|
@requires_pwsh
|
|
def test_an_empty_handoff_is_ignored(tmp_path):
|
|
assert _run(tmp_path, [], env = {HANDOFF: ""})["arch"] is None
|
|
|
|
|
|
@requires_pwsh
|
|
@pytest.mark.parametrize(
|
|
"var", ["HIP_VISIBLE_DEVICES", "ROCR_VISIBLE_DEVICES", "CUDA_VISIBLE_DEVICES"]
|
|
)
|
|
def test_a_mask_suppresses_the_handoff(tmp_path, var):
|
|
"""The mask selects the unrecognized discrete card, so the inference above deliberately
|
|
resolves nothing rather than borrowing the 780M's arch. The installer scans without the mask
|
|
and forwards that very arch, and taking it would install for a GPU the mask hides from the
|
|
runtime entirely (ROCR filters below HIP, so masked devices never reach enumeration)."""
|
|
adapters = [(_R780M, 0), ("AMD Radeon RX 5700 XT", 0)]
|
|
assert _run(tmp_path, adapters, env = {var: "1", HANDOFF: "gfx1103"})["arch"] is None
|
|
|
|
|
|
@requires_pwsh
|
|
def test_a_mask_suppresses_the_handoff_even_with_no_adapters_to_check_it_against(tmp_path):
|
|
"""Setup saw no names at all, so it cannot confirm the forwarded arch is the selected device.
|
|
Refuse rather than guess."""
|
|
assert _run(tmp_path, [], env = {"HIP_VISIBLE_DEVICES": "0", HANDOFF: "gfx1151"})["arch"] is None
|
|
|
|
|
|
@requires_pwsh
|
|
def test_a_user_override_is_the_escape_hatch_under_a_mask(tmp_path):
|
|
out = _run(
|
|
tmp_path,
|
|
[(_R780M, 0), ("AMD Radeon RX 5700 XT", 0)],
|
|
env = {"HIP_VISIBLE_DEVICES": "1", "UNSLOTH_ROCM_GFX_ARCH": "gfx1010", HANDOFF: "gfx1103"},
|
|
)
|
|
assert out["arch"] == "gfx1010"
|
|
|
|
|
|
# ── runtime: install.ps1 picks the adapter setup would keep ───────────────────────────────────
|
|
|
|
|
|
def _installer_scan_block() -> str:
|
|
"""install.ps1's own `if (-not $HasROCm)` WMI fallback plus the name table it feeds.
|
|
The report-only peer scan added for #8529 is a SEPARATE block, deliberately outside
|
|
this one: it feeds no label and no arch."""
|
|
src = INSTALL_PS1.read_text(encoding = "utf-8")
|
|
start = src.index(
|
|
" if (-not $HasROCm) {\n try {\n # ConfigManagerErrorCode"
|
|
)
|
|
end = src.index(" # Capture ROCm version for wheel selection", start)
|
|
return src[start:end]
|
|
|
|
|
|
def _run_installer_scan(tmp_path: Path, adapters: list[tuple[str, int]]) -> dict:
|
|
items = ", ".join(
|
|
f"[pscustomobject]@{{ Name = '{name}'; ConfigManagerErrorCode = {code} }}"
|
|
for name, code in adapters
|
|
)
|
|
script = tmp_path / "installer_scan.ps1"
|
|
script.write_text(
|
|
"\n".join(
|
|
[
|
|
"$ErrorActionPreference = 'Stop'",
|
|
# Both names: the routing scan asks WMI (unchanged by #8529), the
|
|
# report-only peer scan asks CIM. Same answer either way here.
|
|
f"function Get-CimInstance {{ param([Parameter(ValueFromRemainingArguments = $true)]$Rest) @({items}) }}",
|
|
f"function Get-WmiObject {{ param([Parameter(ValueFromRemainingArguments = $true)]$Rest) @({items}) }}",
|
|
"function substep { param($a, $b) }",
|
|
"$HasROCm = $false",
|
|
"$ROCmGpuLabel = $null",
|
|
"$ROCmGfxArch = $null",
|
|
_installer_scan_block(),
|
|
"@{ label = $ROCmGpuLabel; arch = $ROCmGfxArch } | ConvertTo-Json -Compress",
|
|
]
|
|
),
|
|
encoding = "utf-8",
|
|
)
|
|
proc = subprocess.run(
|
|
[shutil.which("pwsh") or "pwsh", "-NoProfile", "-NonInteractive", "-File", str(script)],
|
|
capture_output = True,
|
|
text = True,
|
|
timeout = 120,
|
|
env = {"PATH": "/usr/bin:/bin", "HOME": str(tmp_path)},
|
|
)
|
|
assert proc.returncode == 0, f"installer scan failed:\n{proc.stdout}\n{proc.stderr}"
|
|
return json.loads(proc.stdout)
|
|
|
|
|
|
@requires_pwsh
|
|
def test_the_installer_skips_a_disabled_adapter(tmp_path):
|
|
"""A disabled Radeon listed first used to win here, and a mapped arch installs ROCm wheels
|
|
right there, so the dead card got the wheels and the live unsupported one went to nothing.
|
|
Setup filters this adapter out, so forwarding its arch also made the two disagree."""
|
|
out = _run_installer_scan(
|
|
tmp_path, [("AMD Radeon RX 9070 XT", 22), ("AMD Radeon RX 5700 XT", 0)]
|
|
)
|
|
assert out["label"] == "AMD Radeon RX 5700 XT"
|
|
assert out["arch"] is None, "the active card is unsupported, so this host belongs on CPU"
|
|
|
|
|
|
@requires_pwsh
|
|
def test_the_installer_keeps_a_parked_adapter_when_it_is_the_only_one(tmp_path):
|
|
"""Same fallback setup makes: code 45 is routine on a muxless laptop, and with no healthy
|
|
peer there is nothing to prefer."""
|
|
out = _run_installer_scan(tmp_path, [("AMD Radeon RX 9070 XT", 45)])
|
|
assert out["arch"] == "gfx1201"
|
|
|
|
|
|
@requires_pwsh
|
|
def test_a_lone_r9700_is_detected_by_both_scans(tmp_path):
|
|
"""Reported on PR #8398: single R9700 on Windows 11, not detected. One healthy adapter
|
|
and no HIP SDK, so the name is the only evidence left, and it holds neither "9070" nor
|
|
"9080". Both scans must reach gfx1201 (#7624, #7307) or the install lands on CPU torch."""
|
|
assert _run(tmp_path, [(_R9700, 0)])["arch"] == "gfx1201"
|
|
assert _run_installer_scan(tmp_path, [(_R9700, 0)])["arch"] == "gfx1201"
|
|
|
|
|
|
@requires_pwsh
|
|
def test_the_installer_and_setup_agree_on_which_adapter_is_active(tmp_path):
|
|
"""The handoff is only sound because both scans start from the same healthy set."""
|
|
adapters = [("AMD Radeon RX 9070 XT", 22), (_RADEON, 0)]
|
|
setup = _run(tmp_path, adapters)
|
|
assert setup["labels"] == [_RADEON], "setup discards the disabled card"
|
|
assert _run_installer_scan(tmp_path, adapters)["arch"] == setup["arch"] == "gfx1151"
|
|
|
|
|
|
# ── runtime: install.ps1 leaves the caller's environment as it found it ───────────────────────
|
|
|
|
|
|
def _handoff_lifecycle_block() -> str:
|
|
"""install.ps1's save / set / try / finally around the setup call, as shipped."""
|
|
src = INSTALL_PS1.read_text(encoding = "utf-8")
|
|
start = src.index(" $previousRocmGfxHandoff = $env:")
|
|
end = src.index(" if ($setupExit -ne 0) {", start)
|
|
return src[start:end]
|
|
|
|
|
|
def _run_handoff_lifecycle(
|
|
tmp_path: Path, *, arch: str | None, inherited: str | None, fails: bool
|
|
) -> dict:
|
|
body = _handoff_lifecycle_block().replace(
|
|
"Invoke-ManagedUnslothCli -Python $VenvPython -Arguments $studioArgs",
|
|
"throw 'setup exploded'"
|
|
if fails
|
|
else "$script:SeenByChild = $env:_UNSLOTH_ROCM_GFX_ARCH_HANDOFF",
|
|
)
|
|
script = tmp_path / "handoff.ps1"
|
|
script.write_text(
|
|
"\n".join(
|
|
[
|
|
"$ErrorActionPreference = 'Stop'",
|
|
# Not under test, but the shipped finally restores these too and needs them bound.
|
|
"$previousUnslothStudioHome = $null; $hadPreviousUnslothStudioHome = $false",
|
|
"$previousTauriMode = $null; $hadPreviousTauriMode = $false",
|
|
"$previousSetupRuntimeGateHandoff = $null; $hadPreviousSetupRuntimeGateHandoff = $false",
|
|
"$previousProxyHandoff = $null; $hadPreviousProxyHandoff = $false",
|
|
"$UnslothProxyHandoffJson = $null",
|
|
"$UnslothExe = 'stub'; $studioArgs = @(); $setupExit = 0",
|
|
"$script:SeenByChild = '<never ran>'",
|
|
"$ROCmGfxArch = " + ("$null" if arch is None else f"'{arch}'"),
|
|
"try {",
|
|
body,
|
|
"} catch { }",
|
|
"@{",
|
|
" seen_by_child = $script:SeenByChild",
|
|
" after = $(if (Test-Path Env:_UNSLOTH_ROCM_GFX_ARCH_HANDOFF) { $env:_UNSLOTH_ROCM_GFX_ARCH_HANDOFF } else { $null })",
|
|
" after_set = [bool](Test-Path Env:_UNSLOTH_ROCM_GFX_ARCH_HANDOFF)",
|
|
" public = $(if (Test-Path Env:UNSLOTH_ROCM_GFX_ARCH) { $env:UNSLOTH_ROCM_GFX_ARCH } else { $null })",
|
|
"} | ConvertTo-Json -Compress",
|
|
]
|
|
),
|
|
encoding = "utf-8",
|
|
)
|
|
env = {"PATH": "/usr/bin:/bin", "HOME": str(tmp_path), "UNSLOTH_ROCM_GFX_ARCH": "gfx90a"}
|
|
if inherited is not None:
|
|
env[HANDOFF] = inherited
|
|
proc = subprocess.run(
|
|
[shutil.which("pwsh") or "pwsh", "-NoProfile", "-NonInteractive", "-File", str(script)],
|
|
capture_output = True,
|
|
text = True,
|
|
timeout = 120,
|
|
env = env,
|
|
)
|
|
assert proc.returncode == 0, f"handoff block failed:\n{proc.stdout}\n{proc.stderr}"
|
|
return json.loads(proc.stdout)
|
|
|
|
|
|
@requires_pwsh
|
|
@pytest.mark.parametrize("fails", [False, True], ids = ["setup_ok", "setup_throws"])
|
|
@pytest.mark.parametrize(
|
|
"arch, inherited",
|
|
[(None, None), ("gfx1151", None), (None, "gfx1030"), ("gfx1151", "gfx1030")],
|
|
ids = ["nothing", "resolved", "inherited", "resolved_over_inherited"],
|
|
)
|
|
def test_the_caller_environment_survives_the_setup_call(tmp_path, arch, inherited, fails):
|
|
"""`irm ... | iex` runs install.ps1 in the caller's own shell, so anything set for the child
|
|
has to be put back -- on the failure path too, which is the one that rolls back and retries."""
|
|
out = _run_handoff_lifecycle(tmp_path, arch = arch, inherited = inherited, fails = fails)
|
|
assert out["after_set"] is (inherited is not None), "the handoff outlived the setup call"
|
|
assert out["after"] == inherited
|
|
assert out["public"] == "gfx90a", "a user's own override must come back untouched"
|
|
|
|
|
|
@requires_pwsh
|
|
@pytest.mark.parametrize(
|
|
"arch, inherited, expected",
|
|
[("gfx1151", None, "gfx1151"), ("gfx1151", "gfx1030", "gfx1151"), (None, "gfx1030", None)],
|
|
ids = ["resolved", "resolved_over_inherited", "stale_only"],
|
|
)
|
|
def test_only_this_runs_arch_is_handed_to_the_child(tmp_path, arch, inherited, expected):
|
|
"""A value inherited from an outer process is not this run's answer, so it is cleared rather
|
|
than forwarded as though the scan had produced it."""
|
|
out = _run_handoff_lifecycle(tmp_path, arch = arch, inherited = inherited, fails = False)
|
|
assert out["seen_by_child"] == expected
|
|
|
|
|
|
# ── the guard on the guards ───────────────────────────────────────────────────────────────────
|
|
|
|
|
|
@requires_pwsh
|
|
def test_these_assertions_fail_without_the_array_wraps(tmp_path):
|
|
"""A regression test that passes on the unfixed source is not one, and the first version of
|
|
this file was exactly that. Undo just the two wraps and confirm the failures come back."""
|
|
unfixed = _without_the_array_wraps(_setup_source())
|
|
before = _run(tmp_path, [(_RADEON, 0)], source = unfixed, ps51 = True)
|
|
pinned = _run(tmp_path, [(_RADEON, 0)], source = unfixed, env = {"HIP_VISIBLE_DEVICES": "0"})
|
|
assert not before["wmi_array"], "the unwrapped scan should collapse to a scalar"
|
|
assert before["label"] is None, "the unwrapped scan should report no GPU under 5.1 semantics"
|
|
assert pinned["arch"] is None, "the unwrapped name list should infer nothing when pinned"
|