Fix propagateEnv = false to not propagate env (#238)

Originated from com-lihaoyi/mill#2881 Pull request: #238
com-lihaoyi · Nov 18, 2023 · 7000353 · 7000353
1 parent 7ab5c0f
commit 7000353
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 15 deletions.
diff --git 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
@@ -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
@@ -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") {