Skip to content

Commit

Permalink
Add API to redirect os.Inherited streams globally to be consistent …
Browse files Browse the repository at this point in the history
…with std streams redirections (#283)

Fixes #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
  • Loading branch information
lihaoyi authored Jul 20, 2024
1 parent 6c3a300 commit 59b5fd9
Show file tree
Hide file tree
Showing 4 changed files with 65 additions and 6 deletions.
5 changes: 5 additions & 0 deletions Readme.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
25 changes: 20 additions & 5 deletions os/src/ProcessOps.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
20 changes: 19 additions & 1 deletion os/src/SubProcess.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

/**
Expand Down
21 changes: 21 additions & 0 deletions os/test/src-jvm/OpTestsJvmOnly.scala
Original file line number Diff line number Diff line change
Expand Up @@ -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"))
}
}
}
}

0 comments on commit 59b5fd9

Please sign in to comment.