Repository navigation
libutil: Expose spawnProgram and implement fd redirections - #16504
Merged
Merged
Conversation
github-actions
Bot
added
new-cli
xokdvium
force-pushed
the
spawn-program
branch
from
September 21, 2026 23:52
3d2849c to
bb5cccb
Compare
xokdvium
commented
Sep 22, 2026
xokdvium
commented
Sep 22, 2026
xokdvium
force-pushed
the
spawn-program
branch
from
September 22, 2026 21:31
bb5cccb to
a509e23
Compare
Ericson2314
reviewed
Oct 5, 2026
SpawnOptions is a bundle of parameters describing how to spawn an executable into a process, while RunOptions also has additional information about how to run that process to completion. This split would allow us to expose a spawnProcess (probably renamed to spawnProgram for clarity) for the unix case (once we have FD redirections plumbed through) and get rid of most instances of startProcess. It would have been nice to use inheritance for this, but we can't have designated initialisers for them without something like [1]. [1]: https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2024/p2287r3.html
Addresses the TODO. The logic is already identical.
The distinction from startProcess should be much more clear. spawnProgram is the precursor to runProgram2 (or rather a soon-to-be more general version of it).
Now there's only a single runProgram2 implementation in platform-independent code and spawnProgram fd redirection semantics closely mirror that of posix_spawn. Also some windows bugs are fixed along the way, namely the fact that mergeStderrToStdout wasn't respected and stdin wasn't inherited by default. The inheritance semantics are now also properly documented in the code (although they are suboptimal - but at least consistent).
It's only implemented for Linux (although we should probably grab some insights from QEMU's exit-with-parent and implement that on darwin and FreeBSD), so technically the change to unix/processes.cc is redundant.
This removes windows ifdefs since it can now compile, but it won't really work without other changes. For now it's just switching the unix case to the nicer interface.
xokdvium
force-pushed
the
spawn-program
branch
from
October 5, 2026 20:22
a509e23 to
116ae93
Compare
Ericson2314
approved these changes
Oct 5, 2026
xokdvium
enabled auto-merge
October 5, 2026 21:24
xokdvium
commented
Oct 5, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Motivation
Mostly replaces #16454 in a way that allows us to simplify the code rather than making it more convoluted (well, other than the file descriptor redirection dance, but that's unavoidable).
After this it should be quite easy to get rid of most non-test calls to
startProcess. That way we get better performance viavforkon linux and gain more windows portability.Context
Add 👍 to pull requests you find important.
The Nix maintainer team uses a GitHub project board to schedule and track reviews.