From 59b5fd93e56a3b86acbdb16d0c96e218fca6f320 Mon Sep 17 00:00:00 2001 From: Li Haoyi Date: Sat, 20 Jul 2024 21:05:45 +0800 Subject: [PATCH] Add API to redirect `os.Inherit`ed streams globally to be consistent with std streams redirections (#283) Fixes https://github.com/com-lihaoyi/os-lib/issues/282 Unlike usage of `System.in`/`System.out`/`System.err` programmatically, or the `scala.Console` equivalents, output from subprocesses with `os.Inherit` is not affected by `System.setIn`/`setOut`/`setErr`. This is often counterintuitive, because you expect redirecting these streams to redirect all output, only to find output from subprocess leaking through a side channel to the original process `in`/`out`/`err`. This PR adds the `os.Inherit.in`/`out`/`err` dynamic variables, which can be used to re-assign `os.Inherit` on a scoped threadlocal basis. This allows developers who use `System.setOut` or `Console.withOut` to also redirect subprocess output within the scope of the stdout redirect, without users working within those scopes to have to remember to redirect them manually at every callsite. Because we do not control `System.setOut` or `Console.withOut`, there isn't really a way for us to detect and do this automatically (e.g. redirects may happen even before OS-Lib has been even classloaded) and so we have to rely on it being opt-in. As an escape hatch, in case users do not want this behavior, we provide a `os.Inherit0` redirect which performs identically to the original `os.Inherit` even in the process of redirecting `os.Inherit.{in,out,err}` Covered by an added unit test --- Readme.adoc | 5 +++++ os/src/ProcessOps.scala | 25 ++++++++++++++++++++----- os/src/SubProcess.scala | 20 +++++++++++++++++++- os/test/src-jvm/OpTestsJvmOnly.scala | 21 +++++++++++++++++++++ 4 files changed, 65 insertions(+), 6 deletions(-) diff --git a/Readme.adoc b/Readme.adoc index 888c1727..925304ce 100644 --- a/Readme.adoc +++ b/Readme.adoc @@ -2224,6 +2224,11 @@ string, int or set representations of the `os.PermSet` via: == Changelog +[#main] +=== main + +* `os.Inherit` + [#0-10-2] === 0.10.2 diff --git a/os/src/ProcessOps.scala b/os/src/ProcessOps.scala index 1ba98468..5983aaa8 100644 --- a/os/src/ProcessOps.scala +++ b/os/src/ProcessOps.scala @@ -114,16 +114,31 @@ case class proc(command: Shellable*) { mergeErrIntoOut: Boolean = false, propagateEnv: Boolean = true ): SubProcess = { - val builder = - buildProcess(commandChunks, cwd, env, stdin, stdout, stderr, mergeErrIntoOut, propagateEnv) val cmdChunks = commandChunks val commandStr = cmdChunks.mkString(" ") + + def resolve[T](x: T, y: T) = if (x == os.Inherit) y else x + val resolvedStdin = resolve(stdin, os.Inherit.in.value) + val resolvedStdout = resolve(stdout, os.Inherit.out.value) + val resolvedStderr = resolve(stderr, os.Inherit.err.value) + + val builder = buildProcess( + commandChunks, + cwd, + env, + resolvedStdin, + resolvedStdout, + resolvedStderr, + mergeErrIntoOut, + propagateEnv + ) + lazy val proc: SubProcess = new SubProcess( builder.start(), - stdin.processInput(proc.stdin).map(new Thread(_, commandStr + " stdin thread")), - stdout.processOutput(proc.stdout).map(new Thread(_, commandStr + " stdout thread")), - stderr.processOutput(proc.stderr).map(new Thread(_, commandStr + " stderr thread")) + resolvedStdin.processInput(proc.stdin).map(new Thread(_, commandStr + " stdin thread")), + resolvedStdout.processOutput(proc.stdout).map(new Thread(_, commandStr + " stdout thread")), + resolvedStderr.processOutput(proc.stderr).map(new Thread(_, commandStr + " stderr thread")) ) proc.inputPumperThread.foreach(_.start()) diff --git a/os/src/SubProcess.scala b/os/src/SubProcess.scala index 72f8d7ec..61961619 100644 --- a/os/src/SubProcess.scala +++ b/os/src/SubProcess.scala @@ -440,13 +440,31 @@ object ProcessOutput { } /** - * Inherit the input/output stream from the current process + * Inherit the input/output stream from the current process. + * + * Can be overriden on a thread local basis for the various + * kinds of streams (stdin, stdout, stderr) via [[in]], [[out]], and [[err]] */ object Inherit extends ProcessInput with ProcessOutput { def redirectTo = ProcessBuilder.Redirect.INHERIT def redirectFrom = ProcessBuilder.Redirect.INHERIT def processInput(stdin: => SubProcess.InputStream) = None def processOutput(stdin: => SubProcess.OutputStream) = None + + val in = new scala.util.DynamicVariable[ProcessInput](Inherit) + val out = new scala.util.DynamicVariable[ProcessOutput](Inherit) + val err = new scala.util.DynamicVariable[ProcessOutput](Inherit) +} + +/** + * Inherit the input/output stream from the current process. + * Identical of [[os.Inherit]], except it cannot be redirected globally + */ +object InheritRaw extends ProcessInput with ProcessOutput { + def redirectTo = ProcessBuilder.Redirect.INHERIT + def redirectFrom = ProcessBuilder.Redirect.INHERIT + def processInput(stdin: => SubProcess.InputStream) = None + def processOutput(stdin: => SubProcess.OutputStream) = None } /** diff --git a/os/test/src-jvm/OpTestsJvmOnly.scala b/os/test/src-jvm/OpTestsJvmOnly.scala index e75e6d07..f5c950bb 100644 --- a/os/test/src-jvm/OpTestsJvmOnly.scala +++ b/os/test/src-jvm/OpTestsJvmOnly.scala @@ -70,5 +70,26 @@ object OpTestsJvmOnly extends TestSuite { val d = testFolder / "readWrite" intercept[nio.NoSuchFileException](os.list(d / "nonexistent")) } + + // Not sure why this doesn't work on native + test("redirectSubprocessInheritedOutput") { + if (Unix()) { // relies on bash scripts that don't run on windows + val scriptFolder = os.pwd / "os" / "test" / "resources" / "test" + val lines = collection.mutable.Buffer.empty[String] + os.Inherit.out.withValue(os.ProcessOutput.Readlines(lines.append(_))) { + // Redirected + os.proc(scriptFolder / "misc" / "echo_with_wd", "HELLO\nWorld").call( + cwd = os.root / "usr", + stdout = os.Inherit + ) + // Not Redirected + os.proc(scriptFolder / "misc" / "echo_with_wd", "hello\nWORLD").call( + cwd = os.root / "usr", + stdout = os.InheritRaw + ) + } + assert(lines == Seq("HELLO", "World /usr")) + } + } } }