mirror of
https://github.com/unslothai/unsloth.git
synced 2026-08-21 14:53:58 +00:00
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.
This commit is contained in:
parent
2a613431fb
commit
de486ff873
4 changed files with 106 additions and 15 deletions
|
|
@ -1860,7 +1860,13 @@ if (-not $HasGit) {
|
|||
if ($_localLlamaDir) {
|
||||
# Same layout candidates as the reuse check in Phase 4.
|
||||
foreach ($_c in @("llama-server.exe", "build\bin\llama-server.exe", "build\bin\Release\llama-server.exe")) {
|
||||
if (Test-Path -LiteralPath (Join-Path $_localLlamaDir $_c)) { $_localLlamaBuilt = $true; break }
|
||||
# Denied here terminated the run under "Stop" long before Phase 4's
|
||||
# guarded probes, so this scan needs the same three-state handling.
|
||||
$_cState = Get-PathState -Path (Join-Path $_localLlamaDir $_c)
|
||||
if ($_cState -eq "Denied") {
|
||||
Exit-PathAccessDenied -Path $_localLlamaDir -Label "the UNSLOTH_LOCAL_LLAMA_CPP_DIR build" -UserSupplied
|
||||
}
|
||||
if ($_cState -eq "Present") { $_localLlamaBuilt = $true; break }
|
||||
}
|
||||
}
|
||||
if (-not $_localLlamaBuilt) {
|
||||
|
|
@ -2906,12 +2912,25 @@ $StudioHomeIsCustom = ($_studioHomeCanon -ne $LegacyStudioHome)
|
|||
# llama.cpp or whisper.cpp predating the .unsloth-studio-owned marker (see
|
||||
# setup.sh). Only Unsloth prebuilt markers count; source builds are
|
||||
# indistinguishable from a user clone on Windows and stay under the strict guard.
|
||||
# Test-PathQuiet: an unreadable tree must reach the errors below, not throw here.
|
||||
# "Yes" / "No" / "Denied". A denied marker is not evidence of absence: reading it
|
||||
# as "No" makes the guard below call an Unsloth tree an unrelated directory and
|
||||
# tell the user to move it aside, when the real problem is permissions.
|
||||
function Get-StudioAdoptableState {
|
||||
param([Parameter(Mandatory = $true)][string]$Path)
|
||||
$denied = $false
|
||||
foreach ($marker in @("UNSLOTH_PREBUILT_INFO.json", "UNSLOTH_WHISPER_PREBUILT_INFO.json")) {
|
||||
switch (Get-PathState -Path (Join-Path $Path $marker) -PathType Leaf) {
|
||||
"Present" { return "Yes" }
|
||||
"Denied" { $denied = $true }
|
||||
}
|
||||
}
|
||||
if ($denied) { return "Denied" }
|
||||
return "No"
|
||||
}
|
||||
# Boolean view for callers that only gate a cosmetic cleanup on adoption.
|
||||
function Test-StudioOwnedAdoptable {
|
||||
param([Parameter(Mandatory = $true)][string]$Path)
|
||||
if (Test-PathQuiet (Join-Path $Path "UNSLOTH_PREBUILT_INFO.json") "Leaf") { return $true }
|
||||
if (Test-PathQuiet (Join-Path $Path "UNSLOTH_WHISPER_PREBUILT_INFO.json") "Leaf") { return $true }
|
||||
return $false
|
||||
return ((Get-StudioAdoptableState -Path $Path) -eq "Yes")
|
||||
}
|
||||
function Assert-StudioOwnedOrAbsent {
|
||||
param(
|
||||
|
|
@ -2934,7 +2953,11 @@ function Assert-StudioOwnedOrAbsent {
|
|||
Exit-PathAccessDenied -Path $Path -Label $Label
|
||||
}
|
||||
if ($StudioHomeIsCustom -and $markerState -ne "Present") {
|
||||
if (Test-StudioOwnedAdoptable $Path) {
|
||||
$adoptState = Get-StudioAdoptableState -Path $Path
|
||||
if ($adoptState -eq "Denied") {
|
||||
Exit-PathAccessDenied -Path $Path -Label $Label
|
||||
}
|
||||
if ($adoptState -eq "Yes") {
|
||||
Mark-StudioOwned $Path
|
||||
return
|
||||
}
|
||||
|
|
@ -3984,10 +4007,10 @@ if ($LocalLlamaCppSrc) {
|
|||
# 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
|
||||
# -UserSupplied even when this is the canonical location: the
|
||||
# override says the tree is the user's build, so never advise
|
||||
# deleting it, managed path or not.
|
||||
Exit-PathAccessDenied -Path $ResolvedLocal -Label "the UNSLOTH_LOCAL_LLAMA_CPP_DIR build" -UserSupplied
|
||||
}
|
||||
if ($candState -eq "Present") { $LocalLlamaServerFound = $true; break }
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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") == 8
|
||||
assert SETUP_PS1.count("Exit-PathAccessDenied -Path") == 9
|
||||
|
||||
|
||||
def test_denied_install_reports_an_actionable_failure():
|
||||
|
|
@ -148,6 +148,38 @@ def test_local_llama_dir_probes_are_three_state():
|
|||
# whether a preserved binary is usable, and an unreadable one is not.
|
||||
|
||||
|
||||
def test_phase_1b_git_scan_is_guarded_too():
|
||||
"""The git prerequisite scan probes the same candidate binaries in Phase 1b,
|
||||
thousands of lines before the Phase 4 guards, and under "Stop". A denial
|
||||
there reproduced the original raw termination."""
|
||||
scan = SETUP_PS1.split("$_localLlamaBuilt = $false", 1)[1].split(
|
||||
"if (-not $_localLlamaBuilt) {", 1
|
||||
)[0]
|
||||
assert "$_cState = Get-PathState -Path (Join-Path $_localLlamaDir $_c)" in scan
|
||||
assert '$_cState -eq "Denied"' in scan
|
||||
assert '$_cState -eq "Present"' in scan
|
||||
assert "Test-Path -LiteralPath (Join-Path $_localLlamaDir $_c)" not in scan
|
||||
assert "-UserSupplied" in scan
|
||||
|
||||
|
||||
def test_adoption_markers_keep_their_denial():
|
||||
"""A denied prebuilt marker is not evidence of absence. Collapsing it made the
|
||||
ownership guard call an Unsloth tree an unrelated directory and tell the user
|
||||
to move it aside, hiding a permissions problem."""
|
||||
assert "function Get-StudioAdoptableState" in SETUP_PS1
|
||||
state = SETUP_PS1.split("function Get-StudioAdoptableState", 1)[1].split("\nfunction ", 1)[0]
|
||||
assert "Get-PathState -Path (Join-Path $Path $marker) -PathType Leaf" in state
|
||||
for verdict in ('return "Yes"', 'return "No"', 'return "Denied"'):
|
||||
assert verdict in state
|
||||
guard = SETUP_PS1.split("function Assert-StudioOwnedOrAbsent", 1)[1].split("\nfunction ", 1)[0]
|
||||
assert "$adoptState = Get-StudioAdoptableState -Path $Path" in guard
|
||||
assert '$adoptState -eq "Denied"' in guard
|
||||
# The denial must be reported before the "not Unsloth-owned" wording.
|
||||
assert guard.index('$adoptState -eq "Denied"') < guard.index(
|
||||
"is not marked as an Unsloth-owned"
|
||||
)
|
||||
|
||||
|
||||
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."""
|
||||
|
|
@ -157,9 +189,14 @@ def test_user_supplied_paths_are_never_told_to_delete_themselves():
|
|||
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.
|
||||
# Every call site that reports a path the user pointed us at must pass the
|
||||
# switch, including the canonical location: the override says that tree is
|
||||
# the user's build, so "delete it, we reinstall it" is wrong there too.
|
||||
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:
|
||||
local_block = SETUP_PS1.split("$LocalLlamaCppSrc = $env:UNSLOTH_LOCAL_LLAMA_CPP_DIR", 1)[1]
|
||||
local_block = local_block.split("if ($LocalLlamaCppLinked) {", 1)[0]
|
||||
for line in local_block.splitlines():
|
||||
if "Exit-PathAccessDenied" in line:
|
||||
assert "-UserSupplied" in line, line
|
||||
|
|
|
|||
|
|
@ -22,7 +22,8 @@ $setupPath = [System.IO.Path]::Combine($repoRoot, "studio", "setup.ps1")
|
|||
. ([System.IO.Path]::Combine($repoRoot, "tests", "studio_setup_ps1", "Get-FunctionSource.ps1"))
|
||||
|
||||
foreach ($fn in @("Test-AccessDeniedError", "Get-PathState", "Test-PathQuiet",
|
||||
"Get-PathDenialDetail", "Test-StudioOwnedAdoptable")) {
|
||||
"Get-PathDenialDetail", "Get-StudioAdoptableState",
|
||||
"Test-StudioOwnedAdoptable")) {
|
||||
$src = Get-FunctionSource -Path $setupPath -Name $fn
|
||||
Check "setup.ps1 defines $fn" ($null -ne $src)
|
||||
if ($src) { . ([scriptblock]::Create($src)) }
|
||||
|
|
@ -142,6 +143,34 @@ Remove-Item -Recurse -Force -LiteralPath $parityRoot -ErrorAction SilentlyContin
|
|||
Check "Get-PathState matches bare Test-Path on every non-throwing probe ($probed)" ($mismatch -eq 0)
|
||||
Check "Denied never appears where the old probe did not throw" ($deniedWithoutThrow -eq 0)
|
||||
|
||||
# ── A denied marker FILE under a readable directory ──
|
||||
# Windows only: POSIX keeps a mode-000 file stat-able, so this state cannot be
|
||||
# built on Unix. Collapsing it to "No" made the ownership guard report an
|
||||
# Unsloth tree as an unrelated directory instead of a permissions problem.
|
||||
$adoptRoot = Join-Path ([System.IO.Path]::GetTempPath()) ("uns_adopt_" + [guid]::NewGuid().ToString("N"))
|
||||
New-Item -ItemType Directory -Force -Path $adoptRoot | Out-Null
|
||||
$adoptMarker = Join-Path $adoptRoot "UNSLOTH_PREBUILT_INFO.json"
|
||||
Set-Content -LiteralPath $adoptMarker -Value '{"release_tag":"app-1"}'
|
||||
Check "a readable marker reports Yes" ((Get-StudioAdoptableState -Path $adoptRoot) -eq "Yes")
|
||||
if ($onWindows) {
|
||||
$who = "$env:USERDOMAIN\$env:USERNAME"
|
||||
icacls $adoptMarker /deny "${who}:(R)" *>$null
|
||||
try {
|
||||
$markerThrew = $false
|
||||
try { $null = Test-Path -LiteralPath $adoptMarker -PathType Leaf -ErrorAction Stop } catch { $markerThrew = $true }
|
||||
if ($markerThrew) {
|
||||
Check "a denied marker reports Denied, not No" ((Get-StudioAdoptableState -Path $adoptRoot) -eq "Denied")
|
||||
Check "the boolean view still refuses to adopt it" (-not (Test-StudioOwnedAdoptable $adoptRoot))
|
||||
} else {
|
||||
Write-Host " SKIP this host would not deny the marker file" -ForegroundColor Yellow
|
||||
}
|
||||
} finally { icacls $adoptMarker /remove:d "$who" *>$null }
|
||||
} else {
|
||||
Write-Host " SKIP denied marker file is a Windows-ACL-only state (POSIX keeps mode-000 files stat-able)" -ForegroundColor Yellow
|
||||
}
|
||||
Remove-Item -Recurse -Force -LiteralPath $adoptRoot -ErrorAction SilentlyContinue
|
||||
Check "a missing marker reports No" ((Get-StudioAdoptableState -Path ([System.IO.Path]::GetTempPath())) -eq "No")
|
||||
|
||||
# ── The reporting path must not itself fail ──
|
||||
# Get-PathDenialDetail runs while a failure is being reported, so a null or empty
|
||||
# path must not replace the actionable message with a binding exception.
|
||||
|
|
|
|||
|
|
@ -253,7 +253,9 @@ def test_setup_ps1_prebuilt_llama_cpp_has_ownership_guard():
|
|||
|
||||
def test_setup_ps1_adopts_existing_whisper_prebuilt_marker():
|
||||
text = SETUP_PS1.read_text(encoding = "utf-8")
|
||||
helper_start = text.index("function Test-StudioOwnedAdoptable")
|
||||
# The marker scan lives in Get-StudioAdoptableState; Test-StudioOwnedAdoptable
|
||||
# is the boolean view of it.
|
||||
helper_start = text.index("function Get-StudioAdoptableState")
|
||||
helper_end = text.index("function Assert-StudioOwnedOrAbsent", helper_start)
|
||||
helper = text[helper_start:helper_end]
|
||||
assert "UNSLOTH_WHISPER_PREBUILT_INFO.json" in helper
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue