2 commits

Author	SHA1	Message	Date
Michael Han	ebfefcf84e	Windows: do not abort setup on an unreadable llama.cpp install (#7735) ... * Windows: do not abort setup on an unreadable llama.cpp install Test-Path raises UnauthorizedAccessException instead of returning $false when an ACL denies the probe. setup.ps1 runs under $ErrorActionPreference = "Stop", so the bare probe of UNSLOTH_PREBUILT_INFO.json in the llama.cpp prebuilt phase killed setup with a raw "Test-Path : Access is denied" and exit code 1. The desktop app had nothing but [TAURI:ERROR_DEFAULT] to fall back on, so it showed "unsloth studio setup failed (exit code 1)". ~/.unsloth/llama.cpp sits beside the app, not inside it, so reinstalling reused the unreadable folder and hit the same line again, including a reinstall to a different drive. Add Get-PathState (Present / Absent / Denied) plus Test-PathQuiet, route the probes that read inside install trees we do not own through them, and report a denied llama.cpp install through Exit-SetupFailure so the reason and the recovery steps reach the desktop UI. Reported in unsloth-test/unsloth-test#9 * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Stop on every denied path, and split the recovery commands Review follow-ups: - Assert-StudioOwnedOrAbsent treated a denied root as absent and returned, so the caller could go on to replace a tree it cannot read. Probe the root three-state and stop on Denied, still gated on $StudioHomeIsCustom so default-home behaviour is unchanged. - The source-build .git probe treated a denied checkout as "no checkout" and cloned a replacement. The swap that follows recursively removes the original and moves the temp tree over it under "Continue" and unchecked, so a denied child could leave a half-deleted install. Stop instead. This path already treated denied as absent before the previous commit (that probe runs under "Continue", so it printed an error and took the false branch), so the hazard is older than this branch, but it is in scope for the same reason. - Probe $LlamaCppDir itself three-state, so an unreadable parent is reported rather than dying on the bare probe under "Stop". - takeown and icacls were printed joined by "then", which is not a PowerShell separator: takeown would swallow the rest as arguments and icacls would never run. Print them on separate lines. Fold the repeated guidance into Exit-PathAccessDenied so all five denial routes report the same thing. * Harden the denial reporting path, found by simulation Ran the real decision blocks against simulated filesystems (denied file, denied parent, traverse-only and list-only dirs, symlinks, dangling links, wildcard and unicode paths, 3000 random paths) plus PSScriptAnalyzer's 5.1/6.2/7.0 syntax check. Two things came out of it: - Get-PathDenialDetail threw a parameter-binding exception on an empty path. It runs while a failure is being reported, so it would have replaced the actionable message with a raw binding error at exactly the wrong moment. Null and empty are now accepted and return no detail. - The link-target lookup used an empty catch, which PSScriptAnalyzer flags and which hid the intent. It assigns $null explicitly now. Both are covered by new checks. Also promoted the strongest invariant from the simulation into the suite: Get-PathState must agree with a bare Test-Path on every probe that did not throw, and Denied may only appear where the old probe threw, so no path that worked before can take a different branch now. Verified: PSUseCompatibleSyntax reports nothing for 5.1, 6.2 and 7.0; the Python contract tests pass on 3.10 through 3.13 in separate uv venvs; the tauri install:: unit tests pass (17), which is the code that prefers the [TAURI:ERROR] line over the generic exit-code message. * Trigger the Windows PowerShell tests when they change studio-windows-inference-smoke.yml runs six PowerShell unit tests out of tests/studio, but its pull_request paths filter matched none of them, and no other workflow runs them. A PR touching only one of those tests never ran it. Five predate this branch; the sixth is the ACL test added here. Scope the filter to tests/studio/*.ps1 rather than tests/studio/**, so a python-only change under that directory does not pull in the GGUF smoke jobs. This matches what the other two workflows already do: parity-ci lists its .ps1 test outright and update-smoke uses a scoped glob. Guard it in test_ci_shell_suite_coverage.py, which exists for this exact failure (tests/sh had the same hole): every tests/*.ps1 a workflow invokes must be matched by that workflow's paths filter, and must exist. The GitHub glob matcher it needs has its own table-driven test, since a wrong matcher would make the guard pass on everything. Verified by reverting the one-line filter change: the guard then names all six unrun tests. * Make the Windows PowerShell test step fail when a test fails Verifying the path-filter fix turned up a second hole in the same step. A `shell: pwsh` step inherits only the LAST command's exit code, and this step ran five tests as five bare commands, so only the last one could fail the build. test_resolve_cuda_toolkit.ps1 has been printing FAIL exits non-zero (scenario 2, forced source build) FAIL exits non-zero (scenario 6, no toolkit, forced) 2 check(s) FAILED on every Windows run, exiting 1, and the job reported success. Confirmed on main (run 30723608191, sha c67410a7), so it predates this branch, and it reproduces locally. The cause is in the test, not the installer. Resolve-CudaToolkit -RequireOrExit leaves through Exit-SetupFailure, which the child harness never stubbed, so under ErrorActionPreference=Continue the call was an ignored command-not-found, the child fell through and exited 0. The harness already injects the real Resolve-CudaToolkit and Write-CudaDriverToolkitMismatch by AST, so inject the real Exit-SetupFailure the same way. That test now passes 25/25. With it green, add the exit-code checks after each invocation, matching what studio-windows-update-smoke.yml already does, and guard the pattern in test_ci_shell_suite_coverage.py: any step running more than one PowerShell test must check $LASTEXITCODE after each. Verified by reverting each piece: dropping one check makes the guard name that test, and dropping the Exit-SetupFailure injection brings both scenario failures straight back. * Stop on a denied --with-llama-cpp-dir instead of reinstalling over it When UNSLOTH_LOCAL_LLAMA_CPP_DIR points at the canonical $LlamaCppDir and the llama-server.exe there is ACL-denied, Test-PathQuiet collapsed Denied to $false, $LocalLlamaServerFound stayed false, and the canonical branch reported "nothing built there yet; running the normal install". The prebuilt installer then moves that tree aside and replaces it, which is exactly what the branch's own comment says it exists to prevent. The old bare probe stopped first, by throwing under "Stop". Probe the candidates three-state and stop on Denied. Same for the directory probe itself, which reported an unreadable dir as "does not exist" and sent the user after the wrong problem. The generic message could not be reused as-is here: it tells the user to delete the folder because Unsloth reinstalls it, which is true of the managed cache and wrong for a build they pointed us at. Exit-PathAccessDenied takes -UserSupplied, which swaps that advice for restoring access or repointing UNSLOTH_LOCAL_LLAMA_CPP_DIR, and keeps the takeown/icacls lines. Verified by driving the real block through every state: a readable build is still reused, a genuinely empty canonical dir still falls through to the normal install, a missing dir still reports "does not exist", and a denied build now stops with exit 1 instead of being replaced. Reverting the probe puts the fall-through back, and the user-supplied path never prints "delete or rename" or "managed cache". * Carry the denial through three more probes Three review points, all reproduced before fixing: - The canonical --with-llama-cpp-dir override still got the managed advice ("delete it, Unsloth reinstalls it"). The override means the user asked to reuse whatever is in that tree, so deleting it is wrong wherever it sits. Both candidate denials now pass -UserSupplied, which collapses the branch to one call. - Phase 1b's git prerequisite scan probes the same candidate binaries with a bare Test-Path under "Stop", thousands of lines before the Phase 4 guards, so a denied override terminated the run with the raw error this change exists to replace. Reproduced, then guarded. - Test-StudioOwnedAdoptable collapsed a denied prebuilt marker to $false, so Assert-StudioOwnedOrAbsent called an Unsloth tree an unrelated directory and told the user to move it aside. Get-StudioAdoptableState returns Yes/No/Denied and the guard reports the denial first; Test-StudioOwnedAdoptable stays as the boolean view for the cosmetic cleanup gate. A denied file under a readable directory is a Windows-ACL-only state: POSIX keeps a mode-000 file stat-able, and a symlink into a denied directory still answers Test-Path. The local run injects that one state at the lowest seam and lets the real functions run; the Windows leg of test_path_probe_access_denied.ps1 builds it for real with icacls and skips elsewhere with the reason. test_setup_ps1_adopts_existing_whisper_prebuilt_marker sliced between two function names and the marker scan moved, so its anchor now points at Get-StudioAdoptableState. Its assertion is unchanged. Reverting each fix individually puts the original behaviour back: the ownership misdiagnosis, the raw "Access to the path ... is denied" from Phase 1b, and the delete-your-own-build advice. * Close the remaining denial gaps and the stale probe anchors for PR #7735 - tests/sh/test_with_llama_cpp_dir_flag.sh anchored the literal 'if ($ResolvedLocal -eq $LlamaCppDir) {', which 2a61343 hoisted into $LocalIsCanonical. Re-pin it to the comparison, not the branch. - The junction path deleted and replaced $LlamaCppDir behind a bare Test-Path, so a denied destination still threw raw under a default home. Probe it three-state, and treat Denied as surviving removal. - Get-Content on the prebuilt metadata still globbed while the probes gating it went literal, so a path holding [ or ] passed the probe and threw into the catch. Make both reads literal, with a test. - Get-PathDenialDetail could throw on a non-filesystem provider item whose .Attributes has no -band overload, replacing the failure being reported. - Win32Exception keeps E_FAIL in HResult and the code in NativeErrorCode, so the HRESULT check never matched it. Fix the comment and the check. - test_windows_git_gate.py ran the layout scan in a child that never defined Get-PathState, so de486ff had it silently report nothing built. Inject the real helpers, as test_resolve_cuda_toolkit.ps1 does. - Relax the exact Exit-PathAccessDenied count to a floor and key the -UserSupplied rule on the path reported rather than on position. * Stop advising deletion of a tree whose ownership cannot be read The ownership guard stops precisely because it could not read the marker, so it cannot claim the folder is ours either. It was still emitting the managed-cache text, telling the user to delete it. Eleven lines below, the readable-but-unowned branch says 'move it aside' instead, so we were being gentler when we could prove the tree was not ours than when we could not read it at all. New -OwnershipUnverified wording for those three stops. Also from re-reading the previous commit: - The reparse-point unlink above the junction path ran before the new three-state probe, and a link reports Present, so the probe could not cover it. A denied unlink still terminated on the raw .Delete() throw. - That junction destination probe had no test at all; reverting it left the suite green, since the count floor cannot see a swap. Pinned by name like every other route. - The re-pinned shell anchor accepted an assignment that nothing consumed. Pin the branch that uses it too. - Get-PathDenialDetail now checks the item type rather than the attribute type, which also covers a provider item with no Attributes at all. * Check both destructive steps of the temp-dir swap The guard above the clone path probes only .git, but its own comment names any unreadable child as the risk. A forced source build over a non-git install with a denied child elsewhere reads Absent, clones into a temp dir, and reaches the swap. Both steps there are non-terminating under Continue and neither was checked. Reproduced: Remove-Item partially fails, the original survives, and Move-Item then moves the temp dir INSIDE it, so the new binary lands at llama.cpp/llama.cpp.build.<pid>/llama-server.exe while $LlamaServerBin points at llama.cpp/build/bin/Release. Setup carries on reporting success with no usable server and a half-deleted install. Check the removal before moving, so the stop happens while the temp build is still whole, and check the move afterwards. Denied routes through Exit-PathAccessDenied; anything else surviving exits 3 like the other blocked-replacement paths. * [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@gmail.com>	2026-08-02 07:25:39 -07:00
Daniel Han	9bfa18cdb0	Windows: unblock the consumer install on clean and no-winget machines (#7549) ... * Windows: unblock the consumer install on clean and no-winget machines Four independent things stop a clean Windows box today. git was a hard Exit-SetupFailure in setup.ps1, justified as required by pip for git+https:// deps and by npm. Neither holds on the consumer path: the unsloth-zoo git+https URL is only used under STUDIO_LOCAL_INSTALL, node is a pinned nodejs.org prebuilt that never touches system npm, and the frontend lockfile has no VCS dependencies. It stays fatal for --local, where it really is needed. Ensure-VCRedist was winget-only, so on hosts without winget (LTSC, Server, managed corporate images) it silently did nothing while the install reported success, and torch then failed to import on a missing VCRUNTIME140.dll. Adds a direct aka.ms/vs/17/release/vc_redist.<arch>.exe download with /quiet /norestart, accepting exit codes 0 and 3010. The redistributable stays required: it is the runtime the prebuilt llama-server and torch link against, not the MSVC compiler, which is already detection-only. Windows on ARM has no PyTorch at all. Measured with uv against download.pytorch.org/whl/cpu and PyPI for aarch64-pc-windows-msvc / cp313: torch, torchvision and torchaudio all resolve to nothing, wheels exist only for win_amd64 and the manylinux targets. The installer burned three uv retries on an unsatisfiable resolution and reported a bare 'Failed to install PyTorch (exit code 1)'. Now it says what is actually wrong and points at --no-torch, which works because llama.cpp does publish windows-arm64-cpu. install_node_prebuilt.py hit '[WinError 5] Access is denied' on os.replace of the freshly extracted directory during a FRESH install, which is a scanner or indexer holding handles for a moment. Retries only winerror 5, 32 and 145 with capped exponential backoff; any other OSError still raises immediately. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Give the ARM64 dead end a recovery that works for web installs The only remedy printed was .\install.ps1 --no-torch, but the documented path is irm | iex, where no file exists and flags cannot be forwarded. Name the env var the script already honours at line 145. * Windows on ARM: drop torchaudio, do not abort the install The fail-fast was based on a wrong premise. Counted against download.pytorch.org/whl/cpu: torch has 42 win_arm64 wheels and torchvision 60; only torchaudio has none. PyTorch has shipped Arm-native Windows builds since April 2025, so aborting blocked a platform that mostly works. Drop the one unsatisfiable pin instead. Decide from the interpreter uv will resolve for, not the PowerShell host: an x64 CPython under emulation gets working win_amd64 wheels on an ARM64 box, and powershell.exe inherits PROCESSOR_ARCHITECTURE from its parent. * Carry the ARM64 torchaudio omission into studio setup Dropping it from the first PyTorch command was not enough: install.ps1 then runs studio setup with SKIP_STUDIO_BASE=1 and setup.ps1 reinstalls the bare trio from the CPU index, so the ARM64 path still aborted. Apply the same interpreter-based test there. An unreadable platform keeps the full trio. * Build the torch spec list outside the verbose branch The ARM64 guard landed inside `if ($script:UnslothVerbose)`, so on the default path $_torchTrio was never assigned and the splat expanded to nothing: uv ran as `uv pip install --index-url ...` with no package, exit 2, straight to Exit-SetupFailure. That broke the ordinary Windows install. Hoist it above the branch and use substep, which prints on both paths. Realign the two parity guards to the splat form; they asserted the pre-refactor literal command and were the actual cause of the red parity legs. Both halves are still checked: the bounded list is built, and it reaches the install. * Tighten the comments on the Windows install path * Windows install: honour the ARM64 torchaudio skip everywhere and keep git for source builds Hoist the venv-interpreter platform probe above every torch branch in studio/setup.ps1 so the win_arm64 torchaudio omission applies to the ROCm, CPU and CUDA/custom paths. A pinned index whose leaf is not cpu routed an ARM64 host into the CUDA/custom branch, which still asked for torchaudio. Require git again when a llama.cpp source build is opted into up front (UNSLOTH_LLAMA_FORCE_COMPILE, UNSLOTH_LLAMA_PR / PR_FORCE, a non-upstream source). Those paths git clone in phase 4, so setup used to report git as not required, install the build toolchain, then fail at the clone. A local llama.cpp dir overrides them, and the automatic source fallback after a failed prebuilt download stays non-fatal. Also tighten the comments across the changed install paths. * Install the x64 VC++ runtime unconditionally in the direct-download fallback The winget branch always installs Microsoft.VCRedist.2015+.x64, but the direct-download fallback picked the package from PROCESSOR_ARCHITECTURE, which reports the architecture of the running PowerShell process rather than the interpreter that will load the DLLs. Find-CompatiblePython in install.ps1 selects an interpreter on version and non-Conda status alone, with no architecture predicate, so a native ARM64 shell can settle on an emulated x64 Python whose win_amd64 torch and prebuilt llama-server need the x64 runtime, while the fallback had just installed the ARM64-only package. Ensure-VCRedist also runs well before the venv exists, so the interpreter cannot be probed at that point. Microsoft ships the x64 redistributable as an Arm64X superset that carries both ARM64 and x64 binaries, so it is correct on both machines and the manual instruction printed on failure already pointed at it. * Windows on ARM: prefer an x64 Python interpreter An ARM64 host cannot complete the install with a native ARM64 interpreter. pyarrow, pulled in by unsloth -> datasets, has never published a win_arm64 wheel on any version, and neither has hf-transfer, a direct dependency. Both therefore fall back to a source build: pyarrow dies in scikit-build-core CMake configuration and hf-transfer dies in openssl-sys for want of perl, several minutes into a run that looked healthy. torch and torchvision are not the problem, they have win_arm64 wheels and install fine. Windows 11 on ARM runs x64 binaries under emulation and both packages ship win_amd64 wheels, so an x64 interpreter installs cleanly. Find-CompatiblePython accepted an interpreter on version and non-Conda status alone. It now ranks candidates by architecture on ARM64 hosts and returns an x64 one when present, asking each interpreter for its own sysconfig.get_platform() rather than guessing from its path. Host architecture comes from PROCESSOR_ARCHITEW6432 and OSArchitecture as well as PROCESSOR_ARCHITECTURE, which describes only the current process and reads AMD64 in an emulated shell. This is a preference, not a requirement. If only ARM64 is found, x64 is bootstrapped through winget --architecture x64 or the python.org fallback, and if neither works the installer names pyarrow and hf-transfer up front instead of failing later on a CMake or Rust error. The ARM64 torchaudio skip stays live for that path. Non-ARM hosts return on the first match exactly as before, with no extra interpreter probing. * Windows install: three correctness fixes on the ARM64 and git-less paths Ensure-VCRedist never reached its x64 download on an ARM64 machine that already had the arm64 redistributable: Test-VCRedistInstalled accepted System32\vcruntime140_1.dll regardless of architecture, and there that file can be the pure-ARM64 package. An ARM64 PE cannot load into an emulated x64 process, so the x64 Python this branch now prefers would have been left without a usable runtime. The x64 registry entry is the only x64-specific proof, and Microsoft registers Runtimes\{x86|x64|arm64} per architecture, so vc_redist.x64.exe still writes Runtimes\x64 on an ARM64 host and the check cannot loop. The DLL probe stays for x64 hosts. Phase 1 demanded git for any non-blank UNSLOTH_LLAMA_PR_FORCE, but the promotion that actually turns it into a source build requires a positive integer, so PR_FORCE=0 or a non-numeric value aborted a git-less consumer install for a build that never runs. Both sites now use the same predicate. The automatic fallback after a failed prebuilt llama.cpp download reached git clone with no git check anywhere in between, and Invoke-SetupCommand returns 0 for a command-not-found, so a git-less host did not stop there: it continued into an empty directory and reported a cmake configure failure instead. Git is now resolved where the source build is decided, with a last winget attempt, and a missing git degrades exactly like a missing cmake rather than aborting, since the opt-in source triggers already required git in Phase 1. Also tightened the comments across the changed Windows install code, keeping the reasons on the guards that prevent a specific failure. * Rank ARM64 Python candidates by minor version before architecture The x64 preference filtered the whole candidate list on architecture, which outranks the version preference the candidates were collected in. With UNSLOTH_PYTHON=3.12 on a Windows ARM64 box holding an ARM64 3.12 and an x64 3.13, it returned the x64 3.13: the explicit pin was silently broken, and because a x64 interpreter was found the caller never ran Install-X64Python to fetch an x64 3.12. With no pin it was worse still, since an x64 3.11 outranked a newer ARM64 3.13 and defeated the newest-first fallback. Walk $minors in order and take the x64 build of the best minor available, falling back to that minor's ARM64 build so the caller bootstraps x64 for the version actually requested. x64 still wins within a minor, and non-ARM hosts are untouched. * Windows install: see every registered Python, order git before the toolchain Find-CompatiblePython only ever probed `py -3.X`, which runs the launcher's preferred build for that minor. On an ARM64 box that is the native ARM64 interpreter, so a same-minor x64 install that is registered with the launcher but neither preferred nor on PATH never became a candidate. The x64 preference then lost to ARM64, and Install-X64Python re-downloaded an x64 CPython that was already on the machine; when that download is unavailable the install continues on ARM64 and source-builds pyarrow and hf-transfer, which publish no win_arm64 wheels. Enumerate `py -0p` on ARM64 hosts and probe each listed path. The `-3.12-64` suffix cannot be used for this: it has meant "not 32-bit" since 3.11 and does not distinguish arm64 from amd64. studio/setup.ps1 ran Ensure-BuildToolsForLlamaSourceBuild before checking git in Phase 4. That helper calls Exit-SetupFailure when Visual Studio Build Tools cannot be installed, so on a clean no-winget box the git degraded path added by this PR was unreachable and a standalone update aborted instead of finishing in limited mode; where winget does exist it spent a multi-GB Build Tools download on a clone that could never run. Check and install git first, skip the toolchain helper when git is still missing, and report the git branch before the cmake branch so the message names the real cause. _swap_into_place retried the forward rename for about 16 seconds but rolled back with a bare os.replace. A scanner holding the backup for the same WinError 5/32 then left no install_dir at all and stranded the working runtime in .old-*, and its exception replaced the original failure. The rollback now uses the same backoff and logs instead of masking the error it is recovering from. * Installer: use an already installed x64 Python on ARM64 when none can be downloaded Find-CompatiblePython ranks x64 within one minor and returns the native build when that minor is ARM64-only, leaving Install-X64Python to bootstrap x64. On an offline or winget-less box that bootstrap fails, and the retry went through the same resolver, so an x64 build of a lower-priority supported minor already on the machine was never picked up and setup continued on ARM64 Python, where pyarrow and hf-transfer have no wheels. Add an -X64Only mode that returns the best installed x64 interpreter or nothing, and call it as the last resort in Install-X64Python. The version-first preference is unchanged: x64 of the requested minor is still bootstrapped first. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Tighten comments in the Windows ARM64 installer changes * Setup: require Git for a source build behind an unbuilt local llama.cpp dir UNSLOTH_LOCAL_LLAMA_CPP_DIR only overrides the source-build opt-ins once the directory holds a reusable llama-server.exe. Pointing it at the canonical install location with nothing built there falls through to the normal install, so the Phase 1 gate now probes the same layout candidates as the Phase 4 reuse check before dropping the requirement. * Setup: require Git when UNSLOTH_LLAMA_TAG=master forces a source build * Tighten comments in the Windows installer changes * Setup: negotiate TLS 1.2 for the direct VC++ runtime download --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: danielhanchen <unslothai@gmail.com>	2026-07-28 22:24:40 -07:00