Repository: pypa/pip
Pull request #12477 (2024-01-14 12:57:51)

Update legacy editables support for compatibility with setuptools >= 69
Since version 69, setuptools preserves `_`  in Name metadata and `.egg-link` files (before it converted them to `-`).

This is an attempt to make pip freeze and pip uninstall compatible with that behaviour, as well as fix various part of the testsuite that depended on that. The changes should be compatible with previous setuptools versions too.

This includes and extends https://github.com/pypa/pip/pull/12458.




notatallshaw: Obvious question, but is there an easy way to run the Pip suite using setuptools<69?

Some users pin their build dependencies either via `PIP_CONSTRAINTS` or using pip-tools.

Patch:
diff --git a/news/12477.feature.rst b/news/12477.feature.rst
new file mode 100644
index 000000000..e318d2924
--- /dev/null
+++ b/news/12477.feature.rst
@@ -0,0 +1,2 @@
+Make pip freeze and pip uninstall of legacy editable installs of packages whose name
+contains ``_`` compatible with ``setuptools>=69``.
diff --git a/src/pip/_internal/utils/egg_link.py b/src/pip/_internal/utils/egg_link.py
index eb57ed151..4a384a636 100644
--- a/src/pip/_internal/utils/egg_link.py
+++ b/src/pip/_internal/utils/egg_link.py
@@ -15,24 +15,31 @@ __all__ = [
 ]
 
 
-def _egg_link_name(raw_name: str) -> str:
+def _egg_link_names(raw_name: str) -> List[str]:
     """
     Convert a Name metadata value to a .egg-link name, by applying
     the same substitution as pkg_resources's safe_name function.
     Note: we cannot use canonicalize_name because it has a different logic.
+
+    We also look for the raw name (without normalization) as setuptools 69 changed
+    the way it names .egg-link files (https://github.com/pypa/setuptools/issues/4167).
     """
-    return re.sub("[^A-Za-z0-9.]+", "-", raw_name) + ".egg-link"
+    return [
+        re.sub("[^A-Za-z0-9.]+", "-", raw_name) + ".egg-link",
+        f"{raw_name}.egg-link",
+    ]
 
 
 def egg_link_path_from_sys_path(raw_name: str) -> Optional[str]:
     """
     Look for a .egg-link file for project name, by walking sys.path.
     """
-    egg_link_name = _egg_link_name(raw_name)
+    egg_link_names = _egg_link_names(raw_name)
     for path_item in sys.path:
-        egg_link = os.path.join(path_item, egg_link_name)
-        if os.path.isfile(egg_link):
-            return egg_link
+        for egg_link_name in egg_link_names:
+            egg_link = os.path.join(path_item, egg_link_name)
+            if os.path.isfile(egg_link):
+                return egg_link
     return None
 
 
@@ -64,9 +71,10 @@ def egg_link_path_from_location(raw_name: str) -> Optional[str]:
             sites.append(user_site)
         sites.append(site_packages)
 
-    egg_link_name = _egg_link_name(raw_name)
+    egg_link_names = _egg_link_names(raw_name)
     for site in sites:
-        egglink = os.path.join(site, egg_link_name)
-        if os.path.isfile(egglink):
-            return egglink
+        for egg_link_name in egg_link_names:
+            egglink = os.path.join(site, egg_link_name)
+            if os.path.isfile(egglink):
+                return egglink
     return None


Test patch:
diff --git a/tests/functional/test_check.py b/tests/functional/test_check.py
index e2b1c60ef..79b6df39c 100644
--- a/tests/functional/test_check.py
+++ b/tests/functional/test_check.py
@@ -119,7 +119,10 @@ def test_check_complicated_name_missing(script: PipTestEnvironment) -> None:
 
     # Without dependency
     result = script.pip("install", "--no-index", package_a_path, "--no-deps")
-    assert "Successfully installed package-A-1.0" in result.stdout, str(result)
+    assert (
+        "Successfully installed package_A-1.0" in result.stdout
+        or "Successfully installed package-A-1.0" in result.stdout
+    ), str(result)
 
     result = script.pip("check", expect_error=True)
     expected_lines = ("package-a 1.0 requires dependency-b, which is not installed.",)
@@ -142,7 +145,10 @@ def test_check_complicated_name_broken(script: PipTestEnvironment) -> None:
 
     # With broken dependency
     result = script.pip("install", "--no-index", package_a_path, "--no-deps")
-    assert "Successfully installed package-A-1.0" in result.stdout, str(result)
+    assert (
+        "Successfully installed package_A-1.0" in result.stdout
+        or "Successfully installed package-A-1.0" in result.stdout
+    ), str(result)
 
     result = script.pip(
         "install",
@@ -175,7 +181,10 @@ def test_check_complicated_name_clean(script: PipTestEnvironment) -> None:
     )
 
     result = script.pip("install", "--no-index", package_a_path, "--no-deps")
-    assert "Successfully installed package-A-1.0" in result.stdout, str(result)
+    assert (
+        "Successfully installed package_A-1.0" in result.stdout
+        or "Successfully installed package-A-1.0" in result.stdout
+    ), str(result)
 
     result = script.pip(
         "install",
@@ -203,7 +212,10 @@ def test_check_considers_conditional_reqs(script: PipTestEnvironment) -> None:
     )
 
     result = script.pip("install", "--no-index", package_a_path, "--no-deps")
-    assert "Successfully installed package-A-1.0" in result.stdout, str(result)
+    assert (
+        "Successfully installed package_A-1.0" in result.stdout
+        or "Successfully installed package-A-1.0" in result.stdout
+    ), str(result)
 
     result = script.pip("check", expect_error=True)
     expected_lines = ("package-a 1.0 requires dependency-b, which is not installed.",)
diff --git a/tests/functional/test_freeze.py b/tests/functional/test_freeze.py
index b2fd1d629..b7af974ea 100644
--- a/tests/functional/test_freeze.py
+++ b/tests/functional/test_freeze.py
@@ -220,7 +220,7 @@ def test_freeze_editable_not_vcs(script: PipTestEnvironment) -> None:
     # the freeze code does.
     expected = textwrap.dedent(
         f"""\
-    ...# Editable install with no version control (version-pkg==0.1)
+    ...# Editable install with no version control (version...pkg==0.1)
     -e {os.path.normcase(pkg_path)}
     ..."""
     )
@@ -245,7 +245,7 @@ def test_freeze_editable_git_with_no_remote(
     # the freeze code does.
     expected = textwrap.dedent(
         f"""\
-    ...# Editable Git install with no remote (version-pkg==0.1)
+    ...# Editable Git install with no remote (version...pkg==0.1)
     -e {os.path.normcase(pkg_path)}
     ..."""
     )
@@ -483,7 +483,7 @@ def test_freeze_git_remote(script: PipTestEnvironment) -> None:
     expected = os.path.normcase(
         textwrap.dedent(
             f"""
-            ...# Editable Git...(version-pkg...)...
+            ...# Editable Git...(version...pkg...)...
             # '{other_remote}'
             -e {repo_dir}...
         """
diff --git a/tests/functional/test_install.py b/tests/functional/test_install.py
index b18fabc84..3411f66d8 100644
--- a/tests/functional/test_install.py
+++ b/tests/functional/test_install.py
@@ -358,7 +358,7 @@ def test_basic_install_editable_from_svn(script: PipTestEnvironment) -> None:
     checkout_path = _create_test_package(script.scratch_path)
     repo_url = _create_svn_repo(script.scratch_path, checkout_path)
     result = script.pip("install", "-e", "svn+" + repo_url + "#egg=version-pkg")
-    result.assert_installed("version-pkg", with_files=[".svn"])
+    result.assert_installed("version_pkg", with_files=[".svn"])
 
 
 def _test_install_editable_from_git(script: PipTestEnvironment) -> None:
diff --git a/tests/functional/test_install_reqs.py b/tests/functional/test_install_reqs.py
index c2d951f26..993f25a2a 100644
--- a/tests/functional/test_install_reqs.py
+++ b/tests/functional/test_install_reqs.py
@@ -300,7 +300,7 @@ def test_install_local_editable_with_subdirectory(script: PipTestEnvironment) ->
         ),
     )
 
-    result.assert_installed("version-subpkg", sub_dir="version_subdir")
+    result.assert_installed("version_subpkg", sub_dir="version_subdir")
 
 
 @pytest.mark.network
diff --git a/tests/functional/test_install_vcs_git.py b/tests/functional/test_install_vcs_git.py
index 2abc7aa0f..e59b269a6 100644
--- a/tests/functional/test_install_vcs_git.py
+++ b/tests/functional/test_install_vcs_git.py
@@ -449,7 +449,7 @@ def test_git_with_ambiguous_revs(script: PipTestEnvironment) -> None:
     assert "Could not find a tag or branch" not in result.stdout
     # it is 'version-pkg' instead of 'version_pkg' because
     # egg-link name is version-pkg.egg-link because it is a single .py module
-    result.assert_installed("version-pkg", with_files=[".git"])
+    result.assert_installed("version_pkg", with_files=[".git"])
 
 
 def test_editable__no_revision(script: PipTestEnvironment) -> None:
diff --git a/tests/functional/test_new_resolver.py b/tests/functional/test_new_resolver.py
index 62e56adb2..afe07cbf3 100644
--- a/tests/functional/test_new_resolver.py
+++ b/tests/functional/test_new_resolver.py
@@ -5,6 +5,7 @@ import textwrap
 from typing import TYPE_CHECKING, Callable, Dict, List, Tuple
 
 import pytest
+from packaging.utils import canonicalize_name
 
 from tests.conftest import ScriptFactory
 from tests.lib import (
@@ -27,9 +28,13 @@ def assert_editable(script: PipTestEnvironment, *args: str) -> None:
     # This simply checks whether all of the listed packages have a
     # corresponding .egg-link file installed.
     # TODO: Implement a more rigorous way to test for editable installations.
-    egg_links = {f"{arg}.egg-link" for arg in args}
-    assert egg_links <= set(
-        os.listdir(script.site_packages_path)
+    egg_links = {f"{canonicalize_name(arg)}.egg-link" for arg in args}
+    actual_egg_links = {
+        f"{canonicalize_name(p.stem)}.egg-link"
+        for p in script.site_packages_path.glob("*.egg-link")
+    }
+    assert (
+        egg_links <= actual_egg_links
     ), f"{args!r} not all found in {script.site_packages_path!r}"
 
 
@@ -1847,7 +1852,7 @@ def test_new_resolver_succeeds_on_matching_constraint_and_requirement(
 
     script.assert_installed(test_pkg="0.1.0")
     if editable:
-        assert_editable(script, "test-pkg")
+        assert_editable(script, "test_pkg")
 
 
 def test_new_resolver_applies_url_constraint_to_dep(script: PipTestEnvironment) -> None:
diff --git a/tests/functional/test_show.py b/tests/functional/test_show.py
index b8ec0510a..05fccbbe3 100644
--- a/tests/functional/test_show.py
+++ b/tests/functional/test_show.py
@@ -277,7 +277,10 @@ def test_show_required_by_packages_basic(
     lines = result.stdout.splitlines()
 
     assert "Name: simple" in lines
-    assert "Required-by: requires-simple" in lines
+    assert (
+        "Required-by: requires_simple" in lines
+        or "Required-by: requires-simple" in lines
+    )
 
 
 def test_show_required_by_packages_capitalized(
@@ -294,7 +297,10 @@ def test_show_required_by_packages_capitalized(
     lines = result.stdout.splitlines()
 
     assert "Name: simple" in lines
-    assert "Required-by: Requires-Capitalized" in lines
+    assert (
+        "Required-by: Requires_Capitalized" in lines
+        or "Required-by: Requires-Capitalized" in lines
+    )
 
 
 def test_show_required_by_packages_requiring_capitalized(
@@ -314,8 +320,13 @@ def test_show_required_by_packages_requiring_capitalized(
     lines = result.stdout.splitlines()
     print(lines)
 
-    assert "Name: Requires-Capitalized" in lines
-    assert "Required-by: requires-requires-capitalized" in lines
+    assert (
+        "Name: Requires_Capitalized" in lines or "Name: Requires-Capitalized" in lines
+    )
+    assert (
+        "Required-by: requires_requires_capitalized" in lines
+        or "Required-by: requires-requires-capitalized" in lines
+    )
 
 
 def test_show_skip_work_dir_pkg(script: PipTestEnvironment) -> None:
diff --git a/tests/lib/__init__.py b/tests/lib/__init__.py
index f14837e24..ea755c838 100644
--- a/tests/lib/__init__.py
+++ b/tests/lib/__init__.py
@@ -41,6 +41,7 @@ from pip._internal.models.search_scope import SearchScope
 from pip._internal.models.selection_prefs import SelectionPreferences
 from pip._internal.models.target_python import TargetPython
 from pip._internal.network.session import PipSession
+from pip._internal.utils.egg_link import _egg_link_names
 from tests.lib.venv import VirtualEnvironment
 from tests.lib.wheel import make_wheel
 
@@ -305,6 +306,12 @@ class TestPipResult:
     def files_deleted(self) -> FoundFiles:
         return FoundFiles(self._impl.files_deleted)
 
+    def _get_egg_link_path_created(self, egg_link_paths: List[str]) -> Optional[str]:
+        for egg_link_path in egg_link_paths:
+            if egg_link_path in self.files_created:
+                return egg_link_path
+        return None
+
     def assert_installed(
         self,
         pkg_name: str,
@@ -320,7 +327,7 @@ class TestPipResult:
         e = self.test_env
 
         if editable:
-            pkg_dir = e.venv / "src" / pkg_name.lower()
+            pkg_dir = e.venv / "src" / canonicalize_name(pkg_name)
             # If package was installed in a sub directory
             if sub_dir:
                 pkg_dir = pkg_dir / sub_dir
@@ -329,22 +336,30 @@ class TestPipResult:
             pkg_dir = e.site_packages / pkg_name
 
         if use_user_site:
-            egg_link_path = e.user_site / f"{pkg_name}.egg-link"
+            egg_link_paths = [
+                e.user_site / egg_link_name
+                for egg_link_name in _egg_link_names(pkg_name)
+            ]
         else:
-            egg_link_path = e.site_packages / f"{pkg_name}.egg-link"
+            egg_link_paths = [
+                e.site_packages / egg_link_name
+                for egg_link_name in _egg_link_names(pkg_name)
+            ]
 
+        egg_link_path_created = self._get_egg_link_path_created(egg_link_paths)
         if without_egg_link:
-            if egg_link_path in self.files_created:
+            if egg_link_path_created:
                 raise TestFailure(
-                    f"unexpected egg link file created: {egg_link_path!r}\n{self}"
+                    f"unexpected egg link file created: {egg_link_path_created!r}\n"
+                    f"{self}"
                 )
         else:
-            if egg_link_path not in self.files_created:
+            if not egg_link_path_created:
                 raise TestFailure(
-                    f"expected egg link file missing: {egg_link_path!r}\n{self}"
+                    f"expected egg link file missing: {egg_link_paths!r}\n{self}"
                 )
 
-            egg_link_file = self.files_created[egg_link_path]
+            egg_link_file = self.files_created[egg_link_path_created]
             egg_link_contents = egg_link_file.bytes.replace(os.linesep, "\n")
 
             # FIXME: I don't understand why there's a trailing . here