GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — impl preexec_fn by youknowone · Pull Request #6479 · RustPython/RustPython

Conversation

@youknowone

@youknowone youknowone commented Dec 24, 2025 •
edited by coderabbitai Bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

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.

All reactions
@coderabbitai

coderabbitai Bot commented Dec 24, 2025 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

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
  • X
  • Mastodon
  • Reddit
  • LinkedIn

Comment @coderabbitai help to get the list of available commands and usage tips.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@youknowone
youknowone force-pushed the preexec_fn branch from b83a64a to 0cb5dfd Compare December 24, 2025 07:10
@youknowone
youknowone marked this pull request as ready for review December 24, 2025 07:22
@youknowone
impl preexec_fn
c7fcd56
@youknowone
youknowone force-pushed the preexec_fn branch from 0cb5dfd to c7fcd56 Compare December 24, 2025 08:02
@youknowone
youknowone merged commit 4f0b940 into RustPython:main Dec 24, 2025
12 of 13 checks passed

Uh oh!

There was an error while loading. Please reload this page.

@youknowone
youknowone deleted the preexec_fn branch December 24, 2025 08:02
This was referenced Dec 30, 2025
Fix subprocess and Update subprocess from Python 3.13.11 #6583
Merged
Add PythonFinallizationError and upgrade multiprocessing #6592
Merged
@coderabbitai coderabbitai Bot mentioned this pull request Feb 1, 2026
clear_after_fork #6933
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

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.

1 participant

@youknowone
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.
You are visitor 01402520 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.