GeezerVison is showing github.com — impl preexec_fn by youknowone · Pull Request #6479 · RustPython/RustPython
Conversation
youknowone
commented
Dec 24, 2025
•
edited by coderabbitai
Bot
Loading
Uh oh!
There was an error while loading. Please reload this page.
Summary by CodeRabbit
Bug Fixes
- preexec_fn is now supported in subprocess execution (previously blocked)
- Enhanced error reporting for preexec_fn failures with clearer, more descriptive error messages distinguishing subprocess-level errors from OS-level errors
✏️ Tip: You can customize this high-level summary in your review settings.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
coderabbitai
Bot
commented
Dec 24, 2025
•
edited
Loading
Uh oh!
There was an error while loading. Please reload this page.
Caution
Review failed
The pull request is closed.
📝 Walkthrough
Walkthrough
The changes enable preexec_fn support in subprocess execution by removing a not-implemented guard, threading the VM reference through function signatures from fork_exec to exec_inner, adding a new PreExec variant to ExecErrorContext for error distinction, and implementing preexec_fn invocation in the child process after setup but before file descriptor closing.
Changes
| Cohort / File(s) |
Summary |
preexec_fn invocation and error context crates/stdlib/src/posixsubprocess.rs |
Removed preexec_fn guard allowing its execution; added PreExec variant to ExecErrorContext enum with corresponding message "Exception occurred in preexec_fn."; enhanced error reporting to distinguish PreExec errors (SubprocessError:0:) from OS errors (OSError:<errno_hex>:) |
Function signature updates for VM threading crates/stdlib/src/posixsubprocess.rs |
Updated exec() signature to accept vm: &VirtualMachine parameter; updated exec_inner() signature to accept vm: &VirtualMachine parameter; updated call site in fork_exec() to pass vm to exec() and all exec_inner() call sites to forward the VM reference |
preexec_fn execution logic crates/stdlib/src/posixsubprocess.rs |
Implemented preexec_fn invocation in child process after setup steps but before file descriptor closing; on failure, sets error context to PreExec and returns UnknownErrno; on success, execution continues to execve/execv |
Estimated code review effort
🎯 2 (Simple) | ⏱️ ~12 minutes
Poem
🐰 A preexec hop through code we go,
With VMs passed where children flow,
Callbacks bloom before execve's call,
Error messages catch one and all—
Subprocess magic, clean and bright!
✨ Finishing touches
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
📜 Recent review details
Configuration used: Path: .coderabbit.yml
Review profile: CHILL
Plan: Pro
📥 Commits
Reviewing files that changed from the base of the PR and between 309b2ad and c7fcd56.
⛔ Files ignored due to path filters (1)
Lib/test/test_subprocess.py is excluded by !Lib/**
📒 Files selected for processing (1)
crates/stdlib/src/posixsubprocess.rs
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.
❤️ Share
Comment @coderabbitai help to get the list of available commands and usage tips.
|
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
This was referenced Dec 30, 2025
Uh oh!
There was an error while loading. Please reload this page.
Assignees
No one assigned
Milestone
No milestone
Development
Successfully merging this pull request may close these issues.
Uh oh!
There was an error while loading. Please reload this page.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.