From 1c96c5c15a7019092e94e7a8ad2dccf43e880684 Mon Sep 17 00:00:00 2001 From: Siddhartha Bagaria Date: Wed, 14 Jun 2023 07:51:08 +0000 Subject: [PATCH 01/16] Sync direct references with hashes --- piptools/_compat/pip_compat.py | 13 ++++++------- piptools/scripts/sync.py | 2 +- piptools/sync.py | 32 +++++++++++++++++++++++++++++--- piptools/utils.py | 12 +++++++++--- tests/conftest.py | 29 ++++++++++++++++++++++------- tests/test_sync.py | 28 +++++++++++++++++++++------- 6 files changed, 88 insertions(+), 28 deletions(-) diff --git a/piptools/_compat/pip_compat.py b/piptools/_compat/pip_compat.py index cb6df94a0..ff0f4427f 100644 --- a/piptools/_compat/pip_compat.py +++ b/piptools/_compat/pip_compat.py @@ -1,7 +1,7 @@ from __future__ import annotations import optparse -from typing import Callable, Iterable, Iterator, cast +from typing import Iterable, Iterator import pip from pip._internal.cache import WheelCache @@ -52,13 +52,12 @@ def _uses_pkg_resources() -> bool: uses_pkg_resources = _uses_pkg_resources() if uses_pkg_resources: - from operator import methodcaller - from pip._vendor.pkg_resources import Distribution - dist_requires = cast( - Callable[[Distribution], Iterable[Requirement]], methodcaller("requires") - ) + def dist_requires(dist: Distribution) -> Iterable[Requirement]: + res: Iterable[Requirement] = dist._dist.requires() + return res + else: from pip._internal.metadata import select_backend @@ -68,7 +67,7 @@ def dist_requires(dist: Distribution) -> Iterable[Requirement]: """Mimics pkg_resources.Distribution.requires for the case of no extras. This doesn't fulfill that API's `extras` parameter but satisfies the needs of pip-tools.""" - reqs = (Requirement.parse(req) for req in (dist.requires or ())) + reqs = (Requirement.parse(req) for req in (dist._dist.requires or ())) return [ req for req in reqs diff --git a/piptools/scripts/sync.py b/piptools/scripts/sync.py index d504d889a..d7a1f6a18 100755 --- a/piptools/scripts/sync.py +++ b/piptools/scripts/sync.py @@ -303,4 +303,4 @@ def _get_installed_distributions( user_only=user_only, skip=[], ) - return [cast(Distribution, dist)._dist for dist in dists] + return [cast(Distribution, dist) for dist in dists] diff --git a/piptools/sync.py b/piptools/sync.py index 184d78a1b..317bf94b5 100644 --- a/piptools/sync.py +++ b/piptools/sync.py @@ -9,8 +9,13 @@ import click from pip._internal.commands.freeze import DEV_PKGS +from pip._internal.models.direct_url import ArchiveInfo from pip._internal.req import InstallRequirement from pip._internal.utils.compat import stdlib_pkgs +from pip._internal.utils.direct_url_helpers import ( + direct_url_as_pep440_direct_reference, + direct_url_from_link, +) from ._compat.pip_compat import Distribution, dist_requires from .exceptions import IncompatibleRequirements @@ -120,10 +125,10 @@ def diff_key_from_ireq(ireq: InstallRequirement) -> str: """ Calculate a key for comparing a compiled requirement with installed modules. For URL requirements, only provide a useful key if the url includes - #egg=name==version, which will set ireq.req.name and ireq.specifier. + a hash, e.g. #sha1=..., in any of the supported hash algorithms. Otherwise return ireq.link so the key will not match and the package will reinstall. Reinstall is necessary to ensure that packages will reinstall - if the URL is changed but the version is not. + if the contents at the URL have changed but the version has not. """ if is_url_requirement(ireq): if ( @@ -132,10 +137,31 @@ def diff_key_from_ireq(ireq: InstallRequirement) -> str: and ireq.specifier ): return key_from_ireq(ireq) + if getattr(ireq.req, "name", None) and ireq.link.has_hash: + return str( + direct_url_as_pep440_direct_reference( + direct_url_from_link(ireq.link), ireq.req.name + ) + ) + # TODO: Also support VCS and editable installs. return str(ireq.link) return key_from_ireq(ireq) +def diff_key_from_req(req: Distribution) -> str: + """Get a unique key for the requirement.""" + key = key_from_req(req) + if ( + hasattr(req, "direct_url") + and req.direct_url + and type(req.direct_url.info) == ArchiveInfo + and req.direct_url.info.hash + ): + key = direct_url_as_pep440_direct_reference(req.direct_url, key) + # TODO: Also support VCS and editable installs. + return key + + def diff( compiled_requirements: Iterable[InstallRequirement], installed_dists: Iterable[Distribution], @@ -152,7 +178,7 @@ def diff( pkgs_to_ignore = get_dists_to_ignore(installed_dists) for dist in installed_dists: - key = key_from_req(dist) + key = diff_key_from_req(dist) if key not in requirements_lut or not requirements_lut[key].match_markers(): to_uninstall.add(key) elif requirements_lut[key].specifier.contains(dist.version): diff --git a/piptools/utils.py b/piptools/utils.py index 4374b74a9..ff2f80b35 100644 --- a/piptools/utils.py +++ b/piptools/utils.py @@ -68,11 +68,17 @@ def key_from_ireq(ireq: InstallRequirement) -> str: def key_from_req(req: InstallRequirement | Distribution | Requirement) -> str: """Get an all-lowercase version of the requirement's name.""" if hasattr(req, "key"): - # from pkg_resources, such as installed dists for pip-sync + # From pkg_resources, such as installed dists for pip-sync. key = req.key else: - # from packaging, such as install requirements from requirements.txt - key = req.name + # From packaging, such as install requirements from requirements.txt. + # For installed distributions, pip internal backends derive from + # pip._internal.metadata.BaseDistribution, and have canonical_name + # instead of name. + if hasattr(req, "canonical_name"): + key = req.canonical_name + else: + key = req.name return str(canonicalize_name(key)) diff --git a/tests/conftest.py b/tests/conftest.py index 70f0e0e90..868ed44d4 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -18,11 +18,13 @@ from pip._internal.commands.install import InstallCommand from pip._internal.index.package_finder import PackageFinder from pip._internal.models.candidate import InstallationCandidate +from pip._internal.models.link import Link from pip._internal.network.session import PipSession from pip._internal.req.constructors import ( install_req_from_editable, install_req_from_line, ) +from pip._internal.utils.direct_url_helpers import direct_url_from_link from pip._vendor.packaging.version import Version from pip._vendor.pkg_resources import Requirement @@ -125,20 +127,17 @@ def command(self) -> InstallCommand: """Not used""" -class FakeInstalledDistribution: - def __init__(self, line, deps=None): +class InnerFakeInstalledDistribution: + def __init__(self, req, version, deps=None): + self.req = req + self.version = version if deps is None: deps = [] self.dep_strs = deps self.deps = [Requirement.parse(d) for d in deps] - - self.req = Requirement.parse(line) - self.key = key_from_req(self.req) self.specifier = self.req.specifier - self.version = line.split("==")[1] - # The Distribution interface has changed between pkg_resources and # importlib.metadata. if uses_pkg_resources: @@ -153,6 +152,22 @@ def requires(self): return self.dep_strs +class FakeInstalledDistribution: + def __init__(self, line, deps=None): + req = Requirement.parse(line) + if "==" in line: + version = line.split("==")[1] + else: + version = "0+unknown" + + self._dist = InnerFakeInstalledDistribution(req, version, deps) + self.key = self._dist.key + self.canonical_name = self._dist.req.project_name + self.version = version + if req.url: + self.direct_url = direct_url_from_link(Link(req.url)) + + def pytest_collection_modifyitems(config, items): for item in items: # Mark network tests as flaky diff --git a/tests/test_sync.py b/tests/test_sync.py index 2eabaaa78..cd0b26d7b 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -263,27 +263,41 @@ def test_diff_with_editable(fake_dist, from_editable): assert package.link.url == path_to_url(path_to_package) -def test_diff_with_matching_url_versions(fake_dist, from_line): +def test_diff_with_matching_url_hash(fake_dist, from_line): # if URL version is explicitly provided, use it to avoid reinstalling - installed = [fake_dist("example==1.0")] - reqs = [from_line("file:///example.zip#egg=example==1.0")] + line = "example@file:///example.zip#sha1=abc" + installed = [fake_dist(line)] + reqs = [from_line(line)] to_install, to_uninstall = diff(reqs, installed) assert to_install == set() assert to_uninstall == set() -def test_diff_with_no_url_versions(fake_dist, from_line): - # if URL version is not provided, assume the contents have +def test_diff_with_no_url_hash(fake_dist, from_line): + # if URL hash is not provided, assume the contents have # changed and reinstall - installed = [fake_dist("example==1.0")] - reqs = [from_line("file:///example.zip#egg=example")] + line = "example@file:///example.zip" + installed = [fake_dist(line)] + reqs = [from_line(line)] to_install, to_uninstall = diff(reqs, installed) assert to_install == set(reqs) assert to_uninstall == {"example"} +def test_diff_with_unequal_url_hash(fake_dist, from_line): + # if URL hashes mismatch, assume the contents have + # changed and reinstall + line = "example@file:///example.zip#" + installed = [fake_dist(line + "#sha1=abc")] + reqs = [from_line(line + "#sha1=def")] + + to_install, to_uninstall = diff(reqs, installed) + assert to_install == set(reqs) + assert to_uninstall == {"example @ file:///example.zip#sha1=abc"} + + def test_sync_install_temporary_requirement_file( from_line, from_editable, mocked_tmp_req_file ): From cfb0b91a2120cffdbb8470a0a8e6a7034817175f Mon Sep 17 00:00:00 2001 From: Siddhartha Bagaria Date: Wed, 14 Jun 2023 17:57:29 +0000 Subject: [PATCH 02/16] Clean up typing for Distribution --- piptools/_compat/pip_compat.py | 5 +++-- piptools/utils.py | 18 ++++++++---------- 2 files changed, 11 insertions(+), 12 deletions(-) diff --git a/piptools/_compat/pip_compat.py b/piptools/_compat/pip_compat.py index ff0f4427f..505105760 100644 --- a/piptools/_compat/pip_compat.py +++ b/piptools/_compat/pip_compat.py @@ -52,7 +52,7 @@ def _uses_pkg_resources() -> bool: uses_pkg_resources = _uses_pkg_resources() if uses_pkg_resources: - from pip._vendor.pkg_resources import Distribution + from pip._internal.metadata.pkg_resources import Distribution def dist_requires(dist: Distribution) -> Iterable[Requirement]: res: Iterable[Requirement] = dist._dist.requires() @@ -60,8 +60,9 @@ def dist_requires(dist: Distribution) -> Iterable[Requirement]: else: from pip._internal.metadata import select_backend + from pip._internal.metadata.importlib import Distribution - Distribution = select_backend().Distribution + assert select_backend().Distribution is Distribution def dist_requires(dist: Distribution) -> Iterable[Requirement]: """Mimics pkg_resources.Distribution.requires for the case of no diff --git a/piptools/utils.py b/piptools/utils.py index ff2f80b35..052f0296f 100644 --- a/piptools/utils.py +++ b/piptools/utils.py @@ -19,6 +19,7 @@ import click from click.utils import LazyFile +from ._compat.pip_compat import Distribution from pip._internal.req import InstallRequirement from pip._internal.req.constructors import install_req_from_line, parse_req_from_line from pip._internal.utils.misc import redact_auth_from_url @@ -27,7 +28,7 @@ from pip._vendor.packaging.specifiers import SpecifierSet from pip._vendor.packaging.utils import canonicalize_name from pip._vendor.packaging.version import Version -from pip._vendor.pkg_resources import Distribution, Requirement, get_distribution +from pip._vendor.pkg_resources import Requirement, get_distribution from piptools._compat import PIP_VERSION from piptools.locations import CONFIG_FILE_NAME @@ -67,18 +68,15 @@ def key_from_ireq(ireq: InstallRequirement) -> str: def key_from_req(req: InstallRequirement | Distribution | Requirement) -> str: """Get an all-lowercase version of the requirement's name.""" + if isinstance(req, Distribution): + # Use the wrapped distribution object, not the pip internal one. + req = req._dist if hasattr(req, "key"): - # From pkg_resources, such as installed dists for pip-sync. + # from pkg_resources, such as installed dists for pip-sync. key = req.key else: - # From packaging, such as install requirements from requirements.txt. - # For installed distributions, pip internal backends derive from - # pip._internal.metadata.BaseDistribution, and have canonical_name - # instead of name. - if hasattr(req, "canonical_name"): - key = req.canonical_name - else: - key = req.name + # from packaging, such as install requirements from requirements.txt + key = req.name return str(canonicalize_name(key)) From 7088e695e33d6e6aee508cbe77f5d3625113e8c7 Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 14 Jun 2023 18:13:38 +0000 Subject: [PATCH 03/16] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- piptools/utils.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/piptools/utils.py b/piptools/utils.py index 052f0296f..ceec22ab5 100644 --- a/piptools/utils.py +++ b/piptools/utils.py @@ -19,7 +19,6 @@ import click from click.utils import LazyFile -from ._compat.pip_compat import Distribution from pip._internal.req import InstallRequirement from pip._internal.req.constructors import install_req_from_line, parse_req_from_line from pip._internal.utils.misc import redact_auth_from_url @@ -34,6 +33,8 @@ from piptools.locations import CONFIG_FILE_NAME from piptools.subprocess_utils import run_python_snippet +from ._compat.pip_compat import Distribution + if TYPE_CHECKING: from typing import Protocol else: From 4e04da0809cb58a3da1ed89cd5e53e4b12b24152 Mon Sep 17 00:00:00 2001 From: Siddhartha Bagaria Date: Wed, 14 Jun 2023 20:05:18 +0000 Subject: [PATCH 04/16] Clean up test class --- piptools/utils.py | 11 ++++++++--- tests/conftest.py | 6 +++--- tests/test_sync.py | 2 +- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/piptools/utils.py b/piptools/utils.py index ceec22ab5..badd8520c 100644 --- a/piptools/utils.py +++ b/piptools/utils.py @@ -69,11 +69,16 @@ def key_from_ireq(ireq: InstallRequirement) -> str: def key_from_req(req: InstallRequirement | Distribution | Requirement) -> str: """Get an all-lowercase version of the requirement's name.""" - if isinstance(req, Distribution): - # Use the wrapped distribution object, not the pip internal one. + if ( + isinstance(req, Distribution) + or req.__class__.__name__ == "FakeInstalledDistribution" + ): + # If this is a pip internal installed distribution (or the fake + # installed distribution used in tests), use the wrapped distribution + # object, not the pip internal one. req = req._dist if hasattr(req, "key"): - # from pkg_resources, such as installed dists for pip-sync. + # from pkg_resources, such as installed dists for pip-sync key = req.key else: # from packaging, such as install requirements from requirements.txt diff --git a/tests/conftest.py b/tests/conftest.py index 868ed44d4..4376b9a4e 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -128,6 +128,8 @@ def command(self) -> InstallCommand: class InnerFakeInstalledDistribution: + # Emulate relevant parts of the _dist attribute of + # piptools._compat.pip_compat.Distribution. def __init__(self, req, version, deps=None): self.req = req self.version = version @@ -153,16 +155,14 @@ def requires(self): class FakeInstalledDistribution: + # Emulate relevant parts of piptools._compat.pip_compat.Distribution. def __init__(self, line, deps=None): req = Requirement.parse(line) if "==" in line: version = line.split("==")[1] else: version = "0+unknown" - self._dist = InnerFakeInstalledDistribution(req, version, deps) - self.key = self._dist.key - self.canonical_name = self._dist.req.project_name self.version = version if req.url: self.direct_url = direct_url_from_link(Link(req.url)) diff --git a/tests/test_sync.py b/tests/test_sync.py index cd0b26d7b..a466334f1 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -68,7 +68,7 @@ def mocked_tmp_req_file(mocked_tmp_file): ) def test_dependency_tree(fake_dist, installed, root, expected): installed = { - distribution.key: distribution + distribution._dist.key: distribution for distribution in (fake_dist(name, deps) for name, deps in installed) } From 04569746758c7857b7042faabba7a0b0fc46ea93 Mon Sep 17 00:00:00 2001 From: Siddhartha Bagaria Date: Fri, 16 Jun 2023 18:19:32 +0000 Subject: [PATCH 05/16] Some more comments for the test class --- tests/conftest.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 4376b9a4e..050a83d15 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -129,7 +129,8 @@ def command(self) -> InstallCommand: class InnerFakeInstalledDistribution: # Emulate relevant parts of the _dist attribute of - # piptools._compat.pip_compat.Distribution. + # piptools._compat.pip_compat.Distribution. See note below in + # FakeInstalledDistribution. def __init__(self, req, version, deps=None): self.req = req self.version = version @@ -155,7 +156,12 @@ def requires(self): class FakeInstalledDistribution: - # Emulate relevant parts of piptools._compat.pip_compat.Distribution. + # Emulate relevant parts of piptools._compat.pip_compat.Distribution, which + # is currently aliasing a pip internal class implementing the protocol + # pip._internal.metadata.base.BaseDistribution. + # piptools uses only the version and direct_url fields directly from this + # type, and uses the delegate instance in the _dist attribute for the other + # values. def __init__(self, line, deps=None): req = Requirement.parse(line) if "==" in line: From 22cb2ca610252eb9c591c8406c7c0cdb7671b0c2 Mon Sep 17 00:00:00 2001 From: Siddhartha Bagaria Date: Fri, 16 Jun 2023 21:27:26 +0000 Subject: [PATCH 06/16] Adapter class --- piptools/_compat/__init__.py | 9 +++- piptools/_compat/pip_compat.py | 94 +++++++++++++++++----------------- piptools/scripts/sync.py | 5 +- piptools/sync.py | 10 +--- piptools/utils.py | 15 ++---- tests/conftest.py | 60 +++++----------------- tests/test_sync.py | 2 +- 7 files changed, 76 insertions(+), 119 deletions(-) diff --git a/piptools/_compat/__init__.py b/piptools/_compat/__init__.py index 78d29b387..27d7372f8 100644 --- a/piptools/_compat/__init__.py +++ b/piptools/_compat/__init__.py @@ -1,5 +1,10 @@ from __future__ import annotations -from .pip_compat import PIP_VERSION, create_wheel_cache, parse_requirements +from .pip_compat import ( + PIP_VERSION, + Distribution, + create_wheel_cache, + parse_requirements, +) -__all__ = ["PIP_VERSION", "parse_requirements", "create_wheel_cache"] +__all__ = ["PIP_VERSION", "Distribution", "parse_requirements", "create_wheel_cache"] diff --git a/piptools/_compat/pip_compat.py b/piptools/_compat/pip_compat.py index 505105760..951c629bf 100644 --- a/piptools/_compat/pip_compat.py +++ b/piptools/_compat/pip_compat.py @@ -1,11 +1,16 @@ from __future__ import annotations import optparse -from typing import Iterable, Iterator +from dataclasses import dataclass +from typing import Iterable, Iterator, Optional import pip from pip._internal.cache import WheelCache from pip._internal.index.package_finder import PackageFinder +from pip._internal.metadata import BaseDistribution +from pip._internal.metadata.importlib import Distribution as _ImportLibDist +from pip._internal.metadata.pkg_resources import Distribution as _PkgResourcesDist +from pip._internal.models.direct_url import DirectUrl from pip._internal.network.session import PipSession from pip._internal.req import InstallRequirement from pip._internal.req import parse_requirements as _parse_requirements @@ -15,11 +20,48 @@ PIP_VERSION = tuple(map(int, parse_version(pip.__version__).base_version.split("."))) -__all__ = [ - "dist_requires", - "uses_pkg_resources", - "Distribution", -] +# The Distribution interface has changed between pkg_resources and +# importlib.metadata, so this compat layer allows for a consistent access +# pattern. In pip 22.1, importlib.metadata became the default on Python 3.11 +# (and later), but is overridable. `select_backend` returns what's being used. + +@dataclass(frozen=True) +class Distribution: + key: str + version: str + requires: Iterable[Requirement] + direct_url: Optional[DirectUrl] + + @classmethod + def from_pip_distribution(cls, dist: BaseDistribution) -> Distribution: + # TODO: Use only the BaseDistribution protocol properties and methods + # instead of specializing by type. + if isinstance(dist, _PkgResourcesDist): + return cls._from_pkg_resources(dist) + if isinstance(dist, _ImportLibDist): + return cls._from_importlib(dist) + raise ValueError( + f"unsupported installed distribution class ({dist.__class__}): {dist}" + ) + + @classmethod + def _from_pkg_resources(cls, dist: _PkgResourcesDist) -> Distribution: + return cls( + dist._dist.key, dist._dist.version, dist._dist.requires(), dist.direct_url + ) + + @classmethod + def _from_importlib(cls, dist: _ImportLibDist) -> Distribution: + """Mimics pkg_resources.Distribution.requires for the case of no + extras. This doesn't fulfill that API's `extras` parameter but + satisfies the needs of pip-tools.""" + reqs = (Requirement.parse(req) for req in (dist._dist.requires or ())) + requires = [ + req + for req in reqs + if not req.marker or req.marker.evaluate({"extra": None}) + ] + return cls(dist._dist.name, dist._dist.version, requires, dist.direct_url) def parse_requirements( @@ -36,46 +78,6 @@ def parse_requirements( yield install_req_from_parsed_requirement(parsed_req, isolated=isolated) -# The Distribution interface has changed between pkg_resources and -# importlib.metadata, so this compat layer allows for a consistent access -# pattern. In pip 22.1, importlib.metadata became the default on Python 3.11 -# (and later), but is overridable. `select_backend` returns what's being used. - - -def _uses_pkg_resources() -> bool: - from pip._internal.metadata import select_backend - from pip._internal.metadata.pkg_resources import Distribution as _Dist - - return select_backend().Distribution is _Dist - - -uses_pkg_resources = _uses_pkg_resources() - -if uses_pkg_resources: - from pip._internal.metadata.pkg_resources import Distribution - - def dist_requires(dist: Distribution) -> Iterable[Requirement]: - res: Iterable[Requirement] = dist._dist.requires() - return res - -else: - from pip._internal.metadata import select_backend - from pip._internal.metadata.importlib import Distribution - - assert select_backend().Distribution is Distribution - - def dist_requires(dist: Distribution) -> Iterable[Requirement]: - """Mimics pkg_resources.Distribution.requires for the case of no - extras. This doesn't fulfill that API's `extras` parameter but - satisfies the needs of pip-tools.""" - reqs = (Requirement.parse(req) for req in (dist._dist.requires or ())) - return [ - req - for req in reqs - if not req.marker or req.marker.evaluate({"extra": None}) - ] - - def create_wheel_cache(cache_dir: str, format_control: str | None = None) -> WheelCache: kwargs: dict[str, str | None] = {"cache_dir": cache_dir} if PIP_VERSION[:2] <= (23, 0): diff --git a/piptools/scripts/sync.py b/piptools/scripts/sync.py index d7a1f6a18..984882a73 100755 --- a/piptools/scripts/sync.py +++ b/piptools/scripts/sync.py @@ -15,8 +15,7 @@ from pip._internal.metadata import get_environment from .. import sync -from .._compat import parse_requirements -from .._compat.pip_compat import Distribution +from .._compat import Distribution, parse_requirements from ..exceptions import PipToolsError from ..locations import CONFIG_FILE_NAME from ..logging import log @@ -303,4 +302,4 @@ def _get_installed_distributions( user_only=user_only, skip=[], ) - return [cast(Distribution, dist) for dist in dists] + return [Distribution.from_pip_distribution(dist) for dist in dists] diff --git a/piptools/sync.py b/piptools/sync.py index 317bf94b5..0cb722211 100644 --- a/piptools/sync.py +++ b/piptools/sync.py @@ -17,7 +17,7 @@ direct_url_from_link, ) -from ._compat.pip_compat import Distribution, dist_requires +from ._compat import Distribution from .exceptions import IncompatibleRequirements from .logging import log from .utils import ( @@ -66,7 +66,7 @@ def dependency_tree( dependencies.add(key) - for dep_specifier in dist_requires(v): + for dep_specifier in v.requires: dep_name = key_from_req(dep_specifier) if dep_name in installed_keys: dep = installed_keys[dep_name] @@ -131,12 +131,6 @@ def diff_key_from_ireq(ireq: InstallRequirement) -> str: if the contents at the URL have changed but the version has not. """ if is_url_requirement(ireq): - if ( - ireq.req - and (getattr(ireq.req, "key", None) or getattr(ireq.req, "name", None)) - and ireq.specifier - ): - return key_from_ireq(ireq) if getattr(ireq.req, "name", None) and ireq.link.has_hash: return str( direct_url_as_pep440_direct_reference( diff --git a/piptools/utils.py b/piptools/utils.py index badd8520c..d41770954 100644 --- a/piptools/utils.py +++ b/piptools/utils.py @@ -24,17 +24,16 @@ from pip._internal.utils.misc import redact_auth_from_url from pip._internal.vcs import is_url from pip._vendor.packaging.markers import Marker +from pip._vendor.packaging.requirements import Requirement from pip._vendor.packaging.specifiers import SpecifierSet from pip._vendor.packaging.utils import canonicalize_name from pip._vendor.packaging.version import Version -from pip._vendor.pkg_resources import Requirement, get_distribution +from pip._vendor.pkg_resources import get_distribution -from piptools._compat import PIP_VERSION +from piptools._compat import PIP_VERSION, Distribution from piptools.locations import CONFIG_FILE_NAME from piptools.subprocess_utils import run_python_snippet -from ._compat.pip_compat import Distribution - if TYPE_CHECKING: from typing import Protocol else: @@ -69,14 +68,6 @@ def key_from_ireq(ireq: InstallRequirement) -> str: def key_from_req(req: InstallRequirement | Distribution | Requirement) -> str: """Get an all-lowercase version of the requirement's name.""" - if ( - isinstance(req, Distribution) - or req.__class__.__name__ == "FakeInstalledDistribution" - ): - # If this is a pip internal installed distribution (or the fake - # installed distribution used in tests), use the wrapped distribution - # object, not the pip internal one. - req = req._dist if hasattr(req, "key"): # from pkg_resources, such as installed dists for pip-sync key = req.key diff --git a/tests/conftest.py b/tests/conftest.py index 050a83d15..0459cef44 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -28,7 +28,7 @@ from pip._vendor.packaging.version import Version from pip._vendor.pkg_resources import Requirement -from piptools._compat.pip_compat import PIP_VERSION, uses_pkg_resources +from piptools._compat import PIP_VERSION, Distribution from piptools.cache import DependencyCache from piptools.exceptions import NoCandidateFound from piptools.locations import CONFIG_FILE_NAME @@ -40,7 +40,6 @@ as_tuple, is_url_requirement, key_from_ireq, - key_from_req, make_install_requirement, ) @@ -127,51 +126,18 @@ def command(self) -> InstallCommand: """Not used""" -class InnerFakeInstalledDistribution: - # Emulate relevant parts of the _dist attribute of - # piptools._compat.pip_compat.Distribution. See note below in - # FakeInstalledDistribution. - def __init__(self, req, version, deps=None): - self.req = req - self.version = version - if deps is None: - deps = [] - self.dep_strs = deps - self.deps = [Requirement.parse(d) for d in deps] - self.key = key_from_req(self.req) - self.specifier = self.req.specifier - - # The Distribution interface has changed between pkg_resources and - # importlib.metadata. - if uses_pkg_resources: - - def requires(self): - return self.deps - +def parse_fake_distribution(line, deps=[]): + req = Requirement.parse(line) + key = req.key + if "==" in line: + version = line.split("==")[1] else: - - @property - def requires(self): - return self.dep_strs - - -class FakeInstalledDistribution: - # Emulate relevant parts of piptools._compat.pip_compat.Distribution, which - # is currently aliasing a pip internal class implementing the protocol - # pip._internal.metadata.base.BaseDistribution. - # piptools uses only the version and direct_url fields directly from this - # type, and uses the delegate instance in the _dist attribute for the other - # values. - def __init__(self, line, deps=None): - req = Requirement.parse(line) - if "==" in line: - version = line.split("==")[1] - else: - version = "0+unknown" - self._dist = InnerFakeInstalledDistribution(req, version, deps) - self.version = version - if req.url: - self.direct_url = direct_url_from_link(Link(req.url)) + version = "0+unknown" + requires = [Requirement.parse(d) for d in deps] + direct_url = None + if req.url: + direct_url = direct_url_from_link(Link(req.url)) + return Distribution(key, version, requires, direct_url) def pytest_collection_modifyitems(config, items): @@ -183,7 +149,7 @@ def pytest_collection_modifyitems(config, items): @pytest.fixture def fake_dist(): - return FakeInstalledDistribution + return parse_fake_distribution @pytest.fixture diff --git a/tests/test_sync.py b/tests/test_sync.py index a466334f1..cd0b26d7b 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -68,7 +68,7 @@ def mocked_tmp_req_file(mocked_tmp_file): ) def test_dependency_tree(fake_dist, installed, root, expected): installed = { - distribution._dist.key: distribution + distribution.key: distribution for distribution in (fake_dist(name, deps) for name, deps in installed) } From d7b86a075da6d91f5cd2c2735f7b66af7ffbebdc Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Fri, 16 Jun 2023 22:31:09 +0000 Subject: [PATCH 07/16] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- piptools/_compat/pip_compat.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/piptools/_compat/pip_compat.py b/piptools/_compat/pip_compat.py index 951c629bf..592c31835 100644 --- a/piptools/_compat/pip_compat.py +++ b/piptools/_compat/pip_compat.py @@ -25,12 +25,13 @@ # pattern. In pip 22.1, importlib.metadata became the default on Python 3.11 # (and later), but is overridable. `select_backend` returns what's being used. + @dataclass(frozen=True) class Distribution: key: str version: str requires: Iterable[Requirement] - direct_url: Optional[DirectUrl] + direct_url: DirectUrl | None @classmethod def from_pip_distribution(cls, dist: BaseDistribution) -> Distribution: From d5cb1dce4df0690ed1e8faf9c3b7c8ff446c0813 Mon Sep 17 00:00:00 2001 From: Siddhartha Bagaria Date: Fri, 16 Jun 2023 23:08:53 +0000 Subject: [PATCH 08/16] Add dep on importlib-metadata --- piptools/_compat/pip_compat.py | 2 +- pyproject.toml | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/piptools/_compat/pip_compat.py b/piptools/_compat/pip_compat.py index 592c31835..5e98840e1 100644 --- a/piptools/_compat/pip_compat.py +++ b/piptools/_compat/pip_compat.py @@ -2,7 +2,7 @@ import optparse from dataclasses import dataclass -from typing import Iterable, Iterator, Optional +from typing import Iterable, Iterator import pip from pip._internal.cache import WheelCache diff --git a/pyproject.toml b/pyproject.toml index 47fa90566..ff99767fb 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -39,6 +39,7 @@ dependencies = [ # direct dependencies "build", "click >= 8", + "importlib-metadata == 1.4; python_version < '3.8'", "pip >= 22.2", "tomli; python_version < '3.11'", # indirect dependencies From 3f5722c973463f2ce0e086eb8fc8c7fd9aa47d44 Mon Sep 17 00:00:00 2001 From: Siddhartha Bagaria Date: Fri, 16 Jun 2023 23:24:59 +0000 Subject: [PATCH 09/16] Try to resolve mypy error --- piptools/utils.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/piptools/utils.py b/piptools/utils.py index d41770954..03ba1a1cd 100644 --- a/piptools/utils.py +++ b/piptools/utils.py @@ -68,7 +68,9 @@ def key_from_ireq(ireq: InstallRequirement) -> str: def key_from_req(req: InstallRequirement | Distribution | Requirement) -> str: """Get an all-lowercase version of the requirement's name.""" - if hasattr(req, "key"): + if isinstance(req, Distribution): + key = req.key + elif hasattr(req, "key"): # from pkg_resources, such as installed dists for pip-sync key = req.key else: From 6a440975b79e26fa40cd6d4e65042dd33360578e Mon Sep 17 00:00:00 2001 From: Siddhartha Bagaria Date: Sat, 17 Jun 2023 00:44:06 +0000 Subject: [PATCH 10/16] Simplify a little more --- piptools/sync.py | 9 ++++----- piptools/utils.py | 14 +++----------- 2 files changed, 7 insertions(+), 16 deletions(-) diff --git a/piptools/sync.py b/piptools/sync.py index 0cb722211..f0e430993 100644 --- a/piptools/sync.py +++ b/piptools/sync.py @@ -60,7 +60,7 @@ def dependency_tree( while queue: v = queue.popleft() - key = key_from_req(v) + key = v.key if key in dependencies: continue @@ -86,7 +86,7 @@ def get_dists_to_ignore(installed: Iterable[Distribution]) -> list[str]: locally, click should also be installed/uninstalled depending on the given requirements. """ - installed_keys = {key_from_req(r): r for r in installed} + installed_keys = {r.key: r for r in installed} return list( flat_map(lambda req: dependency_tree(installed_keys, req), PACKAGES_TO_IGNORE) ) @@ -144,10 +144,9 @@ def diff_key_from_ireq(ireq: InstallRequirement) -> str: def diff_key_from_req(req: Distribution) -> str: """Get a unique key for the requirement.""" - key = key_from_req(req) + key = req.key if ( - hasattr(req, "direct_url") - and req.direct_url + req.direct_url and type(req.direct_url.info) == ArchiveInfo and req.direct_url.info.hash ): diff --git a/piptools/utils.py b/piptools/utils.py index 03ba1a1cd..cea776fa6 100644 --- a/piptools/utils.py +++ b/piptools/utils.py @@ -30,7 +30,7 @@ from pip._vendor.packaging.version import Version from pip._vendor.pkg_resources import get_distribution -from piptools._compat import PIP_VERSION, Distribution +from piptools._compat import PIP_VERSION from piptools.locations import CONFIG_FILE_NAME from piptools.subprocess_utils import run_python_snippet @@ -66,17 +66,9 @@ def key_from_ireq(ireq: InstallRequirement) -> str: return key_from_req(ireq.req) -def key_from_req(req: InstallRequirement | Distribution | Requirement) -> str: +def key_from_req(req: InstallRequirement | Requirement) -> str: """Get an all-lowercase version of the requirement's name.""" - if isinstance(req, Distribution): - key = req.key - elif hasattr(req, "key"): - # from pkg_resources, such as installed dists for pip-sync - key = req.key - else: - # from packaging, such as install requirements from requirements.txt - key = req.name - return str(canonicalize_name(key)) + return str(canonicalize_name(req.name)) def comment(text: str) -> str: From 13fe77d59cc1deda4f9b9c829b17c821a956609d Mon Sep 17 00:00:00 2001 From: Siddhartha Bagaria Date: Sat, 17 Jun 2023 16:21:22 +0000 Subject: [PATCH 11/16] remove dep on importlib --- piptools/_compat/pip_compat.py | 8 ++------ pyproject.toml | 1 - 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/piptools/_compat/pip_compat.py b/piptools/_compat/pip_compat.py index 5e98840e1..39f57266d 100644 --- a/piptools/_compat/pip_compat.py +++ b/piptools/_compat/pip_compat.py @@ -8,7 +8,6 @@ from pip._internal.cache import WheelCache from pip._internal.index.package_finder import PackageFinder from pip._internal.metadata import BaseDistribution -from pip._internal.metadata.importlib import Distribution as _ImportLibDist from pip._internal.metadata.pkg_resources import Distribution as _PkgResourcesDist from pip._internal.models.direct_url import DirectUrl from pip._internal.network.session import PipSession @@ -39,11 +38,8 @@ def from_pip_distribution(cls, dist: BaseDistribution) -> Distribution: # instead of specializing by type. if isinstance(dist, _PkgResourcesDist): return cls._from_pkg_resources(dist) - if isinstance(dist, _ImportLibDist): + else: return cls._from_importlib(dist) - raise ValueError( - f"unsupported installed distribution class ({dist.__class__}): {dist}" - ) @classmethod def _from_pkg_resources(cls, dist: _PkgResourcesDist) -> Distribution: @@ -52,7 +48,7 @@ def _from_pkg_resources(cls, dist: _PkgResourcesDist) -> Distribution: ) @classmethod - def _from_importlib(cls, dist: _ImportLibDist) -> Distribution: + def _from_importlib(cls, dist) -> Distribution: """Mimics pkg_resources.Distribution.requires for the case of no extras. This doesn't fulfill that API's `extras` parameter but satisfies the needs of pip-tools.""" diff --git a/pyproject.toml b/pyproject.toml index ff99767fb..47fa90566 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -39,7 +39,6 @@ dependencies = [ # direct dependencies "build", "click >= 8", - "importlib-metadata == 1.4; python_version < '3.8'", "pip >= 22.2", "tomli; python_version < '3.11'", # indirect dependencies From a2e3010754e3c0909cbd00aafbbab356c0bdc996 Mon Sep 17 00:00:00 2001 From: Siddhartha Bagaria Date: Sat, 17 Jun 2023 16:25:45 +0000 Subject: [PATCH 12/16] add back type hints --- piptools/_compat/pip_compat.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/piptools/_compat/pip_compat.py b/piptools/_compat/pip_compat.py index 39f57266d..892b8b125 100644 --- a/piptools/_compat/pip_compat.py +++ b/piptools/_compat/pip_compat.py @@ -2,7 +2,7 @@ import optparse from dataclasses import dataclass -from typing import Iterable, Iterator +from typing import Iterable, Iterator, TYPE_CHECKING import pip from pip._internal.cache import WheelCache @@ -23,7 +23,8 @@ # importlib.metadata, so this compat layer allows for a consistent access # pattern. In pip 22.1, importlib.metadata became the default on Python 3.11 # (and later), but is overridable. `select_backend` returns what's being used. - +if TYPE_CHECKING: + from pip._internal.metadata.importlib import Distribution as _ImportLibDist @dataclass(frozen=True) class Distribution: @@ -48,7 +49,7 @@ def _from_pkg_resources(cls, dist: _PkgResourcesDist) -> Distribution: ) @classmethod - def _from_importlib(cls, dist) -> Distribution: + def _from_importlib(cls, dist: "_ImportLibDist") -> Distribution: """Mimics pkg_resources.Distribution.requires for the case of no extras. This doesn't fulfill that API's `extras` parameter but satisfies the needs of pip-tools.""" From 6913a009411ad522947dcb3d476bf63580459abe Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Sat, 17 Jun 2023 16:27:13 +0000 Subject: [PATCH 13/16] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- piptools/_compat/pip_compat.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/piptools/_compat/pip_compat.py b/piptools/_compat/pip_compat.py index 892b8b125..700576455 100644 --- a/piptools/_compat/pip_compat.py +++ b/piptools/_compat/pip_compat.py @@ -2,7 +2,7 @@ import optparse from dataclasses import dataclass -from typing import Iterable, Iterator, TYPE_CHECKING +from typing import TYPE_CHECKING, Iterable, Iterator import pip from pip._internal.cache import WheelCache @@ -26,6 +26,7 @@ if TYPE_CHECKING: from pip._internal.metadata.importlib import Distribution as _ImportLibDist + @dataclass(frozen=True) class Distribution: key: str @@ -49,7 +50,7 @@ def _from_pkg_resources(cls, dist: _PkgResourcesDist) -> Distribution: ) @classmethod - def _from_importlib(cls, dist: "_ImportLibDist") -> Distribution: + def _from_importlib(cls, dist: _ImportLibDist) -> Distribution: """Mimics pkg_resources.Distribution.requires for the case of no extras. This doesn't fulfill that API's `extras` parameter but satisfies the needs of pip-tools.""" From 5d65fd7cffbf2f133b29174ddde18baa2d14e20d Mon Sep 17 00:00:00 2001 From: Siddhartha Bagaria Date: Wed, 28 Jun 2023 13:48:56 -0700 Subject: [PATCH 14/16] Review comment: use isinstance instead of type Co-authored-by: Albert Tugushev --- piptools/sync.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/piptools/sync.py b/piptools/sync.py index f0e430993..2dd57a976 100644 --- a/piptools/sync.py +++ b/piptools/sync.py @@ -147,7 +147,7 @@ def diff_key_from_req(req: Distribution) -> str: key = req.key if ( req.direct_url - and type(req.direct_url.info) == ArchiveInfo + and isinstance(req.direct_url.info, ArchiveInfo) and req.direct_url.info.hash ): key = direct_url_as_pep440_direct_reference(req.direct_url, key) From aadc4ec01f2704160cb953a22d51b7ff9613defc Mon Sep 17 00:00:00 2001 From: Siddhartha Bagaria Date: Wed, 28 Jun 2023 23:00:37 +0000 Subject: [PATCH 15/16] Review comments --- tests/conftest.py | 30 +++++++++++++++--------------- tests/test_sync.py | 4 ++-- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 0459cef44..5d83bf935 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -126,20 +126,6 @@ def command(self) -> InstallCommand: """Not used""" -def parse_fake_distribution(line, deps=[]): - req = Requirement.parse(line) - key = req.key - if "==" in line: - version = line.split("==")[1] - else: - version = "0+unknown" - requires = [Requirement.parse(d) for d in deps] - direct_url = None - if req.url: - direct_url = direct_url_from_link(Link(req.url)) - return Distribution(key, version, requires, direct_url) - - def pytest_collection_modifyitems(config, items): for item in items: # Mark network tests as flaky @@ -149,7 +135,21 @@ def pytest_collection_modifyitems(config, items): @pytest.fixture def fake_dist(): - return parse_fake_distribution + def _fake_dist(line, deps=None): + if deps is None: + deps = [] + req = Requirement.parse(line) + key = req.key + if "==" in line: + version = line.split("==")[1] + else: + version = "0+unknown" + requires = [Requirement.parse(d) for d in deps] + direct_url = None + if req.url: + direct_url = direct_url_from_link(Link(req.url)) + return Distribution(key, version, requires, direct_url) + return _fake_dist @pytest.fixture diff --git a/tests/test_sync.py b/tests/test_sync.py index cd0b26d7b..ebd26a21e 100644 --- a/tests/test_sync.py +++ b/tests/test_sync.py @@ -290,8 +290,8 @@ def test_diff_with_unequal_url_hash(fake_dist, from_line): # if URL hashes mismatch, assume the contents have # changed and reinstall line = "example@file:///example.zip#" - installed = [fake_dist(line + "#sha1=abc")] - reqs = [from_line(line + "#sha1=def")] + installed = [fake_dist(line + "sha1=abc")] + reqs = [from_line(line + "sha1=def")] to_install, to_uninstall = diff(reqs, installed) assert to_install == set(reqs) From a37dca07d5d7cd700f269917768dac1fdd63e61d Mon Sep 17 00:00:00 2001 From: "pre-commit-ci[bot]" <66853113+pre-commit-ci[bot]@users.noreply.github.com> Date: Wed, 28 Jun 2023 23:01:02 +0000 Subject: [PATCH 16/16] [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci --- tests/conftest.py | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/conftest.py b/tests/conftest.py index 5d83bf935..b4dd9182d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -149,6 +149,7 @@ def _fake_dist(line, deps=None): if req.url: direct_url = direct_url_from_link(Link(req.url)) return Distribution(key, version, requires, direct_url) + return _fake_dist