Implement maybe_pyc_file and .pyc file execution #6539
Conversation
📝 WalkthroughWalkthroughAdds CPython-aligned .pyc magic constants, detects and executes .pyc files via a sourceless loader in the compiler path, removes dynamic git-derived MAGIC_NUMBER setup in import initialization, and exposes the pyc magic token to Python via the _imp module attribute. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant CLI as RustPython CLI
participant Detector as Pyc Detector (compile.rs)
participant Loader as Import Loader (Sourceless/SourceFileLoader)
participant VM as VirtualMachine
CLI->>Detector: execute_file(path)
Detector->>Detector: check extension and PYC magic
alt .pyc detected
Detector->>Loader: set_main_loader(..., "SourcelessFileLoader")
Loader->>VM: get_code(module_name="__main__")
VM->>VM: execute PyCode (bytecode)
else source file
Detector->>Loader: set_main_loader(..., "SourceFileLoader")
Loader->>Loader: read source -> compile to PyCode
Loader->>VM: run compiled PyCode
end
VM-->>CLI: execution result
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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 |
youknowone
force-pushed
the
pyc
branch
2 times, most recently
from
0299bef to
c979a0b
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/vm/src/vm/compile.rs (1)
172-181: Consider usingread_exactand clarify the half-magic rationale.
read_exact(&mut buf)is more idiomatic than checking the return value ofread().- The "half magic" (2 bytes) comparison is unusual—CPython checks all 4 bytes. If this is intentional for some compatibility reason, the docstring should explain why.
🔎 Suggested improvement
- let mut file = std::fs::File::open(path)?; - let mut buf = [0u8; 2]; - - use std::io::Read; - if file.read(&mut buf)? != 2 || magic_number.len() < 2 { + use std::io::Read; + let mut file = std::fs::File::open(path)?; + let mut buf = [0u8; 4]; + + if file.read_exact(&mut buf).is_err() || magic_number.len() < 4 { return Ok(false); } - // Compare with first 2 bytes of magic number (half magic) - Ok(buf == magic_number[..2]) + // Compare full 4-byte magic number + Ok(buf == magic_number[..4])
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
Lib/test/test_cmd_line_script.pyis excluded by!Lib/**Lib/test/test_compileall.pyis excluded by!Lib/**
📒 Files selected for processing (2)
crates/vm/src/version.rscrates/vm/src/vm/compile.rs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.rs: Follow the default rustfmt code style by runningcargo fmtto format Rust code
Always run clippy to lint Rust code (cargo clippy) before completing tasks and fix any warnings or lints introduced by changes
Follow Rust best practices for error handling and memory management
Use the macro system (pyclass,pymodule,pyfunction, etc.) when implementing Python functionality in Rust
Files:
crates/vm/src/version.rscrates/vm/src/vm/compile.rs
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T12:17:28.606Z
Learning: Use `cargo run -- script.py` instead of the standard `python` command to run and test Python code with RustPython
Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T12:17:28.606Z
Learning: Applies to **/*.rs : Use the macro system (`pyclass`, `pymodule`, `pyfunction`, etc.) when implementing Python functionality in Rust
📚 Learning: 2025-11-29T12:17:28.606Z
Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T12:17:28.606Z
Learning: Use `cargo run -- script.py` instead of the standard `python` command to run and test Python code with RustPython
Applied to files:
crates/vm/src/vm/compile.rs
📚 Learning: 2025-11-29T12:17:28.606Z
Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T12:17:28.606Z
Learning: Applies to **/*.rs : Use the macro system (`pyclass`, `pymodule`, `pyfunction`, etc.) when implementing Python functionality in Rust
Applied to files:
crates/vm/src/vm/compile.rs
🧬 Code graph analysis (2)
crates/vm/src/version.rs (2)
crates/literal/src/float.rs (1)
rand(255-255)crates/common/src/rand.rs (1)
os_random(9-13)
crates/vm/src/vm/compile.rs (1)
crates/vm/src/version.rs (1)
get_magic_number(131-138)
🔇 Additional comments (3)
crates/vm/src/vm/compile.rs (3)
78-88: Pyc execution flow looks correct, assuming upstream magic number issue is resolved.The flow correctly:
- Sets up
SourcelessFileLoaderfor.pycfiles- Retrieves
get_codemethod and calls it- Downcasts to
PyCodewith appropriate error handling- Executes via
run_code_obj
135-149: LGTM!Clean refactoring to parameterize the loader type. The function correctly imports machinery, retrieves the loader class by name, and instantiates it with the module name and filename.
151-163: Logic is sound for the detection strategy.The two-tier approach (extension check, then magic check) is reasonable. However, the effectiveness depends on resolving the magic number issues noted in
version.rs.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
crates/vm/src/vm/compile.rs (1)
151-162: Consider logging IO errors during .pyc detection.Line 161 silently treats IO errors as "not a .pyc file" via
unwrap_or(false). While defensive, this can make debugging harder if a .pyc file exists but cannot be read due to permissions or filesystem issues.💡 Optional improvement: log IO errors
- maybe_pyc_file_with_magic(path, &crate::version::PYC_MAGIC_NUMBER_BYTES).unwrap_or(false) + match maybe_pyc_file_with_magic(path, &crate::version::PYC_MAGIC_NUMBER_BYTES) { + Ok(result) => result, + Err(e) => { + warn!("Error checking if {} is a .pyc file: {}", path, e); + false + } + }
📜 Review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
Lib/test/test_cmd_line_script.pyis excluded by!Lib/**Lib/test/test_compileall.pyis excluded by!Lib/**Lib/test/test_importlib/source/test_file_loader.pyis excluded by!Lib/**Lib/test/test_importlib/test_util.pyis excluded by!Lib/**
📒 Files selected for processing (4)
crates/vm/src/import.rscrates/vm/src/stdlib/imp.rscrates/vm/src/version.rscrates/vm/src/vm/compile.rs
💤 Files with no reviewable changes (1)
- crates/vm/src/import.rs
🧰 Additional context used
📓 Path-based instructions (1)
**/*.rs
📄 CodeRabbit inference engine (.github/copilot-instructions.md)
**/*.rs: Follow the default rustfmt code style by runningcargo fmtto format Rust code
Always run clippy to lint Rust code (cargo clippy) before completing tasks and fix any warnings or lints introduced by changes
Follow Rust best practices for error handling and memory management
Use the macro system (pyclass,pymodule,pyfunction, etc.) when implementing Python functionality in Rust
Files:
crates/vm/src/version.rscrates/vm/src/stdlib/imp.rscrates/vm/src/vm/compile.rs
🧠 Learnings (5)
📚 Learning: 2025-06-27T14:47:28.810Z
Learnt from: moreal
Repo: RustPython/RustPython PR: 5847
File: vm/src/stdlib/stat.rs:547-567
Timestamp: 2025-06-27T14:47:28.810Z
Learning: In RustPython's stat module implementation, platform-specific constants like SF_SUPPORTED and SF_SYNTHETIC should be conditionally declared only for the platforms where they're available (e.g., macOS), following CPython's approach of optional declaration using #ifdef checks rather than providing fallback values for other platforms.
Applied to files:
crates/vm/src/version.rs
📚 Learning: 2025-06-27T14:47:28.810Z
Learnt from: moreal
Repo: RustPython/RustPython PR: 5847
File: vm/src/stdlib/stat.rs:547-567
Timestamp: 2025-06-27T14:47:28.810Z
Learning: In RustPython's stat module implementation, platform-specific constants like SF_SUPPORTED and SF_SYNTHETIC should be conditionally declared only for the platforms where they're available (e.g., macOS), following CPython's approach of optional declaration rather than providing fallback values for other platforms.
Applied to files:
crates/vm/src/version.rs
📚 Learning: 2025-08-09T22:56:24.527Z
Learnt from: jackoconnordev
Repo: RustPython/RustPython PR: 6086
File: benches/microbenchmarks/sort.py:1-3
Timestamp: 2025-08-09T22:56:24.527Z
Learning: In RustPython's microbenchmarks (benches/microbenchmarks/*.py), the variable `ITERATIONS` is intentionally used without being defined in the Python files. It is injected by the cargo bench harness at runtime. This pattern should be maintained for consistency across all microbenchmarks, and F821 lint warnings for undefined `ITERATIONS` are expected and acceptable in this context.
Applied to files:
crates/vm/src/version.rs
📚 Learning: 2025-11-29T12:17:28.606Z
Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T12:17:28.606Z
Learning: Use `cargo run -- script.py` instead of the standard `python` command to run and test Python code with RustPython
Applied to files:
crates/vm/src/vm/compile.rs
📚 Learning: 2025-11-29T12:17:28.606Z
Learnt from: CR
Repo: RustPython/RustPython PR: 0
File: .github/copilot-instructions.md:0-0
Timestamp: 2025-11-29T12:17:28.606Z
Learning: Applies to **/*.rs : Use the macro system (`pyclass`, `pymodule`, `pyfunction`, etc.) when implementing Python functionality in Rust
Applied to files:
crates/vm/src/vm/compile.rs
🧬 Code graph analysis (1)
crates/vm/src/stdlib/imp.rs (2)
crates/vm/src/vm/mod.rs (1)
import(571-575)crates/vm/src/stdlib/sys.rs (1)
version(241-243)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Run rust tests (ubuntu-latest)
- GitHub Check: Run snippets and cpython tests (windows-latest)
- GitHub Check: Run tests under miri
- GitHub Check: Run rust tests (windows-latest)
- GitHub Check: Run snippets and cpython tests (ubuntu-latest)
- GitHub Check: Ensure compilation on various targets
- GitHub Check: Check the WASM package and demo
- GitHub Check: Check Rust code with clippy
🔇 Additional comments (5)
crates/vm/src/version.rs (1)
133-139: LGTM! Correct CPython-compatible .pyc magic format.The token construction correctly follows CPython's format:
magic_number | ('\r' << 16) | ('\n' << 24), protecting against text-mode corruption. The little-endian byte representation aligns with CPython's .pyc header format.crates/vm/src/stdlib/imp.rs (1)
88-88: LGTM! Correctly exposes pyc magic number to Python.The constant is properly imported from the version module and exposed via the
_impmodule with appropriate Python naming conventions, matching CPython's approach for bytecode magic number access.Also applies to: 97-98
crates/vm/src/vm/compile.rs (3)
78-88: LGTM! Correct .pyc execution flow.The implementation properly detects .pyc files, configures the appropriate loader (
SourcelessFileLoader), obtains the code object, and handles downcasting errors with a clear message. This matches CPython's sourceless execution path.
135-149: LGTM! Loader parameterization enables .pyc handling.The updated signature correctly accepts
loader_nameas a parameter, enabling the selection betweenSourceFileLoaderandSourcelessFileLoaderbased on file type. All call sites have been updated appropriately.
164-184: LGTM! Correct magic number detection with text-mode safeguard.The 2-byte magic check (lines 176-183) correctly implements CPython's detection strategy, which only examines the first 2 bytes to avoid text-mode corruption issues with
\r\nbytes. The comment clearly documents this rationale.
| // Must be aligned to Lib/importlib/_bootstrap_external.py | ||
| pub const PYC_MAGIC_NUMBER: u16 = 3531; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Python 3.13.0 alpha pyc magic number
💡 Result:
Python 3.13 (alpha) .pyc magic number: bytes = 0xF3 0x0D 0x0D 0x0A (i.e. little-endian 2‑byte value 3571 then b'\r\n'). [1][2]
Sources:
- importlib._bootstrap_external MAGIC_NUMBER definition (CPython 3.13 source snapshot). [1]
- importlib.util.MAGIC_NUMBER (docs). [2]
Change the magic number from 3531 to 3571 to match Python 3.13.0alpha.
The hardcoded value must be 3571 (bytes 0xF3 0x0D 0x0D 0x0A in little-endian) to align with CPython's Python 3.13.0alpha magic number. The current value 3531 is incorrect and will cause .pyc file incompatibility.
🤖 Prompt for AI Agents
In crates/vm/src/version.rs around lines 130 to 131, the PYC_MAGIC_NUMBER
constant is incorrectly set to 3531; update its value to 3571 to match Python
3.13.0alpha (ensure the constant uses the new numeric value 3571 so generated
.pyc files use the correct magic number).
1 participant
fix #6273
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.