-
-
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 all 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,11 @@ _VERBOSE=false | |
| _SHORTCUTS_ONLY=false | ||
| _next_is_package=false | ||
| _next_is_python=false | ||
| _next_is_llama_cpp_dir=false | ||
| # Seed from the environment so a caller who exports UNSLOTH_LOCAL_LLAMA_CPP_DIR | ||
| # (the documented piped-install style) is honored; the --with-llama-cpp-dir | ||
| # flag below overrides it when given. | ||
| _WITH_LLAMA_CPP_DIR="${UNSLOTH_LOCAL_LLAMA_CPP_DIR:-}" | ||
| for arg in "$@"; do | ||
| if [ "$_next_is_package" = true ]; then | ||
| PACKAGE_NAME="$arg" | ||
|
|
@@ -64,6 +69,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 +82,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 | ||
|
|
||
|
|
@@ -255,6 +266,10 @@ if [ "$_next_is_python" = true ]; then | |
| echo "❌ ERROR: --python requires a version argument (e.g. --python 3.12)." >&2 | ||
| exit 1 | ||
| fi | ||
| if [ "$_next_is_llama_cpp_dir" = true ]; then | ||
| echo "❌ ERROR: --with-llama-cpp-dir requires a path argument." >&2 | ||
| exit 1 | ||
| fi | ||
|
|
||
| # Validate --package to prevent injection into shell/Python commands. | ||
| # Must start with a letter/digit (rejects leading dashes that uv would parse as flags). | ||
|
|
@@ -3023,6 +3038,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="$(CDPATH= cd -P -- "$_WITH_LLAMA_CPP_DIR" && pwd -P)" | ||
| fi | ||
| if [ "$STUDIO_LOCAL_INSTALL" = true ]; then | ||
| _run_setup_with_studio_home env \ | ||
| SKIP_STUDIO_BASE="$_SKIP_BASE" \ | ||
|
|
@@ -3031,6 +3053,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 | ||
|
|
@@ -3045,6 +3068,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 |
|---|---|---|
|
|
@@ -1223,6 +1223,25 @@ def _backfill_usage_from_timings(usage, timings): | |
| return out | ||
|
|
||
|
|
||
| def _is_external_link(path: Path) -> bool: | ||
| """True when ``path`` is a --with-llama-cpp-dir local link: a POSIX symlink | ||
| or a Windows directory junction / reparse point. Such a link resolves into | ||
| the user's own llama.cpp checkout, which Studio does not own.""" | ||
| try: | ||
| if os.path.islink(path): | ||
| return True | ||
| except OSError: | ||
| return False | ||
| if os.name == "nt": | ||
| try: | ||
| import stat | ||
| attrs = os.lstat(path).st_file_attributes # type: ignore[attr-defined] | ||
| return bool(attrs & stat.FILE_ATTRIBUTE_REPARSE_POINT) | ||
| except (OSError, AttributeError): | ||
| return False | ||
| return False | ||
|
|
||
|
|
||
| class LlamaCppBackend: | ||
| """Manages a llama-server subprocess for GGUF model inference. | ||
|
|
||
|
|
@@ -7166,6 +7185,13 @@ def _kill_orphaned_servers() -> int: | |
| resolved_roots: list[Path] = [] | ||
| for root in install_roots: | ||
| try: | ||
| # A --with-llama-cpp-dir local link (symlink/junction) | ||
| # resolves into the user's own checkout. Adding it would let | ||
| # us treat the user's externally-launched llama-server as our | ||
| # orphan and kill it, so leave such roots out of the | ||
| # allowlist (we forgo orphan-reaping for local-link installs). | ||
| if _is_external_link(root): | ||
| continue | ||
|
Comment on lines
+7193
to
+7194
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 👍 / 👎. |
||
| resolved_roots.append(root.resolve()) | ||
| except OSError: | ||
| pass | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,137 @@ | ||
| # SPDX-License-Identifier: AGPL-3.0-only | ||
| # Copyright 2026-present the Unsloth AI Inc. team. All rights reserved. See /studio/LICENSE.AGPL-3.0 | ||
|
|
||
| """Behavioral tests for the --with-llama-cpp-dir 'unmanaged local link' contract. | ||
|
|
||
| When the canonical llama.cpp dir is a symlink (POSIX) / junction (Windows) to a | ||
| user's own checkout, Studio must treat it as externally managed: | ||
| - the in-app updater must not offer or apply a prebuilt over the link | ||
| - orphan cleanup must not kill a llama-server the user launched from that tree | ||
|
|
||
| These exercise real link behavior rather than grepping the scripts. | ||
| """ | ||
|
|
||
| import os | ||
| import subprocess | ||
| from pathlib import Path | ||
|
|
||
| import pytest | ||
|
|
||
| from utils import llama_cpp_update as u | ||
| from core.inference.llama_cpp import LlamaCppBackend | ||
|
|
||
|
|
||
| def _make_link(link: Path, target: Path) -> None: | ||
| """Create a directory junction (Windows) / symlink (POSIX); neither needs | ||
| elevation.""" | ||
| target.mkdir(parents = True, exist_ok = True) | ||
| if os.name == "nt": | ||
| subprocess.run( | ||
| ["cmd", "/c", "mklink", "/J", str(link), str(target)], | ||
| check = True, | ||
| capture_output = True, | ||
| text = True, | ||
| ) | ||
| else: | ||
| link.symlink_to(target, target_is_directory = True) | ||
|
|
||
|
|
||
| def _server_subpath() -> Path: | ||
| return Path( | ||
| "build/bin/Release/llama-server.exe" if os.name == "nt" else "build/bin/llama-server" | ||
| ) | ||
|
|
||
|
|
||
| class _FakeProc: | ||
| def __init__(self, pid: int, exe: str) -> None: | ||
| self.info = {"pid": pid, "name": "llama-server", "exe": exe} | ||
| self.killed = False | ||
|
|
||
| def kill(self) -> None: | ||
| self.killed = True | ||
|
|
||
|
|
||
| def test_is_external_link_detects_link_vs_plain_dir(tmp_path: Path) -> None: | ||
| plain = tmp_path / "plain" | ||
| plain.mkdir() | ||
| assert u._is_external_link(plain) is False | ||
|
|
||
| link = tmp_path / "link" | ||
| _make_link(link, tmp_path / "tgt") | ||
| assert u._is_external_link(link) is True | ||
|
|
||
|
|
||
| def test_active_install_is_local_link(tmp_path: Path) -> None: | ||
| link = tmp_path / "llama.cpp" | ||
| _make_link(link, tmp_path / "tgt") | ||
| binary = str(link / _server_subpath()) | ||
| assert u._active_install_is_local_link(binary) is True | ||
|
|
||
| # A plain (non-link) llama.cpp dir is Studio-managed, not a local link. | ||
| plain = tmp_path / "plain" / "llama.cpp" | ||
| plain.mkdir(parents = True) | ||
| assert u._active_install_is_local_link(str(plain / _server_subpath())) is False | ||
|
|
||
|
|
||
| def test_get_update_status_reports_local_link(tmp_path: Path, monkeypatch) -> None: | ||
| link = tmp_path / "llama.cpp" | ||
| _make_link(link, tmp_path / "tgt") | ||
| monkeypatch.setattr(u, "_find_binary", lambda: str(link / _server_subpath())) | ||
| st = u.get_update_status() | ||
| assert st["supported"] is False | ||
| assert st["update_available"] is False | ||
| assert st["local_link"] is True | ||
|
|
||
|
|
||
| def test_start_update_refuses_local_link(tmp_path: Path, monkeypatch) -> None: | ||
| link = tmp_path / "llama.cpp" | ||
| _make_link(link, tmp_path / "tgt") | ||
| monkeypatch.setattr(u, "_find_binary", lambda: str(link / _server_subpath())) | ||
| res = u.start_update() | ||
| assert res["started"] is False | ||
| assert res["reason"] == "local_link" | ||
|
|
||
|
|
||
| def _run_orphan_scan(monkeypatch, studio_root: Path, fake: _FakeProc) -> int: | ||
| # psutil drives the cross-platform process scan; skip (rather than error) if a | ||
| # minimal test env lacks it. CI installs it so these tests actually run. | ||
| psutil = pytest.importorskip("psutil") | ||
|
|
||
| monkeypatch.setattr( | ||
| LlamaCppBackend, | ||
| "_resolved_studio_root_and_is_legacy", | ||
| staticmethod(lambda: (studio_root.resolve(), False)), | ||
| ) | ||
| monkeypatch.setattr(LlamaCppBackend, "_reap_recorded_pid", staticmethod(lambda: 0)) | ||
| monkeypatch.setattr(psutil, "process_iter", lambda attrs = None: iter([fake])) | ||
| return LlamaCppBackend._kill_orphaned_servers() | ||
|
|
||
|
|
||
| def test_orphan_cleanup_spares_local_link_tree(tmp_path: Path, monkeypatch) -> None: | ||
| studio_root = tmp_path / "studio-home" | ||
| studio_root.mkdir() | ||
| external = tmp_path / "external" | ||
| (external / _server_subpath().parent).mkdir(parents = True) | ||
| (external / _server_subpath()).write_text("x") | ||
| _make_link(studio_root / "llama.cpp", external) | ||
|
|
||
| exe_under_link = str((external / _server_subpath()).resolve()) | ||
| fake = _FakeProc(os.getpid() + 777, exe_under_link) | ||
| killed = _run_orphan_scan(monkeypatch, studio_root, fake) | ||
| assert killed == 0 | ||
| assert fake.killed is False | ||
|
|
||
|
|
||
| def test_orphan_cleanup_kills_under_real_root(tmp_path: Path, monkeypatch) -> None: | ||
| # Control: a real (non-link) managed root still gets its orphan reaped, so | ||
| # the spare-the-link test above is meaningful (not a no-op). | ||
| studio_root = tmp_path / "studio-home" | ||
| bin_dir = studio_root / "llama.cpp" / _server_subpath().parent | ||
| bin_dir.mkdir(parents = True) | ||
| exe = studio_root / "llama.cpp" / _server_subpath() | ||
| exe.write_text("x") | ||
|
|
||
| fake = _FakeProc(os.getpid() + 888, str(exe.resolve())) | ||
| killed = _run_orphan_scan(monkeypatch, studio_root, fake) | ||
| assert killed == 1 | ||
| assert fake.killed is True |
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 👍 / 👎.