mirror of
https://github.com/unslothai/unsloth.git
synced 2026-08-18 05:13:54 +00:00
4 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
|
|
a151ac875c
|
Make install.ps1 work with the user's PowerShell profile loaded (#8161)
* Make install.ps1 work with the user's PowerShell profile loaded Installing from a normal console failed where the same install from a console started with -NoProfile succeeded. A profile runs before `irm https://unsloth.ai/install.ps1 | iex` does and shares its scope, and that entry point has no script file to re-launch without it, so the individual couplings are cut instead. install.ps1, at the top of Install-UnslothStudio: - Set-StrictMode -Off. The script tests environment variables that are legitimately unset and reads $script: state only some branches assign, both of which a profile's `Set-StrictMode -Version Latest` turns into terminating errors. - $PSDefaultParameterValues is filtered down to proxy keys. An entry like 'Start-Process:WindowStyle' silently rebinds cmdlets here and fails the install with an error naming none of it. Proxy entries are kept because they can only ever enable a download, and on a locked-down host may be the only route to python.org and the uv release. - $PSNativeCommandUseErrorActionPreference = $false. With a profile turning it on, the "Stop" preference makes a failing native command throw out of the `unsloth studio setup` handoff instead of reaching Exit-InstallFailure, skipping rollback and the Tauri error record. All three assign without a scope qualifier, so they apply to the installer and everything it calls and leave the caller's session alone. uv is resolved once through Resolve-UvExecutable, which uses `Get-Command uv -CommandType Application -All` and falls back to the bare token when nothing is on PATH. PowerShell ranks aliases and functions above PATH, so a profile `Set-Alias uv ...` was answering the version probe and ending the install at "uv could not be installed" on machines that had a working uv. Test-UvVersionOk pins the executable that answered in $script:UvExe, and the 27 install scriptblocks invoke that path. $script:UvExe and $script:UvInstallDestDir are reset per invocation, since $script: is the caller's session under irm | iex. unsloth_cli/commands/studio.py passes -NoProfile to setup.ps1 unconditionally. It was only added when stdout was not a tty, which is never the case for the console install this fixes, so setup.ps1 ran under the profile with its own bare uv calls exposed. tests/test_installer_profile_hardening.py runs the extracted prologue and uv probe under a hostile profile and checks the caller's session is left intact. The four existing tests that anchored on the literal `uv venv $VenvDir` are re-anchored past the command token. * Plant the real-profile fixture where pwsh actually looks test_a_real_profile_reproduces_the_same_state failed on ubuntu-latest with every probed setting at its default, meaning the planted profile never loaded. It passed here and on macos-14. PowerShell resolves $PROFILE from $XDG_CONFIG_HOME when that is set and only falls back to $HOME/.config when it is not, and GitHub's ubuntu image writes XDG_CONFIG_HOME into /etc/environment. The fixture redirected HOME alone, so on a hosted runner the inherited value went on naming the real account and the profile was written to a path pwsh never opened. Setting XDG_CONFIG_HOME to the same directory HOME already implies reproduces the CI failure exactly on this machine, and removing it makes the failure go away. _hostile_env now redirects XDG_CONFIG_HOME alongside HOME, so the two rules agree whichever one the host applies, and the test asks pwsh for the path instead of hardcoding the fallback branch. The two guards are precise rather than blanket: a machine-wide profile loads into the real leg only, and a $PROFILE that lands outside the fixture cannot be planted into. Neither is reachable on Linux or macOS with the redirect in place. The other tests in the file never shared the premise; every other pwsh launch there passes -NoProfile and dot-sources the profile explicitly. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Close the uv wrapper hole, harden winget the same way, and restore module autoloading Validating the profile hardening against real pwsh turned up three things the first pass missed. Six of the nineteen hostile-profile scenarios I exercised are genuinely broken on main and genuinely fixed by this branch, so the shape of the fix is right -- these are gaps in its coverage, not a change of direction. Resolve-UvExecutable still handed back the bare token when nothing named uv was on PATH. That was meant to keep a working non-Application uv working, but it reopens the exact hole the function exists to close: a profile `function uv { Write-Output "uv 99.0.0" }` clears the version gate, gets pinned into $script:UvExe, and then receives every install command the script runs, with the user's torch, index URL and venv path as arguments. The existing test missed it because its hostile alias reports no version at all, and an alias to a missing file fails loudly. Follow an alias as far as an Application and return that resolved path, since aliasing uv at a specific build is a legitimate thing to do; return $null for anything else, which puts the caller back on its install-uv branch and the gate re-probes against the real thing. winget had the identical defect and was left untouched. It is detected with a bare Get-Command and invoked as a bare token at five sites, and it is what installs both Python and uv -- so a `function winget` wrapper, which people write to inject --accept-* or pin a source, owns the whole bootstrap. Same treatment: resolve once to an Application and invoke through the path. A profile setting $PSModuleAutoLoadingPreference to 'None' is fatal here and was not covered. PowerShell 7 loads no modules at startup, so that one line removes Test-Path, Write-Host, Select-Object, ConvertFrom-Json, Get-FileHash, Invoke-WebRequest, Expand-Archive, Start-Process and Get-Content, and the script dies on its first step naming a cmdlet the reader assumes is always there. Windows PowerShell 5.1 preloads Utility and Management and survives, which is exactly what makes this reproduce on one machine and not another. Also: use [regex]::IsMatch in the defaults filter so it leaves no $Matches behind; add -NoProfile unconditionally in _refresh_desktop_shortcuts, which launches install.ps1 and had it gated on the hidden branch, so the visible console path -- the one where a profile IS loaded -- was the one that missed it; and record that the preserved proxy defaults do not reach setup.ps1, which is launched with -NoProfile, along with why that trade is accepted. Tests: the assertion that the bare token must come back now asserts the opposite, and there are new ones for a convincing uv function, an alias to a real uv, the winget call sites and the autoloading reset. The two new pwsh tests execute against a genuinely planted profile rather than reading source. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Carry the profile proxy across the setup handoff, not just inside install.ps1 install.ps1 deliberately keeps proxy-shaped $PSDefaultParameterValues entries out of the profile table it discards, because on a locked-down corporate host that entry can be the only route out. Adding -NoProfile to the setup launch unconditionally then threw them away one process later, and setup.ps1 downloads on its own: the VC++ runtime through Invoke-WebRequest and the uv installer through Invoke-RestMethod. A PowerShell variable does not cross a process boundary, so the kept entries travel as JSON in _UNSLOTH_PS_PROXY_DEFAULTS and the child re-applies them before running setup.ps1. Nothing else from the profile comes with them. A credential is left behind on purpose: PSCredential does not survive ConvertTo-Json, and the environment is the wrong place for one. A stale variable is cleared when there is nothing to hand off. Three tests, one static and two driving real pwsh, covering the round trip, the credential and non-proxy keys being dropped, and the prelude staying silent when the variable is absent, empty or corrupt. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Harden the proxy handoff: ordering, key casing, uri values, scope, and the standalone update Five follow-ups, all on the handoff added last round. The handoff serializes with ConvertTo-Json, from Microsoft.PowerShell.Utility, and ran before the module-autoloading reset. Under a profile's $PSModuleAutoLoadingPreference = 'None' a fresh PowerShell 7 session therefore died right there, taking out the one configuration the handoff exists to support. The reset moves to the front of the prologue. The key filter was a case-sensitive .NET regex, but cmdlet and parameter names bind case-insensitively, so 'invoke-webrequest:proxy' was dropped. And [uri] is the type the Proxy parameter actually takes, so a careful profile assigns one; the serializer accepted only string and bool, and it disappeared at the process boundary. Both are accepted now, a uri by its AbsoluteUri. A PSCredential is still deliberately left behind. Under "irm ... | iex" the prologue runs in the caller's own session, so writing the environment variable there outlived the install on every path, early returns included, and a later `unsloth studio update` from that console would reapply stale JSON over a proxy that had since changed. The prologue now holds the value and it is published around the setup child only, saved and restored beside the other child-scoped variables. A standalone `unsloth studio update` has no installer above it, so there was nothing to restore and -NoProfile left it with no route out. It now asks: a throwaway PowerShell that does load the profile prints just the proxy-shaped defaults as JSON, validated before use, entirely best effort. Same filter as install.ps1's. Five tests, two driving real pwsh, including one against a profile with strict mode on, autoloading off, a lowercase key and a uri value. * Ask the profile the caller actually has, and follow a uv alias first - the standalone update probed powershell.exe only, so a proxy living in the PowerShell 7 profile never reached the -NoProfile child; both editions are asked now, the caller's first, and their answers merged. - Resolve-UvExecutable checked PATH before the alias, which is the reverse of PowerShell's own resolution and made the alias branch unreachable on any machine with some uv on PATH. - the parity workflow did not run this suite when unsloth_cli/commands/studio.py changed, though the suite asserts that module directly. Its own path-filter parser also treated a comment inside the list as the end of it, which would have hidden the addition. * Give the parity job the imports it needs, and fold proxy keys the way PowerShell does Three tests in the profile-hardening suite import unsloth_cli.commands.studio to drive the profile probe directly, and that pulls typer, pyyaml, pydantic and click. The job installed pip and pytest only, so on a clean setup-python both matrix legs died with ModuleNotFoundError before a single test ran. Installed, with a test that keeps the step in step with what the suite imports. $PSDefaultParameterValues keys are case-insensitive and a Python dict is not, so "Invoke-WebRequest:Proxy" from the caller's own host and "invoke-webrequest:proxy" from the other one both crossed over; the prelude then replayed them in order and the lower-priority host's value landed last, reversing the earlier-host-wins rule this merge exists for. Keys are folded now, first spelling seen wins, within one profile's answer as well as across two. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Frame the proxy record, and quote the union so 3.9 can still import the CLI The probe runs after the profile, and the profile is free to print: a MOTD, a "loading personal and system profiles took 812ms" line, a corporate banner. With the record bare, that arrived ahead of the JSON, the parse threw and the whole answer was dropped -- so the locked-down host that needed the proxy handed the -NoProfile child nothing and every download failed, which is worse than before, since the old visible-console path loaded the profile itself. The record is emitted between two markers now and cut out of whatever else was said. And `str | list[str]` was evaluated at def time in a module with no postponed annotations, so on the 3.9 this project still supports it raised TypeError and took the whole CLI import with it. Quoted, with a test that walks every annotation in the module for an unquoted PEP 604 union. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Let the uv gate move past a stale alias, and decode the probe output lossily An alias pointing at a real but stale uv was the only binary the version gate ever probed, so a current uv already on PATH -- or one winget or the pinned release had just installed -- could not rescue the run and the install ended at "uv could not be installed" on a machine that had one. The resolver hands back every candidate in the order the bare token would pick them, alias first, and the gate walks them until one passes, pinning the one that answered. The profile probe decoded its child with text=True alone, which is the locale codec with STRICT errors. A UTF-8 banner on an ANSI console then raised UnicodeDecodeError, which is neither OSError nor SubprocessError, so it escaped the handler and took the update down before the -NoProfile child ever ran -- and before the framing could discard the banner. UTF-8 with replacement now; the record itself is ASCII. rich is named in the parity job's install line too. It arrives through typer today, but unsloth_cli imports it directly, and this suite's imports should not rest on somebody else's dependency list. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Assert the proxy handoff by parsing it, not by substring CodeQL reads the bare membership test as an incomplete URL sanitization, which is a fair reading of the shape even though this is an assertion on a compressed JSON payload rather than a check on untrusted input. Parsing it and comparing the value exactly is the stronger assertion anyway. * Read the caller edition by order, pin the probe's encoding, claim cmdlets whole A machine can carry both PowerShell module trees on PSModulePath at once, so inferring the caller from the absence of the other edition handed precedence to the wrong profile and let its proxy override the console the command was typed into. Each host puts its own module directory first, so the earliest tree names the caller; neither present keeps the previous order. Windows PowerShell 5.1 writes redirected output in the console code page while this process decodes UTF-8, so a non-ASCII proxy value came back with replacement characters, still parsed as JSON, and handed setup a proxy that does not resolve. The probe pins its own output encoding first. And the merge claims a cmdlet whole rather than filling missing companion parameters from the other profile, which built a configuration neither host had -- one profile's proxy with the other's credential forwarding. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Evaluate script-block proxy defaults, read the caller's host profile, drop the secret A profile can set a dynamic default as a script block, which is PowerShell's supported form and which Invoke-WebRequest evaluates per call. Both serializers dropped it, so the caller downloaded fine and the -NoProfile setup child got no proxy at all. Both now invoke the block and hand over the resulting URI or string; executable code does not cross the handoff. The probe spawns pwsh.exe or powershell.exe, which load the CONSOLEHOST profile. A caller in the VS Code Integrated Console or the ISE keeps its defaults in Microsoft.VSCode_profile.ps1 or Microsoft.PowerShellISE_profile.ps1 instead, so the probe reported no proxy on exactly the host that needed one. It dot-sources the caller's other CurrentUser host profiles, from their own directory, before reading the table. And the prelude clears _UNSLOTH_PS_PROXY_DEFAULTS the moment it has read it. A profile proxy routinely carries credentials, and every native process setup.ps1 starts inherited the environment it was launched with. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Probe only the caller's own host profile, on one shared budget Sourcing every Microsoft.*_profile.ps1 in the profile directory ran profiles belonging to hosts nobody was using: they can overwrite the console's own $PSDefaultParameterValues, have side effects, or exit before the framed record is written. The probe now sources exactly one, named by _UNSLOTH_PS_HOST_PROFILE, and only when the caller identifies itself (VS Code does, via TERM_PROGRAM). A host we cannot name gets no extra profile rather than someone else's. install.ps1 removed the handoff variable when it had no proxy to pass, and its absence is precisely how the CLI recognises a standalone update -- so an installer launch, including one started with -NoProfile or by the desktop app, went and reloaded the profiles it had deliberately discarded. It publishes an explicit empty handoff instead, and the CLI keys on presence. And the probe's timeout is one budget for the whole call rather than one per host, so two installed editions with two hung profiles no longer cost twice the documented best-effort delay before setup starts. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Run the proxy probe with -NoProfile and dot-source the caller's own two Without -NoProfile the probe host loaded its own ConsoleHost profile before the script ran, so an unrelated profile could print, rewrite $PSDefaultParameterValues or exit before the record was written -- and it still was not the profile a VS Code caller keeps its defaults in. The child runs with -NoProfile now and dot-sources exactly the two the caller's session would have loaded: $PROFILE.CurrentUserAllHosts and either the host profile named in _UNSLOTH_PS_HOST_PROFILE or $PROFILE.CurrentUserCurrentHost. $PROFILE is fully populated under -NoProfile, since the paths are computed rather than loaded, so this is exact rather than incidental. Checked against pwsh with a fixture profile directory: a VS Code caller picks up its own profile plus the all-hosts one and never runs the console profile's banner, and a plain console caller picks up the console profile plus the all-hosts one. * Probe the all-users profiles too, and clear profile defaults before emitting A machine-managed proxy commonly lives in AllUsersAllHosts on a domain-joined box while the user's own profile never mentions it, so sourcing only the current-user pair reported no proxy on exactly the host that has one. The probe now walks PowerShell's own startup order, all-users first, so the user's profile still gets the last word. The profile's $PSDefaultParameterValues was also still active when the record was serialized. ConvertTo-Json:AsArray = $true is a legitimate setting and turns the payload into a JSON array, which the reader rejects for not being a dictionary. $out already holds copies by then, so the table is cleared first. * Harden the proxy probe against profile overrides, and drop the handoff copy Five fixes from the review round: install.ps1 kept the serialized proxy defaults in $script:, which under the documented irm | iex path IS the caller's session scope, so an authenticated proxy URI stayed readable in that console after the installer returned. Cleared in the same finally that restores the environment handoff. A profile setting [Console]::OutputEncoding overrode the probe's UTF-8 pin, and the parent decodes that stream as UTF-8, so the framed record could come back corrupted. Re-pinned after the last profile is sourced. The record was emitted through bare Write-Output and ConvertTo-Json, which a profile alias or function shadows; clearing $PSDefaultParameterValues does not cover a command override. Both are module-qualified now. TERM_PROGRAM=vscode is set by every VS Code integrated terminal, not only the PowerShell extension's host, so substituting Microsoft.VSCode_profile.ps1 for the current-host profile missed the proxy a plain pwsh terminal there actually has. The named host profile is added rather than substituted, with the current-host profile last. The per-cmdlet ownership check compared command strings literally, so a wildcard key from one host and a literal key for a matching cmdlet from the other were both merged, which is how one invocation ends up configured from two profiles. Overlap is matched in either direction now. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Hold the proxy handoff in the frame, and treat two wildcards as one family Three fixes from the review round: The serialized handoff lived in $script:, which under the documented irm | iex path is the caller's session scope, and the only cleanup ran after the setup child. Dozens of exits return earlier -- ShortcutsOnly, an argument error, lock contention, a failed dependency install -- so an authenticated proxy URI stayed readable in that console. It is a function-local now, which dies with the frame on every path including a throw. install.ps1 serialized that record through a bare ConvertTo-Json, which a profile alias or function shadows exactly as it does in the probe. Module qualified. The cmdlet-ownership check compared two wildcard patterns as strings, and Invoke-Web* and *-WebRequest both apply to Invoke-WebRequest while neither matches the other. Two patterns are now assumed to overlap: the cost is a second host's unrelated wildcard entry going unmerged, against handing setup a credential setting from a profile that never asked for one. * Tighten the profile-hardening comments Comments, docstrings and whitespace only; no code changes. Each comment keeps the reason it records and drops the retelling. * Cut the profile-hardening comments down again Comments, docstrings and whitespace only. The install.ps1 prologue and the studio.py proxy probe kept one causal claim per decision, with the probe's profile-loading essay split into a short note beside each line it justifies. * Pin the probe's add-both profile order in its own test name * Keep disjoint wildcard proxy families, and give each probed host its own module path * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Co-authored-by: Daniel Han <moonshotaisubstack@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> |
||
|
|
07df95079e
|
Studio: route every Windows installer line through the UTF-8 stdout sink (#8148)
* Studio: route every Windows setup line through the UTF-8 stdout sink The desktop setup log rendered "?? Unsloth Studio Setup" over a rule of replacement characters. Tauri spawns Windows PowerShell 5.1 with CREATE_NO_WINDOW (install.rs), so the [Console]::OutputEncoding setter throws and both entry scripts rebind [Console]::Out to a UTF-8 writer. step/substep already write only through that writer when stdout is redirected, so they came out right. Every other line did not: Write-Host is written by 5.1's console host with its own writer on the OEM code page, and U+1F9A5 has no OEM form while U+2500 becomes a bare 0xC4, which from_utf8_lossy turns into U+FFFD. The banner and the footer are not steps, so they kept arriving as mojibake, and install.ps1 had neither the IsOutputRedirected probe nor a mirror at all. Add Write-StudioLine above the first write in studio/setup.ps1 and install.ps1: console handle when redirected, Write-Host when interactive, since it is the only writer that colorizes. Rewrite 164 call sites in setup.ps1 and 155 in install.ps1 onto it, including install.ps1's own step/substep. Write-Host now survives only inside helpers that have already ruled out the redirected sink, and the launcher script install.ps1 generates keeps its own, since it runs as a separate process. No behaviour change for an interactive console user: same text, same colors, same single record per line. test_windows_setup_output_encoding.py gains byte-level coverage that the real banner and footer, sliced out of setup.ps1, survive both launch shapes as valid UTF-8 exactly once, plus a source contract that runs on Linux and names any file:line that reaches for Write-Host outside the allow-list. Studio.Setup.Output.Tests.ps1 covers Write-StudioLine in both modes and pins install.ps1's copy to setup.ps1's. Harnesses that splice these scripts apart now stub or dot-source Write-StudioLine: two PowerShell harnesses, one Python harness, and the VC++ redist leg of studio-windows-inference-smoke. pytest tests/python tests/test_installer_*.py: 1077 passed (2 pre-existing sandbox failures unrelated to this change). All 16 tests/studio harnesses and 57 Pester cases pass. Both scripts parse clean. * CI: spawn install.ps1 as a child process so its lines reach install.log * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Stub the output sink in the llama.cpp backend PowerShell harness * Guard the console-less spawn on a Windows runner The byte-level cases in this file run with a console attached, and a GitHub runner gives a CREATE_NO_WINDOW child one, so the UTF-8 setter succeeds there and every version of these scripts emits a clean banner. Those cases cannot tell this fix from what preceded it. Add cases that call FreeConsole() in the child first, which is the state install.rs's own comment assumes CREATE_NO_WINDOW produces. There Write-Host has no screen buffer to query, throws, and takes the script down: 2 bytes of stdout and exit 1 rather than the banner. The probe is assembled entirely out of text sliced from the script under test and spawned with install.rs's own interpreter, flags and creation flags. No Windows job ran this file, so its byte-level half was only ever exercised under pwsh 7 on the Linux Backend CI leg, which is UTF-8 by default. Add it to the cross-platform parity matrix, which already has a windows-latest row and already triggers on install.ps1 and studio/setup.ps1. * Report skips in the parity step A platform-gated case that stopped running on the row it exists for still reports green with -q alone. * Slice the error preference too It is what turns the Write-Host throw into a dead script rather than a skipped line, so restating it would be assuming the result. * Say what the comments actually mean * Make the console-less cases fail on a lost banner, not just a mangled one * Stub the output sink in every harness that splices these scripts The Write-Host rewrite left four spliced-source harnesses reaching Write-StudioLine without defining it. An undefined command is a terminating error, so each one either aborted or was swallowed by the harness's own catch, and the test kept passing while no longer testing anything. - test_windows_python_venv_hardening.py, partial-rollback case: the five-line split-move warning was lost. The assertion that "both halves are named" only stayed green because $existing is a prefix of the rollback dir, so it matched the dir= line instead. Pin it to the warning text. - test_path_probe_access_denied.ps1, ownership guard: the catch scored the command-not-found as the intended failure and never reached Exit-SetupFailure. Pin the check to the EXIT-SETUP message. - test_windows_installer_concurrency_guard.py: the decision block prints before Exit-InstallFailure, so on Windows the active case aborted at exit 1 and never produced RESULT:blocked. - Studio.Setup.Vs2026.Tests.ps1: on a host without cmake, Ensure-BuildToolsForLlamaSourceBuild hits the sink first and the no-op case fails on the throw. Also stub the three remaining harnesses that splice sink-calling helpers but do not reach the sink on the paths they exercise today, so the next case added to them cannot reintroduce this. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> |
||
|
|
6ea2b91d04
|
fix(studio/install): keep the rollback reference when a venv move stops partway (#7810) (#7823)
* fix(studio/install): keep the rollback reference when a venv move stops partway (#7810) Start-StudioVenvRollback moves the existing environment aside with a single Move-Item and treats any failure as "the original is still in place". On Windows that is not the shape the failure takes: an open handle inside the tree -- a running Studio backend holds its own unsloth.exe there -- fails the rename *after* it has already walked part of it, so the entries handled before the locked one sit at the rollback path while the rest stay behind. Both paths then exist, and the catch tests only the source, so it scores the split tree as an untouched original, clears StudioVenvRollbackDir and drops the sole record of where the other half went. The reporter was left with a unsloth_studio\ holding 7 Scripts entries and no python.exe, an intact venv under unsloth_studio.rollback.<stamp>.<pid>\, and no output naming either. Retries cannot recover: the create branch keys off python.exe, and uv refuses to build a venv over the directory the stranded files still occupy. Clear the rollback state only when the destination is genuinely absent. When both paths exist the move is partial, so keep it active -- the existing finally-block Restore-StudioVenvRollback then reverses it -- and print both locations plus the "close Unsloth Studio" hint the launcher-shim path at install.ps1:3078 already gives for the same underlying cause. install.sh is unaffected: POSIX rename ignores open descriptors and both paths are siblings under , so that move really is atomic. Regression test extracts the function from install.ps1 and runs it under real PowerShell with Move-Item stubbed to fail after creating the destination. On current main the partial case reports active=False with an empty rollback dir -- the stranding itself; the clean-failure case is asserted alongside it so the untouched-original path keeps clearing state as before. Signed-off-by: Tai An <antai12232931@outlook.com> * fix(studio/install): merge a split venv back instead of clearing the target Keeping the rollback active after a partway move sent the failure path into Restore-StudioVenvRollback, which removes $target before moving $backup back. In the split case $target is not an incomplete *new* environment -- it holds the half of the previous one the move never reached -- so that removal deleted files present nowhere else and restored a corrupted venv. Flag the split and give restoration a merge path: move each entry of the backup into the target without overwriting, recursing where the move stopped inside a subtree, and only drop the backup once it is empty. Anything ambiguous is left in place and both locations are named. Regression test pins the file that never moved to surviving restoration. * Installer: fix split-move merge nesting siblings and walking through junctions Two problems in Merge-StudioVenvRollbackTree, both only reachable once a partway move leaves the venv split. Sibling nesting. The per-entry variable was named $destination, and PowerShell variable names are case-insensitive, so it reassigned the $Destination parameter. Only the first entry at a level landed correctly; every later sibling was joined onto the previous one's path, so a restored venv came back with pyvenv.cfg inside Lib. Renamed to $entryTarget. Junction traversal. Recursion keyed on "directory on both sides", which a junction or directory symlink satisfies. If the half left behind holds the link, venv files move through it and land outside $StudioHome. If the moved half holds it, the recursion enumerates the link target and pulls those files into the venv, emptying a directory that was never part of the environment. Either way the link is replaced by a real directory, which the whole-tree Move-Item this path replaced never did. Now checks both sides for a reparse point and falls through to keep-both-copies. Attributes are read via Get-Item on both sides, since Get-ChildItem has reported them inconsistently. Tests cover sibling placement and both link directions. The link test uses a junction on Windows so it does not need SeCreateSymbolicLinkPrivilege. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --------- Signed-off-by: Tai An <antai12232931@outlook.com> Co-authored-by: danielhanchen <danielhanchen@gmail.com> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> |
||
|
|
1770182b5a
|
Windows: validate managed Python before package installation (#7763)
* Harden Windows pyenv interpreter handling * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Fix Windows managed Python recovery * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Tighten-installer-comments * Finish the -S probe hardening and run the new tests on Windows for PR #7763 Get-PythonPlatformTag still probed without -S. Its result is compared with -eq "win-amd64", so a sitecustomize banner reads as "unknown", the x64-over-ARM64 preference is lost and Windows on ARM settles for a native ARM64 interpreter. Test-IsCondaPython gets -S for the same reason. Neither query needs site, and base_prefix and get_platform() are unchanged by -S on 3.11, 3.12 and 3.13. The new test module parametrizes over pwsh and powershell, but cross-platform-parity-ci.yml is the only three-OS job and its paths filter and pytest list are hardcoded, so the 5.1 leg never ran. Added the file to both. Also match the py launcher branch to the PATH branch with -LiteralPath -PathType Leaf, and fix the failure message: the empty base home leaked into the Exit-InstallFailure text, and the ownership marker is written before the gate, so a plain re-run already replaces the environment. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com> Co-authored-by: danielhanchen <unslothshared@gmail.com> |