From 25d7b960432b59f0ed8e8b361a7e5711b74ee154 Mon Sep 17 00:00:00 2001 From: Ivan Date: Thu, 23 Apr 2026 02:41:12 -0500 Subject: [PATCH] refactor(repository-server): remove pip dependency from wheel downloading process, streamline to use only urllib for fetching wheels --- .../src/backend/repository_server_manager.py | 113 +----------------- scripts/build/fetch_repository_wheels.py | 2 +- .../backend/test_repository_server_manager.py | 61 ++++------ 3 files changed, 29 insertions(+), 147 deletions(-) diff --git a/meshchatx/src/backend/repository_server_manager.py b/meshchatx/src/backend/repository_server_manager.py index c74c482..ee3ea22 100644 --- a/meshchatx/src/backend/repository_server_manager.py +++ b/meshchatx/src/backend/repository_server_manager.py @@ -12,8 +12,6 @@ import os import re import shutil import socketserver -import subprocess -import sys import threading import urllib.error import urllib.parse @@ -49,7 +47,7 @@ def bundled_pip_targets() -> tuple[str, ...]: def meshchat_bundle_project_root() -> Path | None: - """Directory containing ``pyproject.toml`` for this MeshChatX tree (for local ``pip wheel``).""" + """Directory containing ``pyproject.toml`` for this MeshChatX tree (repo layout helper).""" here = Path(__file__).resolve() for anc in here.parents: meta = anc / "pyproject.toml" @@ -83,29 +81,6 @@ def _pip_spec_stem(spec: str) -> str: return s.strip() or spec.strip() -def _resolve_pip_argv(pip: str | list[str] | None) -> list[str] | None: - if isinstance(pip, list) and pip: - return pip[:] - if isinstance(pip, str) and pip.strip(): - return [pip.strip()] - exe = shutil.which("pip3") or shutil.which("pip") - if exe: - return [exe] - try: - proc = subprocess.run( - [sys.executable, "-m", "pip", "--version"], - capture_output=True, - text=True, - timeout=30, - check=False, - ) - if proc.returncode == 0: - return [sys.executable, "-m", "pip"] - except (OSError, subprocess.TimeoutExpired): - pass - return None - - def _pypi_project_json(canonical_name: str) -> dict[str, Any] | None: safe = urllib.parse.quote(canonical_name) url = f"https://pypi.org/pypi/{safe}/json" @@ -204,102 +179,28 @@ def _download_wheel_via_pypi_index( return True, None -def _pip_download_no_deps( - pip_argv: list[str], package_spec: str, dest: Path -) -> tuple[bool, str | None]: - try: - proc = subprocess.run( - [*pip_argv, "download", "--no-deps", "-d", str(dest), package_spec], - check=False, - capture_output=True, - text=True, - timeout=900, - ) - except subprocess.TimeoutExpired: - return False, "timeout" - except OSError as e: - return False, str(e) - if proc.returncode != 0: - err = (proc.stderr or proc.stdout or "").strip() or f"exit {proc.returncode}" - return False, err[:2000] - return True, None - - -def _bundle_meshchatx_wheel_pip( - pip_argv: list[str], dest: Path -) -> tuple[bool, str | None]: - root = meshchat_bundle_project_root() - if root is None: - return False, "meshchat_project_root_not_found" - try: - proc = subprocess.run( - [*pip_argv, "wheel", "--no-deps", "-w", str(dest), str(root)], - check=False, - capture_output=True, - text=True, - timeout=900, - ) - except subprocess.TimeoutExpired: - return False, "timeout" - except OSError as e: - return False, str(e) - if proc.returncode != 0: - err = (proc.stderr or proc.stdout or "").strip() or f"exit {proc.returncode}" - return False, err[:2000] - return True, None - - def download_bundled_wheels_to_directory( dest: Path, - pip: str | list[str] | None = None, *, on_package: Callable[[int, int, str], None] | None = None, ) -> dict[str, Any]: """Populate ``dest`` with wheels for :func:`bundled_pip_targets`. - Uses PyPI JSON + ``urllib`` (no pip CLI required). If ``pip`` resolves to an executable - or ``python -m pip``, failed HTTP steps fall back to ``pip download`` / ``pip wheel``. + Uses PyPI project metadata JSON and HTTPS downloads via ``urllib`` only. """ dest.mkdir(parents=True, exist_ok=True) packages = list(bundled_pip_targets()) total = len(packages) ok: list[str] = [] failed: dict[str, str] = {} - pip_argv = _resolve_pip_argv(pip) for i, pkg in enumerate(packages): if on_package is not None: on_package(i, total, pkg) - if pkg == _MESHCHATX_BUNDLE_PIP_NAME: - good = False - err: str | None = None - if pip_argv: - good, err = _bundle_meshchatx_wheel_pip(pip_argv, dest) - if not good: - g2, e2 = _download_wheel_via_pypi_index(pkg, dest) - if g2: - good, err = True, None - elif err is None: - err = e2 - elif e2: - err = f"{err}; pypi:{e2}" - if good: - ok.append(pkg) - else: - failed[pkg] = err or "wheel_failed" - continue - g_http, e_http = _download_wheel_via_pypi_index(pkg, dest) if g_http: ok.append(pkg) - continue - if pip_argv: - g_pip, e_pip = _pip_download_no_deps(pip_argv, pkg, dest) - if g_pip: - ok.append(pkg) - continue - failed[pkg] = f"pypi:{e_http or 'failed'}; pip:{e_pip}" else: - failed[pkg] = f"pypi:{e_http or 'failed'}; pip:unavailable" + failed[pkg] = f"pypi:{e_http or 'failed'}" return { "ok": bool(ok), "downloaded": ok, @@ -473,7 +374,7 @@ def _safe_any_upload_filename(name: str) -> str | None: class RepositoryServerManager: - """Keeps user uploads and a ``bundled`` directory of wheels (PyPI HTTP + optional pip).""" + """Keeps user uploads and a ``bundled`` directory of wheels (PyPI over HTTPS, stdlib only).""" def __init__(self, storage_path: str, public_dir: str | None = None) -> None: self.root = os.path.join(storage_path, "repository-server") @@ -721,7 +622,7 @@ class RepositoryServerManager: } def refresh_bundled_wheels(self) -> dict[str, Any]: - """Download wheels into ``bundled_dir`` (PyPI over HTTPS; pip optional fallback).""" + """Download wheels into ``bundled_dir`` (PyPI JSON + ``urllib``).""" self._last_refresh_error = None self._last_refresh_ok = [] self._last_refresh_failed = {} @@ -745,9 +646,7 @@ class RepositoryServerManager: running=True, current=pkg, completed=i, total=t ) - result = download_bundled_wheels_to_directory( - dest, pip=None, on_package=_on_pkg - ) + result = download_bundled_wheels_to_directory(dest, on_package=_on_pkg) ok = result["downloaded"] failed = result["failed"] finally: diff --git a/scripts/build/fetch_repository_wheels.py b/scripts/build/fetch_repository_wheels.py index 4aa65e3..f34573a 100644 --- a/scripts/build/fetch_repository_wheels.py +++ b/scripts/build/fetch_repository_wheels.py @@ -71,7 +71,7 @@ def main(argv: list[str] | None = None) -> int: except OSError as e: logging.warning("Could not remove %s: %s", old, e) - result = download_bundled_wheels_to_directory(dest, pip=None) + result = download_bundled_wheels_to_directory(dest) failed = result.get("failed") or {} ok = result.get("downloaded") or [] if failed: diff --git a/tests/backend/test_repository_server_manager.py b/tests/backend/test_repository_server_manager.py index 0fe79bb..09be1ef 100644 --- a/tests/backend/test_repository_server_manager.py +++ b/tests/backend/test_repository_server_manager.py @@ -2,7 +2,6 @@ import time import urllib.request -from subprocess import CompletedProcess from unittest.mock import patch from meshchatx.src.backend.repository_server_manager import ( @@ -100,59 +99,43 @@ def test_refresh_invokes_bundled_downloader(mock_dl, tmp_path): out = mgr.refresh_bundled_wheels() assert out["ok"] is True mock_dl.assert_called_once() - assert mock_dl.call_args.kwargs.get("pip") is None + assert mock_dl.call_args.kwargs.get("on_package") is not None @patch("meshchatx.src.backend.repository_server_manager._download_wheel_via_pypi_index") -@patch("meshchatx.src.backend.repository_server_manager.subprocess.run") -@patch("meshchatx.src.backend.repository_server_manager.shutil.which") -def test_download_bundled_wheels_to_directory( - mock_which, mock_run, mock_pypi, tmp_path, monkeypatch -): +def test_download_bundled_wheels_to_directory(mock_pypi, tmp_path, monkeypatch): monkeypatch.setenv("MESHCHAT_REPOSITORY_EXTRA_PIP", "") - mock_which.return_value = "/fake/pip3" - mock_pypi.return_value = (False, "offline") - - def fake_run(cmd, **kwargs): - assert cmd[0] == "/fake/pip3" - assert cmd[1] in ("download", "wheel") - assert cmd[2] == "--no-deps" - assert cmd[3] in ("-d", "-w") - return CompletedProcess(cmd, 0, "", "") - - mock_run.side_effect = fake_run + mock_pypi.return_value = (True, None) dest = tmp_path / "out" - out = download_bundled_wheels_to_directory(dest, "/fake/pip3") - assert out["ok"] is True + out = download_bundled_wheels_to_directory(dest) n = len(bundled_pip_targets()) - assert mock_run.call_count == n - assert mock_pypi.call_count == n - 1 + assert out["ok"] is True + assert len(out["downloaded"]) == n + assert not out["failed"] + assert mock_pypi.call_count == n @patch("meshchatx.src.backend.repository_server_manager._download_wheel_via_pypi_index") -@patch("meshchatx.src.backend.repository_server_manager.subprocess.run") -@patch("meshchatx.src.backend.repository_server_manager.shutil.which") -def test_refresh_calls_pip_after_pypi_fails( - mock_which, mock_run, mock_pypi, tmp_path, monkeypatch -): +def test_download_bundled_wheels_records_pypi_failures(mock_pypi, tmp_path, monkeypatch): monkeypatch.setenv("MESHCHAT_REPOSITORY_EXTRA_PIP", "") - mock_which.return_value = "/fake/pip3" mock_pypi.return_value = (False, "offline") + dest = tmp_path / "out" + out = download_bundled_wheels_to_directory(dest) + n = len(bundled_pip_targets()) + assert out["ok"] is False + assert not out["downloaded"] + assert len(out["failed"]) == n + assert mock_pypi.call_count == n - def fake_run(cmd, **kwargs): - assert cmd[0] == "/fake/pip3" - assert cmd[1] in ("download", "wheel") - assert cmd[2] == "--no-deps" - assert cmd[3] in ("-d", "-w") - return CompletedProcess(cmd, 0, "", "") - mock_run.side_effect = fake_run +@patch("meshchatx.src.backend.repository_server_manager._download_wheel_via_pypi_index") +def test_refresh_bundled_wheels_fails_when_pypi_unavailable(mock_pypi, tmp_path, monkeypatch): + monkeypatch.setenv("MESHCHAT_REPOSITORY_EXTRA_PIP", "") + mock_pypi.return_value = (False, "offline") mgr = RepositoryServerManager(str(tmp_path)) out = mgr.refresh_bundled_wheels() - assert out["ok"] is True - n = len(bundled_pip_targets()) - assert mock_run.call_count == n - assert mock_pypi.call_count == n - 1 + assert out["ok"] is False + assert not out["downloaded"] def test_http_start_stop_and_status(tmp_path):