mirror of
https://github.com/unslothai/unsloth.git
synced 2026-08-23 15:53:46 +00:00
21296c4274
* Install Studio on a Windows host whose C# compiler cannot run
A first launch of the desktop app on Windows 10 died with
[ERROR] Could not create the Studio install lock: (0) : Die Quelldatei
'C:\Windows\TEMP\05po312x.0.cs' konnte nicht gefunden werden.
(1) : using System;
which is C# compiler error CS2001, not a locking problem. Get-StudioFinalPath
compiles a GetFinalPathNameByHandleW helper with Add-Type, and Windows
PowerShell 5.1 (the interpreter install.rs spawns) compiles by writing the
source into %TEMP% and running csc.exe. When that directory cannot hold a file,
or a scanner eats the source before csc opens it, Add-Type throws, and the
throw travelled up Get-StudioPathHash into Enter-StudioInstallMutex, where it
was reported as a lock failure. install.rs never sets or validates TEMP, so the
installer inherits whatever the app was started with.
Three parts:
Probe TMP and TEMP once with a write, read back and delete, and if the
inherited one cannot hold a file, point both at a per-user directory for the
rest of the run. The compiler is not the only thing that stages through there;
so do the Python, uv and VC++ downloads. It is restored on every exit path.
Split Get-StudioFinalPath into a cached native initializer and a compiler-free
resolver. The initializer skips compiling under Constrained Language Mode,
retries once with a private %TEMP%, and then remembers the answer. The fallback
resolves reparse points component-wise from the root, since a link on a parent
component is the ordinary Windows shape. Callers get an Exact flag so
Test-StudioPathEqual can answer "unknown" rather than "different", which the
runtime lock already reads as "take both locks".
Stop the in-use check failing open. Get-RunningStudioVenvProcesses called the
compiled helper inside catch { continue }, so a host that could not compile
found no running processes and would happily overwrite a venv Studio had open.
It now falls back to the process image path and Win32_Process, and says so.
With the native helper present every output is byte-identical to before: the
install mutex name, the runtime lock names, path identity and the process scan.
Fixes #9140
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Put the private temp directory where uninstall already looks
The fallback temp directory outlives the install on purpose: a Studio started
from it inherits it as its own %TEMP%. That means it has to sit somewhere
scripts/uninstall.ps1 reclaims, and LOCALAPPDATA\UnslothStudio was a folder of
my own invention that nothing would ever clean.
The USERPROFILE candidate was worse than litter. ~\.unsloth is removed only when
it is empty, so a leftover ~\.unsloth\temp would have stopped the uninstaller
clearing the directory at all.
Now LOCALAPPDATA\"Unsloth Studio"\temp, which the uninstaller deletes wholesale
as the data dir, and ~\.unsloth\.cache\temp, which is on its explicit sibling
list. Creating the data dir early cannot be mistaken for an install: the desktop
app decides that from find_unsloth_binary(), not from this directory existing.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Prove the private temp retry recovers, not just that it survives
Surviving a dead compiler is the floor. The retry is supposed to get the native
resolver back, and nothing asserted that it does. Add-Type is stubbed to behave
the way 5.1's CodeDom does, writing the source into %TEMP% and failing while that
cannot hold a file, then calling the real cmdlet once it can, so the type really
is defined on the second attempt.
Pins the whole sequence: one failure then exactly one retry, the type loaded
afterwards, no degraded warning, TMP and TEMP restored to the broken values they
came in with, and the retry directory cleaned up.
* Reclaim a stale junction in the private temp sweep instead of leaking it
Running the sweep on Windows PowerShell 5.1 showed the reparse-point branch
never removing the link: Remove-Item without -Recurse reports the junction
target's contents and refuses as "directory not empty", so a junction left
under the private temp root would sit there forever. Directory.Delete with
recursive:$false removes the reparse point itself and, unlike -Recurse on
5.1, cannot follow it into the target.
* Say what the junction branch actually guards against
Measured on windows-latest under PowerShell 5.1: Remove-Item -Recurse on a
junction removed only the link and left the target alone, while Remove-Item
without -Recurse threw a NullReferenceException that -ErrorAction did not
suppress. The comment claimed the first case as the hazard; the second is
what is reproducible today. Directory.Delete avoids both.
* Make the installer resolver tests hold on Windows too
Running them on windows-latest turned up three things the Linux-only run could
not see. The junction-alias test extracts Get-StudioFinalPath alone, which is a
dispatcher now, so every call in it was undefined and it read as "could not
resolve"; give it the whole resolver chain. The link-shape cases compared a
POSIX fixture against a GetFullPath result, which is drive-rooted and
backslashed on Windows, so compare paths rather than spellings. And utime with
follow_symlinks=False does not exist on Windows, where aging a link any other
way writes through onto the target; skip there and say why.
Resolve-StudioLinkTarget now normalises $Path before the self-reference check,
so a caller passing an unnormalised spelling still trips the guard instead of
being handed a link that points at itself.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Restore the caller's temp on every early exit, and keep a live owner's directory
Four review findings, all reachable through paths this change itself opened.
Two early throws sit before the try/finally that owns the locks: rejecting a
custom root under --tauri, and --shortcuts-only finding no managed Python.
Resolving a path can redirect TMP and TEMP, so under irm | iex both left the
caller's own session pointed at an installer-owned directory. The tauri branch
restores before it throws; the shortcuts-only block is now a try/finally, which
also covers the tauri return inside it.
The stale sweep treated age as proof a directory was unused. A Studio
autostarted by an earlier install inherits one as its %TEMP% and can outlive the
one-day cutoff without writing to it, and the sweep runs before the runtime
mutex is taken. The owner PID is in the name, so a directory whose owner is
still running is left alone.
The in-use scan resolved the venv and each process image through the same
resolver, which is enough while it is exact. Without the native helper an alias
it cannot canonicalize keeps its own spelling while the process reports the
physical one, so the prefix test found nothing and the install would overwrite a
venv Studio had open. When the identity is inexact it now also compares the two
paths below their roots, and fails closed.
* Fold SUBST aliases, fix the UNC device form, record the real temp owner
Three follow-ups, each on a path the earlier round opened rather than closed.
A SUBST drive kept its own spelling on a compiler-blocked host, so one directory
reached as X:\venv and as its physical target produced two different install
mutexes and hid a running Studio from the in-use scan. The Python runtime gate
resolves it, since Path.resolve does. Measured on windows-latest that subst.exe
is the only source available without a compiler: Get-PSDrive.DisplayRoot,
Win32_LogicalDisk.ProviderName, GetFullPath and Resolve-Path all reveal nothing.
Get-StudioLexicalPath now folds the alias before it walks components, so the
identity, the mutex and the scan all agree with the runtime gate.
Stripping the four-character device prefix from \??\UNC\server\share left
UNC\server\share, which reads as relative and was combined with the link's own
parent. It becomes \\server\share instead.
The PID baked into a ust-<pid>- name is the installer's, and the installer is
gone by the time the next sweep runs; the process that keeps using the directory
is the Studio it autostarted. That one is recorded in owner.pid when the
autostart happens, and the sweep prefers it over the name.
* Keep a mounted folder's volume GUID target rooted
Same trap as the UNC device form, in a different shape. A mounted folder
reports its target as \??\Volume{GUID}\..., and after the four-character strip
Volume{...}\... is not rooted either, so it was combined with the link's own
parent and the resulting identity named a directory that does not exist. It
becomes the extended-length spelling \\?\Volume{GUID}\... instead, which is the
same device path and stays the volume it names.
* Keep the volume GUID prefix through final normalization
The rewrite in the resolver was undone one step later. Resolve-StudioFinalPathInfo
strips a leading extended-length prefix, which is right for \\?\C:\x -- that still
names a drive afterwards -- and wrong for \\?\Volume{GUID}\x, which becomes the
unrooted Volume{GUID}\x. That hashes to a different identity than the same
directory reached by its drive letter, and leaves GetPathRoot empty so the
relaxed process comparison cannot run either. The volume GUID branch is tested
before the general one and keeps the prefix.
* Say only what was measured about the volume GUID root
The previous commit gave two reasons for keeping the prefix and one of them is
false. Measured on Windows PowerShell 5.1: GetPathRoot returns empty for both
\\?\Volume{GUID}\x and Volume{GUID}\x, so keeping the prefix does not restore a
root and does not re-enable the relaxed process comparison. The real and only
reason is IsPathRooted, which is true for the extended form and false for the
bare one, so the link resolver stops combining the target with the link's own
parent and inventing a directory that does not exist.
* Drop the root-relative fallback in the in-use scan
It was written to catch an aliased root the lexical resolver could not
canonicalize, and it was too broad to keep. Without the native helper every path
is inexact, so it compared path tails across unrelated drives: an ordinary
D:\env\python.exe matched a protected C:\env and aborted a legitimate install
as "the managed Python environment is still in use". That is every
compiler-blocked host, which is the population this change exists to serve.
The alias it was written for was SUBST, and that is now folded in
Get-StudioLexicalPath instead, which is the right place and costs no false
positives. A volume reached by GUID and the same volume reached by drive letter
still cannot be matched without the compiler, and a tail match is not a safe
price to pay for it.
* Probe TMP whenever Windows would use it
GetTempPath takes the first of TMP/TEMP that is merely non-empty, so a
whitespace-only TMP is the one Windows and every child process resolve through.
IsNullOrWhiteSpace read that as unset, probed a healthy TEMP, found it fine and
returned, leaving the compile and every later Python, uv and VC++ download
pointed at a path that cannot exist. Only an absent or empty TMP falls through
to TEMP now.
* Tighten the comments on the temp and path resolver fallbacks
Collapse the explanatory prose to one line where it still reads clearly and
drop the restatements, keeping every measured Windows PowerShell 5.1 fact the
code is shaped around. No code changes: the PowerShell token stream is
identical with comments and newlines dropped, and the two test files pass the
comments-only AST check.
* Resolve the data dir the same way the installer does when uninstalling
install.ps1 falls back to GetFolderPath("LocalApplicationData") when
LOCALAPPDATA is unset, which is the service and CI case the fallback exists for,
and puts its private temp under "Unsloth Studio\temp" there. uninstall.ps1
derived the default data dir from the variable alone and skipped the removal
when it was absent, so that tree survived an uninstall on exactly those hosts.
It uses _AppDataRoot now, the same resolver the WebView2 profile removal
already used a few lines below.
* Resolve an inherited temp path before probing it, and pin what was probed
A relative TMP or TEMP (temp, or the drive-relative C:temp) is resolved by
whoever reads it, and the install relocates out of a Windows system directory
later, so the value that was probed could afterwards name somewhere else or
nowhere. The absolute form is now pinned into both variables, and the caller's
own spelling is still what the restore hands back.
Resolving happens BEFORE the probe rather than after, because the two halves of
the probe disagree on relative paths: Test-Path is relative to PowerShell's
location while the .NET file APIs are relative to the process working directory,
and Set-Location moves only the first. Probing the absolute path checks the same
directory the writes will use.
The override now records whether the directory is one this run created, so
owner.pid is written only there and never into the host's own temp.
* Do not resolve a whitespace-only temp value before probing it
Resolving it first turns " " or a tab into the working directory plus that
name, which is creatable on some filesystems, so the probe would manufacture a
junk directory and then trust it as the host's temp. Leaving it untouched lets
the probe reject it, which is what sends the install to a private directory.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Resolve a drive-less rooted link target on the link's own drive
A directory symlink can store a target like "\real": rooted as far as
IsPathRooted is concerned, but naming no drive. The compiler-free resolver
therefore skipped anchoring it to the link and handed it to GetFullPath, which
resolves a drive-less path against the PROCESS current drive. A link on D:
could normalize to C:\real, so the fallback mutex and the in-use process scan
would guard a different directory than the one being installed into.
Windows resolves such a target on the link's own volume, so anchor it to
GetPathRoot of the link before normalizing. A volume-GUID spelling has no root
to anchor to (GetPathRoot is empty there, measured on windows-latest), and
those keep today's behaviour rather than a guess.
* Run the Windows guard tests from a file, not a 32 KB command line
The Windows job started failing every test in this file as WinError 206, "The
filename or extension is too long". These scripts embed the whole extracted
helper chain, which has grown past the 32767 character command line cap
Windows enforces, so nothing was being tested on the platform the tests exist
for. Written to a temp .ps1 and run with -File instead, with a BOM so 5.1 does
not read it as ANSI.
test_running_venv_process_is_reported also raced: a six-ping child can exit
before Windows PowerShell 5.1 has finished its cold start and the csc.exe
compile of the native helper, which reads as the in-use scan missing a running
process. Same fix already applied to the 32-bit test: a long-lived child and a
deadline that allows for a slow shell.
* Reclaim both LocalAppData spellings on uninstall
New-StudioPrivateTempDirectory tries $env:LOCALAPPDATA first and falls through
to the LocalApplicationData known folder when that path is set but not usable,
so a non-blank variable does not tell you where "Unsloth Studio\temp" actually
landed. The uninstaller resolved a single root and stopped at the first
non-blank candidate, so on those hosts it removed a directory that was never
used and left the real tree behind.
It now collects both spellings, deduplicated, and stops servers by port file,
adds stop roots and removes the data dir for each. On an ordinary host the two
agree and the list has one entry, which is exactly today's behaviour.
* [pre-commit.ci] auto fixes from pre-commit.com hooks
for more information, see https://pre-commit.ci
* Sweep abandoned private temp directories on a healthy host too
The stale sweep only ran from New-StudioPrivateTempDirectory, which a run with
a usable inherited TMP/TEMP never reaches. So once a host's temp was fixed, by
an ACL correction or a cleaned environment, nothing ever collected what the
degraded runs had left, including the remains of interrupted downloads, and it
sat there until an uninstall.
The candidate roots move into Get-StudioPrivateTempRoots so both paths can
share them, and the healthy path now sweeps each one. A root that does not
exist is a no-op, which is every ordinary host.
* Require the temp probe file to actually be deleted
The usability probe created a file, read it back, then deleted it with the
error suppressed and reported success regardless. A directory that accepts a
file and refuses to give it back, a denied Delete ACE or a scanner sitting on
the handle, therefore passed as usable. That is the shape behind this whole
issue: csc.exe writes its source and its output into the temp directory and
then cleans up, so the private-temp fallback was being skipped for exactly the
hosts that needed it.
The delete is now verified. Retried up to three times first, since a scanner
holding a file for a moment is not the same as a directory that denies
deletion and only the second should cost a healthy host its own temp.
* Run every Windows guard script from a file, and decode its output as utf-8
The command line cap caught the other two spawn sites as well: the Add-Type
fallback tests and the two mutex holder processes still passed their script
inline, so seven tests died as WinError 206 on windows-latest without testing
anything. All of them now go through a temp .ps1 with -File, the same as the
rest of the file.
Output is also decoded as utf-8 with replacement rather than the console
codepage. cp1252 cannot decode what PowerShell writes into an error message,
and the reader thread raised UnicodeDecodeError from inside subprocess.
* [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>
|
||
|---|---|---|
| .. | ||
| data | ||
| build_prequant_checkpoint.py | ||
| build_te_prequant_checkpoint.py | ||
| build_whisper_cpp.sh | ||
| check_frontend_dep_removal.py | ||
| check_new_install_scripts.py | ||
| compare_engines.py | ||
| compile_probe.py | ||
| diffusion_bench.py | ||
| diffusion_quality.py | ||
| enforce_kwargs_spacing.py | ||
| fbcache_flux_probe.py | ||
| fp8_overflow_check.py | ||
| image_speedmem_bench.py | ||
| install_gemma4_mlx.sh | ||
| install_qwen3_6_mlx.sh | ||
| install_rocm_wsl_strixhalo.sh | ||
| int8_linear_probe.py | ||
| leverage_probe.py | ||
| lint_backend_python_floor.py | ||
| lint_no_parallel_clamp.py | ||
| lint_workflow_triggers.py | ||
| lockfile_supply_chain_audit.py | ||
| make_dmg_background.py | ||
| notebook_to_python.py | ||
| notebook_validator.py | ||
| nvfp4_probe.py | ||
| nvfp4_t211_probe.py | ||
| online_tokenization_ab.py | ||
| perf_levers_probe.py | ||
| perf_verify.py | ||
| prequant_probe.py | ||
| profile_startup.py | ||
| quant_probe.py | ||
| run_ruff_format.py | ||
| scan_npm_packages.py | ||
| scan_npm_packages_baseline.json | ||
| scan_packages.py | ||
| scan_packages_baseline.json | ||
| sd_cpp_smoke.py | ||
| sdpa_mask_backend_probe.py | ||
| sparse_accum_probe.py | ||
| stamp_studio_release.py | ||
| sync_allow_scripts_pins.py | ||
| uninstall.ps1 | ||
| uninstall.sh | ||
| verify_comment_only_diff.py | ||
| verify_import_hoist.py | ||
| verify_prequant_backend.py | ||
| video_quality.py | ||
| virustotal_scan.py | ||