From 613cc01c0b2a4fbb2d2c399a20003ec07d91474a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul=20=28ACSONE=29?= Date: Sun, 10 Nov 2019 21:41:41 +0100 Subject: [PATCH 1/4] Include subdirectory URL fragment in the cache key --- news/7333.bugfix | 1 + src/pip/_internal/cache.py | 10 +++++++++ tests/unit/test_cache.py | 45 ++++++++++++++++++++++++++++++++------ 3 files changed, 49 insertions(+), 7 deletions(-) create mode 100644 news/7333.bugfix diff --git a/news/7333.bugfix b/news/7333.bugfix new file mode 100644 index 00000000000..8ddcba76a39 --- /dev/null +++ b/news/7333.bugfix @@ -0,0 +1 @@ +Include ``subdirectory`` URL fragments in cache keys. diff --git a/src/pip/_internal/cache.py b/src/pip/_internal/cache.py index c5431e14d14..b3327ea9a57 100644 --- a/src/pip/_internal/cache.py +++ b/src/pip/_internal/cache.py @@ -58,6 +58,10 @@ def _get_cache_path_parts(self, link): key_parts = [link.url_without_fragment] if link.hash_name is not None and link.hash is not None: key_parts.append("=".join([link.hash_name, link.hash])) + if link.subdirectory_fragment: + key_parts.append( + "=".join(["subdirectory", link.subdirectory_fragment]) + ) key_url = "#".join(key_parts) # Encode our key url with sha224, we'll use this because it has similar @@ -168,11 +172,17 @@ def get( # type: (...) -> Link candidates = [] + canonical_package_name = None + if package_name: + canonical_package_name = canonicalize_name(package_name) for wheel_name in self._get_candidates(link, package_name): try: wheel = Wheel(wheel_name) except InvalidWheelFilename: continue + assert canonical_package_name + if wheel.name != canonical_package_name: + continue if not wheel.supported(supported_tags): # Built for a different python/arch/etc continue diff --git a/tests/unit/test_cache.py b/tests/unit/test_cache.py index d75cd2c654f..79e4f624d19 100644 --- a/tests/unit/test_cache.py +++ b/tests/unit/test_cache.py @@ -1,13 +1,44 @@ +import os + from pip._internal.cache import WheelCache +from pip._internal.models.format_control import FormatControl +from pip._internal.models.link import Link from pip._internal.utils.compat import expanduser +from pip._internal.utils.misc import ensure_dir + + +def test_expands_path(): + wc = WheelCache("~/.foo/", None) + assert wc.cache_dir == expanduser("~/.foo/") + + +def test_falsey_path_none(): + wc = WheelCache(False, None) + assert wc.cache_dir is None -class TestWheelCache: +def test_subdirectory_fragment(): + """ + Test the subdirectory URL fragment is part of the cache key. + """ + wc = WheelCache("~/.foo/", None) + link1 = Link("git+https://g.c/o/r#subdirectory=d1") + link2 = Link("git+https://g.c/o/r#subdirectory=d2") + assert wc.get_path_for_link(link1) != wc.get_path_for_link(link2) - def test_expands_path(self): - wc = WheelCache("~/.foo/", None) - assert wc.cache_dir == expanduser("~/.foo/") - def test_falsey_path_none(self): - wc = WheelCache(False, None) - assert wc.cache_dir is None +def test_wheel_name_filter(tmpdir): + """ + Test the wheel cache filters on wheel name when several wheels + for different package are stored under the same cache directory. + """ + wc = WheelCache(tmpdir, FormatControl()) + link = Link("https://g.c/package.tar.gz") + cache_path = wc.get_path_for_link(link) + ensure_dir(cache_path) + with open(os.path.join(cache_path, "package-1.0-py3-none-any.whl"), "w"): + pass + # package matches wheel name + assert wc.get(link, "package", [("py3", "none", "any")]) is not link + # package2 does not match wheel name + assert wc.get(link, "package2", [("py3", "none", "any")]) is link From dcb8669f215cd488200c0d5e8ca1026977a4a5af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul=20=28ACSONE=29?= Date: Wed, 13 Nov 2019 11:43:56 +0100 Subject: [PATCH 2/4] Simplify SimpleWheelCache.get in case package_name is falsy --- src/pip/_internal/cache.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/pip/_internal/cache.py b/src/pip/_internal/cache.py index b3327ea9a57..432b652ec1e 100644 --- a/src/pip/_internal/cache.py +++ b/src/pip/_internal/cache.py @@ -172,15 +172,15 @@ def get( # type: (...) -> Link candidates = [] - canonical_package_name = None - if package_name: - canonical_package_name = canonicalize_name(package_name) + if not package_name: + return link + + canonical_package_name = canonicalize_name(package_name) for wheel_name in self._get_candidates(link, package_name): try: wheel = Wheel(wheel_name) except InvalidWheelFilename: continue - assert canonical_package_name if wheel.name != canonical_package_name: continue if not wheel.supported(supported_tags): From a4d12f807fb4be7998b7f83572f020a447fc9956 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul=20=28ACSONE=29?= Date: Wed, 13 Nov 2019 11:57:30 +0100 Subject: [PATCH 3/4] Avoid double package name canonicalization --- src/pip/_internal/cache.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/pip/_internal/cache.py b/src/pip/_internal/cache.py index 432b652ec1e..5173a070fed 100644 --- a/src/pip/_internal/cache.py +++ b/src/pip/_internal/cache.py @@ -77,19 +77,18 @@ def _get_cache_path_parts(self, link): return parts - def _get_candidates(self, link, package_name): + def _get_candidates(self, link, canonical_package_name): # type: (Link, Optional[str]) -> List[Any] can_not_cache = ( not self.cache_dir or - not package_name or + not canonical_package_name or not link ) if can_not_cache: return [] - canonical_name = canonicalize_name(package_name) formats = self.format_control.get_allowed_formats( - canonical_name + canonical_package_name ) if not self.allowed_formats.intersection(formats): return [] @@ -176,12 +175,12 @@ def get( return link canonical_package_name = canonicalize_name(package_name) - for wheel_name in self._get_candidates(link, package_name): + for wheel_name in self._get_candidates(link, canonical_package_name): try: wheel = Wheel(wheel_name) except InvalidWheelFilename: continue - if wheel.name != canonical_package_name: + if canonicalize_name(wheel.name) != canonical_package_name: continue if not wheel.supported(supported_tags): # Built for a different python/arch/etc From 47ae034da8f10b854c5f6febb54a7794deea86e1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?St=C3=A9phane=20Bidoul=20=28ACSONE=29?= Date: Wed, 13 Nov 2019 15:13:04 +0100 Subject: [PATCH 4/4] Debug logging in case of unexpected wheel name in cache --- src/pip/_internal/cache.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/pip/_internal/cache.py b/src/pip/_internal/cache.py index 5173a070fed..7aa72b9a774 100644 --- a/src/pip/_internal/cache.py +++ b/src/pip/_internal/cache.py @@ -181,6 +181,12 @@ def get( except InvalidWheelFilename: continue if canonicalize_name(wheel.name) != canonical_package_name: + logger.debug( + "Ignoring cached wheel {} for {} as it " + "does not match the expected distribution name {}.".format( + wheel_name, link, package_name + ) + ) continue if not wheel.supported(supported_tags): # Built for a different python/arch/etc