Udpate ensurepip from CPython 3.13.2 #5740
Conversation
|
Need #5590 first from what it seems |
|
Well typing is now broken in new and interesting ways :) |
|
The error is a typing_extensions bug: |
WalkthroughThe ensurepip module was refactored to streamline pip bootstrapping by focusing exclusively on pip, removing multi-package support and simplifying wheel discovery. The new implementation uses pathlib.Path for wheel directory handling, introduces context managers for locating the pip wheel, and updates tests to match the new logic and data structures. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant ensurepip
participant SystemWheelDir
participant BundledResource
User->>ensurepip: _bootstrap()
ensurepip->>SystemWheelDir: _find_wheel_pkg_dir_pip()
alt pip wheel found in system dir
SystemWheelDir-->>ensurepip: Return pip wheel path context
else pip wheel not found
ensurepip->>BundledResource: Use bundled pip wheel path context
end
ensurepip->>ensurepip: Copy pip wheel to temp dir
ensurepip->>ensurepip: _run_pip("pip", [copied_wheel_path])
Poem
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://web.archive.org/web/20250624110215im_/https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
Lib/test/test_ensurepip.py (1)
9-9: Remove unused import.The
Traversableimport is not used anywhere in this test file.-from importlib.resources.abc import Traversable from pathlib import PathLib/ensurepip/__init__.py (1)
53-61: Consider more robust version extraction.While the current string manipulation works for the standard wheel naming convention, consider using a more robust approach to handle edge cases.
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] - ) + # Extract version from wheel filename: pip-{version}-py3-none-any.whl + if wheel_name.startswith('pip-') and wheel_name.endswith('.whl'): + # Remove prefix and suffix, then split on '-' + name_parts = wheel_name[4:-4].split('-', 1) + if name_parts: + return name_parts[0] + # Fallback to the bundled version if parsing fails + return _PIP_VERSION
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
Lib/ensurepip/__init__.py(5 hunks)Lib/test/test_ensurepip.py(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (2)
Lib/test/test_ensurepip.py (2)
Lib/importlib/resources/abc.py (1)
Traversable(63-150)Lib/ensurepip/__init__.py (2)
version(90-94)_get_pip_whl_path_ctx(41-50)
Lib/ensurepip/__init__.py (3)
Lib/contextlib.py (1)
nullcontext(761-785)Lib/shutil.py (1)
copy2(441-479)Lib/importlib/resources/_common.py (1)
as_file(162-167)
🪛 Pylint (3.3.7)
Lib/ensurepip/__init__.py
[error] 19-19: Module 'sysconfig' has no 'get_config_var' member
(E1101)
⏰ Context from checks skipped due to timeout of 90000ms (11)
- GitHub Check: Run tests under miri
- GitHub Check: Check the WASM package and demo
- GitHub Check: Run snippets and cpython tests on wasm-wasi
- GitHub Check: Run snippets and cpython tests (windows-latest)
- GitHub Check: Run snippets and cpython tests (macos-latest)
- GitHub Check: Run snippets and cpython tests (ubuntu-latest)
- GitHub Check: Check Rust code with rustfmt and clippy
- GitHub Check: Run rust tests (macos-latest)
- GitHub Check: Run rust tests (ubuntu-latest)
- GitHub Check: Run rust tests (windows-latest)
- GitHub Check: Ensure compilation on various targets
🔇 Additional comments (9)
Lib/test/test_ensurepip.py (4)
25-25: LGTM!Correctly updated to patch with a
Pathobject, aligning with the main module's type change.
28-38: Good test coverage for the no-directory case.These new tests properly verify the fallback behavior when
_WHEEL_PKG_DIRis None, ensuring the bundled pip wheel is used.
40-53: Well-structured test for wheel selection.The test properly verifies that
_get_pip_whl_path_ctx()selects only the pip wheel file from the directory, correctly ignoring other wheels and non-wheel files.
68-69: Comment accurately updated.The comment now correctly references
_find_wheel_pkg_dir_pip()instead of the old function name.Lib/ensurepip/__init__.py (5)
6-9: LGTM!All new imports are appropriately used in the refactored code.
13-13: Pip version updated as intended.The update to pip 25.1.1 aligns with the PR objective of updating from CPython 3.13.2.
19-22: Clean handling of optional wheel directory.The code properly converts the config variable to a resolved
Pathobject when present, orNoneotherwise. The static analysis warning aboutget_config_varis a false positive.🧰 Tools
🪛 Pylint (3.3.7)
[error] 19-19: Module 'sysconfig' has no 'get_config_var' member
(E1101)
156-172: Clean refactoring of the bootstrap logic.The simplified approach focusing solely on pip makes the code more maintainable. Good use of context managers and proper file copying with metadata preservation.
203-203: Consistent with single-package approach.The change correctly reflects the module's focus on pip only.
This is weird. We have typing.ParamSpec but it hits the else block |
Summary by CodeRabbit
Refactor
Tests