From 034cbde2f7b650067e4ca5b33dfd2ca0b6c78f1c Mon Sep 17 00:00:00 2001 From: emil Date: Tue, 23 Jun 2026 23:01:32 +0000 Subject: [PATCH] DEVX-19: fix: retry pip install with --ignore-installed only on failure --- .taskid | 2 +- src/devx/tools/setup.py | 23 ++++++++++++++++------- tests/unit/test_setup.py | 37 +++++++++++++++++++++++++++++-------- 3 files changed, 46 insertions(+), 16 deletions(-) diff --git a/.taskid b/.taskid index 5e9ca08..85d64d7 100644 --- a/.taskid +++ b/.taskid @@ -1 +1 @@ -DEVX-18 +DEVX-19 diff --git a/src/devx/tools/setup.py b/src/devx/tools/setup.py index 7243df4..18092a8 100644 --- a/src/devx/tools/setup.py +++ b/src/devx/tools/setup.py @@ -26,15 +26,24 @@ def _run(cmd: list[str]) -> None: def _install_python_deps(bin_dir: str, extras: str = "dev") -> None: - """Install the project with the specified extras in editable mode.""" + """Install the project with the specified extras in editable mode. + + In CI (system Python with PIP_BREAK_SYSTEM_PACKAGES=1), a first attempt + uses --break-system-packages. If that fails (e.g. debian-installed + packages without RECORD files), retry with --ignore-installed to skip + uninstalling system packages entirely. + """ pip = str(Path(bin_dir) / "pip") cmd = [pip, "install", "-e", f".[{extras}]"] - # In CI (system Python), --break-system-packages allows installing to - # system site-packages, and --ignore-installed avoids uninstall failures - # for debian-installed packages (e.g. platformdirs) that lack RECORD files. if os.environ.get("PIP_BREAK_SYSTEM_PACKAGES") == "1": - cmd.extend(["--break-system-packages", "--ignore-installed"]) - _run(cmd) + cmd.append("--break-system-packages") + result = subprocess.run(cmd, check=False) # nosec B603 + if result.returncode != 0 and os.environ.get("PIP_BREAK_SYSTEM_PACKAGES") == "1": + click.echo(" Retrying with --ignore-installed to bypass system packages...") + cmd.append("--ignore-installed") + _run(cmd) + elif result.returncode != 0: + raise subprocess.CalledProcessError(result.returncode, cmd) def _install_pre_commit_hooks(bin_dir: str) -> None: @@ -46,7 +55,7 @@ def _install_pre_commit_hooks(bin_dir: str) -> None: def _install_ansible_collections(bin_dir: str) -> None: """Install required Ansible Galaxy collections if requirements exist.""" - galaxy = str(Path(bin_dir) / "ansible-galaxy") + galaxy = shutil.which("ansible-galaxy") or str(Path(bin_dir) / "ansible-galaxy") requirements = Path("ansible/requirements.yml") if not requirements.exists(): click.echo(" ansible/requirements.yml not found — skipping collections.") diff --git a/tests/unit/test_setup.py b/tests/unit/test_setup.py index 024bdf5..ea8ae46 100644 --- a/tests/unit/test_setup.py +++ b/tests/unit/test_setup.py @@ -33,29 +33,49 @@ class TestRun: class TestInstallPythonDeps: - @patch("devx.tools.setup._run") + @patch("devx.tools.setup.subprocess.run") def test_install_dev(self, mock_run: MagicMock) -> None: + mock_run.return_value = MagicMock(returncode=0) _install_python_deps(".venv/bin", "dev") - mock_run.assert_called_once_with([".venv/bin/pip", "install", "-e", ".[dev]"]) + mock_run.assert_called_once_with([".venv/bin/pip", "install", "-e", ".[dev]"], check=False) - @patch("devx.tools.setup._run") + @patch("devx.tools.setup.subprocess.run") def test_install_ci(self, mock_run: MagicMock) -> None: + mock_run.return_value = MagicMock(returncode=0) _install_python_deps(".venv/bin", "ci") - mock_run.assert_called_once_with([".venv/bin/pip", "install", "-e", ".[ci]"]) + mock_run.assert_called_once_with([".venv/bin/pip", "install", "-e", ".[ci]"], check=False) - @patch("devx.tools.setup._run") + @patch("devx.tools.setup.subprocess.run") def test_install_custom_extras(self, mock_run: MagicMock) -> None: + mock_run.return_value = MagicMock(returncode=0) _install_python_deps(".venv/bin", "ci,lint") - mock_run.assert_called_once_with([".venv/bin/pip", "install", "-e", ".[ci,lint]"]) + mock_run.assert_called_once_with([".venv/bin/pip", "install", "-e", ".[ci,lint]"], check=False) + + @patch("devx.tools.setup.subprocess.run") + def test_install_with_break_system_packages(self, mock_run: MagicMock) -> None: + mock_run.return_value = MagicMock(returncode=0) + with patch.dict(os.environ, {"PIP_BREAK_SYSTEM_PACKAGES": "1"}): + _install_python_deps(".venv/bin", "ci") + mock_run.assert_called_once_with( + [".venv/bin/pip", "install", "-e", ".[ci]", "--break-system-packages"], check=False + ) @patch("devx.tools.setup._run") - def test_install_with_break_system_packages(self, mock_run: MagicMock) -> None: + @patch("devx.tools.setup.subprocess.run") + def test_install_retry_with_ignore_installed(self, mock_subprocess: MagicMock, mock_run: MagicMock) -> None: + mock_subprocess.return_value = MagicMock(returncode=1) with patch.dict(os.environ, {"PIP_BREAK_SYSTEM_PACKAGES": "1"}): _install_python_deps(".venv/bin", "ci") mock_run.assert_called_once_with( [".venv/bin/pip", "install", "-e", ".[ci]", "--break-system-packages", "--ignore-installed"] ) + @patch("devx.tools.setup.subprocess.run") + def test_install_failure_without_break_system(self, mock_run: MagicMock) -> None: + mock_run.return_value = MagicMock(returncode=1) + with pytest.raises(subprocess.CalledProcessError): + _install_python_deps(".venv/bin", "ci") + class TestInstallPreCommitHooks: @patch("devx.tools.setup._run") @@ -69,8 +89,9 @@ class TestInstallPreCommitHooks: class TestInstallAnsibleCollections: + @patch("devx.tools.setup.shutil.which", return_value="/usr/local/bin/ansible-galaxy") @patch("devx.tools.setup._run") - def test_installs_from_requirements(self, mock_run: MagicMock, tmp_path: Path) -> None: + def test_installs_from_requirements(self, mock_run: MagicMock, mock_which: MagicMock, tmp_path: Path) -> None: req = tmp_path / "ansible" / "requirements.yml" req.parent.mkdir(parents=True) req.write_text("collections: []")