Correctly normalize relative paths for 'pip show'

Author: uranusjr

src/pip/_internal/commands/show.py

@@ -1,8 +1,8 @@
 import csv
 import logging
-import os
+import pathlib
 from optparse import Values
-from typing import Iterator, List, NamedTuple, Optional
+from typing import Iterator, List, NamedTuple, Optional, Tuple
 
 from pip._vendor.packaging.utils import canonicalize_name
 
@@ -66,6 +66,33 @@ class _PackageInfo(NamedTuple):
     files: Optional[List[str]]
 
 
+def _covert_legacy_entry(entry: Tuple[str, ...], info: Tuple[str, ...]) -> str:
+    """Convert a legacy installed-files.txt path into modern RECORD path.
+
+    The legacy format stores paths relative to the info directory, while the
+    modern format stores paths relative to the package root, e.g. the
+    site-packages directory.
+
+    :param entry: Path parts of the installed-files.txt entry.
+    :param info: Path parts of the egg-info directory relative to package root.
+    :returns: The converted entry.
+
+    For best compatibility with symlinks, this does not use ``abspath()`` or
+    ``Path.resolve()``, but tries to work with path parts:
+
+    1. While ``entry`` starts with ``..``, remove the equal amounts of parts
+       from ``info``; if ``info`` is empty, start appending ``..`` instead.
+    2. Join the two directly.
+    """
+    while entry and entry[0] == "..":
+        if not info or info[-1] == "..":
+            info += ("..",)
+        else:
+            info = info[:-1]
+        entry = entry[1:]
+    return str(pathlib.Path(*info, *entry))
+
+
 def search_packages_info(query: List[str]) -> Iterator[_PackageInfo]:
     """
     Gather details from installed distributions. Print distribution name,
@@ -100,14 +127,29 @@ def _files_from_record(dist: BaseDistribution) -> Optional[Iterator[str]]:
             text = dist.read_text('RECORD')
         except FileNotFoundError:
             return None
-        return (row[0] for row in csv.reader(text.splitlines()))
+        # This extra Path-str cast normalizes entries.
+        return (str(pathlib.Path(row[0])) for row in csv.reader(text.splitlines()))
 
-    def _files_from_installed_files(dist: BaseDistribution) -> Optional[Iterator[str]]:
+    def _files_from_legacy(dist: BaseDistribution) -> Optional[Iterator[str]]:
         try:
             text = dist.read_text('installed-files.txt')
         except FileNotFoundError:
             return None
-        return (p for p in text.splitlines(keepends=False) if p)
+        paths = (p for p in text.splitlines(keepends=False) if p)
+        root = dist.location
+        info = dist.info_directory
+        if root is None or info is None:
+            return paths
+        try:
+            info_rel = pathlib.Path(info).relative_to(root)
+        except ValueError:  # info is not relative to root.
+            return paths
+        if not info_rel.parts:  # info *is* root.
+            return paths
+        return (
+            _covert_legacy_entry(pathlib.Path(p).parts, info_rel.parts)
+            for p in paths
+        )
 
     for query_name in query_names:
         try:
@@ -121,11 +163,11 @@ def _files_from_installed_files(dist: BaseDistribution) -> Optional[Iterator[str
         except FileNotFoundError:
             entry_points = []
 
-        files_iter = _files_from_record(dist) or _files_from_installed_files(dist)
+        files_iter = _files_from_record(dist) or _files_from_legacy(dist)
         if files_iter is None:
             files: Optional[List[str]] = None
         else:
-            files = sorted(os.path.relpath(p, dist.location) for p in files_iter)
+            files = sorted(files_iter)
 
         metadata = dist.metadata
 

src/pip/_internal/metadata/base.py

@@ -57,6 +57,26 @@ def location(self) -> Optional[str]:
         A string value is not necessarily a filesystem path, since distributions
         can be loaded from other sources, e.g. arbitrary zip archives. ``None``
         means the distribution is created in-memory.
+
+        Do not canonicalize this value with e.g. ``pathlib.Path.resolve()``. If
+        this is a symbolic link, we want to preserve the relative path between
+        it and files in the distribution.
+        """
+        raise NotImplementedError()
+
+    @property
+    def info_directory(self) -> Optional[str]:
+        """Location of the .[egg|dist]-info directory.
+
+        Similarly to ``location``, a string value is not necessarily a
+        filesystem path. ``None`` means the distribution is created in-memory.
+
+        For a modern .dist-info installation on disk, this should be something
+        like ``{location}/{raw_name}-{version}.dist-info``.
+
+        Do not canonicalize this value with e.g. ``pathlib.Path.resolve()``. If
+        this is a symbolic link, we want to preserve the relative path between
+        it and other files in the distribution.
         """
         raise NotImplementedError()
 

src/pip/_internal/metadata/pkg_resources.py

@@ -48,6 +48,10 @@ def from_wheel(cls, path: str, name: str) -> "Distribution":
     def location(self) -> Optional[str]:
         return self._dist.location
 
+    @property
+    def info_directory(self) -> Optional[str]:
+        return self._dist.egg_info
+
     @property
     def canonical_name(self) -> "NormalizedName":
         return canonicalize_name(self._dist.project_name)

src/pip/_internal/operations/install/legacy.py

@@ -25,6 +25,46 @@ def __init__(self):
         self.parent = sys.exc_info()
 
 
+def write_installed_files_from_setuptools_record(
+    record_lines: List[str],
+    root: Optional[str],
+    req_description: str,
+) -> None:
+    def prepend_root(path):
+        # type: (str) -> str
+        if root is None or not os.path.isabs(path):
+            return path
+        else:
+            return change_root(root, path)
+
+    for line in record_lines:
+        directory = os.path.dirname(line)
+        if directory.endswith('.egg-info'):
+            egg_info_dir = prepend_root(directory)
+            break
+    else:
+        message = (
+            "{} did not indicate that it installed an "
+            ".egg-info directory. Only setup.py projects "
+            "generating .egg-info directories are supported."
+        ).format(req_description)
+        raise InstallationError(message)
+
+    new_lines = []
+    for line in record_lines:
+        filename = line.strip()
+        if os.path.isdir(filename):
+            filename += os.path.sep
+        new_lines.append(
+            os.path.relpath(prepend_root(filename), egg_info_dir)
+        )
+    new_lines.sort()
+    ensure_dir(egg_info_dir)
+    inst_files_path = os.path.join(egg_info_dir, 'installed-files.txt')
+    with open(inst_files_path, 'w') as f:
+        f.write('\n'.join(new_lines) + '\n')
+
+
 def install(
     install_options,  # type: List[str]
     global_options,  # type: Sequence[str]
@@ -88,38 +128,5 @@ def install(
         with open(record_filename) as f:
             record_lines = f.read().splitlines()
 
-    def prepend_root(path):
-        # type: (str) -> str
-        if root is None or not os.path.isabs(path):
-            return path
-        else:
-            return change_root(root, path)
-
-    for line in record_lines:
-        directory = os.path.dirname(line)
-        if directory.endswith('.egg-info'):
-            egg_info_dir = prepend_root(directory)
-            break
-    else:
-        message = (
-            "{} did not indicate that it installed an "
-            ".egg-info directory. Only setup.py projects "
-            "generating .egg-info directories are supported."
-        ).format(req_description)
-        raise InstallationError(message)
-
-    new_lines = []
-    for line in record_lines:
-        filename = line.strip()
-        if os.path.isdir(filename):
-            filename += os.path.sep
-        new_lines.append(
-            os.path.relpath(prepend_root(filename), egg_info_dir)
-        )
-    new_lines.sort()
-    ensure_dir(egg_info_dir)
-    inst_files_path = os.path.join(egg_info_dir, 'installed-files.txt')
-    with open(inst_files_path, 'w') as f:
-        f.write('\n'.join(new_lines) + '\n')
-
+    write_installed_files_from_setuptools_record(record_lines, root, req_description)
     return True

tests/functional/test_show.py

@@ -1,10 +1,12 @@
 import os
 import re
 
-import pytest
-
 from pip import __version__
 from pip._internal.commands.show import search_packages_info
+from pip._internal.operations.install.legacy import (
+    write_installed_files_from_setuptools_record,
+)
+from pip._internal.utils.unpacking import untar_file
 from tests.lib import create_test_package_with_setup
 
 
@@ -41,7 +43,7 @@ def test_show_with_files_not_found(script, data):
 
 def test_show_with_files_from_wheel(script, data):
     """
-    Test that a wheel's files can be listed
+    Test that a wheel's files can be listed.
     """
     wheel_file = data.packages.joinpath('simple.dist-0.1-py2.py3-none-any.whl')
     script.pip('install', '--no-index', wheel_file)
@@ -50,18 +52,36 @@ def test_show_with_files_from_wheel(script, data):
     assert 'Name: simple.dist' in lines
     assert 'Cannot locate RECORD or installed-files.txt' not in lines[6], lines[6]
     assert re.search(r"Files:\n(  .+\n)+", result.stdout)
+    assert f"  simpledist{os.sep}__init__.py" in lines[6:]
 
 
-@pytest.mark.network
-def test_show_with_all_files(script):
+def test_show_with_files_from_legacy(tmp_path, script, data):
     """
-    Test listing all files in the show command.
+    Test listing files in the show command (legacy installed-files.txt).
     """
-    script.pip('install', 'initools==0.2')
-    result = script.pip('show', '--files', 'initools')
+    # Since 'pip install' now always tries to build a wheel from sdist, it
+    # cannot properly generate a setup. The legacy code path is basically
+    # 'setup.py install' plus installed-files.txt, which we manually generate.
+    source_dir = tmp_path.joinpath("unpacked-sdist")
+    setuptools_record = tmp_path.joinpath("installed-record.txt")
+    untar_file(data.packages.joinpath("simple-1.0.tar.gz"), str(source_dir))
+    script.run(
+        "python", "setup.py", "install",
+        "--single-version-externally-managed",
+        "--record", str(setuptools_record),
+        cwd=source_dir,
+    )
+    write_installed_files_from_setuptools_record(
+        setuptools_record.read_text().splitlines(),
+        root=None,
+        req_description="simple==1.0",
+    )
+
+    result = script.pip('show', '--files', 'simple')
     lines = result.stdout.splitlines()
     assert 'Cannot locate RECORD or installed-files.txt' not in lines[6], lines[6]
     assert re.search(r"Files:\n(  .+\n)+", result.stdout)
+    assert f"  simple{os.sep}__init__.py" in lines[6:]
 
 
 def test_missing_argument(script):