Fix a bug where pip-sync would unexpectedly uninstall some packages (#1919)

Author: atugushev

piptools/sync.py

@@ -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
@@ -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
 
@@ -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)
     )
@@ -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)

tests/conftest.py

@@ -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:

tests/test_sync.py

@@ -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
@@ -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