Skip to content

Support Darwin subprocess working directories with posix_spawn - #4951

Closed
ecpeterson wants to merge 1 commit into
google:mainfrom
ecpeterson:codex/upstream-darwin-subprocess
Closed

ecpeterson wants to merge 1 commit into
google:mainfrom
ecpeterson:codex/upstream-darwin-subprocess

Conversation

@ecpeterson

Copy link
Copy Markdown
Contributor

This PR was primarily authored by Codex.

Darwin builds of xls/common/subprocess.cc currently encounter Linux-only memfd support and the /proc/self/fd helper path. Use posix_spawnp directly on Darwin, with a child working-directory file action when requested. The Linux embedded-helper path and shared spawn setup retain their existing variable names and comments.

Apple's macOS 26 SDK introduces posix_spawn_file_actions_addchdir and deprecates the _np spelling. XLS still targets macOS 10.15. When the SDK declares the standard function, an availability check selects it on macOS 26 or later and retains _np for older supported systems. Builds whose minimum target is already macOS 26 use only the standard function; builds using older SDKs retain the older spelling. This avoids requiring a macOS 26 symbol on the earlier supported deployment targets.

The child keeps the existing argument, environment, output-capture, and timeout handling. New tests exercise PATH lookup with a child working directory, verify that the parent directory stays unchanged, and run an executable addressed relative to the child directory. Existing tests cover output capture, exit status, timeouts, and environment additions.

Validation: formatting, absolute-include checks, and git diff --check pass. A standalone probe of the API-selection helper compiled with deprecation and unguarded-availability warnings treated as errors for x86_64/macOS 10.15, arm64/macOS 11, and arm64/macOS 26 deployment targets; its native spawn/chdir smoke test passed. The full //xls/common:subprocess_test target passed all 14 tests on native Apple Silicon, including the new working-directory cases and the existing timeout/output/environment cases. Linux execution and execution on older macOS versions remain untested.

@cdleary

cdleary commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator

@ecpeterson nice contribution! just to avoid platform specific ifdefs being too mingled in the code (which we try to avoid) I made a commit that stacks on top of yours that factors out a subprocess_for_os layer with an associated bazel configuration flag for which source file to use in #4972

@ecpeterson

Copy link
Copy Markdown
Contributor Author

@cdleary back at you, nice project :) Prasahnt put me on to it, and it’s been a treat to explore. I hope union types for tagged tuples land soon!

I’ll close this in favor of your version.

@ecpeterson ecpeterson closed this Sep 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants