Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 34k
GH-80789: Bundle ensurepip wheels at build time#109130
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
0d8c94fc2c9a5576de0b8252ee8e800c4874f0b3a20aaf135a163396ccaad2ddb6f0db8fae12a161f304File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Uh oh!
There was an error while loading. Please reload this page.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| * | ||
| !.gitignore | ||
| !README.rst |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| Bundling ensurepip wheels | ||
| ========================= | ||
| ``ensurepip`` expects wheels for ``pip`` to be located in this directory. | ||
| These need to be 'bundled' by each distributor of Python, | ||
| ordinarily by running the ``Tools/build/bundle_ensurepip_wheels.py`` script. | ||
| This will download the version of ``pip`` specified by the | ||
| ``ensurepip._PIP_VERSION`` variable to this directory, | ||
| and verify it against the stored SHA-256 checksum. |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -76,6 +76,12 @@ def setUp(self): | ||
| patched_os.path = os.path | ||
| self.os_environ = patched_os.environ = os.environ.copy() | ||
| # ensure the pip wheel exists | ||
| pip_filename = os.path.join(test.support.STDLIB_DIR, 'ensurepip', '_bundled', | ||
| f'pip-{ensurepip._PIP_VERSION}-py3-none-any.whl') | ||
| if not os.path.isfile(pip_filename): | ||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel like if it didn't exist, there should be some code to clean it up after testing so that the dummy file doesn't get forgotten on disk. Tests should avoid side effects... | ||
| open(pip_filename, "wb").close() | ||
| class TestBootstrap(EnsurepipMixin, unittest.TestCase): | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,100 @@ | ||
| importsys | ||
| importtempfile | ||
| importunittest | ||
| importunittest.mock | ||
| importurllib.request | ||
| fromhashlibimportsha256 | ||
| fromioimportBytesIO | ||
| frompathlibimportPath | ||
| fromrandomimportrandbytes | ||
| fromtestimportsupport, test_tools | ||
| fromtest.supportimportcaptured_stderr | ||
| importensurepip | ||
| test_tools.skip_if_missing('build') | ||
| withtest_tools.imports_under_tool('build'): | ||
| importbundle_ensurepip_wheelsasbew | ||
| # Disable fancy GitHub actions output during the tests | ||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There should be a test for enabled GHA, just for those helpers, then. It'd be sad not to get coverage there. | ||
| @unittest.mock.patch.object(bew, 'GITHUB_ACTIONS', False) | ||
| classTestBundle(unittest.TestCase): | ||
| contents=randbytes(512) | ||
| checksum=sha256(contents).hexdigest() | ||
| projects= [('pip', '1.2.3', checksum)] | ||
| pip_filename="pip-1.2.3-py3-none-any.whl" | ||
| deftest__wheel_url(self): | ||
| self.assertEqual( | ||
| bew._wheel_url('pip', '1.2.3'), | ||
| 'https://files.pythonhosted.org/packages/py3/p/pip/pip-1.2.3-py3-none-any.whl', | ||
| ) | ||
| deftest__get_projects(self): | ||
| self.assertListEqual( | ||
| bew._get_projects(), | ||
| [('pip', ensurepip._PIP_VERSION, ensurepip._PIP_SHA_256)], | ||
| ) | ||
| deftest__is_valid_wheel(self): | ||
| self.assertTrue(bew._is_valid_wheel(self.contents, checksum=self.checksum)) | ||
| deftest_cached_wheel(self): | ||
| withtempfile.TemporaryDirectory() astmpdir: | ||
| tmpdir=Path(tmpdir) | ||
| (tmpdir/self.pip_filename).write_bytes(self.contents) | ||
| with ( | ||
| captured_stderr() asstderr, | ||
| unittest.mock.patch.object(bew, 'WHEEL_DIR', tmpdir), | ||
| unittest.mock.patch.object(bew, '_get_projects', lambda: self.projects), | ||
| ): | ||
| exit_code=bew.download_wheels() | ||
| self.assertEqual(exit_code, 0) | ||
| stderr=stderr.getvalue() | ||
| self.assertIn("A valid 'pip' wheel already exists!", stderr) | ||
| deftest_invalid_checksum(self): | ||
| classMockedHTTPSOpener: | ||
| @staticmethod | ||
| defopen(url, data, timeout): | ||
| assert'pip'inurl | ||
| assertdataisNone# HTTP GET | ||
hugovk marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading. Please reload this page. | ||
| # Intentionally corrupt the wheel: | ||
| returnBytesIO(self.contents[:-1]) | ||
| with ( | ||
| captured_stderr() asstderr, | ||
| unittest.mock.patch.object(urllib.request, '_opener', None), | ||
| unittest.mock.patch.object(bew, '_get_projects', lambda: self.projects), | ||
| ): | ||
| urllib.request.install_opener(MockedHTTPSOpener()) | ||
| exit_code=bew.download_wheels() | ||
| self.assertEqual(exit_code, 1) | ||
| stderr=stderr.getvalue() | ||
| self.assertIn("Failed to validate checksum for", stderr) | ||
| deftest_download_wheel(self): | ||
| classMockedHTTPSOpener: | ||
| @staticmethod | ||
| defopen(url, data, timeout): | ||
| assert'pip'inurl | ||
| assertdataisNone# HTTP GET | ||
hugovk marked this conversation as resolved. Show resolvedHide resolvedUh oh!There was an error while loading. Please reload this page. | ||
| returnBytesIO(self.contents) | ||
| withtempfile.TemporaryDirectory() astmpdir: | ||
| tmpdir=Path(tmpdir) | ||
| with ( | ||
| captured_stderr() asstderr, | ||
| unittest.mock.patch.object(urllib.request, '_opener', None), | ||
| unittest.mock.patch.object(bew, 'WHEEL_DIR', tmpdir), | ||
| unittest.mock.patch.object(bew, '_get_projects', lambda: self.projects), | ||
| ): | ||
| urllib.request.install_opener(MockedHTTPSOpener()) | ||
| exit_code=bew.download_wheels() | ||
| self.assertEqual((tmpdir/self.pip_filename).read_bytes(), self.contents) | ||
| self.assertEqual(exit_code, 0) | ||
| stderr=stderr.getvalue() | ||
| self.assertIn("Downloading 'https://files.pythonhosted.org/packages/py3/p/pip/pip-1.2.3-py3-none-any.whl'", | ||
| stderr) | ||
| self.assertIn("Writing 'pip-1.2.3-py3-none-any.whl' to disk", stderr) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| Replaced vendored ``pip`` wheels for :mod:`ensurepip` with a new bundler script, | ||
| :file:`Tools/build/bundle_ensurepip_wheels.py`, to be run by distributors. |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,108 @@ | ||||||
| #!/usr/bin/env python3 | ||||||
| """ | ||||||
| Download wheels for 'ensurepip' packages from PyPI. | ||||||
| When GitHub Actions executes the script, output is formatted accordingly. | ||||||
| https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#setting-a-notice-message | ||||||
| """ | ||||||
| importimportlib.util | ||||||
| importos | ||||||
| importsys | ||||||
| fromhashlibimportsha256 | ||||||
| frompathlibimportPath | ||||||
| fromurllib.errorimportURLError | ||||||
| fromurllib.requestimporturlopen | ||||||
| HOST='https://files.pythonhosted.org' | ||||||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we name this something like a | ||||||
| ENSURE_PIP_ROOT=Path(__file__, "..", "..", "..", "Lib", "ensurepip").resolve() | ||||||
| WHEEL_DIR=ENSURE_PIP_ROOT/"_bundled" | ||||||
| ENSURE_PIP_INIT=ENSURE_PIP_ROOT/"__init__.py" | ||||||
| GITHUB_ACTIONS="GITHUB_ACTIONS"inos.environ | ||||||
| defprint_notice(message: str) ->None: | ||||||
| ifGITHUB_ACTIONS: | ||||||
| print(f"::notice::{message}", end="\n\n") | ||||||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nit: this also works if output to stderr FYI. | ||||||
| else: | ||||||
| print(message, file=sys.stderr) | ||||||
| defprint_error(message: str) ->None: | ||||||
| ifGITHUB_ACTIONS: | ||||||
| print(f"::error::{message}", end="\n\n") | ||||||
| else: | ||||||
| print(message, file=sys.stderr) | ||||||
| defdownload_wheels() ->int: | ||||||
| """Download wheels into bundle if they are not there yet.""" | ||||||
| try: | ||||||
| projects=_get_projects() | ||||||
| except (AttributeError, TypeError): | ||||||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think there's an abstraction leak here: it's hard to guess from looking at the | ||||||
| print_error(f"Could not find '_PROJECTS' in {ENSURE_PIP_INIT}.") | ||||||
| return1 | ||||||
| errors=0 | ||||||
| forname, version, checksuminprojects: | ||||||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. FWIW with #109245, looping will stop making sense here. So maybe you could start working with just Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, let's keep them separate. Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I was kinda assuming that the simplification PR would get merged first, in which case, this one would have to adapt.. | ||||||
| wheel_filename=f'{name}-{version}-py3-none-any.whl' | ||||||
| wheel_path=WHEEL_DIR/wheel_filename | ||||||
| ifwheel_path.exists(): | ||||||
| if_is_valid_wheel(wheel_path.read_bytes(), checksum=checksum): | ||||||
| print_notice(f"A valid '{name}' wheel already exists!") | ||||||
| continue | ||||||
| else: | ||||||
| print_error(f"An invalid '{name}' wheel exists.") | ||||||
| os.remove(wheel_path) | ||||||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any reason not to use pathlib's method? Seeing that it's already used everywhere.. | ||||||
| wheel_url=_wheel_url(name, version) | ||||||
| print_notice(f"Downloading {wheel_url!r}") | ||||||
| try: | ||||||
| withurlopen(wheel_url) asresponse: | ||||||
| whl=response.read() | ||||||
| exceptURLErrorasexc: | ||||||
| print_error(f"Failed to download {wheel_url!r}: {exc}") | ||||||
| errors=1 | ||||||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Was this meant to be a counter? Suggested change
| ||||||
| continue | ||||||
| ifnot_is_valid_wheel(whl, checksum=checksum): | ||||||
| print_error(f"Failed to validate checksum for {wheel_url!r}!") | ||||||
| errors=1 | ||||||
| continue | ||||||
| print_notice(f"Writing {wheel_filename!r} to disk") | ||||||
| wheel_path.write_bytes(whl) | ||||||
| returnerrors | ||||||
| def_get_projects() ->list[tuple[str, str, str]]: | ||||||
| spec=importlib.util.spec_from_file_location("ensurepip", ENSURE_PIP_INIT) | ||||||
| ensurepip=importlib.util.module_from_spec(spec) | ||||||
| spec.loader.exec_module(ensurepip) | ||||||
| returnensurepip._PROJECTS | ||||||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @AA-Turner I believe this would need to be updated after #109245, and it'll probably let you work with simpler structures. | ||||||
| def_wheel_url(name: str, version: str, /) ->str: | ||||||
| # The following code was adapted from | ||||||
| # https://warehouse.pypa.io/api-reference/integration-guide.html#predictable-urls | ||||||
| # | ||||||
| # We rely on the fact that pip is, as a matter of policy, portable code. | ||||||
| # We can therefore guarantee that we'll always have the values: | ||||||
| # build_tag = "" | ||||||
| # python_tag = "py3" | ||||||
| # abi_tag = "none" | ||||||
| # platform_tag = "any" | ||||||
| # https://www.python.org/dev/peps/pep-0491/#file-name-convention | ||||||
| filename=f'{name}-{version}-py3-none-any.whl' | ||||||
| returnf'{HOST}/packages/py3/{name[0]}/{name}/{filename}' | ||||||
| def_is_valid_wheel(content: bytes, *, checksum: str) ->bool: | ||||||
| returnchecksum==sha256(content, usedforsecurity=False).hexdigest() | ||||||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @AA-Turner I already know that this effort is going to be rejected for now, but out of curiosity — what's the motivation for setting | ||||||
| if__name__=='__main__': | ||||||
| raiseSystemExit(download_wheels()) | ||||||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would probably be clearer: