Skip to content
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

Sync direct references with hashes #1885

Merged
merged 20 commits into from Jul 1, 2023
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
16 changes: 8 additions & 8 deletions 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
Expand Down Expand Up @@ -52,23 +52,23 @@ def _uses_pkg_resources() -> bool:
uses_pkg_resources = _uses_pkg_resources()

if uses_pkg_resources:
from operator import methodcaller
from pip._internal.metadata.pkg_resources import Distribution

from pip._vendor.pkg_resources import Distribution
def dist_requires(dist: Distribution) -> Iterable[Requirement]:
res: Iterable[Requirement] = dist._dist.requires()
return res

dist_requires = cast(
Callable[[Distribution], Iterable[Requirement]], methodcaller("requires")
)
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
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
Expand Down
2 changes: 1 addition & 1 deletion piptools/scripts/sync.py
Expand Up @@ -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]
32 changes: 29 additions & 3 deletions piptools/sync.py
Expand Up @@ -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
Expand Down Expand Up @@ -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 (
Expand All @@ -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
siddharthab marked this conversation as resolved.
Show resolved Hide resolved
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],
Expand All @@ -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):
Expand Down
12 changes: 11 additions & 1 deletion piptools/utils.py
Expand Up @@ -27,12 +27,14 @@
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
from piptools.subprocess_utils import run_python_snippet

from ._compat.pip_compat import Distribution

if TYPE_CHECKING:
from typing import Protocol
else:
Expand Down Expand Up @@ -67,6 +69,14 @@ 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
Expand Down
29 changes: 22 additions & 7 deletions tests/conftest.py
Expand Up @@ -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

Expand Down Expand Up @@ -125,20 +127,19 @@ def command(self) -> InstallCommand:
"""Not used"""


class FakeInstalledDistribution:
def __init__(self, line, deps=None):
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
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:
Expand All @@ -153,6 +154,20 @@ def requires(self):
return self.dep_strs


class FakeInstalledDistribution:
chrysle marked this conversation as resolved.
Show resolved Hide resolved
# 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.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
Expand Down
30 changes: 22 additions & 8 deletions tests/test_sync.py
Expand Up @@ -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)
}

Expand Down Expand Up @@ -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")]
siddharthab marked this conversation as resolved.
Show resolved Hide resolved

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
):
Expand Down