Merge pull request #156 from sbidoul/refactor-installer-selection

Refactor installer selection, and use uv pip uninstall, uv pip freeze

news/156.feature

@@ -0,0 +1,2 @@
+Use ``uv`` for freeze and uninstall operations too when ``uvpip`` has been selected.
+Don't do the direct url fixup optimization hack when ``uvpip`` has been selected.

src/pip_deepfreeze/__main__.py

@@ -7,7 +7,7 @@
 from packaging.utils import canonicalize_name
 from packaging.version import Version
 
-from .pip import Installer
+from .pip import Installer, InstallerFlavor
 from .pyproject_toml import load_pyproject_toml
 from .sanity import check_env
 from .sync import sync as sync_operation
@@ -78,7 +78,7 @@ def sync(
             "Can be specified multiple times."
         ),
     ),
-    installer: Installer = typer.Option(
+    installer: InstallerFlavor = typer.Option(
         "pip",
     ),
 ) -> None:
@@ -91,6 +91,7 @@ def sync(
     constraints. Optionally uninstall unneeded dependencies.
     """
     sync_operation(
+        Installer.create(flavor=installer, python=ctx.obj.python),
         ctx.obj.python,
         upgrade_all,
         comma_split(to_upgrade),
@@ -99,7 +100,6 @@ def sync(
         project_root=ctx.obj.project_root,
         pre_sync_commands=pre_sync_commands,
         post_sync_commands=post_sync_commands,
-        installer=installer,
     )
 
 

src/pip_deepfreeze/pip.py

@@ -1,8 +1,8 @@
 import json
-import os
 import shlex
 import sys
 import textwrap
+from abc import ABC, abstractmethod
 from enum import Enum
 from pathlib import Path
 from typing import Any, Dict, Iterable, List, Optional, Sequence, Tuple, TypedDict, cast
@@ -52,38 +52,71 @@ class PipInspectReport(TypedDict, total=False):
     environment: Dict[str, str]
 
 
-class Installer(str, Enum):
+class InstallerFlavor(str, Enum):
     pip = "pip"
     uvpip = "uvpip"
 
 
-def _pip_install_cmd_and_env(python: str) -> Tuple[List[str], Dict[str, str]]:
-    return [*get_pip_command(python), "install"], {}
+class Installer(ABC):
+    @abstractmethod
+    def install_cmd(self, python: str) -> List[str]: ...
 
+    @abstractmethod
+    def uninstall_cmd(self, python: str) -> List[str]: ...
 
-def _uv_pip_install_cmd_and_env(python: str) -> Tuple[List[str], Dict[str, str]]:
-    return [sys.executable, "-m", "uv", "pip", "install", "--python", python], {}
+    @abstractmethod
+    def freeze_cmd(self, python: str) -> List[str]: ...
 
+    @abstractmethod
+    def has_metadata_cache(self) -> bool:
+        """Whether the installer caches metadata preparation results."""
+        ...
 
-def _install_cmd_and_env(
-    installer: Installer, python: str
-) -> Tuple[List[str], Dict[str, str]]:
-    if installer == Installer.pip:
-        return _pip_install_cmd_and_env(python)
-    elif installer == Installer.uvpip:
-        if get_python_version_info(python) < (3, 7):
-            log_error("The 'uv' installer requires Python 3.7 or later.")
-            raise typer.Exit(1)
-        return _uv_pip_install_cmd_and_env(python)
-    raise NotImplementedError(f"Installer {installer} is not implemented.")
+    @classmethod
+    def create(cls, flavor: InstallerFlavor, python: str) -> "Installer":
+        if flavor == InstallerFlavor.pip:
+            return PipInstaller()
+        elif flavor == InstallerFlavor.uvpip:
+            if get_python_version_info(python) < (3, 7):
+                log_error("The 'uv' installer requires Python 3.7 or later.")
+                raise typer.Exit(1)
+            return UvpipInstaller()
+
+
+class PipInstaller(Installer):
+    def install_cmd(self, python: str) -> List[str]:
+        return [*get_pip_command(python), "install"]
+
+    def uninstall_cmd(self, python: str) -> List[str]:
+        return [*get_pip_command(python), "uninstall", "--yes"]
+
+    def freeze_cmd(self, python: str) -> List[str]:
+        return [*get_pip_command(python), "freeze", "--all"]
+
+    def has_metadata_cache(self) -> bool:
+        return False
+
+
+class UvpipInstaller(Installer):
+    def install_cmd(self, python: str) -> List[str]:
+        return [sys.executable, "-m", "uv", "pip", "install", "--python", python]
+
+    def uninstall_cmd(self, python: str) -> List[str]:
+        return [sys.executable, "-m", "uv", "pip", "uninstall", "--python", python]
+
+    def freeze_cmd(self, python: str) -> List[str]:
+        return [sys.executable, "-m", "uv", "pip", "freeze", "--python", python]
+
+    def has_metadata_cache(self) -> bool:
+        return True
 
 
 def pip_upgrade_project(
+    installer: Installer,
     python: str,
     constraints_filename: Path,
     project_root: Path,
     extras: Optional[Sequence[NormalizedName]] = None,
-    installer: Installer = Installer.pip,
     installer_options: Optional[List[str]] = None,
 ) -> None:
     """Upgrade a project.
@@ -138,7 +171,9 @@ def pip_upgrade_project(
     # 2. get installed frozen dependencies of project
     installed_reqs = {
         get_req_name(req_line): normalize_req_line(req_line)
-        for req_line in pip_freeze_dependencies(python, project_root, extras)[0]
+        for req_line in pip_freeze_dependencies(
+            installer, python, project_root, extras
+        )[0]
     }
     assert all(installed_reqs.keys())  # XXX user error instead?
     # 3. uninstall dependencies that do not match constraints
@@ -152,11 +187,11 @@ def pip_upgrade_project(
     if to_uninstall:
         to_uninstall_str = ",".join(to_uninstall)
         log_info(f"Uninstalling dependencies to update: {to_uninstall_str}")
-        pip_uninstall(python, to_uninstall)
+        pip_uninstall(installer, python, to_uninstall)
     # 4. install project with constraints
     project_name = get_project_name(python, project_root)
     log_info(f"Installing/updating {project_name}")
-    cmd, env = _install_cmd_and_env(installer, python)
+    cmd = installer.install_cmd(python)
     if installer_options:
         cmd.extend(installer_options)
     cmd.extend(
@@ -181,7 +216,7 @@ def pip_upgrade_project(
         log_debug(textwrap.indent(constraints, prefix="  "))
     else:
         log_debug(f"with empty {constraints_without_editables_filename}.")
-    check_call(cmd, env=dict(os.environ, **env))
+    check_call(cmd)
 
 
 def _pip_list__env_info_json(python: str) -> InstalledDistributions:
@@ -222,14 +257,18 @@ def pip_list(python: str) -> InstalledDistributions:
         return _pip_list__env_info_json(python)
 
 
-def pip_freeze(python: str) -> Iterable[str]:
+def pip_freeze(installer: Installer, python: str) -> Iterable[str]:
     """Run pip freeze."""
-    cmd = [*get_pip_command(python), "freeze", "--all"]
+    cmd = installer.freeze_cmd(python)
+    log_debug(f"Running {shlex.join(cmd)}")
     return check_output(cmd).splitlines()
 
 
 def pip_freeze_dependencies(
-    python: str, project_root: Path, extras: Optional[Sequence[NormalizedName]] = None
+    installer: Installer,
+    python: str,
+    project_root: Path,
+    extras: Optional[Sequence[NormalizedName]] = None,
 ) -> Tuple[List[str], List[str]]:
     """Run pip freeze, returning only dependencies of the project.
 
@@ -241,7 +280,7 @@ def pip_freeze_dependencies(
     """
     project_name = get_project_name(python, project_root)
     dependencies_names = list_installed_depends(pip_list(python), project_name, extras)
-    frozen_reqs = pip_freeze(python)
+    frozen_reqs = pip_freeze(installer, python)
     dependencies_reqs = []
     unneeded_reqs = []
     for frozen_req in frozen_reqs:
@@ -258,7 +297,10 @@ def pip_freeze_dependencies(
 
 
 def pip_freeze_dependencies_by_extra(
-    python: str, project_root: Path, extras: Sequence[NormalizedName]
+    installer: Installer,
+    python: str,
+    project_root: Path,
+    extras: Sequence[NormalizedName],
 ) -> Tuple[Dict[Optional[NormalizedName], List[str]], List[str]]:
     """Run pip freeze, returning only dependencies of the project.
 
@@ -272,7 +314,7 @@ def pip_freeze_dependencies_by_extra(
     dependencies_by_extras = list_installed_depends_by_extra(
         pip_list(python), project_name
     )
-    frozen_reqs = pip_freeze(python)
+    frozen_reqs = pip_freeze(installer, python)
     dependencies_reqs = {}  # type: Dict[Optional[NormalizedName], List[str]]
     for extra in extras:
         if extra not in dependencies_by_extras:
@@ -301,12 +343,15 @@ def pip_freeze_dependencies_by_extra(
     return dependencies_reqs, unneeded_reqs
 
 
-def pip_uninstall(python: str, requirements: Iterable[str]) -> None:
+def pip_uninstall(
+    installer: Installer, python: str, requirements: Iterable[str]
+) -> None:
     """Uninstall packages."""
     reqs = list(requirements)
     if not reqs:
         return
-    cmd = [*get_pip_command(python), "uninstall", "--yes", *reqs]
+    cmd = [*installer.uninstall_cmd(python), *reqs]
+    log_debug(f"Running {shlex.join(cmd)}")
     check_call(cmd)
 
 

src/pip_deepfreeze/sync.py

@@ -53,6 +53,7 @@ def _constraints_path(project_root: Path) -> Path:
 
 
 def sync(
+    installer: Installer,
     python: str,
     upgrade_all: bool,
     to_upgrade: List[str],
@@ -61,7 +62,6 @@ def sync(
     project_root: Path,
     pre_sync_commands: Sequence[str] = (),
     post_sync_commands: Sequence[str] = (),
-    installer: Installer = Installer.pip,
 ) -> None:
     # run pre-sync commands
     run_commands(pre_sync_commands, project_root, "pre-sync")
@@ -86,16 +86,16 @@ def sync(
             else:
                 print(req_line.raw_line, file=constraints)
     pip_upgrade_project(
+        installer,
         python,
         merged_constraints_path,
         project_root,
         extras=extras,
-        installer=installer,
         installer_options=installer_options,
     )
     # freeze dependencies
     frozen_reqs_by_extra, unneeded_reqs = pip_freeze_dependencies_by_extra(
-        python, project_root, extras
+        installer, python, project_root, extras
     )
     for extra, frozen_reqs in frozen_reqs_by_extra.items():
         frozen_requirements_path = make_frozen_requirements_path(project_root, extra)
@@ -141,14 +141,15 @@ def sync(
             prompted = True
         if uninstall_unneeded:
             log_info(f"Uninstalling unneeded distributions: {unneeded_reqs_str}")
-            pip_uninstall(python, unneeded_req_names)
+            pip_uninstall(installer, python, unneeded_req_names)
         elif not prompted:
             log_debug(
                 f"The following distributions "
                 f"that are not dependencies of {project_name_with_extras} "
                 f"are also installed: {unneeded_reqs_str}"
             )
     # fixup VCS direct_url.json (see fixup-vcs-direct-urls.py for details on why)
-    pip_fixup_vcs_direct_urls(python)
+    if not installer.has_metadata_cache():
+        pip_fixup_vcs_direct_urls(python)
     # run post-sync commands
     run_commands(post_sync_commands, project_root, "post-sync")

tests/test_fixup_direct_url.py

@@ -1,52 +1,58 @@
 import subprocess
 
-from pip_deepfreeze.pip import pip_fixup_vcs_direct_urls, pip_freeze
+import pytest
+
+from pip_deepfreeze.pip import (
+    Installer,
+    PipInstaller,
+    UvpipInstaller,
+    pip_fixup_vcs_direct_urls,
+    pip_freeze,
+)
 
 
 def test_fixup_vcs_direct_url_branch_fake_commit(virtualenv_python: str) -> None:
     """When installing from a git branch, the commit_id in direct_url.json is replaced
     with a fake one.
     """
+    installer = PipInstaller()
     subprocess.check_call(
         [
-            virtualenv_python,
-            "-m",
-            "pip",
-            "install",
+            *installer.install_cmd(virtualenv_python),
             "git+https://github.com/PyPA/pip-test-package",
         ]
     )
-    frozen = "\n".join(pip_freeze(virtualenv_python))
+    frozen = "\n".join(pip_freeze(installer, virtualenv_python))
     assert "git+https://github.com/PyPA/pip-test-package" in frozen
     # fake commit NOT in direct_url.json
     assert f"git+https://github.com/PyPA/pip-test-package@{'f'*40}" not in frozen
     pip_fixup_vcs_direct_urls(virtualenv_python)
-    frozen = "\n".join(pip_freeze(virtualenv_python))
+    frozen = "\n".join(pip_freeze(installer, virtualenv_python))
     # fake commit in direct_url.json
     assert f"git+https://github.com/PyPA/pip-test-package@{'f'*40}" in frozen
 
 
-def test_fixup_vcs_direct_url_commit(virtualenv_python: str) -> None:
+@pytest.mark.parametrize("installer", [PipInstaller(), UvpipInstaller()])
+def test_fixup_vcs_direct_url_commit(
+    installer: Installer, virtualenv_python: str
+) -> None:
     """When installing from a git commit, the commit_id in direct_url.json is left
     untouched.
     """
     subprocess.check_call(
         [
-            virtualenv_python,
-            "-m",
-            "pip",
-            "install",
+            *installer.install_cmd(virtualenv_python),
             "git+https://github.com/PyPA/pip-test-package"
             "@5547fa909e83df8bd743d3978d6667497983a4b7",
         ]
     )
-    frozen = "\n".join(pip_freeze(virtualenv_python))
+    frozen = "\n".join(pip_freeze(installer, virtualenv_python))
     assert (
         "git+https://github.com/PyPA/pip-test-package"
         "@5547fa909e83df8bd743d3978d6667497983a4b7" in frozen
     )
     pip_fixup_vcs_direct_urls(virtualenv_python)
-    frozen = "\n".join(pip_freeze(virtualenv_python))
+    frozen = "\n".join(pip_freeze(installer, virtualenv_python))
     assert (
         "git+https://github.com/PyPA/pip-test-package"
         "@5547fa909e83df8bd743d3978d6667497983a4b7" in frozen