fix: use stdlib url<->path conversion (#3364)

* fix: use stdlib url<->path conversion

Signed-off-by: Frost Ming <[email protected]>

* fix tests

Signed-off-by: Frost Ming <[email protected]>

* fix the error type

Signed-off-by: Frost Ming <[email protected]>

news/3362.bugfix.md

@@ -0,0 +1 @@
+Use stdlib for URL <-> Path conversions.

src/pdm/models/backends.py

@@ -6,7 +6,7 @@
 from pathlib import Path
 from typing import TYPE_CHECKING
 
-from pdm.utils import expand_env_vars, path_to_url
+from pdm.utils import expand_env_vars
 
 if TYPE_CHECKING:
     from typing import TypedDict
@@ -24,7 +24,7 @@ def expand_line(self, line: str, expand_env: bool = True) -> str:
         return line
 
     def relative_path_to_url(self, path: str) -> str:
-        return path_to_url(os.path.join(self.root, path))
+        return self.root.joinpath(path).as_uri()
 
     @classmethod
     @abc.abstractmethod
@@ -52,14 +52,14 @@ def build_system(cls) -> BuildSystem:
 
 class PDMBackend(BuildBackend):
     def expand_line(self, req: str, expand_env: bool = True) -> str:
-        line = req.replace("file:///${PROJECT_ROOT}", path_to_url(self.root.as_posix()))
+        line = req.replace("file:///${PROJECT_ROOT}", self.root.as_uri())
         if expand_env:
             line = expand_env_vars(line)
         return line
 
     def relative_path_to_url(self, path: str) -> str:
         if os.path.isabs(path):
-            return path_to_url(path)
+            return Path(path).as_uri()
         return f"file:///${{PROJECT_ROOT}}/{urllib.parse.quote(path)}"
 
     @classmethod
@@ -79,7 +79,7 @@ def __format__(self, __format_spec: str) -> str:
         if not __format_spec:
             return self.__path.as_posix()
         elif __format_spec == "uri":
-            return path_to_url(self.__path.as_posix())
+            return self.__path.as_uri()
         elif __format_spec == "real":
             return self.__path.resolve().as_posix()
         raise ValueError(f"Unknown format specifier: {__format_spec}")
@@ -110,7 +110,7 @@ def expand_line(self, line: str, expand_env: bool = True) -> str:
 
     def relative_path_to_url(self, path: str) -> str:
         if os.path.isabs(path):
-            return path_to_url(path)
+            return Path(path).as_uri()
         return f"{{root:uri}}/{urllib.parse.quote(path)}"
 
     @classmethod

src/pdm/models/candidates.py

@@ -34,7 +34,6 @@
     filtered_sources,
     get_rev_from_url,
     normalize_name,
-    path_to_url,
     url_without_fragments,
 )
 
@@ -353,7 +352,7 @@ def direct_url(self) -> dict[str, Any] | None:
                 assert self._source_dir
                 return _filter_none(
                     {
-                        "url": path_to_url(self._source_dir.as_posix()),
+                        "url": self._source_dir.as_uri(),
                         "dir_info": {"editable": True},
                         "subdirectory": req.subdirectory,
                     }

src/pdm/models/repositories/lock.py

@@ -3,14 +3,15 @@
 import dataclasses
 import posixpath
 from functools import cached_property
+from pathlib import Path
 from typing import TYPE_CHECKING, Collection, NamedTuple, cast
 
 from pdm.exceptions import CandidateNotFound, PdmException
 from pdm.models.candidates import Candidate
 from pdm.models.markers import EnvSpec
 from pdm.models.repositories.base import BaseRepository, CandidateMetadata
 from pdm.models.requirements import FileRequirement, Requirement, parse_line
-from pdm.utils import cd, path_to_url, url_to_path, url_without_fragments
+from pdm.utils import cd, url_to_path, url_without_fragments
 
 if TYPE_CHECKING:
     from typing import Any, Callable, Iterable, Mapping
@@ -89,7 +90,7 @@ def _read_lockfile(self, lockfile: Mapping[str, Any]) -> None:
                 }
                 req = Requirement.from_req_dict(package_name, req_dict)
                 if req.is_file_or_url and req.path and not req.url:  # type: ignore[attr-defined]
-                    req.url = path_to_url(posixpath.join(root, req.path))  # type: ignore[attr-defined]
+                    req.url = root.joinpath(req.path).as_uri()  # type: ignore[attr-defined]
                 can = Candidate(req, name=package_name, version=version)
                 can.hashes = package.get("files", [])
                 if not static_urls and any("url" in f for f in can.hashes):
@@ -112,7 +113,7 @@ def _identify_candidate(self, candidate: Candidate) -> CandidateKey:
             url = self.environment.project.backend.expand_line(cast(str, url))
             if url.startswith("file://"):
                 path = posixpath.normpath(url_to_path(url))
-                url = path_to_url(path)
+                url = Path(path).as_uri()
         return (
             candidate.identify(),
             candidate.version if not url else None,

src/pdm/models/requirements.py

@@ -28,8 +28,7 @@
     add_ssh_scheme_to_git_uri,
     comparable_version,
     normalize_name,
-    path_to_url,
-    path_without_fragments,
+    split_path_fragments,
     url_to_path,
     url_without_fragments,
 )
@@ -309,17 +308,24 @@ def str_path(self) -> str | None:
         return result
 
     def _parse_url(self) -> None:
-        if not self.url and self.path and self.path.is_absolute():
-            self.url = path_to_url(self.path.as_posix())
-        if not self.path:
-            path = get_relative_path(self.url)
-            if path is None:
+        if self.path:
+            path, fragments = split_path_fragments(self.path)
+            if not self.url and path.is_absolute():
+                self.url = path.as_uri() + fragments
+                self.path = path
+            # For relative path, we don't resolve URL now, so the path may still contain fragments,
+            # it will be handled in `relocate()` method.
+        else:
+            url = url_without_fragments(self.url)
+            relpath = get_relative_path(url)
+            if relpath is None:
                 try:
-                    self.path = path_without_fragments(url_to_path(self.url))
-                except AssertionError:
+                    self.path = Path(url_to_path(url))
+                except ValueError:
                     pass
             else:
-                self.path = path_without_fragments(path)
+                self.path = Path(relpath)
+
         if self.url:
             self._parse_name_from_url()
 
@@ -328,11 +334,12 @@ def relocate(self, backend: BuildBackend) -> None:
         if self.path is None or self.path.is_absolute():
             return
         # self.path is relative
-        self.path = path_without_fragments(os.path.relpath(self.path, backend.root))
-        path = self.path.as_posix()
-        if path == ".":
-            path = ""
-        self.url = backend.relative_path_to_url(path)
+        path, fragments = split_path_fragments(self.path)
+        self.path = Path(os.path.relpath(path, backend.root))
+        relpath = self.path.as_posix()
+        if relpath == ".":
+            relpath = ""
+        self.url = backend.relative_path_to_url(relpath) + fragments
         self._root = backend.root
 
     @property
@@ -492,7 +499,7 @@ def parse_requirement(line: str, editable: bool = False) -> Requirement:
         # We replace the {root.uri} temporarily with a dummy URL header
         # to make it pass through the packaging.requirement parser
         # and then revert it.
-        root_url = path_to_url(Path().as_posix())
+        root_url = Path().absolute().as_uri()
         replaced = "{root:uri}" in line
         if replaced:
             line = line.replace("{root:uri}", root_url)

src/pdm/project/core.py

@@ -39,7 +39,6 @@
     is_conda_base_python,
     is_path_relative_to,
     normalize_name,
-    path_to_url,
 )
 
 if TYPE_CHECKING:
@@ -621,7 +620,7 @@ def make_self_candidate(self, editable: bool = True) -> Candidate:
 
         from pdm.models.candidates import Candidate
 
-        req = parse_requirement(path_to_url(self.root.as_posix()), editable)
+        req = parse_requirement(self.root.as_uri(), editable)
         assert self.name
         req.name = self.name
         can = Candidate(req, name=self.name, link=Link.from_path(self.root))

src/pdm/pytest.py

@@ -63,7 +63,7 @@
 from pdm.models.session import PDMPyPIClient
 from pdm.project.config import Config
 from pdm.project.core import Project
-from pdm.utils import find_python_in_path, normalize_name, parse_version, path_to_url
+from pdm.utils import find_python_in_path, normalize_name, parse_version
 
 if TYPE_CHECKING:
     from typing import Protocol
@@ -501,12 +501,11 @@ def local_finder_artifacts() -> Path:
 
 @pytest.fixture
 def local_finder(project_no_init: Project, local_finder_artifacts: Path) -> None:
-    artifacts_dir = str(local_finder_artifacts)
     project_no_init.pyproject.settings["source"] = [
         {
             "type": "find_links",
             "verify_ssl": False,
-            "url": path_to_url(artifacts_dir),
+            "url": local_finder_artifacts.as_uri(),
             "name": "pypi",
         }
     ]

src/pdm/utils.py

@@ -37,8 +37,6 @@
     from pdm._types import FileHash, RepositoryConfig
     from pdm.compat import Distribution
 
-_egg_fragment_re = re.compile(r"(.*)[#&]egg=[^&]*")
-
 try:
     _packaging_version = importlib_metadata.version("packaging")
 except Exception:
@@ -189,7 +187,8 @@ def url_to_path(url: str) -> str:
 
     WINDOWS = sys.platform == "win32"
 
-    assert url.startswith("file:"), f"You can only turn file: urls into filenames (not {url!r})"
+    if not url.startswith("file:"):
+        raise ValueError(f"You can only turn file: urls into filenames (not {url!r})")
 
     _, netloc, path, _, _ = parse.urlsplit(url)
 
@@ -220,16 +219,10 @@ def url_to_path(url: str) -> str:
     return path
 
 
-def path_to_url(path: str) -> str:
-    """
-    Convert a path to a file: URL.  The path will be made absolute and have
-    quoted path parts.
-    """
-    from urllib.request import pathname2url
-
-    path = os.path.normpath(os.path.abspath(path))
-    url = parse.urljoin("file:", pathname2url(path))
-    return url
+def split_path_fragments(path: Path) -> tuple[Path, str]:
+    """Split a path into fragments"""
+    left, sep, right = path.as_posix().partition("#egg=")
+    return Path(left), sep + right
 
 
 def expand_env_vars(credential: str, quote: bool = False, env: Mapping[str, str] | None = None) -> str:
@@ -448,14 +441,6 @@ def is_pip_compatible_with_python(python_version: Version | str) -> bool:
     return requires_python.contains(python_version, True)
 
 
-def path_without_fragments(path: str) -> Path:
-    """Remove egg fragment from path"""
-    match = _egg_fragment_re.search(path)
-    if not match:
-        return Path(path)
-    return Path(match.group(1))
-
-
 def is_in_zipapp() -> bool:
     """Check if the current process is running in a zipapp"""
     return not os.path.exists(__file__)

tests/cli/test_add.py

@@ -6,7 +6,6 @@
 from pdm.models.markers import EnvSpec
 from pdm.models.specifiers import PySpecSet
 from pdm.pytest import Distribution
-from pdm.utils import path_to_url
 from tests import FIXTURES
 
 
@@ -268,9 +267,9 @@ def test_add_with_prerelease(project, working_set, pdm):
 
 def test_add_editable_package_with_extras(project, working_set, pdm):
     project.environment.python_requires = PySpecSet(">=3.6")
-    dep_path = FIXTURES.joinpath("projects/demo").as_posix()
-    pdm(["add", "-dGdev", "-e", f"{dep_path}[security]"], obj=project, strict=True)
-    assert f"-e {path_to_url(dep_path)}#egg=demo[security]" in project.use_pyproject_dependencies("dev", True)[0]
+    dep_path = FIXTURES.joinpath("projects/demo")
+    pdm(["add", "-dGdev", "-e", f"{dep_path.as_posix()}[security]"], obj=project, strict=True)
+    assert f"-e {dep_path.as_uri()}#egg=demo[security]" in project.use_pyproject_dependencies("dev", True)[0]
     assert "demo" in working_set
     assert "requests" in working_set
     assert "urllib3" in working_set

tests/cli/test_lock.py

@@ -1,3 +1,4 @@
+from pathlib import Path
 from unittest.mock import ANY
 
 import pytest
@@ -8,7 +9,7 @@
 from pdm.models.requirements import parse_requirement
 from pdm.models.specifiers import PySpecSet
 from pdm.project.lockfile import FLAG_CROSS_PLATFORM, Compatibility
-from pdm.utils import parse_version, path_to_url
+from pdm.utils import parse_version
 from tests import FIXTURES
 
 
@@ -418,7 +419,7 @@ def test_lock_for_multiple_targets(project, pdm, repository, nested):
 
 
 @pytest.mark.usefixtures("repository")
-@pytest.mark.parametrize("constraint", [CONSTRAINT_FILE, path_to_url(CONSTRAINT_FILE)])
+@pytest.mark.parametrize("constraint", [CONSTRAINT_FILE, Path(CONSTRAINT_FILE).as_uri()])
 def test_lock_with_override_file(project, pdm, constraint):
     project.add_dependencies(["requests"])
     pdm(["lock", "--override", constraint], obj=project, strict=True)

tests/cli/test_run.py

@@ -10,7 +10,7 @@
 from pdm import termui
 from pdm.cli import actions
 from pdm.cli.utils import get_pep582_path
-from pdm.utils import cd, path_to_url
+from pdm.utils import cd
 
 
 @pytest.fixture
@@ -972,7 +972,7 @@ def test_run_script_with_inline_metadata(project, pdm, local_finder, local_finde
         result = pdm(["run", "test_script.py"], obj=project)
         assert result.exit_code != 0
 
-    local_artifacts_url = path_to_url(str(local_finder_artifacts))
+    local_artifacts_url = local_finder_artifacts.as_uri()
 
     project.root.joinpath("test_script.py").write_text(
         textwrap.dedent(f"""\

tests/conftest.py

@@ -11,7 +11,6 @@
 
 from pdm.models.auth import keyring
 from pdm.project import Project
-from pdm.utils import path_to_url
 from tests import FIXTURES
 
 if TYPE_CHECKING:
@@ -118,12 +117,11 @@ def func(project_name):
         copytree(source, project_no_init.root)
         project_no_init.pyproject.reload()
         if "local_finder" in request.fixturenames:
-            artifacts_dir = str(local_finder_artifacts)
             project_no_init.pyproject.settings["source"] = [
                 {
                     "type": "find_links",
                     "verify_ssl": False,
-                    "url": path_to_url(artifacts_dir),
+                    "url": local_finder_artifacts.as_uri(),
                     "name": "pypi",
                 }
             ]

tests/models/test_backends.py

@@ -5,7 +5,7 @@
 
 from pdm.models.backends import _BACKENDS, get_backend, get_relative_path
 from pdm.project import Project
-from pdm.utils import cd, path_to_url
+from pdm.utils import cd
 from tests import FIXTURES
 
 
@@ -36,8 +36,8 @@ def test_project_backend(project, working_set, backend, pdm):
         assert "idna" in working_set
         assert "demo" in working_set
         dep = project.pyproject.metadata["dependencies"][0]
-        demo_path = project.root.joinpath("demo").as_posix()
-        demo_url = path_to_url(demo_path)
+        demo_path = project.root.joinpath("demo")
+        demo_url = demo_path.as_uri()
         if backend == "pdm-backend":
             assert dep == "demo @ file:///${PROJECT_ROOT}/demo"
         elif backend == "hatchling":
@@ -54,7 +54,7 @@ def test_project_backend(project, working_set, backend, pdm):
 
 def test_hatch_expand_variables(monkeypatch):
     root = Path().absolute()
-    root_url = path_to_url(root.as_posix())
+    root_url = root.as_uri()
     backend = get_backend("hatchling")(root)
     monkeypatch.setenv("BAR", "bar")
     assert backend.expand_line("demo @ {root:uri}/demo") == f"demo @ {root_url}/demo"
@@ -65,7 +65,7 @@ def test_hatch_expand_variables(monkeypatch):
 
 def test_pdm_backend_expand_variables(monkeypatch):
     root = Path().absolute()
-    root_url = path_to_url(root.as_posix())
+    root_url = root.as_uri()
     backend = get_backend("pdm-backend")(root)
     monkeypatch.setenv("BAR", "bar")
     assert backend.expand_line("demo @ file:///${PROJECT_ROOT}/demo") == f"demo @ {root_url}/demo"