import: test_importlib compatibility and bootstrap-traceback parity fixes - #758
Conversation
IOBase.__exit__ and the buffered __exit__ called the native close directly, so a Python subclass close() override never ran inside a with-block — zipfile's _ZipWriteFile.close() was skipped and the archive kept its open-writing-handle state. Dispatch self.close() dynamically, the shape iobase_del already uses. Assisted-by: Claude
install_importlib_bootstrap now inserts zipimport.zipimporter at the front of the live sys.path_hooks list once the external importers are installed, so zip archives on sys.path are importable. A failed zipimport import leaves the hook out instead of failing the bootstrap. Assisted-by: Claude
loads/load only took bytes and bytearray; a sliced memoryview — what SourcelessFileLoader.get_code passes for the pyc payload — raised TypeError. Route non-bytes inputs through buffer_as_bytes_like. Assisted-by: Claude
The stub returned int 0, so _code_to_hash_pyc's len(source_hash) == 8 assert failed when writing hash-based pycs. Port interp_imp.py source_hash: siphash-2-4 keyed by the pyc magic, 8 bytes little-endian. Assisted-by: Claude
The inplace-BinaryOp journal gate declined exact str/bytes receivers into InplaceContainerMutationUnsupported, a permanent abort whose resume path drops the in-flight FOR_ITER item — posixpath.join lost one path component once per process when its loop hit the compile threshold mid-call. str/bytes += yields a fresh object and rebinds the journaled local, the same argument as the existing int/bool/float/tuple arm. Assisted-by: Claude
script_helper spawns children with sys.executable -E -u; the parser rejected -u with exit 2. pyre's stdio wrappers already write through to the fd on every call, so the flag is accepted as a no-op. Assisted-by: Claude
os.chmod was a silent no-op stub, so os_helper.can_chmod() observed an unchanging st_mode and skipped chmod-dependent tests. Register a real path-based chmod next to fchmod; the sandbox name set already neutralises it. Assisted-by: Claude
Add the `w_class` slot to `W_MODULE_GC_PTR_OFFSETS` so a Module keeps its class reachable. For a `types.ModuleType` subclass instance the class is a collectible heap `W_TypeObject`; when the module is its only referent, an untraced slot let a major collection sweep the class and left `type(m)` and slot dispatch pointing at freed memory. Assisted-by: Claude
`frozen_module_served` treated mode 0 the same as a negative override, so at the default `_override_frozen_modules_for_tests` setting only the essential bootstrap set was served. Serve the whole frozen table when mode is non-negative; a negative mode still keeps only the essential bootstrap set. Assisted-by: Claude
…_ raises When a module's `__getattribute__` slot is replaced (for example importlib.util._LazyModule) and the replacement raises AttributeError, route the miss to the receiver type's `__getattr__` per descroperation.py:242-245 instead of the module-dict PEP 562 `__getattr__`; re-raise when the type has no `__getattr__`. Assisted-by: Claude
`dunder_import` now runs `strip_bootstrap_traceback_frames` on the slow-path
`__import__` result, dropping the leading traceback entries whose code
filename belongs to `importlib/_bootstrap{,_external}.py` (or the frozen
pseudo-names). The walk stops at the first non-bootstrap frame, so an
erroring imported module keeps its own frames. Mirrors
`remove_traceback_module_frames` at interp_import.py:98.
Assisted-by: Claude
|
Warning Review limit reached
Next review available in: 44 seconds Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (9)
✨ 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. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e5dcb4b3d1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return Err(crate::PyError::type_error("chmod() requires 2 arguments")); | ||
| } | ||
| let path = extract_path(args[0])?; | ||
| let mode = (unsafe { pyre_object::w_int_get_value(args[1]) }) as u32; |
There was a problem hiding this comment.
In non-sandbox builds this new os.chmod path reads args[1] with w_int_get_value even though the wrapper only checks arity, so calls such as os.chmod(path, "0644") or any non-int mode reinterpret that object's layout as a W_IntObject and still invoke libc::chmod with an arbitrary mode instead of raising TypeError. Convert the mode through the object-space integer/index helper before the syscall so bad input cannot change permissions unpredictably.
Useful? React with 👍 / 👎.
🤖 Codex parity reviewStatic analysis of this diff vs the local RPython/PyPy sources (commit e5dcb4b). Files in the reviewed diff1. Regressions to PyPy parity introduced by this patch
2. Other mismatches introduced by this patch
3. Pre-existing mismatches (already present before this patch)None. 4. Structural adaptations
|
1 participant
Continues the import/stdlib compatibility work after #746, targeting the CPython
test_importlibsuite plus PyPy code-level parity. 11 commits across import, module, io, marshal, posix, and the JIT walk gate.Import / module
__import__result runs a frame stripper that drops the leading traceback entries whose code filename isimportlib/_bootstrap{,_external}.py(or the frozen pseudo-names), stopping at the first non-bootstrap frame. Mirrorsremove_traceback_module_frames(interp_import.py).import nosuchnow shows only the user line; an erroring imported module keeps its own frames.__getattr__when a replaced__getattribute__raises — a module whose__getattribute__slot is replaced (e.g.importlib.util._LazyModule) and raises AttributeError now routes to the receiver type's__getattr__per descroperation.py, not the module-dict PEP 562 hook._override_frozen_modules_for_testsmode 0 (default) serves the whole frozen table; only a negative mode narrows to the essential bootstrap set.w_classin the module GC shape —W_MODULE_GC_PTR_OFFSETSnow tracesob_header.w_classso aModuleTypesubclass instance's collectible class type is not swept when the module is its only referent.zipimport.zipimportertosys.path_hooksonce the bootstrap is installed.imp / marshal / io / posix / pyrex
imp.source_hashimplemented with siphash-2-4.marshal.loads/loadaccept any readable buffer.closedynamically from the context-manager__exit__.-uflag.JIT
str/bytesreceivers.Verification
pyre/check.py: dynasm 300/300, cranelift 300/300.w_classGC survival, frozen mode 0, and module__getattr__fallback each checked against the CPython oracle on both backends.test_importlibsweep: no regressions (remaining per-module failures are the fixture-precaching artifact, Windows-onlywinreg, or pre-existing edge cases).🤖 Generated with Claude Code