mirror of
https://github.com/unslothai/unsloth.git
synced 2026-08-18 05:13:54 +00:00
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".
This commit is contained in:
Michael Han
parent
439afd5828
commit
2a613431fb
2 changed files with 70 additions and 8 deletions
|
|
@ -361,16 +361,27 @@ function Get-PathDenialDetail {
|
|||
function Exit-PathAccessDenied {
|
||||
param(
|
||||
[Parameter(Mandatory = $true)][AllowNull()][AllowEmptyString()][string]$Path,
|
||||
[Parameter(Mandatory = $true)][AllowEmptyString()][string]$Label
|
||||
[Parameter(Mandatory = $true)][AllowEmptyString()][string]$Label,
|
||||
# "delete it, we reinstall it" is true of the managed cache and wrong for
|
||||
# a tree the user pointed us at. Never tell them to delete their build.
|
||||
[switch]$UserSupplied
|
||||
)
|
||||
|
||||
step "permissions" "$Label at $Path cannot be read: access is denied$(Get-PathDenialDetail -Path $Path)" "Red"
|
||||
substep "This folder lives outside the app, so reinstalling Unsloth Studio, to any drive, reuses it and fails the same way" "Yellow"
|
||||
substep "Simplest fix: close Unsloth, delete or rename $Path, then re-run setup (it is a managed cache and gets reinstalled)" "Yellow"
|
||||
substep "If deleting is also denied, run these two in an elevated PowerShell, then re-run setup:" "Yellow"
|
||||
if ($UserSupplied) {
|
||||
substep "Unsloth will not touch a directory you pointed it at, so this has to be fixed at the source" "Yellow"
|
||||
substep "Restore access with these two in an elevated PowerShell, or point UNSLOTH_LOCAL_LLAMA_CPP_DIR at a readable build:" "Yellow"
|
||||
} else {
|
||||
substep "This folder lives outside the app, so reinstalling Unsloth Studio, to any drive, reuses it and fails the same way" "Yellow"
|
||||
substep "Simplest fix: close Unsloth, delete or rename $Path, then re-run setup (it is a managed cache and gets reinstalled)" "Yellow"
|
||||
substep "If deleting is also denied, run these two in an elevated PowerShell, then re-run setup:" "Yellow"
|
||||
}
|
||||
substep "takeown /F `"$Path`" /R /D Y" "Yellow"
|
||||
substep "icacls `"$Path`" /reset /T" "Yellow"
|
||||
substep "Antivirus or Controlled folder access can deny this path too; allow or exclude it, then retry" "Yellow"
|
||||
if ($UserSupplied) {
|
||||
Exit-SetupFailure "Access denied reading $Label at $Path. Restore access with takeown/icacls, or point UNSLOTH_LOCAL_LLAMA_CPP_DIR at a readable build, then re-run setup."
|
||||
}
|
||||
Exit-SetupFailure "Access denied reading the existing $Label at $Path. Delete or rename that folder (Unsloth reinstalls it) or restore access with takeown/icacls, then re-run setup. Reinstalling the app does not reset it."
|
||||
}
|
||||
|
||||
|
|
@ -3947,7 +3958,13 @@ if ($LlamaPr) {
|
|||
$LocalLlamaCppLinked = $false
|
||||
$LocalLlamaCppSrc = $env:UNSLOTH_LOCAL_LLAMA_CPP_DIR
|
||||
if ($LocalLlamaCppSrc) {
|
||||
if (-not (Test-Path -LiteralPath $LocalLlamaCppSrc -PathType Container)) {
|
||||
# Unreadable is not missing: reporting "does not exist" would send the user
|
||||
# looking for the wrong problem.
|
||||
$localSrcState = Get-PathState -Path $LocalLlamaCppSrc -PathType Container
|
||||
if ($localSrcState -eq "Denied") {
|
||||
Exit-PathAccessDenied -Path $LocalLlamaCppSrc -Label "the UNSLOTH_LOCAL_LLAMA_CPP_DIR directory" -UserSupplied
|
||||
}
|
||||
if ($localSrcState -ne "Present") {
|
||||
step "llama.cpp" "UNSLOTH_LOCAL_LLAMA_CPP_DIR does not exist: $LocalLlamaCppSrc" "Red"
|
||||
Exit-SetupFailure "UNSLOTH_LOCAL_LLAMA_CPP_DIR does not exist: $LocalLlamaCppSrc"
|
||||
}
|
||||
|
|
@ -3957,13 +3974,24 @@ if ($LocalLlamaCppSrc) {
|
|||
# layout LlamaCppBackend._layout_candidates() resolves (root-level, build\bin,
|
||||
# or build\bin\Release) so the flag never rejects a tree Unsloth could run.
|
||||
$LocalLlamaServerFound = $false
|
||||
$LocalIsCanonical = ($ResolvedLocal -eq $LlamaCppDir)
|
||||
foreach ($_cand in @(
|
||||
(Join-Path $ResolvedLocal "llama-server.exe"),
|
||||
(Join-Path $ResolvedLocal "build\bin\llama-server.exe"),
|
||||
(Join-Path $ResolvedLocal "build\bin\Release\llama-server.exe"))) {
|
||||
if (Test-PathQuiet $_cand) { $LocalLlamaServerFound = $true; break }
|
||||
# Denied must not read as "nothing built here": the canonical branch
|
||||
# below would then hand the tree to the prebuilt installer, which
|
||||
# replaces the very build this flag asked to reuse.
|
||||
$candState = Get-PathState -Path $_cand
|
||||
if ($candState -eq "Denied") {
|
||||
if ($LocalIsCanonical) {
|
||||
Exit-PathAccessDenied -Path $ResolvedLocal -Label "llama.cpp install"
|
||||
}
|
||||
Exit-PathAccessDenied -Path $ResolvedLocal -Label "the --with-llama-cpp-dir build" -UserSupplied
|
||||
}
|
||||
if ($candState -eq "Present") { $LocalLlamaServerFound = $true; break }
|
||||
}
|
||||
if ($ResolvedLocal -eq $LlamaCppDir) {
|
||||
if ($LocalIsCanonical) {
|
||||
# Points at the canonical install location itself: never delete-then-link
|
||||
# onto itself. Reuse an existing build here (skip prebuilt + source) so the
|
||||
# staged prebuilt installer can't replace a build the user asked to reuse;
|
||||
|
|
|
|||
|
|
@ -50,7 +50,7 @@ def test_every_denial_route_reports_instead_of_proceeding():
|
|||
assert '$llamaGitState -eq "Denied"' in SETUP_PS1
|
||||
assert "$pathState = Get-PathState -Path $Path -PathType Container" in SETUP_PS1
|
||||
assert '$StudioHomeIsCustom -and $pathState -eq "Denied"' in SETUP_PS1
|
||||
assert SETUP_PS1.count("Exit-PathAccessDenied -Path") == 5
|
||||
assert SETUP_PS1.count("Exit-PathAccessDenied -Path") == 8
|
||||
|
||||
|
||||
def test_denied_install_reports_an_actionable_failure():
|
||||
|
|
@ -129,3 +129,37 @@ def test_reporting_helpers_tolerate_an_empty_path():
|
|||
assert "[AllowEmptyString()]" in head, name
|
||||
detail = SETUP_PS1.split("function Get-PathDenialDetail", 1)[1].split("\nfunction ", 1)[0]
|
||||
assert 'if ([string]::IsNullOrWhiteSpace($Path)) { return "" }' in detail
|
||||
|
||||
|
||||
def test_local_llama_dir_probes_are_three_state():
|
||||
"""--with-llama-cpp-dir pointed at the canonical location reuses whatever is
|
||||
built there so the prebuilt installer cannot replace it. A denied binary read
|
||||
as "nothing built" put that replacement back, which is what the branch exists
|
||||
to prevent."""
|
||||
assert "$localSrcState = Get-PathState -Path $LocalLlamaCppSrc -PathType Container" in SETUP_PS1
|
||||
assert '$localSrcState -eq "Denied"' in SETUP_PS1
|
||||
local_block = SETUP_PS1.split("$LocalLlamaCppSrc = $env:UNSLOTH_LOCAL_LLAMA_CPP_DIR", 1)[1]
|
||||
local_block = local_block.split("if ($LocalLlamaCppLinked) {", 1)[0]
|
||||
assert "$candState = Get-PathState -Path $_cand" in local_block
|
||||
assert '$candState -eq "Denied"' in local_block
|
||||
assert '$candState -eq "Present"' in local_block
|
||||
assert "Test-PathQuiet $_cand" not in local_block
|
||||
# The disk-space branch keeps Test-PathQuiet on purpose: it only decides
|
||||
# whether a preserved binary is usable, and an unreadable one is not.
|
||||
|
||||
|
||||
def test_user_supplied_paths_are_never_told_to_delete_themselves():
|
||||
"""The managed advice ("delete it, Unsloth reinstalls it") is wrong for a tree
|
||||
the user pointed us at with UNSLOTH_LOCAL_LLAMA_CPP_DIR."""
|
||||
body = SETUP_PS1.split("function Exit-PathAccessDenied", 1)[1].split("\nfunction ", 1)[0]
|
||||
assert "[switch]$UserSupplied" in body
|
||||
user_branch = body.split("if ($UserSupplied) {", 1)[1].split("} else {", 1)[0]
|
||||
assert "delete or rename" not in user_branch
|
||||
assert "managed cache" not in user_branch
|
||||
assert "UNSLOTH_LOCAL_LLAMA_CPP_DIR at a readable build" in user_branch
|
||||
# Every user-supplied call site must actually pass the switch.
|
||||
for line in SETUP_PS1.splitlines():
|
||||
if "Exit-PathAccessDenied" in line and "UNSLOTH_LOCAL_LLAMA_CPP_DIR" in line:
|
||||
assert "-UserSupplied" in line, line
|
||||
if "Exit-PathAccessDenied" in line and "--with-llama-cpp-dir" in line:
|
||||
assert "-UserSupplied" in line, line
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue