From c32777f19ec035a29ca8612937b3d3e9c8cd8549 Mon Sep 17 00:00:00 2001 From: Sviatoslav Sydorenko Date: Mon, 11 Sep 2023 01:23:21 +0200 Subject: [PATCH 01/14] Get rid of the ``ensurepip`` infra for many wheels This is a refactoring change that aims to simplify ``ensurepip``. Before it, this module had legacy infrastructure that made an assumption that ``ensurepip`` would be provisioning more then just a single wheel. That assumption is no longer true since [[1]][[2]][[3]]. In this change, the improvement is done around removing unnecessary loops and supporting structures to change the assumptions to expect only the bundled or replacement ``pip`` wheel. [1]: https://github.com/python/cpython/commit/ece20db [2]: https://github.com/python/cpython/pull/101039 [3]: https://github.com/python/cpython/issues/95299 --- Lib/ensurepip/__init__.py | 100 +++++++++--------- Lib/test/test_ensurepip.py | 65 +++++++++--- ...-09-10-23-35-20.gh-issue-109245.yCVQbo.rst | 3 + 3 files changed, 101 insertions(+), 67 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2023-09-10-23-35-20.gh-issue-109245.yCVQbo.rst diff --git a/Lib/ensurepip/__init__.py b/Lib/ensurepip/__init__.py index a09bf3201e1fb7..e5f0cd05128dbd 100644 --- a/Lib/ensurepip/__init__.py +++ b/Lib/ensurepip/__init__.py @@ -5,15 +5,13 @@ import sys import sysconfig import tempfile +from contextlib import suppress +from functools import cache from importlib import resources __all__ = ["version", "bootstrap"] -_PACKAGE_NAMES = ('pip',) _PIP_VERSION = "23.3.2" -_PROJECTS = [ - ("pip", _PIP_VERSION, "py3"), -] # Packages bundled in ensurepip._bundled have wheel_name set. # Packages from WHEEL_PKG_DIR have wheel_path set. @@ -27,8 +25,13 @@ _WHEEL_PKG_DIR = sysconfig.get_config_var('WHEEL_PKG_DIR') -def _find_packages(path): - packages = {} +def _find_packages(path: str | None) -> _Package: + if path is None: + raise LookupError( + 'The compile-time `WHEEL_PKG_DIR` is unset so there is ' + 'no place for looking up the wheels.', + ) + try: filenames = os.listdir(path) except OSError: @@ -38,41 +41,39 @@ def _find_packages(path): # of the same package, but don't attempt to implement correct version # comparison since this case should not happen. filenames = sorted(filenames) + pip_pkg = None for filename in filenames: # filename is like 'pip-21.2.4-py3-none-any.whl' if not filename.endswith(".whl"): continue - for name in _PACKAGE_NAMES: - prefix = name + '-' - if filename.startswith(prefix): - break - else: + if not filename.startswith('pip-'): continue # Extract '21.2.4' from 'pip-21.2.4-py3-none-any.whl' - version = filename.removeprefix(prefix).partition('-')[0] + discovered_pip_pkg_version = filename.removeprefix( + 'pip-', + ).partition('-')[0] wheel_path = os.path.join(path, filename) - packages[name] = _Package(version, None, wheel_path) - return packages + pip_pkg = _Package(discovered_pip_pkg_version, None, wheel_path) + + if pip_pkg is None: + raise LookupError( + '`WHEEL_PKG_DIR` does not contain any wheel files for `pip`.', + ) + + return pip_pkg -def _get_packages(): - global _PACKAGES, _WHEEL_PKG_DIR - if _PACKAGES is not None: - return _PACKAGES +@cache +def _get_usable_pip_package() -> _Package: + wheel_name = f"pip-{_PIP_VERSION}-py3-none-any.whl" + pip_pkg = _Package(_PIP_VERSION, wheel_name, None) - packages = {} - for name, version, py_tag in _PROJECTS: - wheel_name = f"{name}-{version}-{py_tag}-none-any.whl" - packages[name] = _Package(version, wheel_name, None) - if _WHEEL_PKG_DIR: - dir_packages = _find_packages(_WHEEL_PKG_DIR) - # only used the wheel package directory if all packages are found there - if all(name in dir_packages for name in _PACKAGE_NAMES): - packages = dir_packages - _PACKAGES = packages - return packages -_PACKAGES = None + with suppress(LookupError): + # only use the wheel package directory if all packages are found there + pip_pkg = _find_packages(_WHEEL_PKG_DIR) + + return pip_pkg def _run_pip(args, additional_paths=None): @@ -105,7 +106,7 @@ def version(): """ Returns a string specifying the bundled version of pip. """ - return _get_packages()['pip'].version + return _get_usable_pip_package().version def _disable_pip_configuration_settings(): @@ -167,24 +168,21 @@ def _bootstrap(*, root=None, upgrade=False, user=False, with tempfile.TemporaryDirectory() as tmpdir: # Put our bundled wheels into a temporary directory and construct the # additional paths that need added to sys.path - additional_paths = [] - for name, package in _get_packages().items(): - if package.wheel_name: - # Use bundled wheel package - wheel_name = package.wheel_name - wheel_path = resources.files("ensurepip") / "_bundled" / wheel_name - whl = wheel_path.read_bytes() - else: - # Use the wheel package directory - with open(package.wheel_path, "rb") as fp: - whl = fp.read() - wheel_name = os.path.basename(package.wheel_path) - - filename = os.path.join(tmpdir, wheel_name) - with open(filename, "wb") as fp: - fp.write(whl) - - additional_paths.append(filename) + package = _get_usable_pip_package() + if package.wheel_name: + # Use bundled wheel package + wheel_name = package.wheel_name + wheel_path = resources.files("ensurepip") / "_bundled" / wheel_name + whl = wheel_path.read_bytes() + else: + # Use the wheel package directory + with open(package.wheel_path, "rb") as fp: + whl = fp.read() + wheel_name = os.path.basename(package.wheel_path) + + filename = os.path.join(tmpdir, wheel_name) + with open(filename, "wb") as fp: + fp.write(whl) # Construct the arguments to be passed to the pip command args = ["install", "--no-cache-dir", "--no-index", "--find-links", tmpdir] @@ -197,7 +195,7 @@ def _bootstrap(*, root=None, upgrade=False, user=False, if verbosity: args += ["-" + "v" * verbosity] - return _run_pip([*args, *_PACKAGE_NAMES], additional_paths) + return _run_pip([*args, "pip"], [filename]) def _uninstall_helper(*, verbosity=0): """Helper to support a clean default uninstall process on Windows @@ -227,7 +225,7 @@ def _uninstall_helper(*, verbosity=0): if verbosity: args += ["-" + "v" * verbosity] - return _run_pip([*args, *reversed(_PACKAGE_NAMES)]) + return _run_pip([*args, "pip"]) def _main(argv=None): diff --git a/Lib/test/test_ensurepip.py b/Lib/test/test_ensurepip.py index 69ab2a4feaa938..e190025c54035a 100644 --- a/Lib/test/test_ensurepip.py +++ b/Lib/test/test_ensurepip.py @@ -6,12 +6,19 @@ import test.support import unittest import unittest.mock +from pathlib import Path import ensurepip import ensurepip._uninstall class TestPackages(unittest.TestCase): + def setUp(self): + ensurepip._get_usable_pip_package.cache_clear() + + def tearDown(self): + ensurepip._get_usable_pip_package.cache_clear() + def touch(self, directory, filename): fullname = os.path.join(directory, filename) open(fullname, "wb").close() @@ -20,42 +27,44 @@ def test_version(self): # Test version() with tempfile.TemporaryDirectory() as tmpdir: self.touch(tmpdir, "pip-1.2.3b1-py2.py3-none-any.whl") - with (unittest.mock.patch.object(ensurepip, '_PACKAGES', None), - unittest.mock.patch.object(ensurepip, '_WHEEL_PKG_DIR', tmpdir)): + with unittest.mock.patch.object( + ensurepip, '_WHEEL_PKG_DIR', tmpdir, + ): self.assertEqual(ensurepip.version(), '1.2.3b1') def test_get_packages_no_dir(self): # Test _get_packages() without a wheel package directory - with (unittest.mock.patch.object(ensurepip, '_PACKAGES', None), - unittest.mock.patch.object(ensurepip, '_WHEEL_PKG_DIR', None)): - packages = ensurepip._get_packages() + with unittest.mock.patch.object(ensurepip, '_WHEEL_PKG_DIR', None): + pip_pkg = ensurepip._get_usable_pip_package() - # when bundled wheel packages are used, we get _PIP_VERSION + # when bundled pip wheel package is used, we get _PIP_VERSION self.assertEqual(ensurepip._PIP_VERSION, ensurepip.version()) - # use bundled wheel packages - self.assertIsNotNone(packages['pip'].wheel_name) + # use bundled pip wheel package + self.assertIsNotNone(pip_pkg.wheel_name) def test_get_packages_with_dir(self): # Test _get_packages() with a wheel package directory + older_pip_filename = "pip-1.2.3-py2.py3-none-any.whl" pip_filename = "pip-20.2.2-py2.py3-none-any.whl" with tempfile.TemporaryDirectory() as tmpdir: + self.touch(tmpdir, older_pip_filename) self.touch(tmpdir, pip_filename) # not used, make sure that it's ignored self.touch(tmpdir, "wheel-0.34.2-py2.py3-none-any.whl") + # not used, make sure that it's ignored + self.touch(tmpdir, "non-whl") - with (unittest.mock.patch.object(ensurepip, '_PACKAGES', None), - unittest.mock.patch.object(ensurepip, '_WHEEL_PKG_DIR', tmpdir)): - packages = ensurepip._get_packages() + with unittest.mock.patch.object( + ensurepip, '_WHEEL_PKG_DIR', tmpdir, + ): + pip_pkg = ensurepip._get_usable_pip_package() - self.assertEqual(packages['pip'].version, '20.2.2') - self.assertEqual(packages['pip'].wheel_path, + self.assertEqual(pip_pkg.version, '20.2.2') + self.assertEqual(pip_pkg.wheel_path, os.path.join(tmpdir, pip_filename)) - # wheel package is ignored - self.assertEqual(sorted(packages), ['pip']) - class EnsurepipMixin: @@ -93,6 +102,30 @@ def test_basic_bootstrapping(self): additional_paths = self.run_pip.call_args[0][1] self.assertEqual(len(additional_paths), 1) + + def test_replacement_wheel_bootstrapping(self): + ensurepip._get_usable_pip_package.cache_clear() + + pip_wheel_name = ( + f'pip-{ensurepip._PIP_VERSION !s}-' + 'py3-none-any.whl' + ) + + with tempfile.TemporaryDirectory() as tmpdir: + tmp_path = Path(tmpdir) + tmp_wheel_path = tmp_path / pip_wheel_name + tmp_wheel_path.touch() + + with unittest.mock.patch.object( + ensurepip, '_WHEEL_PKG_DIR', tmpdir, + ): + ensurepip.bootstrap() + + ensurepip._get_usable_pip_package.cache_clear() + + additional_paths = self.run_pip.call_args[0][1] + self.assertEqual(Path(additional_paths[-1]).name, pip_wheel_name) + def test_bootstrapping_with_root(self): ensurepip.bootstrap(root="/foo/bar/") diff --git a/Misc/NEWS.d/next/Library/2023-09-10-23-35-20.gh-issue-109245.yCVQbo.rst b/Misc/NEWS.d/next/Library/2023-09-10-23-35-20.gh-issue-109245.yCVQbo.rst new file mode 100644 index 00000000000000..ca7adf1c63adcf --- /dev/null +++ b/Misc/NEWS.d/next/Library/2023-09-10-23-35-20.gh-issue-109245.yCVQbo.rst @@ -0,0 +1,3 @@ +Simplified ``ensurepip`` to stop assuming that it can provision multiple +wheels. The refreshed implementation now expects to only provision +a ``pip`` wheel and no other distribution packages. From d6e51b2374628315afe2393a2124f1856a96d96b Mon Sep 17 00:00:00 2001 From: Sviatoslav Sydorenko Date: Fri, 15 Sep 2023 20:33:27 +0200 Subject: [PATCH 02/14] Rename `_find_packages` to `_get_replacement_pip_package` This change is intended to make it clear that the helper now only returns one package and no more. Co-Authored-By: vstinner@python.org --- Lib/ensurepip/__init__.py | 6 +++--- Lib/test/test_ensurepip.py | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Lib/ensurepip/__init__.py b/Lib/ensurepip/__init__.py index e5f0cd05128dbd..4373633c4f4d60 100644 --- a/Lib/ensurepip/__init__.py +++ b/Lib/ensurepip/__init__.py @@ -25,7 +25,7 @@ _WHEEL_PKG_DIR = sysconfig.get_config_var('WHEEL_PKG_DIR') -def _find_packages(path: str | None) -> _Package: +def _get_replacement_pip_package(path: str | None) -> _Package: if path is None: raise LookupError( 'The compile-time `WHEEL_PKG_DIR` is unset so there is ' @@ -70,8 +70,8 @@ def _get_usable_pip_package() -> _Package: pip_pkg = _Package(_PIP_VERSION, wheel_name, None) with suppress(LookupError): - # only use the wheel package directory if all packages are found there - pip_pkg = _find_packages(_WHEEL_PKG_DIR) + # only use the wheel package directory if pip wheel is found there + pip_pkg = _get_replacement_pip_package(_WHEEL_PKG_DIR) return pip_pkg diff --git a/Lib/test/test_ensurepip.py b/Lib/test/test_ensurepip.py index e190025c54035a..5bb5083148ce91 100644 --- a/Lib/test/test_ensurepip.py +++ b/Lib/test/test_ensurepip.py @@ -78,7 +78,7 @@ def setUp(self): real_devnull = os.devnull os_patch = unittest.mock.patch("ensurepip.os") patched_os = os_patch.start() - # But expose os.listdir() used by _find_packages() + # But expose os.listdir() used by _get_replacement_pip_package() patched_os.listdir = os.listdir self.addCleanup(os_patch.stop) patched_os.devnull = real_devnull From 5c3bd26a7b1c68b6d001c2665d6dd81ca47338be Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Sat, 16 Sep 2023 07:41:44 +0100 Subject: [PATCH 03/14] Remove changelog entry --- .../Library/2023-09-10-23-35-20.gh-issue-109245.yCVQbo.rst | 3 --- 1 file changed, 3 deletions(-) delete mode 100644 Misc/NEWS.d/next/Library/2023-09-10-23-35-20.gh-issue-109245.yCVQbo.rst diff --git a/Misc/NEWS.d/next/Library/2023-09-10-23-35-20.gh-issue-109245.yCVQbo.rst b/Misc/NEWS.d/next/Library/2023-09-10-23-35-20.gh-issue-109245.yCVQbo.rst deleted file mode 100644 index ca7adf1c63adcf..00000000000000 --- a/Misc/NEWS.d/next/Library/2023-09-10-23-35-20.gh-issue-109245.yCVQbo.rst +++ /dev/null @@ -1,3 +0,0 @@ -Simplified ``ensurepip`` to stop assuming that it can provision multiple -wheels. The refreshed implementation now expects to only provision -a ``pip`` wheel and no other distribution packages. From 96b8b9e3f7806d4450eb54f6c708a4d1b0f68033 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Sat, 16 Sep 2023 07:49:58 +0100 Subject: [PATCH 04/14] Simplify WHEEL_PKG_DIR search - Exit early if there are no files in the directory - Return a match early by iterating in reverse order - Merge filename checks - Inline the path variable - Remove type annotations --- Lib/ensurepip/__init__.py | 38 ++++++++++++-------------------------- Lib/test/test_ensurepip.py | 2 +- 2 files changed, 13 insertions(+), 27 deletions(-) diff --git a/Lib/ensurepip/__init__.py b/Lib/ensurepip/__init__.py index 4373633c4f4d60..fe2985bca353b0 100644 --- a/Lib/ensurepip/__init__.py +++ b/Lib/ensurepip/__init__.py @@ -25,43 +25,29 @@ _WHEEL_PKG_DIR = sysconfig.get_config_var('WHEEL_PKG_DIR') -def _get_replacement_pip_package(path: str | None) -> _Package: - if path is None: - raise LookupError( - 'The compile-time `WHEEL_PKG_DIR` is unset so there is ' - 'no place for looking up the wheels.', - ) - +def _find_wheel_pkg_dir_pip(): + if _WHEEL_PKG_DIR is None: + return None try: - filenames = os.listdir(path) + filenames = os.listdir(_WHEEL_PKG_DIR) except OSError: # Ignore: path doesn't exist or permission error - filenames = () + return None # Make the code deterministic if a directory contains multiple wheel files # of the same package, but don't attempt to implement correct version # comparison since this case should not happen. - filenames = sorted(filenames) - pip_pkg = None + filenames = sorted(filenames, reverse=True) for filename in filenames: # filename is like 'pip-21.2.4-py3-none-any.whl' - if not filename.endswith(".whl"): - continue - if not filename.startswith('pip-'): + if not filename.startswith("pip-") or not filename.endswith(".whl"): continue # Extract '21.2.4' from 'pip-21.2.4-py3-none-any.whl' - discovered_pip_pkg_version = filename.removeprefix( - 'pip-', - ).partition('-')[0] - wheel_path = os.path.join(path, filename) - pip_pkg = _Package(discovered_pip_pkg_version, None, wheel_path) + version = filename.removeprefix("pip-").partition("-")[0] + wheel_path = os.path.join(_WHEEL_PKG_DIR, filename) + return _Package(version, None, wheel_path) - if pip_pkg is None: - raise LookupError( - '`WHEEL_PKG_DIR` does not contain any wheel files for `pip`.', - ) - - return pip_pkg + return None @cache @@ -71,7 +57,7 @@ def _get_usable_pip_package() -> _Package: with suppress(LookupError): # only use the wheel package directory if pip wheel is found there - pip_pkg = _get_replacement_pip_package(_WHEEL_PKG_DIR) + pip_pkg = _find_wheel_pkg_dir_pip(_WHEEL_PKG_DIR) return pip_pkg diff --git a/Lib/test/test_ensurepip.py b/Lib/test/test_ensurepip.py index 5bb5083148ce91..3eb433c5867ed7 100644 --- a/Lib/test/test_ensurepip.py +++ b/Lib/test/test_ensurepip.py @@ -78,7 +78,7 @@ def setUp(self): real_devnull = os.devnull os_patch = unittest.mock.patch("ensurepip.os") patched_os = os_patch.start() - # But expose os.listdir() used by _get_replacement_pip_package() + # But expose os.listdir() used by _find_wheel_pkg_dir_pip() patched_os.listdir = os.listdir self.addCleanup(os_patch.stop) patched_os.devnull = real_devnull From 7b04736c1a0e7caf17aa1d146c3547817e99fec6 Mon Sep 17 00:00:00 2001 From: Adam Turner <9087854+aa-turner@users.noreply.github.com> Date: Sat, 16 Sep 2023 07:52:35 +0100 Subject: [PATCH 05/14] Simplify the pip wheel info - Return a dict with consistent fields - Remove caching - Remove type annotations - Leverage the known wheel package dir value to calculate full paths --- Lib/ensurepip/__init__.py | 39 +++++++------------- Lib/test/test_ensurepip.py | 73 ++++++++++---------------------------- 2 files changed, 32 insertions(+), 80 deletions(-) diff --git a/Lib/ensurepip/__init__.py b/Lib/ensurepip/__init__.py index fe2985bca353b0..cfc141cd22e255 100644 --- a/Lib/ensurepip/__init__.py +++ b/Lib/ensurepip/__init__.py @@ -1,23 +1,15 @@ -import collections import os import os.path import subprocess import sys import sysconfig import tempfile -from contextlib import suppress -from functools import cache from importlib import resources __all__ = ["version", "bootstrap"] _PIP_VERSION = "23.3.2" -# Packages bundled in ensurepip._bundled have wheel_name set. -# Packages from WHEEL_PKG_DIR have wheel_path set. -_Package = collections.namedtuple('Package', - ('version', 'wheel_name', 'wheel_path')) - # Directory of system wheel packages. Some Linux distribution packaging # policies recommend against bundling dependencies. For example, Fedora # installs wheel packages in the /usr/share/python-wheels/ directory and don't @@ -44,22 +36,17 @@ def _find_wheel_pkg_dir_pip(): # Extract '21.2.4' from 'pip-21.2.4-py3-none-any.whl' version = filename.removeprefix("pip-").partition("-")[0] - wheel_path = os.path.join(_WHEEL_PKG_DIR, filename) - return _Package(version, None, wheel_path) + return {"version": version, "filename": filename, "bundled": False} return None -@cache -def _get_usable_pip_package() -> _Package: - wheel_name = f"pip-{_PIP_VERSION}-py3-none-any.whl" - pip_pkg = _Package(_PIP_VERSION, wheel_name, None) - - with suppress(LookupError): - # only use the wheel package directory if pip wheel is found there - pip_pkg = _find_wheel_pkg_dir_pip(_WHEEL_PKG_DIR) - - return pip_pkg +def _get_pip_info(): + # Prefer pip from the wheel package directory, if present. + if (pip_info := _find_wheel_pkg_dir_pip()) is not None: + return pip_info + filename = f"pip-{_PIP_VERSION}-py3-none-any.whl" + return {"version": _PIP_VERSION, "filename": filename, "bundled": True} def _run_pip(args, additional_paths=None): @@ -92,7 +79,7 @@ def version(): """ Returns a string specifying the bundled version of pip. """ - return _get_usable_pip_package().version + return _get_pip_info()["version"] def _disable_pip_configuration_settings(): @@ -154,17 +141,16 @@ def _bootstrap(*, root=None, upgrade=False, user=False, with tempfile.TemporaryDirectory() as tmpdir: # Put our bundled wheels into a temporary directory and construct the # additional paths that need added to sys.path - package = _get_usable_pip_package() - if package.wheel_name: + package = _get_pip_info() + wheel_name = package["filename"] + if package["bundled"]: # Use bundled wheel package - wheel_name = package.wheel_name wheel_path = resources.files("ensurepip") / "_bundled" / wheel_name whl = wheel_path.read_bytes() else: # Use the wheel package directory - with open(package.wheel_path, "rb") as fp: + with open(os.path.join(_WHEEL_PKG_DIR, wheel_name), "rb") as fp: whl = fp.read() - wheel_name = os.path.basename(package.wheel_path) filename = os.path.join(tmpdir, wheel_name) with open(filename, "wb") as fp: @@ -183,6 +169,7 @@ def _bootstrap(*, root=None, upgrade=False, user=False, return _run_pip([*args, "pip"], [filename]) + def _uninstall_helper(*, verbosity=0): """Helper to support a clean default uninstall process on Windows diff --git a/Lib/test/test_ensurepip.py b/Lib/test/test_ensurepip.py index 3eb433c5867ed7..6b179a8273acbc 100644 --- a/Lib/test/test_ensurepip.py +++ b/Lib/test/test_ensurepip.py @@ -6,19 +6,12 @@ import test.support import unittest import unittest.mock -from pathlib import Path import ensurepip import ensurepip._uninstall class TestPackages(unittest.TestCase): - def setUp(self): - ensurepip._get_usable_pip_package.cache_clear() - - def tearDown(self): - ensurepip._get_usable_pip_package.cache_clear() - def touch(self, directory, filename): fullname = os.path.join(directory, filename) open(fullname, "wb").close() @@ -27,43 +20,39 @@ def test_version(self): # Test version() with tempfile.TemporaryDirectory() as tmpdir: self.touch(tmpdir, "pip-1.2.3b1-py2.py3-none-any.whl") - with unittest.mock.patch.object( - ensurepip, '_WHEEL_PKG_DIR', tmpdir, - ): + with unittest.mock.patch.object(ensurepip, '_WHEEL_PKG_DIR', tmpdir): self.assertEqual(ensurepip.version(), '1.2.3b1') - def test_get_packages_no_dir(self): - # Test _get_packages() without a wheel package directory + def test_get_pip_info_no_dir(self): + # Test _get_pip_info() without a wheel package directory with unittest.mock.patch.object(ensurepip, '_WHEEL_PKG_DIR', None): - pip_pkg = ensurepip._get_usable_pip_package() + pip_info = ensurepip._get_pip_info() - # when bundled pip wheel package is used, we get _PIP_VERSION + # when the bundled pip wheel is used, we get _PIP_VERSION self.assertEqual(ensurepip._PIP_VERSION, ensurepip.version()) - # use bundled pip wheel package - self.assertIsNotNone(pip_pkg.wheel_name) + # use the bundled pip wheel + pip_filename = f'pip-{ensurepip._PIP_VERSION}-py3-none-any.whl' + expected = {"version": ensurepip._PIP_VERSION, "filename": pip_filename, + "bundled": True} + self.assertDictEqual(pip_info, expected) - def test_get_packages_with_dir(self): - # Test _get_packages() with a wheel package directory - older_pip_filename = "pip-1.2.3-py2.py3-none-any.whl" + def test_get_pip_info_with_dir(self): + # Test _get_pip_info() with a wheel package directory pip_filename = "pip-20.2.2-py2.py3-none-any.whl" with tempfile.TemporaryDirectory() as tmpdir: - self.touch(tmpdir, older_pip_filename) self.touch(tmpdir, pip_filename) - # not used, make sure that it's ignored + # not used, make sure that they're ignored + self.touch(tmpdir, "pip-1.2.3-py2.py3-none-any.whl") self.touch(tmpdir, "wheel-0.34.2-py2.py3-none-any.whl") - # not used, make sure that it's ignored - self.touch(tmpdir, "non-whl") + self.touch(tmpdir, "pip-script.py") - with unittest.mock.patch.object( - ensurepip, '_WHEEL_PKG_DIR', tmpdir, - ): - pip_pkg = ensurepip._get_usable_pip_package() + with unittest.mock.patch.object(ensurepip, '_WHEEL_PKG_DIR', tmpdir): + pip_info = ensurepip._get_pip_info() - self.assertEqual(pip_pkg.version, '20.2.2') - self.assertEqual(pip_pkg.wheel_path, - os.path.join(tmpdir, pip_filename)) + expected = {"version": '20.2.2', "filename": pip_filename, "bundled": False} + self.assertDictEqual(pip_info, expected) class EnsurepipMixin: @@ -102,30 +91,6 @@ def test_basic_bootstrapping(self): additional_paths = self.run_pip.call_args[0][1] self.assertEqual(len(additional_paths), 1) - - def test_replacement_wheel_bootstrapping(self): - ensurepip._get_usable_pip_package.cache_clear() - - pip_wheel_name = ( - f'pip-{ensurepip._PIP_VERSION !s}-' - 'py3-none-any.whl' - ) - - with tempfile.TemporaryDirectory() as tmpdir: - tmp_path = Path(tmpdir) - tmp_wheel_path = tmp_path / pip_wheel_name - tmp_wheel_path.touch() - - with unittest.mock.patch.object( - ensurepip, '_WHEEL_PKG_DIR', tmpdir, - ): - ensurepip.bootstrap() - - ensurepip._get_usable_pip_package.cache_clear() - - additional_paths = self.run_pip.call_args[0][1] - self.assertEqual(Path(additional_paths[-1]).name, pip_wheel_name) - def test_bootstrapping_with_root(self): ensurepip.bootstrap(root="/foo/bar/") From f3a49c2e725ed71f5ab2727328d788e9fa06db2f Mon Sep 17 00:00:00 2001 From: Sviatoslav Sydorenko Date: Wed, 11 Oct 2023 04:38:13 +0200 Subject: [PATCH 06/14] Use `pathlib` in `ensurepip` internally It provides us with the ability to write simpler high-level logic that is easier to understand. As a side effect, it became possible to make the code less branchy. --- Lib/ensurepip/__init__.py | 92 ++++++++++++++++++++------------------ Lib/test/test_ensurepip.py | 26 +++++------ 2 files changed, 60 insertions(+), 58 deletions(-) diff --git a/Lib/ensurepip/__init__.py b/Lib/ensurepip/__init__.py index cfc141cd22e255..de808dd6b96816 100644 --- a/Lib/ensurepip/__init__.py +++ b/Lib/ensurepip/__init__.py @@ -1,10 +1,12 @@ import os -import os.path import subprocess import sys import sysconfig import tempfile +from contextlib import nullcontext, suppress from importlib import resources +from pathlib import Path +from shutil import copy2 __all__ = ["version", "bootstrap"] @@ -14,39 +16,51 @@ # policies recommend against bundling dependencies. For example, Fedora # installs wheel packages in the /usr/share/python-wheels/ directory and don't # install the ensurepip._bundled package. -_WHEEL_PKG_DIR = sysconfig.get_config_var('WHEEL_PKG_DIR') +_WHEEL_PKG_DIR = ( + whl_pkg_dir_str := sysconfig.get_config_var('WHEEL_PKG_DIR') +) is not None and Path(whl_pkg_dir_str).resolve() or None def _find_wheel_pkg_dir_pip(): if _WHEEL_PKG_DIR is None: - return None + raise LookupError( + 'The compile-time `WHEEL_PKG_DIR` is unset so there is ' + 'no place for looking up the wheels.', + ) from None + + + dist_matching_wheels = _WHEEL_PKG_DIR.glob(f'pip-*.whl') try: - filenames = os.listdir(_WHEEL_PKG_DIR) - except OSError: - # Ignore: path doesn't exist or permission error - return None - # Make the code deterministic if a directory contains multiple wheel files - # of the same package, but don't attempt to implement correct version - # comparison since this case should not happen. - filenames = sorted(filenames, reverse=True) - for filename in filenames: - # filename is like 'pip-21.2.4-py3-none-any.whl' - if not filename.startswith("pip-") or not filename.endswith(".whl"): - continue - - # Extract '21.2.4' from 'pip-21.2.4-py3-none-any.whl' - version = filename.removeprefix("pip-").partition("-")[0] - return {"version": version, "filename": filename, "bundled": False} - - return None - - -def _get_pip_info(): + last_matching_dist_wheel = sorted(dist_matching_wheels)[-1] + except IndexError as index_err: + raise LookupError( + '`WHEEL_PKG_DIR` does not contain any wheel files for `pip`.', + ) from index_err + + return nullcontext(last_matching_dist_wheel) + + +def _get_pip_whl_path_ctx(): # Prefer pip from the wheel package directory, if present. - if (pip_info := _find_wheel_pkg_dir_pip()) is not None: - return pip_info - filename = f"pip-{_PIP_VERSION}-py3-none-any.whl" - return {"version": _PIP_VERSION, "filename": filename, "bundled": True} + with suppress(LookupError): + return _find_wheel_pkg_dir_pip() + + return resources.as_file( + resources.files('ensurepip') + / '_bundled' + / f'pip-{_PIP_VERSION}-py3-none-any.whl' + ) + + +def _get_pip_version(): + with _get_pip_whl_path_ctx() as bundled_wheel_path: + wheel_name = bundled_wheel_path.name + return ( + # Extract '21.2.4' from 'pip-21.2.4-py3-none-any.whl' + wheel_name. + removeprefix('pip-'). + partition('-')[0] + ) def _run_pip(args, additional_paths=None): @@ -79,7 +93,7 @@ def version(): """ Returns a string specifying the bundled version of pip. """ - return _get_pip_info()["version"] + return _get_pip_version() def _disable_pip_configuration_settings(): @@ -141,20 +155,10 @@ def _bootstrap(*, root=None, upgrade=False, user=False, with tempfile.TemporaryDirectory() as tmpdir: # Put our bundled wheels into a temporary directory and construct the # additional paths that need added to sys.path - package = _get_pip_info() - wheel_name = package["filename"] - if package["bundled"]: - # Use bundled wheel package - wheel_path = resources.files("ensurepip") / "_bundled" / wheel_name - whl = wheel_path.read_bytes() - else: - # Use the wheel package directory - with open(os.path.join(_WHEEL_PKG_DIR, wheel_name), "rb") as fp: - whl = fp.read() - - filename = os.path.join(tmpdir, wheel_name) - with open(filename, "wb") as fp: - fp.write(whl) + tmpdir_path = Path(tmpdir) + with _get_pip_whl_path_ctx() as bundled_wheel_path: + tmp_wheel_path = tmpdir_path / bundled_wheel_path.name + copy2(bundled_wheel_path, tmp_wheel_path) # Construct the arguments to be passed to the pip command args = ["install", "--no-cache-dir", "--no-index", "--find-links", tmpdir] @@ -167,7 +171,7 @@ def _bootstrap(*, root=None, upgrade=False, user=False, if verbosity: args += ["-" + "v" * verbosity] - return _run_pip([*args, "pip"], [filename]) + return _run_pip([*args, "pip"], [os.fsencode(tmp_wheel_path)]) def _uninstall_helper(*, verbosity=0): diff --git a/Lib/test/test_ensurepip.py b/Lib/test/test_ensurepip.py index 6b179a8273acbc..63389856c91157 100644 --- a/Lib/test/test_ensurepip.py +++ b/Lib/test/test_ensurepip.py @@ -6,6 +6,8 @@ import test.support import unittest import unittest.mock +from importlib.resources.abc import Traversable +from pathlib import Path import ensurepip import ensurepip._uninstall @@ -20,22 +22,20 @@ def test_version(self): # Test version() with tempfile.TemporaryDirectory() as tmpdir: self.touch(tmpdir, "pip-1.2.3b1-py2.py3-none-any.whl") - with unittest.mock.patch.object(ensurepip, '_WHEEL_PKG_DIR', tmpdir): + with unittest.mock.patch.object(ensurepip, '_WHEEL_PKG_DIR', Path(tmpdir)): self.assertEqual(ensurepip.version(), '1.2.3b1') - def test_get_pip_info_no_dir(self): - # Test _get_pip_info() without a wheel package directory + def test_version_no_dir(self): + # Test version() without a wheel package directory with unittest.mock.patch.object(ensurepip, '_WHEEL_PKG_DIR', None): - pip_info = ensurepip._get_pip_info() - # when the bundled pip wheel is used, we get _PIP_VERSION self.assertEqual(ensurepip._PIP_VERSION, ensurepip.version()) - # use the bundled pip wheel + def test_selected_wheel_path_no_dir(self): pip_filename = f'pip-{ensurepip._PIP_VERSION}-py3-none-any.whl' - expected = {"version": ensurepip._PIP_VERSION, "filename": pip_filename, - "bundled": True} - self.assertDictEqual(pip_info, expected) + with unittest.mock.patch.object(ensurepip, '_WHEEL_PKG_DIR', None): + with ensurepip._get_pip_whl_path_ctx() as bundled_wheel_path: + self.assertEqual(pip_filename, bundled_wheel_path.name) def test_get_pip_info_with_dir(self): # Test _get_pip_info() with a wheel package directory @@ -48,11 +48,9 @@ def test_get_pip_info_with_dir(self): self.touch(tmpdir, "wheel-0.34.2-py2.py3-none-any.whl") self.touch(tmpdir, "pip-script.py") - with unittest.mock.patch.object(ensurepip, '_WHEEL_PKG_DIR', tmpdir): - pip_info = ensurepip._get_pip_info() - - expected = {"version": '20.2.2', "filename": pip_filename, "bundled": False} - self.assertDictEqual(pip_info, expected) + with unittest.mock.patch.object(ensurepip, '_WHEEL_PKG_DIR', Path(tmpdir)): + with ensurepip._get_pip_whl_path_ctx() as bundled_wheel_path: + self.assertEqual(pip_filename, bundled_wheel_path.name) class EnsurepipMixin: From 3d7ee78a114641995d424e60249aafb4c1fa479a Mon Sep 17 00:00:00 2001 From: Sviatoslav Sydorenko Date: Wed, 11 Oct 2023 05:06:28 +0200 Subject: [PATCH 07/14] Drop `os.fsencode` from wheel path preprocessing It was returning bytes on FreeBSD which was incorrectly injected into the Python code snippet executed in a subprocess and had a b-preffix. --- Lib/ensurepip/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/ensurepip/__init__.py b/Lib/ensurepip/__init__.py index de808dd6b96816..44653d7f37ebae 100644 --- a/Lib/ensurepip/__init__.py +++ b/Lib/ensurepip/__init__.py @@ -171,7 +171,7 @@ def _bootstrap(*, root=None, upgrade=False, user=False, if verbosity: args += ["-" + "v" * verbosity] - return _run_pip([*args, "pip"], [os.fsencode(tmp_wheel_path)]) + return _run_pip([*args, "pip"], [str(tmp_wheel_path)]) def _uninstall_helper(*, verbosity=0): From 72b8ebe0fe1215bdaffa7ac4e66cfb2fdd490092 Mon Sep 17 00:00:00 2001 From: Sviatoslav Sydorenko Date: Wed, 11 Oct 2023 16:57:33 +0200 Subject: [PATCH 08/14] Update replaced `_get_pip_whl_path_ctx` references Some code comments and test function names were still referring to the removed function name. Not anymore! --- Lib/test/test_ensurepip.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_ensurepip.py b/Lib/test/test_ensurepip.py index 63389856c91157..a4b36a90d8815e 100644 --- a/Lib/test/test_ensurepip.py +++ b/Lib/test/test_ensurepip.py @@ -37,8 +37,8 @@ def test_selected_wheel_path_no_dir(self): with ensurepip._get_pip_whl_path_ctx() as bundled_wheel_path: self.assertEqual(pip_filename, bundled_wheel_path.name) - def test_get_pip_info_with_dir(self): - # Test _get_pip_info() with a wheel package directory + def test_selected_wheel_path_with_dir(self): + # Test _get_pip_whl_path_ctx() with a wheel package directory pip_filename = "pip-20.2.2-py2.py3-none-any.whl" with tempfile.TemporaryDirectory() as tmpdir: From 0d94ec981ca9491988a3132c0fd3ba75acba6aeb Mon Sep 17 00:00:00 2001 From: Sviatoslav Sydorenko Date: Thu, 12 Oct 2023 11:05:11 +0200 Subject: [PATCH 09/14] Drop looping from the wheel verification script This script is separate and is only used in CI as opposed to runtime. --- Tools/build/verify_ensurepip_wheels.py | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/Tools/build/verify_ensurepip_wheels.py b/Tools/build/verify_ensurepip_wheels.py index 29897425da6c03..a37da2f70757e5 100755 --- a/Tools/build/verify_ensurepip_wheels.py +++ b/Tools/build/verify_ensurepip_wheels.py @@ -14,7 +14,6 @@ from pathlib import Path from urllib.request import urlopen -PACKAGE_NAMES = ("pip",) ENSURE_PIP_ROOT = Path(__file__).parent.parent.parent / "Lib/ensurepip" WHEEL_DIR = ENSURE_PIP_ROOT / "_bundled" ENSURE_PIP_INIT_PY_TEXT = (ENSURE_PIP_ROOT / "__init__.py").read_text(encoding="utf-8") @@ -97,8 +96,5 @@ def verify_wheel(package_name: str) -> bool: if __name__ == "__main__": - exit_status = 0 - for package_name in PACKAGE_NAMES: - if not verify_wheel(package_name): - exit_status = 1 + exit_status = int(not verify_wheel("pip")) raise SystemExit(exit_status) From 5c97cda21a4d94539ec441477302c33324ac7e57 Mon Sep 17 00:00:00 2001 From: Sviatoslav Sydorenko Date: Sun, 15 Oct 2023 17:21:17 +0200 Subject: [PATCH 10/14] Multiline complex wheel path conditional Co-authored-by: Pradyun Gedam --- Lib/ensurepip/__init__.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Lib/ensurepip/__init__.py b/Lib/ensurepip/__init__.py index 44653d7f37ebae..a5e449a8c60e1d 100644 --- a/Lib/ensurepip/__init__.py +++ b/Lib/ensurepip/__init__.py @@ -16,9 +16,10 @@ # policies recommend against bundling dependencies. For example, Fedora # installs wheel packages in the /usr/share/python-wheels/ directory and don't # install the ensurepip._bundled package. -_WHEEL_PKG_DIR = ( - whl_pkg_dir_str := sysconfig.get_config_var('WHEEL_PKG_DIR') -) is not None and Path(whl_pkg_dir_str).resolve() or None +if (_pkg_dir := sysconfig.get_config_var('WHEEL_PKG_DIR')) is not None: + _WHEEL_PKG_DIR = Path(_pkg_dir).resolve() +else: + _WHEEL_PKG_DIR = None def _find_wheel_pkg_dir_pip(): From bce6e3ade88596ff6d3d4fe0726d0eab3879554c Mon Sep 17 00:00:00 2001 From: Sviatoslav Sydorenko Date: Sun, 15 Oct 2023 17:22:07 +0200 Subject: [PATCH 11/14] Drag the wheel path object through `os.fsdecode` --- Lib/ensurepip/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/ensurepip/__init__.py b/Lib/ensurepip/__init__.py index a5e449a8c60e1d..90115fac2bc86d 100644 --- a/Lib/ensurepip/__init__.py +++ b/Lib/ensurepip/__init__.py @@ -172,7 +172,7 @@ def _bootstrap(*, root=None, upgrade=False, user=False, if verbosity: args += ["-" + "v" * verbosity] - return _run_pip([*args, "pip"], [str(tmp_wheel_path)]) + return _run_pip([*args, "pip"], [os.fsdecode(tmp_wheel_path)]) def _uninstall_helper(*, verbosity=0): From 50efc2add23df1a7789042c25f33f3b50d57d156 Mon Sep 17 00:00:00 2001 From: Sviatoslav Sydorenko Date: Sun, 15 Oct 2023 17:26:28 +0200 Subject: [PATCH 12/14] Use `None`-sentinel for missing alternative wheel Co-authored-by: Pradyun Gedam --- Lib/ensurepip/__init__.py | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/Lib/ensurepip/__init__.py b/Lib/ensurepip/__init__.py index 90115fac2bc86d..2214ef3bad97d8 100644 --- a/Lib/ensurepip/__init__.py +++ b/Lib/ensurepip/__init__.py @@ -3,7 +3,7 @@ import sys import sysconfig import tempfile -from contextlib import nullcontext, suppress +from contextlib import nullcontext from importlib import resources from pathlib import Path from shutil import copy2 @@ -24,11 +24,7 @@ def _find_wheel_pkg_dir_pip(): if _WHEEL_PKG_DIR is None: - raise LookupError( - 'The compile-time `WHEEL_PKG_DIR` is unset so there is ' - 'no place for looking up the wheels.', - ) from None - + return None dist_matching_wheels = _WHEEL_PKG_DIR.glob(f'pip-*.whl') try: @@ -43,8 +39,8 @@ def _find_wheel_pkg_dir_pip(): def _get_pip_whl_path_ctx(): # Prefer pip from the wheel package directory, if present. - with suppress(LookupError): - return _find_wheel_pkg_dir_pip() + if (alternative_pip_wheel_path := _find_wheel_pkg_dir_pip()) is not None: + return alternative_pip_wheel_path return resources.as_file( resources.files('ensurepip') From 865c41f4f197948b14dc4fb04c5cf4b44a3577bb Mon Sep 17 00:00:00 2001 From: Sviatoslav Sydorenko Date: Sun, 15 Oct 2023 17:29:23 +0200 Subject: [PATCH 13/14] Un-f-string alternative wheel path glob It was tripping the linters and became unnecessary after hardcoding the pip wheel filename prefix in the string. --- Lib/ensurepip/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Lib/ensurepip/__init__.py b/Lib/ensurepip/__init__.py index 2214ef3bad97d8..b5d8fb6e92f765 100644 --- a/Lib/ensurepip/__init__.py +++ b/Lib/ensurepip/__init__.py @@ -26,7 +26,7 @@ def _find_wheel_pkg_dir_pip(): if _WHEEL_PKG_DIR is None: return None - dist_matching_wheels = _WHEEL_PKG_DIR.glob(f'pip-*.whl') + dist_matching_wheels = _WHEEL_PKG_DIR.glob('pip-*.whl') try: last_matching_dist_wheel = sorted(dist_matching_wheels)[-1] except IndexError as index_err: From 2472d87e87aaa13405b21c01bb23af188731ae74 Mon Sep 17 00:00:00 2001 From: Sviatoslav Sydorenko Date: Sun, 15 Oct 2023 17:33:29 +0200 Subject: [PATCH 14/14] Melt the second wheel lookup error into sentinel --- Lib/ensurepip/__init__.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/Lib/ensurepip/__init__.py b/Lib/ensurepip/__init__.py index b5d8fb6e92f765..80ee125cfd4ed3 100644 --- a/Lib/ensurepip/__init__.py +++ b/Lib/ensurepip/__init__.py @@ -24,15 +24,16 @@ def _find_wheel_pkg_dir_pip(): if _WHEEL_PKG_DIR is None: + # NOTE: The compile-time `WHEEL_PKG_DIR` is unset so there is no place + # NOTE: for looking up the wheels. return None dist_matching_wheels = _WHEEL_PKG_DIR.glob('pip-*.whl') try: last_matching_dist_wheel = sorted(dist_matching_wheels)[-1] - except IndexError as index_err: - raise LookupError( - '`WHEEL_PKG_DIR` does not contain any wheel files for `pip`.', - ) from index_err + except IndexError: + # NOTE: `WHEEL_PKG_DIR` does not contain any wheel files for `pip`. + return None return nullcontext(last_matching_dist_wheel)