-
-
Notifications
You must be signed in to change notification settings - Fork 6.1k
[Studio] Add --with-llama-cpp-dir installer flag to reuse a local llama.cpp #6472
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 2 commits
22eb2a8
35542c6
fe4b43d
b2587dc
0d16c6a
56b1e3c
d5e9f51
43f2082
d254f7e
5252163
1206943
be8a3c3
48b37bb
829b3b1
9a365dc
3877bea
8d398a2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -53,6 +53,8 @@ _VERBOSE=false | |
| _SHORTCUTS_ONLY=false | ||
| _next_is_package=false | ||
| _next_is_python=false | ||
| _next_is_llama_cpp_dir=false | ||
| _WITH_LLAMA_CPP_DIR="" | ||
| for arg in "$@"; do | ||
| if [ "$_next_is_package" = true ]; then | ||
| PACKAGE_NAME="$arg" | ||
|
|
@@ -64,6 +66,11 @@ for arg in "$@"; do | |
| _next_is_python=false | ||
| continue | ||
| fi | ||
| if [ "$_next_is_llama_cpp_dir" = true ]; then | ||
| _WITH_LLAMA_CPP_DIR="$arg" | ||
| _next_is_llama_cpp_dir=false | ||
| continue | ||
| fi | ||
| case "$arg" in | ||
| --local) STUDIO_LOCAL_INSTALL=true ;; | ||
| --package) _next_is_package=true ;; | ||
|
|
@@ -72,6 +79,7 @@ for arg in "$@"; do | |
| --no-torch) _NO_TORCH_FLAG=true ;; | ||
| --verbose|-v) _VERBOSE=true ;; | ||
| --shortcuts-only) _SHORTCUTS_ONLY=true ;; | ||
| --with-llama-cpp-dir) _next_is_llama_cpp_dir=true ;; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If Useful? React with 👍 / 👎. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This parser records Useful? React with 👍 / 👎.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If Consider adding a validation check after the loop (around line 268) to ensure that |
||
| esac | ||
| done | ||
|
|
||
|
|
@@ -2894,6 +2902,13 @@ _run_setup_with_studio_home() { | |
| "$@" | ||
| fi | ||
| } | ||
| if [ -n "$_WITH_LLAMA_CPP_DIR" ]; then | ||
| if [ ! -d "$_WITH_LLAMA_CPP_DIR" ]; then | ||
| echo "[ERROR] --with-llama-cpp-dir path does not exist: $_WITH_LLAMA_CPP_DIR" >&2 | ||
| exit 1 | ||
| fi | ||
| _WITH_LLAMA_CPP_DIR="$(cd "$_WITH_LLAMA_CPP_DIR" && pwd)" | ||
|
Imagineer99 marked this conversation as resolved.
Outdated
|
||
| fi | ||
| if [ "$STUDIO_LOCAL_INSTALL" = true ]; then | ||
| _run_setup_with_studio_home env \ | ||
| SKIP_STUDIO_BASE="$_SKIP_BASE" \ | ||
|
|
@@ -2902,6 +2917,7 @@ if [ "$STUDIO_LOCAL_INSTALL" = true ]; then | |
| STUDIO_LOCAL_INSTALL=1 \ | ||
| STUDIO_LOCAL_REPO="$_REPO_ROOT" \ | ||
| UNSLOTH_NO_TORCH="$SKIP_TORCH" \ | ||
| UNSLOTH_LOCAL_LLAMA_CPP_DIR="$_WITH_LLAMA_CPP_DIR" \ | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When the POSIX installer is used in the documented piped-install style with Useful? React with 👍 / 👎. |
||
| bash "$SETUP_SH" </dev/null || _SETUP_EXIT=$? | ||
| else | ||
| # Explicitly reset STUDIO_LOCAL_INSTALL / STUDIO_LOCAL_REPO so a stale | ||
|
|
@@ -2916,6 +2932,7 @@ else | |
| STUDIO_LOCAL_INSTALL=0 \ | ||
| STUDIO_LOCAL_REPO= \ | ||
| UNSLOTH_NO_TORCH="$SKIP_TORCH" \ | ||
| UNSLOTH_LOCAL_LLAMA_CPP_DIR="$_WITH_LLAMA_CPP_DIR" \ | ||
| bash "$SETUP_SH" </dev/null || _SETUP_EXIT=$? | ||
| fi | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2754,7 +2754,38 @@ if ($LlamaPr) { | |
| $SkipPrebuiltInstall = $true | ||
| } | ||
|
|
||
| if ($env:UNSLOTH_LLAMA_FORCE_COMPILE -eq "1") { | ||
| $LocalLlamaCppLinked = $false | ||
| $LocalLlamaCppSrc = $env:UNSLOTH_LOCAL_LLAMA_CPP_DIR | ||
| if ($LocalLlamaCppSrc) { | ||
| if (-not (Test-Path -LiteralPath $LocalLlamaCppSrc -PathType Container)) { | ||
| step "llama.cpp" "UNSLOTH_LOCAL_LLAMA_CPP_DIR does not exist: $LocalLlamaCppSrc" "Red" | ||
| exit 1 | ||
| } | ||
| $ResolvedLocal = (Resolve-Path -LiteralPath $LocalLlamaCppSrc).Path | ||
| if ($ResolvedLocal -eq $LlamaCppDir) { | ||
| substep "UNSLOTH_LOCAL_LLAMA_CPP_DIR points to the canonical install location; ignoring" "Yellow" | ||
|
Imagineer99 marked this conversation as resolved.
Outdated
|
||
| } else { | ||
| if ($StudioHomeIsCustom) { | ||
| Assert-StudioOwnedOrAbsent -Path $LlamaCppDir -Label "llama.cpp install" | ||
| } | ||
| if (Test-Path -LiteralPath $LlamaCppDir) { | ||
| Remove-Item -Recurse -Force -LiteralPath $LlamaCppDir -ErrorAction SilentlyContinue | ||
| } | ||
|
Comment on lines
+3237
to
+3247
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Critical Bug: Potential Data Loss via
|
||
| cmd /c "mklink /J `"$LlamaCppDir`" `"$ResolvedLocal`"" 2>&1 | Out-Null | ||
| if ($LASTEXITCODE -ne 0) { | ||
| substep "Could not create directory junction; copying instead..." "Yellow" | ||
| Copy-Item -Recurse -Path $ResolvedLocal -Destination $LlamaCppDir | ||
|
Imagineer99 marked this conversation as resolved.
Outdated
|
||
| } | ||
| Write-Host "" | ||
| step "llama.cpp" "linked local directory: $ResolvedLocal" | ||
| $LocalLlamaCppLinked = $true | ||
| $NeedLlamaSourceBuild = $false | ||
| } | ||
| } | ||
|
|
||
| if ($LocalLlamaCppLinked) { | ||
| # local directory linked above; skip prebuilt install | ||
| } elseif ($env:UNSLOTH_LLAMA_FORCE_COMPILE -eq "1") { | ||
| Write-Host "" | ||
| substep "UNSLOTH_LLAMA_FORCE_COMPILE=1 -- skipping prebuilt llama.cpp install" "Yellow" | ||
| $NeedLlamaSourceBuild = $true | ||
|
|
@@ -2962,7 +2993,7 @@ if (Test-Path -LiteralPath $LlamaServerBin) { | |
| } | ||
| } | ||
|
|
||
| if (-not $NeedLlamaSourceBuild) { | ||
| if (-not $NeedLlamaSourceBuild -and -not $LocalLlamaCppLinked) { | ||
| Write-Host "" | ||
| step "llama.cpp" "prebuilt (validated)" | ||
| } elseif ((Test-Path -LiteralPath $LlamaServerBin) -and -not $NeedRebuild -and $RequestedLlamaTag -ne "master") { | ||
|
Imagineer99 marked this conversation as resolved.
Outdated
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1086,7 +1086,31 @@ fi | |
|
|
||
| verbose_substep "requested llama.cpp tag: $_REQUESTED_LLAMA_TAG (repo: $_HELPER_RELEASE_REPO)" | ||
|
|
||
| if [ "$_LLAMA_FORCE_COMPILE" = "1" ]; then | ||
| _LOCAL_LLAMA_CPP_LINKED=false | ||
| if [ -n "${UNSLOTH_LOCAL_LLAMA_CPP_DIR:-}" ]; then | ||
| if [ ! -d "$UNSLOTH_LOCAL_LLAMA_CPP_DIR" ]; then | ||
| step "llama.cpp" "UNSLOTH_LOCAL_LLAMA_CPP_DIR does not exist: $UNSLOTH_LOCAL_LLAMA_CPP_DIR" "$C_ERR" | ||
| exit 1 | ||
| fi | ||
| _RESOLVED_LOCAL="$(cd "$UNSLOTH_LOCAL_LLAMA_CPP_DIR" && pwd)" | ||
|
Imagineer99 marked this conversation as resolved.
Outdated
|
||
| if [ "$_RESOLVED_LOCAL" = "$LLAMA_CPP_DIR" ]; then | ||
| substep "UNSLOTH_LOCAL_LLAMA_CPP_DIR points to the canonical install location; ignoring" | ||
| else | ||
|
Imagineer99 marked this conversation as resolved.
Outdated
|
||
| if [ "$_STUDIO_HOME_IS_CUSTOM" = true ]; then | ||
| _assert_studio_owned_or_absent "$LLAMA_CPP_DIR" "llama.cpp install" | ||
| fi | ||
| rm -rf "$LLAMA_CPP_DIR" | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When Useful? React with 👍 / 👎. |
||
| ln -sfn "$_RESOLVED_LOCAL" "$LLAMA_CPP_DIR" | ||
|
Comment on lines
+1342
to
+1346
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For a custom Useful? React with 👍 / 👎. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This creates the managed Useful? React with 👍 / 👎. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For local-dir installs that create this symlink, the backend's orphan cleanup resolves known install roots before comparing process executables ( Useful? React with 👍 / 👎. |
||
| step "llama.cpp" "linked local directory: $_RESOLVED_LOCAL" | ||
| _LOCAL_LLAMA_CPP_LINKED=true | ||
| _NEED_LLAMA_SOURCE_BUILD=false | ||
| _SKIP_PREBUILT_INSTALL=true | ||
|
Comment on lines
+1349
to
+1351
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For a normal local llama.cpp source build on Linux/macOS, Useful? React with 👍 / 👎.
Comment on lines
+1348
to
+1351
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When Useful? React with 👍 / 👎. |
||
| fi | ||
| fi | ||
|
|
||
| if [ "$_LOCAL_LLAMA_CPP_LINKED" = true ]; then | ||
| : # local directory linked above; skip prebuilt install | ||
| elif [ "$_LLAMA_FORCE_COMPILE" = "1" ]; then | ||
| step "llama.cpp" "UNSLOTH_LLAMA_FORCE_COMPILE=1 -- skipping prebuilt" "$C_WARN" | ||
| _NEED_LLAMA_SOURCE_BUILD=true | ||
| elif [ "${_SKIP_PREBUILT_INSTALL:-false}" = true ]; then | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,155 @@ | ||
| #!/bin/bash | ||
| # Static analysis: the --with-llama-cpp-dir flag must be wired consistently | ||
| # across both installers (install.sh / install.ps1) and both setup scripts | ||
| # (studio/setup.sh / studio/setup.ps1). | ||
| # | ||
| # The flag lets a user point the installer at a local llama.cpp directory so it | ||
| # skips BOTH the prebuilt download (Phase 3) and the source build (Phase 4), | ||
| # linking the local dir into the canonical install location instead. The path | ||
| # crosses the installer->setup boundary via the UNSLOTH_LOCAL_LLAMA_CPP_DIR env | ||
| # var. These checks pin that contract so a future refactor of either side can't | ||
| # silently break it (e.g. installer parses the flag but setup never reads the | ||
| # env var, or setup links the dir but still runs the build). | ||
| # | ||
| # This is a shape/wiring test, not a behavioral one: it greps the committed | ||
| # scripts. It needs no Python, no GPU, no network. | ||
| set -e | ||
|
|
||
| SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" | ||
| INSTALL_SH="$SCRIPT_DIR/../../install.sh" | ||
| INSTALL_PS1="$SCRIPT_DIR/../../install.ps1" | ||
| SETUP_SH="$SCRIPT_DIR/../../studio/setup.sh" | ||
| SETUP_PS1="$SCRIPT_DIR/../../studio/setup.ps1" | ||
| ENV_VAR="UNSLOTH_LOCAL_LLAMA_CPP_DIR" | ||
| PASS=0 | ||
| FAIL=0 | ||
|
|
||
| assert_contains() { | ||
| _label="$1"; _file="$2"; _needle="$3" | ||
| if grep -qF -- "$_needle" "$_file"; then | ||
| echo " PASS: $_label" | ||
| PASS=$((PASS + 1)) | ||
| else | ||
| echo " FAIL: $_label (expected to find '$_needle' in $(basename "$_file"))" | ||
| FAIL=$((FAIL + 1)) | ||
| fi | ||
| } | ||
|
|
||
| # Count of distinct lines matching a regex, used to assert a guard appears | ||
| # in more than one place (e.g. env var forwarded on both setup invocations). | ||
| assert_min_count() { | ||
| _label="$1"; _file="$2"; _pattern="$3"; _min="$4" | ||
| _n=$(grep -cE -- "$_pattern" "$_file" || true) | ||
| if [ "$_n" -ge "$_min" ]; then | ||
| echo " PASS: $_label (found $_n, need >= $_min)" | ||
| PASS=$((PASS + 1)) | ||
| else | ||
| echo " FAIL: $_label (found $_n in $(basename "$_file"), need >= $_min)" | ||
| FAIL=$((FAIL + 1)) | ||
| fi | ||
| } | ||
|
|
||
| echo "" | ||
| echo "=== install.sh: parses --with-llama-cpp-dir and forwards the env var ===" | ||
|
|
||
| assert_contains \ | ||
| "install.sh: accepts --with-llama-cpp-dir flag" \ | ||
| "$INSTALL_SH" "--with-llama-cpp-dir" | ||
| assert_contains \ | ||
| "install.sh: validates the path exists before forwarding" \ | ||
| "$INSTALL_SH" 'if [ ! -d "$_WITH_LLAMA_CPP_DIR" ]; then' | ||
| # The path must be forwarded to setup.sh on BOTH the local and the | ||
| # non-local setup invocations, else --local users (the documented path) | ||
| # would silently lose the flag. | ||
| assert_min_count \ | ||
| "install.sh: forwards $ENV_VAR on both setup invocations" \ | ||
| "$INSTALL_SH" "$ENV_VAR=\"\\\$_WITH_LLAMA_CPP_DIR\"" 2 | ||
|
|
||
| echo "" | ||
| echo "=== install.ps1: parses --with-llama-cpp-dir and forwards the env var ===" | ||
|
|
||
| assert_contains \ | ||
| "install.ps1: accepts --with-llama-cpp-dir flag" \ | ||
| "$INSTALL_PS1" '"--with-llama-cpp-dir"' | ||
| assert_contains \ | ||
| "install.ps1: errors when flag is given with no path argument" \ | ||
| "$INSTALL_PS1" "--with-llama-cpp-dir requires a path argument" | ||
| assert_contains \ | ||
| "install.ps1: validates the path exists before forwarding" \ | ||
| "$INSTALL_PS1" "--with-llama-cpp-dir path does not exist" | ||
| assert_contains \ | ||
| "install.ps1: exports $ENV_VAR for setup.ps1" \ | ||
| "$INSTALL_PS1" "\$env:$ENV_VAR =" | ||
| # The exported env var must be cleaned up so a later setup invocation in the | ||
| # same shell session doesn't inherit a stale local-dir link. | ||
| assert_contains \ | ||
| "install.ps1: clears $ENV_VAR after the setup run" \ | ||
| "$INSTALL_PS1" "Remove-Item Env:$ENV_VAR" | ||
|
|
||
| echo "" | ||
| echo "=== studio/setup.sh: reads the env var, links, and skips download+build ===" | ||
|
|
||
| assert_contains \ | ||
| "setup.sh: reads $ENV_VAR" \ | ||
| "$SETUP_SH" "$ENV_VAR" | ||
| assert_contains \ | ||
| "setup.sh: symlinks the local dir into the canonical install location" \ | ||
| "$SETUP_SH" 'ln -sfn "$_RESOLVED_LOCAL" "$LLAMA_CPP_DIR"' | ||
| assert_contains \ | ||
| "setup.sh: disables the source build when the local dir is linked" \ | ||
| "$SETUP_SH" "_NEED_LLAMA_SOURCE_BUILD=false" | ||
| assert_contains \ | ||
| "setup.sh: skips the prebuilt download when the local dir is linked" \ | ||
| "$SETUP_SH" "_SKIP_PREBUILT_INSTALL=true" | ||
| # The link branch must short-circuit the FORCE_COMPILE / prebuilt chain rather | ||
| # than fall through into it. | ||
| assert_contains \ | ||
| "setup.sh: link branch gates the prebuilt/compile chain" \ | ||
| "$SETUP_SH" 'if [ "$_LOCAL_LLAMA_CPP_LINKED" = true ]; then' | ||
|
|
||
| echo "" | ||
| echo "=== studio/setup.ps1: reads the env var, junctions, and skips download+build ===" | ||
|
|
||
| assert_contains \ | ||
| "setup.ps1: reads $ENV_VAR" \ | ||
| "$SETUP_PS1" "\$env:$ENV_VAR" | ||
| assert_contains \ | ||
| "setup.ps1: creates a directory junction into the canonical location" \ | ||
| "$SETUP_PS1" "mklink /J" | ||
| assert_contains \ | ||
| "setup.ps1: falls back to a copy when the junction can't be created" \ | ||
| "$SETUP_PS1" "Copy-Item -Recurse -Path \$ResolvedLocal -Destination \$LlamaCppDir" | ||
|
Imagineer99 marked this conversation as resolved.
Outdated
|
||
| assert_contains \ | ||
| "setup.ps1: disables the source build when the local dir is linked" \ | ||
| "$SETUP_PS1" '$NeedLlamaSourceBuild = $false' | ||
| # The link branch must gate the prebuilt-install chain (the elseif on | ||
| # FORCE_COMPILE), and the 'prebuilt (validated)' banner must be suppressed | ||
| # when we linked a local dir instead of validating a real prebuilt. | ||
| assert_contains \ | ||
| "setup.ps1: link branch gates the prebuilt/compile chain" \ | ||
| "$SETUP_PS1" 'if ($LocalLlamaCppLinked) {' | ||
| assert_contains \ | ||
| "setup.ps1: 'prebuilt (validated)' banner excludes the linked-dir case" \ | ||
| "$SETUP_PS1" '-not $NeedLlamaSourceBuild -and -not $LocalLlamaCppLinked' | ||
|
|
||
| echo "" | ||
| echo "=== both setup scripts: a local dir pointing at the canonical path is a no-op ===" | ||
|
|
||
| # Guard against the self-link footgun: if the user passes the canonical install | ||
| # dir itself, neither script should delete-then-link it onto itself. | ||
| assert_contains \ | ||
| "setup.sh: ignores a local dir equal to the canonical install location" \ | ||
| "$SETUP_SH" 'if [ "$_RESOLVED_LOCAL" = "$LLAMA_CPP_DIR" ]; then' | ||
| assert_contains \ | ||
| "setup.ps1: ignores a local dir equal to the canonical install location" \ | ||
| "$SETUP_PS1" 'if ($ResolvedLocal -eq $LlamaCppDir) {' | ||
|
|
||
| echo "" | ||
| echo "=== Results ===" | ||
| echo " PASS: $PASS" | ||
| echo " FAIL: $FAIL" | ||
| if [ "$FAIL" -gt 0 ]; then | ||
| echo "FAILED" | ||
| exit 1 | ||
| fi | ||
| echo "ALL PASSED" | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When
--with-llama-cpp-diris mistyped in a PowerShell session that already hasUNSLOTH_STUDIO_HOMEor the installer state env vars set, this validation returns after the code above has removed/overwritten those env vars but before thetry/finallythat restores them starts. That leaves the caller's session pointed back at the default Studio home (or with staleSTUDIO_LOCAL_INSTALL/skip flags), so a subsequent setup in the same shell can install into the wrong root; move this validation before mutating env vars or include it inside the restorefinally.Useful? React with 👍 / 👎.