feature: Add subprocess execution APIs (run/call/spawn) to IO#454
Conversation
Add IO.run, IO.call, and IO.spawn for cross-platform subprocess execution in wvlet.uni.io, filling the highest-impact gap vs os-lib. - IO.run: execute command, capture stdout/stderr, return CommandResult - IO.call: like run but throws NonZeroExitCodeException on failure - IO.spawn: start process without waiting, return Process handle - ProcessConfig: working directory, env vars, inheritIO, redirect stderr - JVM/Native: java.lang.ProcessBuilder; JS: throws UnsupportedOperationException Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Code Review
This pull request introduces a unified cross-platform API for subprocess execution across JVM, Scala Native, and Scala.js. The implementation includes a ProcessApi trait with methods for running, calling, and spawning processes, along with configuration options for environment variables and working directories. Review feedback suggests deduplicating the nearly identical JVM and Native implementations into a shared source directory, simplifying the destroyForcibly implementation, and enhancing the Process trait with timeout-based waitFor and exitValue methods to prevent potential hangs.
| /** | ||
| * Scala Native implementation of process execution using java.lang.ProcessBuilder. | ||
| */ | ||
| trait IOCompat extends ProcessApi: |
There was a problem hiding this comment.
This file is nearly identical to uni-core/.jvm/src/main/scala/wvlet/uni/io/IOCompat.scala. To avoid code duplication and improve maintainability, consider moving the shared code to a common source directory for both JVM and Native platforms. In sbt, you can create a uni-core/.jvm-native/src/main/scala directory for this purpose and place the shared implementation there.
| override def destroyForcibly(): Unit = | ||
| process.destroyForcibly(); | ||
| () |
There was a problem hiding this comment.
| trait Process: | ||
| def stdin: OutputStream | ||
| def stdout: InputStream | ||
| def stderr: InputStream | ||
| def isAlive: Boolean | ||
| def waitFor(): Int | ||
| def destroy(): Unit | ||
| def destroyForcibly(): Unit |
There was a problem hiding this comment.
The Process trait could be made more robust by adding a timed waitFor and an exitValue method, similar to java.lang.Process. This is crucial for preventing applications or tests from hanging indefinitely when waiting for a process to complete.
Here's a suggested enhancement:
import java.util.concurrent.TimeUnit
// ... in Process trait
def waitFor(timeout: Long, unit: TimeUnit): Boolean
def exitValue(): IntThis would enable safer process handling, like so:
if (proc.waitFor(5, TimeUnit.SECONDS)) {
val exitCode = proc.exitValue()
// ... handle completed process
} else {
// The process timed out
proc.destroyForcibly()
}- Close process stdin in run/call so commands waiting for EOF don't hang - Apply redirectErrorToOutput config in spawn (was only applied in run) - Fix applied to both JVM and Native implementations Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…yForcibly Address Gemini review feedback: - Add waitFor(timeout, unit) and exitValue() to Process trait - Simplify destroyForcibly implementation Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…onflicts Cross-platform projects can have duplicate class names across JVM/JS/Native source directories, which only surfaces during packageSrc. Run packageSrc for all platforms in CI to catch this early. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…etup packageSrc needs Node.js (for JS) and libcurl (for Native), so it should be its own job rather than appended to the Scala 3 test job. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Document IO.run, IO.call, IO.spawn, ProcessConfig, and platform support. Add IO page to sidebar navigation. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
IO.run("ls -la /tmp") now automatically tokenizes into Seq("ls", "-la", "/tmp").
Handles single/double quotes and escaped characters.
Multi-arg calls like IO.run("echo", "hello") still work as before.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…osed quotes - Empty quoted strings (e.g., "") are now preserved as empty arguments - Unclosed quotes now throw IllegalArgumentException - Document space-in-path limitation in resolveCommand and run Scaladoc Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Include IO, FileSystem, and Utilities entries from both branches. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Enhance ProcessApi.tokenize with \n, \t, \r escape sequence support - Replace CommandLineTokenizer implementation with delegation to ProcessApi.tokenize, eliminating ~70 lines of duplicate logic Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
IO.run,IO.call, andIO.spawnfor subprocess execution inwvlet.uni.ioCommandResultcaptures exit code, stdout, and stderrProcessConfigsupports working directory, env vars, inheritIO, and stderr redirectionProcesstrait for spawned process handle (stdin/stdout/stderr streams, waitFor, destroy)java.lang.ProcessBuilder; JS: throwsUnsupportedOperationExceptionTest plan
./sbt "coreJVM/compile"— compiles./sbt "coreJS/compile"— compiles (stub)./sbt "coreNative/compile"— compiles./sbt "uniJVM/testOnly *IOProcessTest"— 11/11 tests pass./sbt scalafmtAll— formatted🤖 Generated with Claude Code