From 7000353f574c694e4c2f6fff156e857ad9f04271 Mon Sep 17 00:00:00 2001
From: Lorenzo Gabriele <lorenzolespaul@gmail.com>
Date: Sat, 18 Nov 2023 15:34:25 +0100
Subject: [PATCH] Fix propagateEnv = false to not propagate env (#238)

Originated from https://github.com/com-lihaoyi/mill/issues/2881

Pull request: https://github.com/com-lihaoyi/os-lib/pull/238
---
 build.sc                              |  3 +-
 os/src-jvm/ProcessOps.scala           | 17 ++++++----
 os/test/src-jvm/SubprocessTests.scala | 45 ++++++++++++++++++++++-----
 3 files changed, 50 insertions(+), 15 deletions(-)

diff --git a/build.sc b/build.sc
index e78696e4..4a8d4344 100644
--- a/build.sc
+++ b/build.sc
@@ -92,7 +92,8 @@ trait OsLibModule
       "LC_ALL" -> "C",
       "TEST_JAR_WRITER_ASSEMBLY" -> testJarWriter.assembly().path.toString,
       "TEST_JAR_READER_ASSEMBLY" -> testJarReader.assembly().path.toString,
-      "TEST_JAR_EXIT_ASSEMBLY" -> testJarExit.assembly().path.toString
+      "TEST_JAR_EXIT_ASSEMBLY" -> testJarExit.assembly().path.toString,
+      "TEST_SUBPROCESS_ENV" -> "value"
     )
   }
 }
diff --git a/os/src-jvm/ProcessOps.scala b/os/src-jvm/ProcessOps.scala
index 6a7db9ba..1ba98468 100644
--- a/os/src-jvm/ProcessOps.scala
+++ b/os/src-jvm/ProcessOps.scala
@@ -337,12 +337,17 @@ private[os] object ProcessOps {
   ): ProcessBuilder = {
     val builder = new java.lang.ProcessBuilder()
 
-    val baseEnv =
-      if (propagateEnv) sys.env
-      else Map()
-    for ((k, v) <- baseEnv ++ Option(env).getOrElse(Map())) {
-      if (v != null) builder.environment().put(k, v)
-      else builder.environment().remove(k)
+    val environment = builder.environment()
+
+    if (!propagateEnv) {
+      environment.clear()
+    }
+
+    if (env != null) {
+      for ((k, v) <- env) {
+        if (v != null) builder.environment().put(k, v)
+        else builder.environment().remove(k)
+      }
     }
 
     builder.directory(Option(cwd).getOrElse(os.pwd).toIO)
diff --git a/os/test/src-jvm/SubprocessTests.scala b/os/test/src-jvm/SubprocessTests.scala
index 2c7e6fe3..16eefe93 100644
--- a/os/test/src-jvm/SubprocessTests.scala
+++ b/os/test/src-jvm/SubprocessTests.scala
@@ -104,17 +104,46 @@ object SubprocessTests extends TestSuite {
 
     test("envArgs") {
       if (Unix()) {
-        val res0 = proc("bash", "-c", "echo \"Hello$ENV_ARG\"").call(env = Map("ENV_ARG" -> "12"))
-        assert(res0.out.lines() == Seq("Hello12"))
+        locally {
+          val res0 = proc("bash", "-c", "echo \"Hello$ENV_ARG\"").call(env = Map("ENV_ARG" -> "12"))
+          assert(res0.out.lines() == Seq("Hello12"))
+        }
 
-        val res1 = proc("bash", "-c", "echo \"Hello$ENV_ARG\"").call(env = Map("ENV_ARG" -> "12"))
-        assert(res1.out.lines() == Seq("Hello12"))
+        locally {
+          val res1 = proc("bash", "-c", "echo \"Hello$ENV_ARG\"").call(env = Map("ENV_ARG" -> "12"))
+          assert(res1.out.lines() == Seq("Hello12"))
+        }
 
-        val res2 = proc("bash", "-c", "echo 'Hello$ENV_ARG'").call(env = Map("ENV_ARG" -> "12"))
-        assert(res2.out.lines() == Seq("Hello$ENV_ARG"))
+        locally {
+          val res2 = proc("bash", "-c", "echo 'Hello$ENV_ARG'").call(env = Map("ENV_ARG" -> "12"))
+          assert(res2.out.lines() == Seq("Hello$ENV_ARG"))
+        }
+
+        locally {
+          val res3 = proc("bash", "-c", "echo 'Hello'$ENV_ARG").call(env = Map("ENV_ARG" -> "123"))
+          assert(res3.out.lines() == Seq("Hello123"))
+        }
 
-        val res3 = proc("bash", "-c", "echo 'Hello'$ENV_ARG").call(env = Map("ENV_ARG" -> "123"))
-        assert(res3.out.lines() == Seq("Hello123"))
+        locally {
+          // TEST_SUBPROCESS_ENV env should be set in forkEnv in build.sc
+          assert(sys.env.get("TEST_SUBPROCESS_ENV") == Some("value"))
+          val res4 = proc("bash", "-c", "echo \"$TEST_SUBPROCESS_ENV\"").call(
+            env = Map.empty,
+            propagateEnv = false
+          ).out.lines()
+          assert(res4 == Seq(""))
+        }
+
+        locally {
+          // TEST_SUBPROCESS_ENV env should be set in forkEnv in build.sc
+          assert(sys.env.get("TEST_SUBPROCESS_ENV") == Some("value"))
+
+          val res5 = proc("bash", "-c", "echo \"$TEST_SUBPROCESS_ENV\"").call(
+            env = Map.empty,
+            propagateEnv = true
+          ).out.lines()
+          assert(res5 == Seq("value"))
+        }
       }
     }
     test("multiChunk") {