Merge pull request #13732 from ichard26/finish-non-bare-egg-fragment-deprecation

Remove support for non-bare egg fragments

Author: notatallshaw

docs/html/topics/vcs-support.md

@@ -163,7 +163,7 @@ pip also supports an `egg` fragment to specify the "project name". This is a leg
 feature and its use is discouraged in favour of the
 {ref}`Direct URL <pypug:dependency-specifiers>` form.
 
-The `egg` fragment **should** be a bare {ref}`project name <pypug:name-normalization>`.
+The `egg` fragment **MUST** be a bare {ref}`project name <pypug:name-normalization>`.
 Anything else is not guaranteed to work.
 ```
 

news/13157.removal.rst

@@ -0,0 +1,2 @@
+Remove support for non-bare project names in egg fragments. Affected users should use
+the `Direct URL requirement syntax <https://packaging.python.org/en/latest/specifications/version-specifiers/#direct-references>`_.

src/pip/_internal/exceptions.py

@@ -31,6 +31,7 @@
     from pip._vendor.requests.models import PreparedRequest, Request, Response
 
     from pip._internal.metadata import BaseDistribution
+    from pip._internal.models.link import Link
     from pip._internal.network.download import _FileDownload
     from pip._internal.req.req_install import InstallRequirement
 
@@ -899,6 +900,29 @@ def __init__(self, failed: list[InstallRequirement]) -> None:
         )
 
 
+class InvalidEggFragment(DiagnosticPipError):
+    reference = "invalid-egg-fragment"
+
+    def __init__(self, link: Link, fragment: str) -> None:
+        hint = ""
+        if ">" in fragment or "=" in fragment or "<" in fragment:
+            hint = (
+                "Version specifiers are silently ignored for URL references. "
+                "Remove them. "
+            )
+        if "[" in fragment and "]" in fragment:
+            hint += "Try using the Direct URL requirement syntax: 'name[extra] @ URL'"
+
+        if not hint:
+            hint = "Egg fragments can only be a valid project name."
+
+        super().__init__(
+            message=f"The '{escape(fragment)}' egg fragment is invalid",
+            context=f"from '{escape(str(link))}'",
+            hint_stmt=escape(hint),
+        )
+
+
 class BuildDependencyInstallError(DiagnosticPipError):
     """Raised when build dependencies cannot be installed."""
 

src/pip/_internal/models/link.py

@@ -16,8 +16,8 @@
     NamedTuple,
 )
 
+from pip._internal.exceptions import InvalidEggFragment
 from pip._internal.utils.datetime import parse_iso_datetime
-from pip._internal.utils.deprecation import deprecated
 from pip._internal.utils.filetypes import WHEEL_EXTENSION
 from pip._internal.utils.hashes import Hashes
 from pip._internal.utils.misc import (
@@ -483,12 +483,7 @@ def _egg_fragment(self) -> str | None:
         # an optional extras specifier. Anything else is invalid.
         project_name = match.group(1)
         if not self._project_name_re.match(project_name):
-            deprecated(
-                reason=f"{self} contains an egg fragment with a non-PEP 508 name.",
-                replacement="to use the req @ url syntax, and remove the egg fragment",
-                gone_in="26.0",
-                issue=13157,
-            )
+            raise InvalidEggFragment(self, project_name)
 
         return project_name
 

src/pip/_internal/req/constructors.py

@@ -258,8 +258,10 @@ def install_req_from_editable(
     permit_editable_wheels: bool = False,
     config_settings: dict[str, str | list[str]] | None = None,
 ) -> InstallRequirement:
-    parts = parse_req_from_editable(editable_req)
+    if constraint:
+        raise InstallationError("Editable requirements are not allowed as constraints")
 
+    parts = parse_req_from_editable(editable_req)
     return InstallRequirement(
         parts.requirement,
         comes_from=comes_from,

tests/functional/test_install_reqs.py

@@ -646,12 +646,12 @@ def test_install_with_extras_from_constraints(
     script: PipTestEnvironment, data: TestData, resolver_variant: ResolverVariant
 ) -> None:
     to_install = data.packages.joinpath("LocalExtras")
-    script.scratch_path.joinpath("constraints.txt").write_text(
-        f"{to_install.as_uri()}#egg=LocalExtras[bar]"
+    file = script.temporary_file(
+        "constraints.txt", f"LocalExtras[bar] @ {to_install.as_uri()}"
     )
     result = script.pip_install_local(
         "-c",
-        script.scratch_path / "constraints.txt",
+        file,
         "LocalExtras",
         allow_stderr_warning=True,
         expect_error=(resolver_variant == "resolvelib"),
@@ -684,12 +684,12 @@ def test_install_with_extras_joined(
     script: PipTestEnvironment, data: TestData, resolver_variant: ResolverVariant
 ) -> None:
     to_install = data.packages.joinpath("LocalExtras")
-    script.scratch_path.joinpath("constraints.txt").write_text(
-        f"{to_install.as_uri()}#egg=LocalExtras[bar]"
+    file = script.temporary_file(
+        "constraints.txt", f"LocalExtras[bar] @ {to_install.as_uri()}"
     )
     result = script.pip_install_local(
         "-c",
-        script.scratch_path / "constraints.txt",
+        file,
         "LocalExtras[baz]",
         allow_stderr_warning=True,
         expect_error=(resolver_variant == "resolvelib"),
@@ -705,12 +705,12 @@ def test_install_with_extras_editable_joined(
     script: PipTestEnvironment, data: TestData, resolver_variant: ResolverVariant
 ) -> None:
     to_install = data.packages.joinpath("LocalExtras")
-    script.scratch_path.joinpath("constraints.txt").write_text(
-        f"-e {to_install.as_uri()}#egg=LocalExtras[bar]"
+    file = script.temporary_file(
+        "constraints.txt", f"-e LocalExtras[bar] @ {to_install.as_uri()}"
     )
     result = script.pip_install_local(
         "-c",
-        script.scratch_path / "constraints.txt",
+        file,
         "LocalExtras[baz]",
         allow_stderr_warning=True,
         expect_error=(resolver_variant == "resolvelib"),

tests/unit/test_link.py

@@ -2,6 +2,7 @@
 
 import pytest
 
+from pip._internal.exceptions import InvalidEggFragment, PipError
 from pip._internal.models.link import Link, links_equivalent
 from pip._internal.utils.hashes import Hashes
 
@@ -80,20 +81,6 @@ def test_fragments(self) -> None:
         assert "eggname" == Link(url).egg_fragment
         assert "subdir" == Link(url).subdirectory_fragment
 
-        # Extras are supported and preserved in the egg fragment,
-        # even the empty extras specifier.
-        # This behavior is deprecated and will change in pip 25.
-        url = "git+https://example.com/package#egg=eggname[extra]"
-        assert "eggname[extra]" == Link(url).egg_fragment
-        assert None is Link(url).subdirectory_fragment
-        url = "git+https://example.com/package#egg=eggname[extra1,extra2]"
-        assert "eggname[extra1,extra2]" == Link(url).egg_fragment
-        assert None is Link(url).subdirectory_fragment
-        url = "git+https://example.com/package#egg=eggname[]"
-        assert "eggname[]" == Link(url).egg_fragment
-        assert None is Link(url).subdirectory_fragment
-
-    @pytest.mark.xfail(reason="Behavior change scheduled for 25.0", strict=True)
     @pytest.mark.parametrize(
         "fragment",
         [
@@ -102,15 +89,32 @@ def test_fragments(self) -> None:
             # Version specifiers are not valid in egg fragments.
             "eggname==1.2.3",
             "eggname>=1.2.3",
-            # The extras specifier must be in PEP 508 form.
+            # Extras are also prohibited.
             "eggname[!]",
+            "eggname[extra]",
+            "eggname[extra1,extra2]",
+            "eggmame[]",
+            "eggname[extra]==1000",
         ],
     )
     def test_invalid_egg_fragments(self, fragment: str) -> None:
         url = f"git+https://example.com/package#egg={fragment}"
-        with pytest.raises(ValueError):
+        with pytest.raises(PipError):
             Link(url)
 
+    def test_invalid_egg_fragment_with_extras_and_version_hint(self) -> None:
+        """Test that fragments with extras and version specifiers get proper hint."""
+
+        url = "git+https://example.com/package#egg=eggname[extra]==1.0"
+        with pytest.raises(InvalidEggFragment) as exc_info:
+            Link(url)
+
+        # The hint should suggest Direct URL syntax, not just "remove version
+        # specifiers" because the extras require Direct URL syntax anyway.
+        hint = str(exc_info.value.hint_stmt)
+        assert r"name\[extra] @ URL" in hint
+        assert "Version specifiers are silently ignored" in hint
+
     @pytest.mark.parametrize(
         "yanked_reason, expected",
         [

tests/unit/test_req.py

@@ -701,7 +701,7 @@ def test_extras_for_line_path_requirement(self) -> None:
         assert req.extras == {"ex1", "ex2"}
 
     def test_extras_for_line_url_requirement(self) -> None:
-        line = "git+https://url#egg=SomeProject[ex1,ex2]"
+        line = "SomeProject[ex1,ex2] @ git+https://url"
         filename = "filename"
         comes_from = f"-r {filename} (line 1)"
         req = install_req_from_line(line, comes_from=comes_from)
@@ -717,7 +717,7 @@ def test_extras_for_editable_path_requirement(self) -> None:
         assert req.extras == {"ex1", "ex2"}
 
     def test_extras_for_editable_url_requirement(self) -> None:
-        url = "git+https://url#egg=SomeProject[ex1,ex2]"
+        url = "SomeProject[ex1,ex2] @ git+https://url"
         filename = "filename"
         comes_from = f"-r {filename} (line 1)"
         req = install_req_from_editable(url, comes_from=comes_from)
@@ -894,10 +894,10 @@ def test_parse_editable_explicit_vcs() -> None:
 
 
 def test_parse_editable_vcs_extras() -> None:
-    assert parse_editable("svn+https://foo#egg=foo[extras]") == (
-        "foo[extras]",
-        "svn+https://foo#egg=foo[extras]",
-        set(),
+    assert parse_editable("foo[extras] @ svn+https://foo") == (
+        "foo",
+        "svn+https://foo",
+        {"extras"},
     )
 
 

tests/unit/test_req_file.py

@@ -325,16 +325,6 @@ def test_yield_editable_requirement(self, line_processor: LineProcessor) -> None
         req = install_req_from_editable(url, comes_from=comes_from)
         assert repr(line_processor(line, filename, 1)[0]) == repr(req)
 
-    def test_yield_editable_constraint(self, line_processor: LineProcessor) -> None:
-        url = "git+https://url#egg=SomeProject"
-        line = f"-e {url}"
-        filename = "filename"
-        comes_from = f"-c {filename} (line {1})"
-        req = install_req_from_editable(url, comes_from=comes_from, constraint=True)
-        found_req = line_processor(line, filename, 1, constraint=True)[0]
-        assert repr(found_req) == repr(req)
-        assert found_req.constraint is True
-
     def test_nested_constraints_file(
         self, monkeypatch: pytest.MonkeyPatch, tmpdir: Path, session: PipSession
     ) -> None: