-
-
Notifications
You must be signed in to change notification settings - Fork 6.8k
fix(install.sh): fix pipx strategy to avoid PEP 668 errors #24327
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
base: main
Are you sure you want to change the base?
Changes from all commits
d13ec1d
5c4301d
2ab9ebc
4611d9b
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 |
|---|---|---|
|
|
@@ -17,12 +17,14 @@ if [ -t 1 ]; then | |
| BOLD='\033[1m' | ||
| GREEN='\033[38;2;78;186;101m' | ||
| GREY='\033[38;2;153;153;153m' | ||
| YELLOW='\033[33m' | ||
| RESET='\033[0m' | ||
| else | ||
| BOLD='' GREEN='' GREY='' RESET='' | ||
| BOLD='' GREEN='' GREY='' YELLOW='' RESET='' | ||
| fi | ||
|
|
||
| info() { printf "${GREY} %s${RESET}\n" "$*"; } | ||
| warn() { printf "${YELLOW} Warning: %s${RESET}\n" "$*" >&2; } | ||
| success() { printf "${GREEN} ✔ %s${RESET}\n" "$*"; } | ||
| header() { printf "${BOLD} %s${RESET}\n" "$*"; } | ||
| die() { printf "\n Error: %s\n\n" "$*" >&2; exit 1; } | ||
|
|
@@ -72,34 +74,89 @@ if [ -z "$PYTHON_BIN" ]; then | |
| Ubuntu: sudo apt install python3 python3-pip" | ||
| fi | ||
|
|
||
| # ── pip detection ────────────────────────────────────────────────────────── | ||
| if ! "$PYTHON_BIN" -m pip --version >/dev/null 2>&1; then | ||
| die "pip is not available. Install it with: | ||
| $PYTHON_BIN -m ensurepip --upgrade" | ||
| fi | ||
|
|
||
| # ── install ──────────────────────────────────────────────────────────────── | ||
| # Prefer pipx or venv to avoid PEP 668 "externally-managed-environment" on | ||
| # Homebrew/ system Python. Neither strategy requires system pip — pipx | ||
| # manages its own venvs and the venv strategy uses Python's built-in | ||
| # ensurepip, so there is no unconditional pip gate here. | ||
| echo "" | ||
| header "Installing litellm[proxy]…" | ||
| echo "" | ||
|
|
||
| "$PYTHON_BIN" -m pip install --upgrade "${LITELLM_PACKAGE}" \ | ||
| || die "pip install failed. Try manually: $PYTHON_BIN -m pip install '${LITELLM_PACKAGE}'" | ||
|
|
||
| # ── find the litellm binary installed by pip for this Python ─────────────── | ||
| # sysconfig.get_path('scripts') is where pip puts console scripts — reliable | ||
| # even when the Python lives in a libexec/ symlink tree (e.g. Homebrew). | ||
| SCRIPTS_DIR="$("$PYTHON_BIN" -c 'import sysconfig; print(sysconfig.get_path("scripts"))')" | ||
| LITELLM_BIN="${SCRIPTS_DIR}/litellm" | ||
| LITELLM_BIN="" | ||
|
|
||
| # Strategy 1: pipx (best for CLI apps, per PEP 668) | ||
| # Try upgrade first to preserve existing flags (e.g. --include-deps); fall | ||
| # back to a fresh install if the package is not yet managed by pipx. | ||
| if command -v pipx >/dev/null 2>&1; then | ||
| info "Using pipx (isolated install)" | ||
| if pipx upgrade litellm 2>/dev/null; then | ||
| # Ensure proxy extras are present (may be absent if originally installed | ||
| # as bare "litellm" without [proxy]). `pipx inject` has been available | ||
| # since pipx 0.12.3 (~2019) and records injected deps in pipx metadata so | ||
| # upgrades stay consistent; the `runpip` subcommand is newer (0.15.x) and would fail | ||
| # on older pipx. | ||
| if ! _inject_err="$(pipx inject litellm "${LITELLM_PACKAGE}" 2>&1)"; then | ||
| warn "could not inject proxy extras (proxy features may fail until manually fixed)." | ||
| printf '%s\n' "$_inject_err" >&2 | ||
| fi | ||
| unset _inject_err 2>/dev/null || true | ||
| _pipx_upgraded=1 | ||
|
Comment on lines
+99
to
+104
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.
In practice this means:
The upgrade note should at minimum be surfaced in the success message or as an info "To upgrade later, run: pipx upgrade litellm --include-injected"This ensures users don't lose the proxy extras on their next upgrade cycle. |
||
| elif pipx install "${LITELLM_PACKAGE}"; then | ||
| _pipx_upgraded=1 | ||
| fi | ||
| if [ "${_pipx_upgraded:-0}" = "1" ]; then | ||
| _pipx_bin_dir="${PIPX_BIN_DIR:-${HOME}/.local/bin}" | ||
| _pipx_home="${PIPX_HOME:-${HOME}/.local/share/pipx}" | ||
| for try in \ | ||
| "${_pipx_bin_dir}/litellm" \ | ||
| "${_pipx_home}/venvs/litellm/bin/litellm"; do | ||
| if [ -n "$try" ] && [ -x "$try" ]; then | ||
| LITELLM_BIN="$try" | ||
| break | ||
| fi | ||
| done | ||
| if [ -z "$LITELLM_BIN" ] && command -v litellm >/dev/null 2>&1; then | ||
| LITELLM_BIN="$(command -v litellm)" | ||
| fi | ||
| if [ -z "$LITELLM_BIN" ]; then | ||
| warn "pipx install/upgrade succeeded but the litellm binary was not found at expected locations (PIPX_BIN_DIR=${_pipx_bin_dir}, PIPX_HOME=${_pipx_home}) and not on PATH." | ||
| warn "Falling back to a dedicated venv — you may still have a pipx-managed copy elsewhere; check: pipx list" | ||
| fi | ||
| else | ||
| warn "pipx install failed (see above), falling back to venv" | ||
| fi | ||
| fi | ||
|
|
||
| if [ ! -x "$LITELLM_BIN" ]; then | ||
| # Fall back to user-base bin (pip install --user) | ||
| USER_BIN="$("$PYTHON_BIN" -c 'import site; print(site.getuserbase())')/bin" | ||
| LITELLM_BIN="${USER_BIN}/litellm" | ||
| # Strategy 2: venv in ~/.litellm (avoids PEP 668 externally-managed-environment) | ||
| if [ -z "$LITELLM_BIN" ]; then | ||
| LITELLM_VENV="${LITELLM_VENV:-${HOME}/.litellm/venv}" | ||
| info "Using isolated venv: $LITELLM_VENV" | ||
| mkdir -p "$(dirname "$LITELLM_VENV")" | ||
| # Preserve an existing venv on repeat runs; use LITELLM_FORCE_VENV_RECREATE=1 to wipe it. | ||
| if [ "${LITELLM_FORCE_VENV_RECREATE:-0}" = "1" ]; then | ||
| "$PYTHON_BIN" -m venv --clear "$LITELLM_VENV" \ | ||
| || die "Failed to recreate venv. Try: $PYTHON_BIN -m venv --clear $LITELLM_VENV | ||
| On Ubuntu/Debian you may first need: sudo apt install python3-venv" | ||
| elif [ ! -d "$LITELLM_VENV" ]; then | ||
| "$PYTHON_BIN" -m venv "$LITELLM_VENV" \ | ||
| || die "Failed to create venv. Try: $PYTHON_BIN -m venv $LITELLM_VENV | ||
| On Ubuntu/Debian you may first need: sudo apt install python3-venv" | ||
| elif [ ! -x "${LITELLM_VENV}/bin/python" ]; then | ||
| info "Existing venv at ${LITELLM_VENV} is incomplete; recreating" | ||
| "$PYTHON_BIN" -m venv --clear "$LITELLM_VENV" \ | ||
| || die "Failed to recreate venv. Try: $PYTHON_BIN -m venv --clear $LITELLM_VENV | ||
| On Ubuntu/Debian you may first need: sudo apt install python3-venv" | ||
| fi | ||
| "${LITELLM_VENV}/bin/pip" install -q --upgrade pip \ | ||
| || die "Failed to upgrade pip in venv. Try: ${LITELLM_VENV}/bin/pip install --upgrade pip" | ||
|
Comment on lines
+151
to
+152
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.
The venv creation path falls through three branches (lines 137–150): "${LITELLM_VENV}/bin/pip" install -q --upgrade pipPython's The already-existing guard on line 145 ( if [ ! -x "${LITELLM_VENV}/bin/pip" ] && [ ! -x "${LITELLM_VENV}/bin/pip3" ]; then
die "pip was not bootstrapped in the venv. Try: sudo apt install python3-pip python3-venv, then re-run."
fi
VENV_PIP="${LITELLM_VENV}/bin/pip3"
[ -x "${LITELLM_VENV}/bin/pip" ] && VENV_PIP="${LITELLM_VENV}/bin/pip"
"${VENV_PIP}" install -q --upgrade pip ... |
||
| "${LITELLM_VENV}/bin/pip" install --upgrade "${LITELLM_PACKAGE}" \ | ||
| || die "pip install failed. Try manually: ${LITELLM_VENV}/bin/pip install '${LITELLM_PACKAGE}'" | ||
| LITELLM_BIN="${LITELLM_VENV}/bin/litellm" | ||
| fi | ||
|
|
||
| if [ ! -x "$LITELLM_BIN" ]; then | ||
| die "litellm binary not found after install. Try: $PYTHON_BIN -m pip install --user '${LITELLM_PACKAGE}'" | ||
| die "litellm binary not found. Try: pipx install '${LITELLM_PACKAGE}' or use a venv." | ||
| fi | ||
|
|
||
| # ── success banner ───────────────────────────────────────────────────────── | ||
|
|
@@ -111,7 +168,8 @@ installed_ver="$("$LITELLM_BIN" --version 2>&1 | grep -oE '[0-9]+\.[0-9]+\.[0-9] | |
|
|
||
| # ── PATH hint ────────────────────────────────────────────────────────────── | ||
| if ! command -v litellm >/dev/null 2>&1; then | ||
| info "Note: add litellm to your PATH: export PATH=\"\$PATH:${SCRIPTS_DIR}\"" | ||
| LITELLM_DIR="$(dirname "$LITELLM_BIN")" | ||
| info "Note: add litellm to your PATH: export PATH=\"\$PATH:${LITELLM_DIR}\"" | ||
| fi | ||
|
|
||
| # ── launch setup wizard ──────────────────────────────────────────────────── | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| """ | ||
| Regression tests for scripts/install.sh (syntax and critical install-path strings). | ||
|
|
||
| These are lightweight static checks so CI catches accidental rewrites of the | ||
| pipx inject / fallback messaging without running a full network install. | ||
| """ | ||
|
|
||
| from __future__ import annotations | ||
|
|
||
| import subprocess | ||
| from pathlib import Path | ||
|
|
||
| import pytest | ||
|
|
||
| _REPO_ROOT = Path(__file__).resolve().parents[3] | ||
| _INSTALL_SH = _REPO_ROOT / "scripts" / "install.sh" | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def install_sh_text() -> str: | ||
| assert _INSTALL_SH.is_file(), f"missing {_INSTALL_SH}" | ||
| return _INSTALL_SH.read_text(encoding="utf-8") | ||
|
|
||
|
|
||
| def test_install_sh_exists(): | ||
| assert _INSTALL_SH.is_file() | ||
|
|
||
|
|
||
| def test_install_sh_passes_bash_syntax_check(): | ||
| result = subprocess.run( | ||
| ["bash", "-n", str(_INSTALL_SH)], | ||
| capture_output=True, | ||
| text=True, | ||
| check=False, | ||
| ) | ||
| assert result.returncode == 0, result.stderr or result.stdout | ||
|
Comment on lines
+29
to
+36
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.
The test only runs Since the whole point of the def test_install_sh_passes_bash_syntax_check():
result = subprocess.run(
["bash", "-n", str(_INSTALL_SH)],
capture_output=True, text=True, check=False,
)
assert result.returncode == 0, result.stderr or result.stdout
@pytest.mark.skipif(
subprocess.run(["which", "dash"], capture_output=True).returncode != 0,
reason="dash not available",
)
def test_install_sh_passes_dash_syntax_check():
result = subprocess.run(
["dash", "-n", str(_INSTALL_SH)],
capture_output=True, text=True, check=False,
)
assert result.returncode == 0, result.stderr or result.stdout |
||
|
|
||
|
|
||
| def test_install_sh_uses_pipx_inject_for_proxy_extras_after_upgrade(install_sh_text: str): | ||
| """Prefer pipx inject over runpip: inject is older / wider pipx support.""" | ||
| assert "pipx inject litellm" in install_sh_text | ||
| assert "pipx runpip litellm" not in install_sh_text | ||
|
|
||
|
|
||
| def test_install_sh_warns_on_pipx_install_failure_fallback(install_sh_text: str): | ||
| assert 'warn "pipx install failed (see above), falling back to venv"' in install_sh_text | ||
|
|
||
|
|
||
| def test_install_sh_warn_message_for_failed_proxy_inject(install_sh_text: str): | ||
| assert ( | ||
| 'warn "could not inject proxy extras (proxy features may fail until manually fixed)."' | ||
| in install_sh_text | ||
| ) | ||
Uh oh!
There was an error while loading. Please reload this page.