From a2f6183c4aafa234b64d9cb24b53b0c452df87a1 Mon Sep 17 00:00:00 2001 From: hawktuahs <32615509+hawktuahs@users.noreply.github.com> Date: Tue, 2 Jun 2026 08:01:59 +0530 Subject: [PATCH] Fix cookbook pip installs in venvs (#723) --- routes/cookbook_helpers.py | 13 +++++++++++++ routes/cookbook_routes.py | 12 ++++++------ tests/test_cookbook_helpers.py | 15 +++++++++++++++ 3 files changed, 34 insertions(+), 6 deletions(-) diff --git a/routes/cookbook_helpers.py b/routes/cookbook_helpers.py index c311b24..ffa2f0f 100644 --- a/routes/cookbook_helpers.py +++ b/routes/cookbook_helpers.py @@ -148,6 +148,19 @@ def _local_tooling_path_export(executable: str) -> str: return f'export PATH="{esc}:$PATH"' +def _pip_install_fallback_chain(package: str, *, python_cmd: str = "python3 -m pip", upgrade: bool = False) -> str: + """Build a bash pip install fallback chain. + + Try the active interpreter/environment first. `--user` is invalid inside + many venvs, so keep the user-site fallback for PEP-668 system Pythons only + after the venv-safe attempt has failed. + """ + upgrade_flag = " -U" if upgrade else "" + base = f"{python_cmd} install -q{upgrade_flag} {package} 2>/dev/null" + user = f"{python_cmd} install --user --break-system-packages -q{upgrade_flag} {package} 2>/dev/null" + return f"{base} || {user}" + + def _cached_model_scan_script(model_dirs: list[str] | None = None) -> str: """Build the standalone Python scanner used by /api/model/cached.""" lines = [ diff --git a/routes/cookbook_routes.py b/routes/cookbook_routes.py index fcfdca7..85b932c 100644 --- a/routes/cookbook_routes.py +++ b/routes/cookbook_routes.py @@ -38,7 +38,7 @@ from routes.cookbook_helpers import ( _ps_squote, _bash_squote, _validate_serve_cmd, _parse_serve_phase, _safe_env_prefix, _local_tooling_path_export, _append_serve_preflight_exit_lines, _append_serve_exit_code_lines, _cached_model_scan_script, - ModelDownloadRequest, ServeRequest, + _pip_install_fallback_chain, ModelDownloadRequest, ServeRequest, ) _HF_TOKEN_STATUS_SNIPPET = ( @@ -432,12 +432,12 @@ def setup_cookbook_routes() -> APIRouter: # throughput. Retries set disable_hf_transfer to fall back to the plain, # slower-but-reliable downloader (resumes cleanly from the .incomplete files). # Use `python3 -m pip` not `pip` — macOS has no bare `pip` command. - lines.append("command -v hf >/dev/null 2>&1 || python3 -m pip install --user --break-system-packages -q -U huggingface_hub 2>/dev/null || python3 -m pip install -q -U huggingface_hub 2>/dev/null") + lines.append(f"command -v hf >/dev/null 2>&1 || {_pip_install_fallback_chain('huggingface_hub', upgrade=True)}") if req.disable_hf_transfer: lines.append("export HF_HUB_ENABLE_HF_TRANSFER=0") lines.append("export HF_HUB_DOWNLOAD_MAX_WORKERS=4") else: - lines.append("python3 -c 'import hf_transfer' 2>/dev/null || python3 -m pip install --user --break-system-packages -q hf_transfer 2>/dev/null || python3 -m pip install -q hf_transfer 2>/dev/null") + lines.append(f"python3 -c 'import hf_transfer' 2>/dev/null || {_pip_install_fallback_chain('hf_transfer')}") lines.append("python3 -c 'import hf_transfer' 2>/dev/null && export HF_HUB_ENABLE_HF_TRANSFER=1") lines.append("export HF_HUB_DOWNLOAD_MAX_WORKERS=8") @@ -533,8 +533,8 @@ def setup_cookbook_routes() -> APIRouter: runner_lines.append('export PATH="$HOME/.local/bin:$PATH"') # Install hf CLI + hf_transfer best-effort so future runs get the fast path. # Use --break-system-packages on PEP-668 systems (Arch, newer Debian) so it doesn't bail. - runner_lines.append("command -v hf >/dev/null 2>&1 || pip install --user --break-system-packages -q -U huggingface_hub 2>/dev/null || pip install -q -U huggingface_hub 2>/dev/null") - runner_lines.append("python3 -c 'import hf_transfer' 2>/dev/null || pip install --user --break-system-packages -q hf_transfer 2>/dev/null || pip install -q hf_transfer 2>/dev/null") + runner_lines.append(f"command -v hf >/dev/null 2>&1 || {_pip_install_fallback_chain('huggingface_hub', python_cmd='pip', upgrade=True)}") + runner_lines.append(f"python3 -c 'import hf_transfer' 2>/dev/null || {_pip_install_fallback_chain('hf_transfer', python_cmd='pip')}") runner_lines.append("python3 -c 'import hf_transfer' 2>/dev/null && export HF_HUB_ENABLE_HF_TRANSFER=1") runner_lines.append("export HF_HUB_DOWNLOAD_MAX_WORKERS=8") # Surface whether the HF token actually reached THIS server, so a gated @@ -975,7 +975,7 @@ def setup_cookbook_routes() -> APIRouter: runner_lines.append(' # If the native build failed, fall back to the Python bindings.') runner_lines.append(' if ! command -v llama-server &>/dev/null && ! python3 -c "import llama_cpp" 2>/dev/null; then') runner_lines.append(' echo "llama-server build failed — installing Python bindings as fallback..."') - runner_lines.append(' pip install --user --break-system-packages -q llama-cpp-python 2>/dev/null || pip install -q llama-cpp-python 2>/dev/null || true') + runner_lines.append(f" {_pip_install_fallback_chain('llama-cpp-python', python_cmd='pip')} || true") runner_lines.append(' fi') runner_lines.append('fi') elif "ollama" in req.cmd: diff --git a/tests/test_cookbook_helpers.py b/tests/test_cookbook_helpers.py index 5124a0c..ef1b2a5 100644 --- a/tests/test_cookbook_helpers.py +++ b/tests/test_cookbook_helpers.py @@ -10,6 +10,7 @@ from routes.cookbook_helpers import ( _append_serve_exit_code_lines, _append_serve_preflight_exit_lines, _local_tooling_path_export, + _pip_install_fallback_chain, _safe_env_prefix, _validate_gpus, _validate_repo_id, @@ -82,6 +83,20 @@ def test_local_tooling_path_export_preserves_spaces_and_expands_path(): assert line.endswith(':$PATH"') # $PATH stays expandable in double quotes +def test_pip_install_fallback_chain_prefers_venv_safe_install(): + chain = _pip_install_fallback_chain("huggingface_hub", upgrade=True) + assert chain.startswith("python3 -m pip install -q -U huggingface_hub") + assert "|| python3 -m pip install --user --break-system-packages -q -U huggingface_hub" in chain + + +def test_pip_install_fallback_chain_allows_custom_python_command(): + chain = _pip_install_fallback_chain("hf_transfer", python_cmd="pip", upgrade=False) + assert chain == ( + "pip install -q hf_transfer 2>/dev/null || " + "pip install --user --break-system-packages -q hf_transfer 2>/dev/null" + ) + + def test_serve_preflight_failure_keeps_tmux_pane_visible(): """Dependency preflight failures should remain visible in tmux output.