Repository: pypa/pip
Pull request #11696 (2023-01-04 10:24:57)

Merge link_hash back into _hashes
I’m not sure how to describe this, but my instinct is trying to tell me something’s wrong.

I was investigating #11692 and suddenly realised that `Link` currently has a totally unused attribute `_hashes`. This was introduced quite a while ago, and was (as the name suggests) used to check whether the linked file is valid. But in bad03ef931d9b3ff4f9e75f35f9c41f45839e2a1, a new attribute `link_hash` was introduced to do the same thing (? not entirely sure) but more limited (only one hash value) and removed all calls to `_hashes`. However, there are still code that populates the entirely unused `_hashes`.

Since the plural variant covers more use cases (a file can be hashed with multiple algorithms), I restored the old logic that uses `_hashes` before the commit, and consolidate `link_hash` back into that attribute.

cc @cosmicexplorer as the person that introduced `link_hash`.


Patch:
diff --git a/.readthedocs-custom-redirects.yml b/.readthedocs-custom-redirects.yml
index d0c072fbf..46c1f819c 100644
--- a/.readthedocs-custom-redirects.yml
+++ b/.readthedocs-custom-redirects.yml
@@ -2,9 +2,9 @@
 # It is related to Read the Docs, but is not a file processed by the platform.
 
 /dev/news-entry-failure: >-
-  https://pip.pypa.io/en/latest/development/contributing/#news-entries
+  https://pip.pypa.io/en/stable/development/contributing/#news-entries
 /errors/resolution-impossible: >-
-  https://pip.pypa.io/en/stable/topics/dependency-resolution/#dealing-with-dependency-conflicts
+  https://pip.pypa.io/en/latest/topics/dependency-resolution/#dealing-with-dependency-conflicts
 /surveys/backtracking: >-
   https://forms.gle/LkZP95S4CfqBAU1N6
 /warnings/backtracking: >-
diff --git a/news/11623.bugfix.rst b/news/11623.bugfix.rst
deleted file mode 100644
index 45b8fe192..000000000
--- a/news/11623.bugfix.rst
+++ /dev/null
@@ -1,1 +0,0 @@
-Fix scripts path in isolated build environment on Debian.
diff --git a/src/pip/_internal/build_env.py b/src/pip/_internal/build_env.py
index 24bfa870b..e67b868e8 100644
--- a/src/pip/_internal/build_env.py
+++ b/src/pip/_internal/build_env.py
@@ -8,6 +8,7 @@ import site
 import sys
 import textwrap
 from collections import OrderedDict
+from sysconfig import get_paths
 from types import TracebackType
 from typing import TYPE_CHECKING, Iterable, List, Optional, Set, Tuple, Type
 
@@ -18,7 +19,6 @@ from pip._vendor.packaging.version import Version
 from pip import __file__ as pip_location
 from pip._internal.cli.spinners import open_spinner
 from pip._internal.locations import (
-    get_isolated_environment_bin_path,
     get_isolated_environment_lib_paths,
     get_platlib,
     get_purelib,
@@ -37,7 +37,10 @@ class _Prefix:
     def __init__(self, path: str) -> None:
         self.path = path
         self.setup = False
-        self.bin_dir = get_isolated_environment_bin_path(path)
+        self.bin_dir = get_paths(
+            "nt" if os.name == "nt" else "posix_prefix",
+            vars={"base": path, "platbase": path},
+        )["scripts"]
         self.lib_dirs = get_isolated_environment_lib_paths(path)
 
 
diff --git a/src/pip/_internal/locations/__init__.py b/src/pip/_internal/locations/__init__.py
index 496844be1..516bd6078 100644
--- a/src/pip/_internal/locations/__init__.py
+++ b/src/pip/_internal/locations/__init__.py
@@ -25,10 +25,9 @@ __all__ = [
     "USER_CACHE_DIR",
     "get_bin_prefix",
     "get_bin_user",
-    "get_isolated_environment_bin_path",
-    "get_isolated_environment_lib_paths",
     "get_major_minor_version",
     "get_platlib",
+    "get_isolated_environment_lib_paths",
     "get_purelib",
     "get_scheme",
     "get_src_prefix",
@@ -527,7 +526,3 @@ def get_isolated_environment_lib_paths(prefix: str) -> List[str]:
         _log_context(prefix=prefix)
 
     return old_lib_paths
-
-
-def get_isolated_environment_bin_path(prefix: str) -> str:
-    return _sysconfig.get_isolated_environment_paths(prefix)["scripts"]
diff --git a/src/pip/_internal/locations/_sysconfig.py b/src/pip/_internal/locations/_sysconfig.py
index 38e400f94..698215720 100644
--- a/src/pip/_internal/locations/_sysconfig.py
+++ b/src/pip/_internal/locations/_sysconfig.py
@@ -213,13 +213,10 @@ def get_platlib() -> str:
     return sysconfig.get_paths()["platlib"]
 
 
-def get_isolated_environment_paths(prefix: str) -> typing.Dict[str, str]:
-    variables = {"base": prefix, "platbase": prefix}
-    if "venv" in sysconfig.get_scheme_names():
-        return sysconfig.get_paths(vars=variables, scheme="venv")
-    return sysconfig.get_paths(vars=variables)
-
-
 def get_isolated_environment_lib_paths(prefix: str) -> typing.Tuple[str, str]:
-    paths = get_isolated_environment_paths(prefix)
+    vars = {"base": prefix, "platbase": prefix}
+    if "venv" in sysconfig.get_scheme_names():
+        paths = sysconfig.get_paths(vars=vars, scheme="venv")
+    else:
+        paths = sysconfig.get_paths(vars=vars)
     return (paths["purelib"], paths["platlib"])
diff --git a/src/pip/_internal/models/link.py b/src/pip/_internal/models/link.py
index c7c4b0e9b..a1e4d5a08 100644
--- a/src/pip/_internal/models/link.py
+++ b/src/pip/_internal/models/link.py
@@ -79,6 +79,9 @@ class LinkHash:
         name, value = match.groups()
         return cls(name=name, value=value)
 
+    def as_dict(self) -> Dict[str, str]:
+        return {self.name: self.value}
+
     def as_hashes(self) -> Hashes:
         """Return a Hashes instance which checks only for the current hash."""
         return Hashes({self.name: [self.value]})
@@ -165,7 +168,6 @@ class Link(KeyBasedCompareMixin):
         "requires_python",
         "yanked_reason",
         "dist_info_metadata",
-        "link_hash",
         "cache_link_parsing",
         "egg_fragment",
     ]
@@ -177,7 +179,6 @@ class Link(KeyBasedCompareMixin):
         requires_python: Optional[str] = None,
         yanked_reason: Optional[str] = None,
         dist_info_metadata: Optional[str] = None,
-        link_hash: Optional[LinkHash] = None,
         cache_link_parsing: bool = True,
         hashes: Optional[Mapping[str, str]] = None,
     ) -> None:
@@ -200,16 +201,11 @@ class Link(KeyBasedCompareMixin):
             attribute, if present, in a simple repository HTML link. This may be parsed
             into its own `Link` by `self.metadata_link()`. See PEP 658 for more
             information and the specification.
-        :param link_hash: a checksum for the content the link points to. If not
-            provided, this will be extracted from the link URL, if the URL has
-            any checksum.
         :param cache_link_parsing: A flag that is used elsewhere to determine
-                                   whether resources retrieved from this link
-                                   should be cached. PyPI index urls should
-                                   generally have this set to False, for
-                                   example.
+            whether resources retrieved from this link should be cached. PyPI
+            URLs should generally have this set to False, for example.
         :param hashes: A mapping of hash names to digests to allow us to
-                       determine the validity of a download.
+            determine the validity of a download.
         """
 
         # url can be a UNC windows share
@@ -220,13 +216,18 @@ class Link(KeyBasedCompareMixin):
         # Store the url as a private attribute to prevent accidentally
         # trying to set a new value.
         self._url = url
-        self._hashes = hashes if hashes is not None else {}
+
+        link_hash = LinkHash.split_hash_name_and_value(url)
+        hashes_from_link = {} if link_hash is None else link_hash.as_dict()
+        if hashes is None:
+            self._hashes = hashes_from_link
+        else:
+            self._hashes = {**hashes, **hashes_from_link}
 
         self.comes_from = comes_from
         self.requires_python = requires_python if requires_python else None
         self.yanked_reason = yanked_reason
         self.dist_info_metadata = dist_info_metadata
-        self.link_hash = link_hash or LinkHash.split_hash_name_and_value(self._url)
 
         super().__init__(key=url, defining_class=Link)
 
@@ -401,29 +402,26 @@ class Link(KeyBasedCompareMixin):
         if self.dist_info_metadata is None:
             return None
         metadata_url = f"{self.url_without_fragment}.metadata"
-        link_hash: Optional[LinkHash] = None
         # If data-dist-info-metadata="true" is set, then the metadata file exists,
         # but there is no information about its checksum or anything else.
         if self.dist_info_metadata != "true":
             link_hash = LinkHash.split_hash_name_and_value(self.dist_info_metadata)
-        return Link(metadata_url, link_hash=link_hash)
+        else:
+            link_hash = None
+        if link_hash is None:
+            return Link(metadata_url)
+        return Link(metadata_url, hashes=link_hash.as_dict())
 
-    def as_hashes(self) -> Optional[Hashes]:
-        if self.link_hash is not None:
-            return self.link_hash.as_hashes()
-        return None
+    def as_hashes(self) -> Hashes:
+        return Hashes({k: [v] for k, v in self._hashes.items()})
 
     @property
     def hash(self) -> Optional[str]:
-        if self.link_hash is not None:
-            return self.link_hash.value
-        return None
+        return next(iter(self._hashes.values()), None)
 
     @property
     def hash_name(self) -> Optional[str]:
-        if self.link_hash is not None:
-            return self.link_hash.name
-        return None
+        return next(iter(self._hashes), None)
 
     @property
     def show_url(self) -> str:
@@ -452,15 +450,15 @@ class Link(KeyBasedCompareMixin):
 
     @property
     def has_hash(self) -> bool:
-        return self.link_hash is not None
+        return bool(self._hashes)
 
     def is_hash_allowed(self, hashes: Optional[Hashes]) -> bool:
         """
         Return True if the link has a hash and it is allowed by `hashes`.
         """
-        if self.link_hash is None:
+        if hashes is None:
             return False
-        return self.link_hash.is_hash_allowed(hashes)
+        return any(hashes.is_hash_allowed(k, v) for k, v in self._hashes.items())
 
 
 class _CleanResult(NamedTuple):


Test patch:
diff --git a/tests/unit/test_collector.py b/tests/unit/test_collector.py
index 55676a4fc..47307c00e 100644
--- a/tests/unit/test_collector.py
+++ b/tests/unit/test_collector.py
@@ -6,7 +6,7 @@ import re
 import uuid
 from pathlib import Path
 from textwrap import dedent
-from typing import List, Optional, Tuple
+from typing import Dict, List, Optional, Tuple
 from unittest import mock
 
 import pytest
@@ -538,7 +538,7 @@ def test_parse_links_json() -> None:
         metadata_link.url
         == "https://example.com/files/holygrail-1.0-py3-none-any.whl.metadata"
     )
-    assert metadata_link.link_hash == LinkHash("sha512", "aabdd41")
+    assert metadata_link._hashes == {"sha512": "aabdd41"}
 
 
 @pytest.mark.parametrize(
@@ -575,41 +575,41 @@ _pkg1_requirement = Requirement("pkg1==1.0")
 
 
 @pytest.mark.parametrize(
-    "anchor_html, expected, link_hash",
+    "anchor_html, expected, hashes",
     [
         # Test not present.
         (
             '<a href="/pkg1-1.0.tar.gz"></a>',
             None,
-            None,
+            {},
         ),
         # Test with value "true".
         (
             '<a href="/pkg1-1.0.tar.gz" data-dist-info-metadata="true"></a>',
             "true",
-            None,
+            {},
         ),
         # Test with a provided hash value.
         (
             '<a href="/pkg1-1.0.tar.gz" data-dist-info-metadata="sha256=aa113592bbe"></a>',  # noqa: E501
             "sha256=aa113592bbe",
-            None,
+            {},
         ),
         # Test with a provided hash value for both the requirement as well as metadata.
         (
             '<a href="/pkg1-1.0.tar.gz#sha512=abc132409cb" data-dist-info-metadata="sha256=aa113592bbe"></a>',  # noqa: E501
             "sha256=aa113592bbe",
-            LinkHash("sha512", "abc132409cb"),
+            {"sha512": "abc132409cb"},
         ),
     ],
 )
 def test_parse_links__dist_info_metadata(
     anchor_html: str,
     expected: Optional[str],
-    link_hash: Optional[LinkHash],
+    hashes: Dict[str, str],
 ) -> None:
     link = _test_parse_links_data_attribute(anchor_html, "dist_info_metadata", expected)
-    assert link.link_hash == link_hash
+    assert link._hashes == hashes
 
 
 def test_parse_links_caches_same_page_by_url() -> None: