Skip to content

Commit

Permalink
Fix a bug where pip-sync would unexpectedly uninstall some packages (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
atugushev committed Jul 18, 2023
1 parent 9636428 commit 6b162ac
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 6 deletions.
7 changes: 4 additions & 3 deletions piptools/sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
direct_url_as_pep440_direct_reference,
direct_url_from_link,
)
from pip._vendor.packaging.utils import canonicalize_name

from ._compat import Distribution, get_dev_pkgs
from .exceptions import IncompatibleRequirements
Expand Down Expand Up @@ -59,7 +60,7 @@ def dependency_tree(

while queue:
v = queue.popleft()
key = v.key
key = str(canonicalize_name(v.key))
if key in dependencies:
continue

Expand All @@ -85,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 = {r.key: r for r in installed}
installed_keys = {str(canonicalize_name(r.key)): r for r in installed}
return list(
flat_map(lambda req: dependency_tree(installed_keys, req), PACKAGES_TO_IGNORE)
)
Expand Down Expand Up @@ -143,7 +144,7 @@ def diff_key_from_ireq(ireq: InstallRequirement) -> str:

def diff_key_from_req(req: Distribution) -> str:
"""Get a unique key for the requirement."""
key = req.key
key = str(canonicalize_name(req.key))
if (
req.direct_url
and isinstance(req.direct_url.info, ArchiveInfo)
Expand Down
2 changes: 1 addition & 1 deletion tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ def _fake_dist(line, deps=None):
if deps is None:
deps = []
req = Requirement.parse(line)
key = req.key
key = req.name
if "==" in line:
version = line.split("==")[1]
else:
Expand Down
27 changes: 25 additions & 2 deletions tests/test_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -226,16 +226,18 @@ def test_diff_leave_packaging_packages_alone(fake_dist, from_line):
assert to_uninstall == {"first"}


def test_diff_leave_piptools_alone(fake_dist, from_line):
def test_diff_leave_piptools_and_its_dependencies_alone(fake_dist, from_line):
# Suppose an env contains Django, and pip-tools itself (including all of
# its dependencies)
installed = [
fake_dist("django==1.7"),
fake_dist("first==2.0.1"),
fake_dist("pip-tools==1.1.1", ["click>=4", "first", "six"]),
fake_dist("pip-tools==1.1.1", ["click>=4", "first", "six", "build"]),
fake_dist("six==1.9.0"),
fake_dist("click==4.1"),
fake_dist("foobar==0.3.6"),
fake_dist("build==0.10.0", ["pyproject_hooks"]),
fake_dist("pyproject_hooks==1.0.0"),
]

# Then this Django-only requirement should keep pip around (i.e. NOT
Expand Down Expand Up @@ -274,6 +276,27 @@ def test_diff_with_matching_url_hash(fake_dist, from_line):
assert to_uninstall == set()


@pytest.mark.parametrize(
("installed_dist", "compiled_req"),
(
pytest.param("Django==1.7", "django==1.7", id="case insensitive"),
pytest.param(
"jaraco.classes==3.2",
"jaraco-classes==3.2",
id="different namespace notation",
),
),
)
def test_diff_respects_canonical_package_names(
fake_dist, from_line, installed_dist, compiled_req
):
installed = [fake_dist(installed_dist)]
reqs = [from_line(compiled_req)]
to_install, to_uninstall = diff(reqs, installed)
assert to_install == set()
assert to_uninstall == set()


def test_diff_with_no_url_hash(fake_dist, from_line):
# if URL hash is not provided, assume the contents have
# changed and reinstall
Expand Down

0 comments on commit 6b162ac

Please sign in to comment.