Update ensurepip from CPython 3.13.9 - #5740
Conversation
Sorry, something went wrong.
|
Need #5590 first from what it seems |
Sorry, something went wrong.
|
Well typing is now broken in new and interesting ways :) |
Sorry, something went wrong.
|
The error is a typing_extensions bug: |
Sorry, something went wrong.
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (5)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including **/dist/** will override the default block on the dist directory, by removing the pattern from both the lists. You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file. """ 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 🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. ❤️ ShareComment @coderabbitai help to get the list of available commands and usage tips. |
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)Lib/test/test_ensurepip.py (1)📜 Review detailsLib/ensurepip/__init__.py (1)9-9: Remove unused import.
The Traversable import is not used anywhere in this test file.
-from importlib.resources.abc import Traversable from pathlib import Path53-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
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Reviewing files that changed from the base of the PR and between 4e094ea and a542757.
📒 Files selected for processing (2)- Lib/ensurepip/__init__.py (5 hunks)
- Lib/test/test_ensurepip.py (3 hunks)
Lib/importlib/resources/abc.py (1)Lib/ensurepip/__init__.py (3)Lib/ensurepip/__init__.py (2)
- Traversable (63-150)
- version (90-94)
- _get_pip_whl_path_ctx (41-50)
Lib/contextlib.py (1)🪛 Pylint (3.3.7) Lib/ensurepip/__init__.pyLib/shutil.py (1)
- nullcontext (761-785)
Lib/importlib/resources/_common.py (1)
- copy2 (441-479)
- as_file (162-167)
[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
Lib/test/test_ensurepip.py (4)25-25: LGTM!
Correctly updated to patch with a Path object, 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_DIR is 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 Path object when present, or None otherwise. The static analysis warning about get_config_var is 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.
Sorry, something went wrong.
Sorry, something went wrong.
File "/tmp/tmp3a6kinnx/pip-25.1.1-py3-none-any.whl/pip/_vendor/typing_extensions.py", line 1768, in __new__ This is weird. We have typing.ParamSpec but it hits the else block |
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)Lib/test/test_ensurepip.py (1)📜 Review detailsLib/ensurepip/__init__.py (1)9-9: Remove unused import or clarify its purpose.
The Traversable import appears unused in the current code. If it's intended for future use or type annotations, consider adding a comment to clarify its purpose.
-from importlib.resources.abc import Traversable25-39: Consider semantic version-aware sorting.
The function uses lexicographical sorting (sorted()[-1]) to select the latest pip wheel, which may not work correctly for semantic versioning. For example, "pip-10.0.0.whl" would sort before "pip-9.0.0.whl" lexicographically.
Consider using a more robust version comparison:
- last_matching_dist_wheel = sorted(dist_matching_wheels)[-1] + from packaging import version + def extract_version(wheel_path): + wheel_name = wheel_path.name + return version.parse(wheel_name.removeprefix('pip-').partition('-')[0]) + + last_matching_dist_wheel = max(dist_matching_wheels, key=extract_version)However, if the current approach works reliably in practice for the expected wheel naming patterns, the change may not be necessary.
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Reviewing files that changed from the base of the PR and between a542757 and a337c76.
📒 Files selected for processing (2)- Lib/ensurepip/__init__.py (5 hunks)
- Lib/test/test_ensurepip.py (3 hunks)
Lib/**/*: Files in the Lib/ directory (Python standard library copied from CPython) should be edited very conservatively; modifications should be minimal and only to work around RustPython limitations.
📄 Source: CodeRabbit Inference Engine (.github/copilot-instructions.md)
List of files the instruction was applied to:
- Lib/ensurepip/__init__.py
- Lib/test/test_ensurepip.py
Lib/test/**/*: Tests in Lib/test often use markers such as '# TODO: RUSTPYTHON', 'unittest.skip("TODO: RustPython ")', or 'unittest.expectedFailure' with a '# TODO: RUSTPYTHON ' comment when modifications are made.
NEVER comment out or delete any test code lines except for removing '@unittest.expectedFailure' decorators and upper TODO comments.
NEVER modify test assertions, test logic, or test data in Lib/test files.
The only acceptable modifications to test files are: (1) removing '@unittest.expectedFailure' decorators and the upper TODO comments when tests actually pass, (2) adding '@unittest.expectedFailure' decorators when tests cannot be fixed.
When a test cannot pass due to missing language features, keep it as expectedFailure and document the reason.
📄 Source: CodeRabbit Inference Engine (.github/copilot-instructions.md)
List of files the instruction was applied to:
- Lib/test/test_ensurepip.py
Lib/contextlib.py (1)Lib/test/test_ensurepip.py (2)Lib/shutil.py (1)
- nullcontext (761-785)
Lib/importlib/resources/_common.py (1)
- copy2 (441-479)
- as_file (162-167)
Lib/importlib/resources/abc.py (1)🪛 Pylint (3.3.7) Lib/ensurepip/__init__.pyLib/ensurepip/__init__.py (1)
- Traversable (63-150)
- _get_pip_whl_path_ctx (41-50)
[error] 19-19: Module 'sysconfig' has no 'get_config_var' member
(E1101)
🪛 Flake8 (7.2.0) Lib/test/test_ensurepip.py[error] 9-9: 'importlib.resources.abc.Traversable' imported but unused
(F401)
⏰ Context from checks skipped due to timeout of 90000ms (5)- GitHub Check: Run snippets and cpython tests (windows-latest)
- GitHub Check: Run rust tests (macos-latest)
- GitHub Check: Run rust tests (windows-latest)
- GitHub Check: Ensure compilation on various targets
- GitHub Check: Run rust tests (ubuntu-latest)
Lib/test/test_ensurepip.py (3)25-25: LGTM! Correctly updated to use Path object.
The change from string to Path(tmpdir) aligns with the refactored implementation where _WHEEL_PKG_DIR is now a Path object.
28-53: Excellent test coverage for the new functionality.
The new test methods properly cover:
- Version extraction when _WHEEL_PKG_DIR is None (bundled wheel scenario)
- Context manager behavior in both bundled and system wheel directory scenarios
- Proper wheel selection from directories with multiple files
The test logic and assertions are correct and align with the refactored implementation.
68-68: LGTM! Comment correctly updated.
The comment properly reflects the new function name _find_wheel_pkg_dir_pip() that replaced _find_packages().
Lib/ensurepip/__init__.py (8)6-9: LGTM! All new imports are properly utilized.
The added imports support the refactored functionality:
- nullcontext for conditional context management
- Path for wheel directory handling
- copy2 for wheel file copying
19-23: LGTM! Clean implementation of wheel directory handling.
The logic properly:
- Uses walrus operator for concise conditional assignment
- Converts string path to resolved Path object when available
- Defaults to None when no system wheel directory is configured
41-50: LGTM! Well-designed context manager for wheel path resolution.
The function properly:
- Prioritizes system wheel directory over bundled wheels
- Uses resources.as_file() for proper resource handling
- Returns appropriate context managers in both scenarios
- Uses clear path construction with the / operator
53-61: LGTM! Clean version extraction implementation.
The function correctly:
- Uses the context manager to access the wheel path
- Parses the version from wheel filename using appropriate string methods
- Follows the wheel naming convention (pip-VERSION-py3-none-any.whl)
- Uses readable method chaining with proper line breaks
94-94: LGTM! Version function now correctly returns dynamic version.
The change from returning a hardcoded constant to calling _get_pip_version() ensures the version reflects the actual wheel being used, whether bundled or from the system directory.
156-172: LGTM! Bootstrap logic simplified and improved.
The refactored bootstrap code:
- Uses the new context manager for consistent wheel path handling
- Properly copies wheel files with copy2 (preserving metadata)
- Explicitly passes "pip" as the package argument for clarity
- Uses os.fsdecode for proper path encoding in subprocess calls
The logic is cleaner and more maintainable than the previous multi-package approach.
203-203: LGTM! Explicit package specification improves clarity.
The change to explicitly pass "pip" as an argument makes the uninstall behavior clear and consistent with the refactored bootstrap logic.
13-13: Verify the pip version update.
The pip version was updated from "23.2.1" to "25.1.1", which represents a significant jump. Please verify this version number is correct for CPython 3.13.2 and that the corresponding wheel file exists in the _bundled directory.
#!/bin/bash # Verify the pip wheel file exists with the updated version fd "pip-25.1.1-py3-none-any.whl" Lib/ensurepip/_bundled/
Sorry, something went wrong.
|
Still failing, but with different error: ERROR: Exception:
Traceback (most recent call last):
File "/tmp/tmpcrk8exvy/pip-25.2-py3-none-any.whl/pip/_internal/cli/base_command.py", line 107, in _run_wrapper
status = _inner_run()
File "/tmp/tmpcrk8exvy/pip-25.2-py3-none-any.whl/pip/_internal/cli/base_command.py", line 100, in _inner_run
self.handle_pip_version_check(options)
File "/tmp/tmpcrk8exvy/pip-25.2-py3-none-any.whl/pip/_internal/cli/base_command.py", line 98, in _inner_run
return self.run(options, args)
File "/tmp/tmpcrk8exvy/pip-25.2-py3-none-any.whl/pip/_internal/cli/base_command.py", line 98, in _inner_run
return self.run(options, args)
File "/tmp/tmpcrk8exvy/pip-25.2-py3-none-any.whl/pip/_internal/cli/req_command.py", line 77, in wrapper
raise
File "/tmp/tmpcrk8exvy/pip-25.2-py3-none-any.whl/pip/_internal/cli/req_command.py", line 71, in wrapper
return func(self, options, args)
File "/tmp/tmpcrk8exvy/pip-25.2-py3-none-any.whl/pip/_internal/commands/install.py", line 339, in run
session = self.get_default_session(options)
File "/tmp/tmpcrk8exvy/pip-25.2-py3-none-any.whl/pip/_internal/cli/index_command.py", line 80, in get_default_session
self._session = self.enter_context(self._build_session(options))
File "/tmp/tmpcrk8exvy/pip-25.2-py3-none-any.whl/pip/_internal/cli/index_command.py", line 99, in _build_session
ssl_context = _create_truststore_ssl_context()
File "/home/runner/work/RustPython/RustPython/pylib/Lib/functools.py", line 584, in wrapper
result = user_function(*args, **kwds)
File "/tmp/tmpcrk8exvy/pip-25.2-py3-none-any.whl/pip/_internal/cli/index_command.py", line 47, in _create_truststore_ssl_context
return None
File "/tmp/tmpcrk8exvy/pip-25.2-py3-none-any.whl/pip/_internal/cli/index_command.py", line 44, in _create_truststore_ssl_context
from pip._vendor import truststore
File "/tmp/tmpcrk8exvy/pip-25.2-py3-none-any.whl/pip/_vendor/truststore/__init__.py", line 31, in <module>
from ._api import SSLContext, extract_from_ssl, inject_into_ssl # noqa: E402
File "/tmp/tmpcrk8exvy/pip-25.2-py3-none-any.whl/pip/_vendor/truststore/_api.py", line 75, in <module>
class SSLContext(_truststore_SSLContext_super_class): # type: ignore[misc]
File "/tmp/tmpcrk8exvy/pip-25.2-py3-none-any.whl/pip/_vendor/truststore/_api.py", line 107, in SSLContext
session: ssl.SSLSession | None = None,
AttributeError: module 'ssl' has no attribute 'SSLSession'
Traceback (most recent call last):
File "/home/runner/work/RustPython/RustPython/pylib/Lib/runpy.py", line 197, in _run_module_as_main
"__main__", mod_spec)
File "/home/runner/work/RustPython/RustPython/pylib/Lib/runpy.py", line 86, in _run_code
exec(code, run_globals)
File "/home/runner/work/RustPython/RustPython/pylib/Lib/ensurepip/__main__.py", line 5, in <module>
sys.exit(ensurepip._main())
File "/home/runner/work/RustPython/RustPython/pylib/Lib/ensurepip/__init__.py", line 263, in _main
default_pip=args.default_pip,
File "/home/runner/work/RustPython/RustPython/pylib/Lib/ensurepip/__init__.py", line 153, in _bootstrap
with tempfile.TemporaryDirectory() as tmpdir:
File "/home/runner/work/RustPython/RustPython/pylib/Lib/ensurepip/__init__.py", line 172, in _bootstrap
return _run_pip([*args, "pip"], [os.fsdecode(tmp_wheel_path)])
File "/home/runner/work/RustPython/RustPython/pylib/Lib/ensurepip/__init__.py", line 87, in _run_pip
return subprocess.run(cmd, check=True).returncode
File "/home/runner/work/RustPython/RustPython/pylib/Lib/subprocess.py", line 548, in run
with Popen(*popenargs, **kwargs) as process:
File "/home/runner/work/RustPython/RustPython/pylib/Lib/subprocess.py", line 572, in run
output=stdout, stderr=stderr)
CalledProcessError: (2, ['/home/runner/work/RustPython/RustPython/target/release/rustpython', '-W', 'ignore::DeprecationWarning', '-c', '\nimport runpy\nimport sys\nsys.path = [\'/tmp/tmpcrk8exvy/pip-25.2-py3-none-any.whl\'] + sys.path\nsys.argv[1:] = [\'install\', \'--no-cache-dir\', \'--no-index\', \'--find-links\', \'/tmp/tmpcrk8exvy\', \'--user\', \'pip\']\nrunpy.run_module("pip", run_name="__main__", alter_sys=True)\n'])
|
Sorry, something went wrong.
|
Finally! |
Sorry, something went wrong.
fix #5681
fix #4332
fix #2671
Summary by CodeRabbit
Refactor
Tests