From f677c908f24399f6bef677986e10a1655a4c9be9 Mon Sep 17 00:00:00 2001 From: Lorenzo Gabriele Date: Thu, 29 Aug 2024 15:33:35 +0200 Subject: [PATCH 1/9] Introduce `os.proc.env` `DynamicVariable` to override default `env` In Mill and any client-server application, it is handy to start processes on the server passing the client environment. This requires to pass the environment manually in all the `os.proc(...).call()`s. An easier alternative is to override the environment as we to for `os.Inherit` `out`, `in` and `err` pipes. So users can forget about the environment and have the right one passed automatically. --- os/src/ProcessOps.scala | 24 ++++++++++++++++-------- os/test/src/SubprocessTests.scala | 16 ++++++++++++++++ 2 files changed, 32 insertions(+), 8 deletions(-) diff --git a/os/src/ProcessOps.scala b/os/src/ProcessOps.scala index 389b34f0..1f6a97b1 100644 --- a/os/src/ProcessOps.scala +++ b/os/src/ProcessOps.scala @@ -251,6 +251,10 @@ case class proc(command: Shellable*) { def pipeTo(next: proc): ProcGroup = ProcGroup(Seq(this, next)) } +object proc { + val env = new scala.util.DynamicVariable[Map[String, String]](sys.env) +} + /** * A group of processes that are piped together, corresponding to e.g. `ls -l | grep .scala`. * You can create a `ProcGroup` by calling `.pipeTo` on a [[proc]] multiple times. @@ -485,18 +489,22 @@ private[os] object ProcessOps { val builder = new java.lang.ProcessBuilder() val environment = builder.environment() + environment.clear() - if (!propagateEnv) { - environment.clear() - } - - if (env != null) { - for ((k, v) <- env) { - if (v != null) builder.environment().put(k, v) - else builder.environment().remove(k) + def addToProcessEnv(env: Map[String, String]) = + if (env != null) { + for ((k, v) <- env) { + if (v != null) builder.environment().put(k, v) + else builder.environment().remove(k) + } } + + if (propagateEnv) { + addToProcessEnv(os.proc.env.value) } + addToProcessEnv(env) + builder.directory(Option(cwd).getOrElse(os.pwd).toIO) builder diff --git a/os/test/src/SubprocessTests.scala b/os/test/src/SubprocessTests.scala index 16eefe93..2bb9f1c0 100644 --- a/os/test/src/SubprocessTests.scala +++ b/os/test/src/SubprocessTests.scala @@ -146,6 +146,22 @@ object SubprocessTests extends TestSuite { } } } + test("envWithValue") { + if (Unix()) { + def envValue() = os.proc("bash", "-c", "echo \"$TEST_ENV_FOO\"").call().out.lines().head + + val before = envValue() + assert(before == "") + + os.proc.env.withValue(Map("TEST_ENV_FOO" -> "bar")) { + val res = envValue() + assert(res == "bar") + } + + val after = envValue() + assert(after == "") + } + } test("multiChunk") { // Make sure that in the case where multiple chunks are being read from // the subprocess in quick succession, we ensure that the output handler From c7dc57f41fa275cdbd384907b81d20402d9781f6 Mon Sep 17 00:00:00 2001 From: Lorenzo Gabriele Date: Thu, 29 Aug 2024 16:41:25 +0200 Subject: [PATCH 2/9] Fix and ignore binary compatibility issues --- build.sc | 5 +++-- os/src/ProcessOps.scala | 9 +++++++++ os/test/src/SubprocessTests.scala | 4 ++-- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/build.sc b/build.sc index dd40a544..168928c2 100644 --- a/build.sc +++ b/build.sc @@ -51,11 +51,12 @@ trait SafeDeps extends ScalaModule { } trait MiMaChecks extends Mima { - def mimaPreviousVersions = Seq("0.9.0", "0.9.1", "0.9.2", "0.9.3", "0.10.0") + def mimaPreviousVersions = Seq("0.9.0", "0.9.1", "0.9.2", "0.9.3", "0.10.0", "0.10.1", "0.10.2", "0.10.3", "0.10.4") override def mimaBinaryIssueFilters: T[Seq[ProblemFilter]] = Seq( ProblemFilter.exclude[ReversedMissingMethodProblem]("os.PathConvertible.isCustomFs"), // this is fine, because ProcessLike is sealed (and its subclasses should be final) - ProblemFilter.exclude[ReversedMissingMethodProblem]("os.ProcessLike.joinPumperThreadsHook") + ProblemFilter.exclude[ReversedMissingMethodProblem]("os.ProcessLike.joinPumperThreadsHook"), + ProblemFilter.exclude[MissingTypesProblem]("os.proc$") ) } diff --git a/os/src/ProcessOps.scala b/os/src/ProcessOps.scala index 1f6a97b1..a82b7931 100644 --- a/os/src/ProcessOps.scala +++ b/os/src/ProcessOps.scala @@ -252,7 +252,16 @@ case class proc(command: Shellable*) { } object proc { + + /** + * The env passed by default to child processes + */ val env = new scala.util.DynamicVariable[Map[String, String]](sys.env) + + // TODO: Delete when in binary compatibity breaking window + def andThen[T](f: proc => T): Seq[Shellable] => T = s => f(apply(s)) + // TODO: Delete when in binary compatibity breaking window + def compose[T](f: T => Seq[Shellable]): T => proc = t => apply(f(t)) } /** diff --git a/os/test/src/SubprocessTests.scala b/os/test/src/SubprocessTests.scala index 2bb9f1c0..bef51954 100644 --- a/os/test/src/SubprocessTests.scala +++ b/os/test/src/SubprocessTests.scala @@ -149,10 +149,10 @@ object SubprocessTests extends TestSuite { test("envWithValue") { if (Unix()) { def envValue() = os.proc("bash", "-c", "echo \"$TEST_ENV_FOO\"").call().out.lines().head - + val before = envValue() assert(before == "") - + os.proc.env.withValue(Map("TEST_ENV_FOO" -> "bar")) { val res = envValue() assert(res == "bar") From 9db0b1d2482d577252a412f0c2ab1721a05709ea Mon Sep 17 00:00:00 2001 From: Lorenzo Gabriele Date: Fri, 30 Aug 2024 11:25:58 +0200 Subject: [PATCH 3/9] Change `os.proc.env` to `os.SubProcess.env` --- build.sc | 3 +-- os/src/ProcessOps.scala | 15 +-------------- os/src/SubProcess.scala | 5 +++++ os/test/src/SubprocessTests.scala | 2 +- 4 files changed, 8 insertions(+), 17 deletions(-) diff --git a/build.sc b/build.sc index 168928c2..1323e4d5 100644 --- a/build.sc +++ b/build.sc @@ -55,8 +55,7 @@ trait MiMaChecks extends Mima { override def mimaBinaryIssueFilters: T[Seq[ProblemFilter]] = Seq( ProblemFilter.exclude[ReversedMissingMethodProblem]("os.PathConvertible.isCustomFs"), // this is fine, because ProcessLike is sealed (and its subclasses should be final) - ProblemFilter.exclude[ReversedMissingMethodProblem]("os.ProcessLike.joinPumperThreadsHook"), - ProblemFilter.exclude[MissingTypesProblem]("os.proc$") + ProblemFilter.exclude[ReversedMissingMethodProblem]("os.ProcessLike.joinPumperThreadsHook") ) } diff --git a/os/src/ProcessOps.scala b/os/src/ProcessOps.scala index a82b7931..bfde53b1 100644 --- a/os/src/ProcessOps.scala +++ b/os/src/ProcessOps.scala @@ -251,19 +251,6 @@ case class proc(command: Shellable*) { def pipeTo(next: proc): ProcGroup = ProcGroup(Seq(this, next)) } -object proc { - - /** - * The env passed by default to child processes - */ - val env = new scala.util.DynamicVariable[Map[String, String]](sys.env) - - // TODO: Delete when in binary compatibity breaking window - def andThen[T](f: proc => T): Seq[Shellable] => T = s => f(apply(s)) - // TODO: Delete when in binary compatibity breaking window - def compose[T](f: T => Seq[Shellable]): T => proc = t => apply(f(t)) -} - /** * A group of processes that are piped together, corresponding to e.g. `ls -l | grep .scala`. * You can create a `ProcGroup` by calling `.pipeTo` on a [[proc]] multiple times. @@ -509,7 +496,7 @@ private[os] object ProcessOps { } if (propagateEnv) { - addToProcessEnv(os.proc.env.value) + addToProcessEnv(os.SubProcess.env.value) } addToProcessEnv(env) diff --git a/os/src/SubProcess.scala b/os/src/SubProcess.scala index 075b2a83..18fb76a8 100644 --- a/os/src/SubProcess.scala +++ b/os/src/SubProcess.scala @@ -162,6 +162,11 @@ class SubProcess( object SubProcess { + /** + * The env passed by default to child processes + */ + val env = new scala.util.DynamicVariable[Map[String, String]](sys.env) + /** * A [[BufferedWriter]] with the underlying [[java.io.OutputStream]] exposed * diff --git a/os/test/src/SubprocessTests.scala b/os/test/src/SubprocessTests.scala index bef51954..c1d47d9d 100644 --- a/os/test/src/SubprocessTests.scala +++ b/os/test/src/SubprocessTests.scala @@ -153,7 +153,7 @@ object SubprocessTests extends TestSuite { val before = envValue() assert(before == "") - os.proc.env.withValue(Map("TEST_ENV_FOO" -> "bar")) { + os.SubProcess.env.withValue(Map("TEST_ENV_FOO" -> "bar")) { val res = envValue() assert(res == "bar") } From 75a8c51faeb95b9b407c5bf49d9034b4cb4253ea Mon Sep 17 00:00:00 2001 From: Lorenzo Gabriele Date: Fri, 30 Aug 2024 11:34:12 +0200 Subject: [PATCH 4/9] Optimization: Avoid recreating the environment map when os.SubProcess.env was not set --- os/src/ProcessOps.scala | 17 ++++++++++++----- os/src/SubProcess.scala | 5 +++-- 2 files changed, 15 insertions(+), 7 deletions(-) diff --git a/os/src/ProcessOps.scala b/os/src/ProcessOps.scala index bfde53b1..267a67e0 100644 --- a/os/src/ProcessOps.scala +++ b/os/src/ProcessOps.scala @@ -485,18 +485,25 @@ private[os] object ProcessOps { val builder = new java.lang.ProcessBuilder() val environment = builder.environment() - environment.clear() def addToProcessEnv(env: Map[String, String]) = if (env != null) { for ((k, v) <- env) { - if (v != null) builder.environment().put(k, v) - else builder.environment().remove(k) + if (v != null) environment.put(k, v) + else environment.remove(k) } } - if (propagateEnv) { - addToProcessEnv(os.SubProcess.env.value) + os.SubProcess.env.value match { + case null => + if (!propagateEnv) { + environment.clear() + } + case subProcessEnvValue => + environment.clear() + if (propagateEnv) { + addToProcessEnv(subProcessEnvValue) + } } addToProcessEnv(env) diff --git a/os/src/SubProcess.scala b/os/src/SubProcess.scala index 18fb76a8..120ab3f2 100644 --- a/os/src/SubProcess.scala +++ b/os/src/SubProcess.scala @@ -163,9 +163,10 @@ class SubProcess( object SubProcess { /** - * The env passed by default to child processes + * The env passed by default to child processes. + * When `null`, the system environment is used. */ - val env = new scala.util.DynamicVariable[Map[String, String]](sys.env) + val env = new scala.util.DynamicVariable[Map[String, String]](null) /** * A [[BufferedWriter]] with the underlying [[java.io.OutputStream]] exposed From d07f8a40553908dae118abc46f3776085b1ed36a Mon Sep 17 00:00:00 2001 From: Lorenzo Gabriele Date: Fri, 30 Aug 2024 11:48:34 +0200 Subject: [PATCH 5/9] Make test verify the env variable is unset --- os/test/src/SubprocessTests.scala | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/os/test/src/SubprocessTests.scala b/os/test/src/SubprocessTests.scala index c1d47d9d..617085e6 100644 --- a/os/test/src/SubprocessTests.scala +++ b/os/test/src/SubprocessTests.scala @@ -148,18 +148,20 @@ object SubprocessTests extends TestSuite { } test("envWithValue") { if (Unix()) { - def envValue() = os.proc("bash", "-c", "echo \"$TEST_ENV_FOO\"").call().out.lines().head + val variableName = "TEST_ENV_FOO" + val variableValue = "bar" + def envValue() = os.proc("bash", "-c", s"if [ -z $${$variableName+x} ]; then echo \"unset\"; else echo \"$$$variableName\"; fi").call().out.lines().head val before = envValue() - assert(before == "") + assert(before == "unset") - os.SubProcess.env.withValue(Map("TEST_ENV_FOO" -> "bar")) { + os.SubProcess.env.withValue(Map(variableName -> variableValue)) { val res = envValue() - assert(res == "bar") + assert(res == variableValue) } val after = envValue() - assert(after == "") + assert(after == "unset") } } test("multiChunk") { From da754db1c03b3fa0928a4740d0ac2e265375a682 Mon Sep 17 00:00:00 2001 From: Lorenzo Gabriele Date: Fri, 30 Aug 2024 11:50:28 +0200 Subject: [PATCH 6/9] Reformat --- build.sc | 3 ++- os/test/src/SubprocessTests.scala | 6 +++++- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/build.sc b/build.sc index 1323e4d5..f2dd57b1 100644 --- a/build.sc +++ b/build.sc @@ -51,7 +51,8 @@ trait SafeDeps extends ScalaModule { } trait MiMaChecks extends Mima { - def mimaPreviousVersions = Seq("0.9.0", "0.9.1", "0.9.2", "0.9.3", "0.10.0", "0.10.1", "0.10.2", "0.10.3", "0.10.4") + def mimaPreviousVersions = + Seq("0.9.0", "0.9.1", "0.9.2", "0.9.3", "0.10.0", "0.10.1", "0.10.2", "0.10.3", "0.10.4") override def mimaBinaryIssueFilters: T[Seq[ProblemFilter]] = Seq( ProblemFilter.exclude[ReversedMissingMethodProblem]("os.PathConvertible.isCustomFs"), // this is fine, because ProcessLike is sealed (and its subclasses should be final) diff --git a/os/test/src/SubprocessTests.scala b/os/test/src/SubprocessTests.scala index 617085e6..c80370c1 100644 --- a/os/test/src/SubprocessTests.scala +++ b/os/test/src/SubprocessTests.scala @@ -150,7 +150,11 @@ object SubprocessTests extends TestSuite { if (Unix()) { val variableName = "TEST_ENV_FOO" val variableValue = "bar" - def envValue() = os.proc("bash", "-c", s"if [ -z $${$variableName+x} ]; then echo \"unset\"; else echo \"$$$variableName\"; fi").call().out.lines().head + def envValue() = os.proc( + "bash", + "-c", + s"if [ -z $${$variableName+x} ]; then echo \"unset\"; else echo \"$$$variableName\"; fi" + ).call().out.lines().head val before = envValue() assert(before == "unset") From c8ad9e6c000f1733da9190ed2def179a972ca2ab Mon Sep 17 00:00:00 2001 From: Lorenzo Gabriele Date: Fri, 30 Aug 2024 11:59:14 +0200 Subject: [PATCH 7/9] Fix string literal for Scala 2.12 --- os/test/src/SubprocessTests.scala | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/os/test/src/SubprocessTests.scala b/os/test/src/SubprocessTests.scala index c80370c1..ac01bd41 100644 --- a/os/test/src/SubprocessTests.scala +++ b/os/test/src/SubprocessTests.scala @@ -153,7 +153,7 @@ object SubprocessTests extends TestSuite { def envValue() = os.proc( "bash", "-c", - s"if [ -z $${$variableName+x} ]; then echo \"unset\"; else echo \"$$$variableName\"; fi" + s"""if [ -z $${$variableName+x} ]; then echo "unset"; else echo "$$$variableName"; fi""" ).call().out.lines().head val before = envValue() From 380c775b7cff882ed42a45d163fa373ce54d0104 Mon Sep 17 00:00:00 2001 From: Lorenzo Gabriele Date: Fri, 30 Aug 2024 16:47:21 +0200 Subject: [PATCH 8/9] Add docs about `os.SubProcess.env` --- Readme.adoc | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/Readme.adoc b/Readme.adoc index ac5b7dca..0e9fd134 100644 --- a/Readme.adoc +++ b/Readme.adoc @@ -1667,6 +1667,19 @@ val sha = os.spawn(cmd = ("shasum", "-a", "256"), stdin = gzip.stdout) sha.stdout.trim ==> "acc142175fa520a1cb2be5b97cbbe9bea092e8bba3fe2e95afa645615908229e -" ---- +==== Customizing the default environment + +Client-server CLI applications sometimes want to run subprocesses on the server based on the environment of the client. +It is possible to customize the default environment passed to subprocesses by setting the `os.SubProcess.env` threadlocal: + +[source,scala] +---- +val clientEnvironment: Map[String, String] = ??? +os.SubProcess.env.withValue(clientEnvironment) { + os.proc(command).call() // clientEnvironment is passed by default instead of the system environment +} +---- + == Spawning Pipelines of Subprocesses After constructing a subprocess with `os.proc`, you can use the `pipeTo` method From 927309f4bf1843126a96b467f21976dbe716c553 Mon Sep 17 00:00:00 2001 From: Lorenzo Gabriele Date: Fri, 30 Aug 2024 16:55:50 +0200 Subject: [PATCH 9/9] Use newer syntax in readme Co-authored-by: Tobias Roeser --- Readme.adoc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Readme.adoc b/Readme.adoc index 0e9fd134..20da5325 100644 --- a/Readme.adoc +++ b/Readme.adoc @@ -1676,7 +1676,7 @@ It is possible to customize the default environment passed to subprocesses by se ---- val clientEnvironment: Map[String, String] = ??? os.SubProcess.env.withValue(clientEnvironment) { - os.proc(command).call() // clientEnvironment is passed by default instead of the system environment + os.call(command) // clientEnvironment is passed by default instead of the system environment } ----