Add Shell.processLauncher — subprocess dispatch primitive#2
Merged
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1b23a7e343
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
3 tasks
…tive Closes #1. ProcessLauncher is the missing real-exec entry point: every shell host (SwiftBash, SwiftScript, a future JS runtime) goes through one shared primitive instead of reaching for Foundation.Process directly. The default delegates to swift-subprocess; embedders can install SandboxedDenyLauncher to refuse exec, or wrap a builtin-aware launcher in front of the default with ChainLauncher so registered builtins shadow PATH while unknown names hit real exec. The dependency is platform-conditional — swift-subprocess pins iOS / tvOS / watchOS to "99.0" because those kernels forbid posix_spawn / fork. On those platforms the launcher type still exists but throws ProcessLaunchUnsupportedOnThisPlatform, and embedders are expected to install a virtual launcher backed by ProcessTable.spawn instead. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1b23a7e to
56da8dd
Compare
The default swift-subprocess `SubprocessSpan` trait pulls in Span-typed overloads that link a back-deployment shim (`libswiftCompatibilitySpan.dylib`). The shim ships with Xcode 26 in `usr/lib/swift-6.2/macosx/`, but its `@rpath` isn't on SwiftPM's test runtime search path, so `swift test --skip-build` fails on macOS 13–15 runners with `Library not loaded: @rpath/libswiftCompatibilitySpan.dylib`. ShellKit doesn't use the Span overloads — `DefaultProcessLauncher` captures stdout / stderr through `Foundation.Data` — so opting out removes the dependency on the back-deployment shim entirely. Tools version bumps to 6.1 to gain access to the package-trait API; this is no constraint that swift-subprocess (already 6.1 tools) doesn't already impose on the same consumers. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Tests that drive `/bin/sh -c …` (env-var expansion, exit-code plumbing, `pwd -P`, stdin piping through `/bin/cat`) can't easily express the same logic against `cmd.exe` / PowerShell. Skip them via a new ``requirePosixShell()`` helper rather than rewriting per-shell versions. The dispatch primitive is still exercised on Windows by tests that use only `.name(\"echo\")` (which resolves on every supported platform), and by the launcher-wiring / SandboxedDeny / Chain suites that don't invoke real exec at all. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Xcode 26's Swift 6.2 back-deploys Span to macOS 13 / 14 via libswiftCompatibilitySpan.dylib, but SwiftPM's test runtime doesn't search the toolchain's swift-6.2/macosx/ dir. swift-subprocess references Span unconditionally on compiler(>=6.2), so the test binary's LC_LOAD_DYLIB resolves to nothing and dlopen fails. Plumb the back-deployment dir via DYLD_LIBRARY_PATH for the test step. Hard-fail if the expected dir is missing (we'd rather know than silently skip the workaround). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Swift Testing has no "skip" status — `throw SkipRealExec()` surfaced
as a failure on Windows. Replace the throwing helpers with simple
`guard supportsRealExec else { return }` / `guard supportsPosixShell
else { return }` patterns: an unconditional early return passes the
test silently, which is the desired behaviour for platform-gated work.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
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
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.
Closes #1.
Summary
ProcessLauncherprotocol +Executable/Arguments/ExecutionRecord/TerminationStatustypes mirroring swift-subprocess so consumers (SwiftScript, SwiftBash, future JS runtime) hit one dispatch primitive instead of reaching forFoundation.Process.DefaultProcessLauncher(delegates to swift-subprocess),SandboxedDenyLauncher(typed-deny),ChainLauncher(composes primary + fallback, catches onlyProcessLaunchUnresolved).Shell.processLauncher(defaults toDefaultProcessLauncher); propagated throughinitandcopy().ProcessLaunchUnsupportedOnThisPlatformbecause the kernel forbidsposix_spawn/fork.Test plan
swift test— 32 tests in 6 suites pass on macOSxcodebuild -destination 'generic/platform=iOS'— module builds clean for iOS (unsupported-platform throw path)Shell.processLauncher— protocol-shaped subprocess dispatch primitive #1 covered:Shell.processLauncherexists, defaults toDefaultProcessLauncheronShell.processDefaultlaunch(.name("echo"), arguments: ["hi"], …)round-tripsSandboxedDenyLaunchercauses the same call to throwProcessLaunchDeniedChainLauncherfalls through onProcessLaunchUnresolved, surfaces other errors unchanged🤖 Generated with Claude Code