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

Update legacy editables support for compatibility with setuptools >= 69 #12477

Merged
merged 3 commits into from
Jan 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions news/12477.feature.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
Make pip freeze and pip uninstall of legacy editable installs of packages whose name
contains ``_`` compatible with ``setuptools>=69``.
28 changes: 18 additions & 10 deletions src/pip/_internal/utils/egg_link.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,24 +15,31 @@
]


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",
pradyunsg marked this conversation as resolved.
Show resolved Hide resolved
]


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


Expand Down Expand Up @@ -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
20 changes: 16 additions & 4 deletions tests/functional/test_check.py
Original file line number Diff line number Diff line change
Expand Up @@ -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.",)
Expand All @@ -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",
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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.",)
Expand Down
6 changes: 3 additions & 3 deletions tests/functional/test_freeze.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)}
..."""
)
Expand All @@ -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)}
..."""
)
Expand Down Expand Up @@ -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}...
"""
Expand Down
2 changes: 1 addition & 1 deletion tests/functional/test_install.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion tests/functional/test_install_reqs.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion tests/functional/test_install_vcs_git.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
13 changes: 9 additions & 4 deletions tests/functional/test_new_resolver.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
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 (
Expand All @@ -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}"


Expand Down Expand Up @@ -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:
Expand Down
19 changes: 15 additions & 4 deletions tests/functional/test_show.py
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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(
Expand All @@ -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:
Expand Down
31 changes: 23 additions & 8 deletions tests/lib/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
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

Expand Down Expand Up @@ -305,6 +306,12 @@ def files_updated(self) -> FoundFiles:
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,
Expand All @@ -320,7 +327,7 @@ def assert_installed(
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
Expand All @@ -329,22 +336,30 @@ def assert_installed(
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
Expand Down