impl preexec_fn by youknowone · Pull Request #6479 · RustPython/RustPython

youknowone

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 docstrings

🧪 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.}