Repository: scala-steward-org/scala-steward
Pull request #1702 (2020-10-30 13:44:09)

Pass environment variables as List to process.slurp
Using a `Map` there is unnecessary because we never look up the values
by key. This PR also unifies how commands are logged.


Patch:
diff --git a/modules/core/src/main/scala/org/scalasteward/core/io/ProcessAlg.scala b/modules/core/src/main/scala/org/scalasteward/core/io/ProcessAlg.scala
index ba02481a..5168370b 100644
--- a/modules/core/src/main/scala/org/scalasteward/core/io/ProcessAlg.scala
+++ b/modules/core/src/main/scala/org/scalasteward/core/io/ProcessAlg.scala
@@ -49,22 +49,24 @@ object ProcessAlg {
       timer: Timer[F],
       F: Concurrent[F]
   ): ProcessAlg[F] = {
-    val envVars = config.envVars.map(v => (v.name, v.value))
+    val configEnv = config.envVars.map(v => (v.name, v.value))
     new UsingFirejail[F](config.sandboxCfg) {
       override def exec(
           command: Nel[String],
           cwd: File,
           extraEnv: (String, String)*
-      ): F[List[String]] =
-        logger.debug(s"Execute ${command.mkString_(" ")}") >>
+      ): F[List[String]] = {
+        val envVars = extraEnv.toList ::: configEnv
+        logger.debug(s"Execute ${process.showCmd(command, envVars)}") >>
           process.slurp[F](
             command,
             Some(cwd.toJava),
-            (extraEnv ++ envVars).toMap,
+            envVars,
             config.processTimeout,
             logger.trace(_),
             blocker
           )
+      }
     }
   }
 }
diff --git a/modules/core/src/main/scala/org/scalasteward/core/io/process.scala b/modules/core/src/main/scala/org/scalasteward/core/io/process.scala
index 0160bff8..038b09dd 100644
--- a/modules/core/src/main/scala/org/scalasteward/core/io/process.scala
+++ b/modules/core/src/main/scala/org/scalasteward/core/io/process.scala
@@ -29,7 +29,7 @@ object process {
   def slurp[F[_]](
       cmd: Nel[String],
       cwd: Option[File],
-      extraEnv: Map[String, String],
+      extraEnv: List[(String, String)],
       timeout: FiniteDuration,
       log: String => F[Unit],
       blocker: Blocker
@@ -41,17 +41,16 @@ object process {
           out.evalMap(line => F.delay(appendBounded(buffer, line, 4096)) >> log(line)).compile.drain
         }
 
-        val showCmd = (extraEnv.map { case (k, v) => s"$k=$v" }.toList ++ cmd.toList).mkString_(" ")
         val result = readOut >> F.delay(process.waitFor()) >>= { exitValue =>
           if (exitValue === 0) F.pure(buffer.toList)
           else {
-            val msg = s"'$showCmd' exited with code $exitValue"
+            val msg = s"'${showCmd(cmd, extraEnv)}' exited with code $exitValue"
             F.raiseError[List[String]](new IOException(makeMessage(msg, buffer.toList)))
           }
         }
 
         val fallback = F.delay(process.destroyForcibly()) >> {
-          val msg = s"'$showCmd' timed out after ${timeout.toString}"
+          val msg = s"'${showCmd(cmd, extraEnv)}' timed out after ${timeout.toString}"
           F.raiseError[List[String]](new TimeoutException(makeMessage(msg, buffer.toList)))
         }
 
@@ -59,10 +58,13 @@ object process {
       }
     }
 
+  def showCmd(cmd: Nel[String], extraEnv: List[(String, String)]): String =
+    (extraEnv.map { case (k, v) => s"$k=$v" } ++ cmd.toList).mkString_(" ")
+
   private def createProcess[F[_]](
       cmd: Nel[String],
       cwd: Option[File],
-      extraEnv: Map[String, String]
+      extraEnv: List[(String, String)]
   )(implicit F: Sync[F]): F[Process] =
     F.delay {
       val pb = new ProcessBuilder(cmd.toList: _*)


Test patch:
diff --git a/modules/core/src/test/scala/org/scalasteward/core/io/processTest.scala b/modules/core/src/test/scala/org/scalasteward/core/io/processTest.scala
index d7744135..d6b84aad 100644
--- a/modules/core/src/test/scala/org/scalasteward/core/io/processTest.scala
+++ b/modules/core/src/test/scala/org/scalasteward/core/io/processTest.scala
@@ -10,10 +10,10 @@ import scala.concurrent.duration._
 
 class processTest extends AnyFunSuite with Matchers {
   def slurp1(cmd: Nel[String]): IO[List[String]] =
-    process.slurp[IO](cmd, None, Map.empty, 1.minute, _ => IO.unit, blocker)
+    process.slurp[IO](cmd, None, List.empty, 1.minute, _ => IO.unit, blocker)
 
   def slurp2(cmd: Nel[String], timeout: FiniteDuration): IO[List[String]] =
-    process.slurp[IO](cmd, None, Map.empty, timeout, _ => IO.unit, blocker)
+    process.slurp[IO](cmd, None, List.empty, timeout, _ => IO.unit, blocker)
 
   test("echo hello") {
     slurp1(Nel.of("echo", "-n", "hello")).unsafeRunSync() shouldBe List("hello")