Repository: scala-steward-org/scala-steward
Pull request #1654 (2020-10-11 07:52:17)

Pass extra env vars to all exec calls
This passes extra environement variables (that are provided via
`--env-var` options) to all `exec` calls instead of only `execSandboxed`
calls. I think it was an oversight that they were only used in
`execSandboxed`.

codecov[bot]: # [Codecov](https://codecov.io/gh/scala-steward-org/scala-steward/pull/1654?src=pr&el=h1) Report
> Merging [#1654](https://codecov.io/gh/scala-steward-org/scala-steward/pull/1654?src=pr&el=desc) into [master](https://codecov.io/gh/scala-steward-org/scala-steward/commit/fd46985e8023974cbd84e25eabf6c4289a007ff9?el=desc) will **not change** coverage.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/scala-steward-org/scala-steward/pull/1654/graphs/tree.svg?width=650&height=150&src=pr&token=udzMsc1Dne)](https://codecov.io/gh/scala-steward-org/scala-steward/pull/1654?src=pr&el=tree)

```diff
@@           Coverage Diff           @@
##           master    #1654   +/-   ##
=======================================
  Coverage   71.76%   71.76%           
=======================================
  Files         114      114           
  Lines        1877     1877           
  Branches       55       56    +1     
=======================================
  Hits         1347     1347           
  Misses        530      530           
```


| [Impacted Files](https://codecov.io/gh/scala-steward-org/scala-steward/pull/1654?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...in/scala/org/scalasteward/core/io/ProcessAlg.scala](https://codecov.io/gh/scala-steward-org/scala-steward/pull/1654/diff?src=pr&el=tree#diff-bW9kdWxlcy9jb3JlL3NyYy9tYWluL3NjYWxhL29yZy9zY2FsYXN0ZXdhcmQvY29yZS9pby9Qcm9jZXNzQWxnLnNjYWxh) | `100.00% <100.00%> (ø)` | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/scala-steward-org/scala-steward/pull/1654?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/scala-steward-org/scala-steward/pull/1654?src=pr&el=footer). Last update [fd46985...4555859](https://codecov.io/gh/scala-steward-org/scala-steward/pull/1654?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


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 908c4664..c11647d8 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
@@ -20,7 +20,6 @@ import better.files.File
 import cats.effect.{Blocker, Concurrent, ContextShift, Timer}
 import cats.syntax.all._
 import io.chrisdavenport.log4cats.Logger
-import org.scalasteward.core.application.Cli.EnvVar
 import org.scalasteward.core.application.Config
 import org.scalasteward.core.util.Nel
 
@@ -32,18 +31,16 @@ trait ProcessAlg[F[_]] {
 
 object ProcessAlg {
   abstract class UsingFirejail[F[_]](config: Config) extends ProcessAlg[F] {
-    override def execSandboxed(command: Nel[String], cwd: File): F[List[String]] = {
-      val envVars = config.envVars.map(EnvVar.unapply(_).get)
+    override def execSandboxed(command: Nel[String], cwd: File): F[List[String]] =
       if (config.disableSandbox)
-        exec(command, cwd, envVars: _*)
+        exec(command, cwd)
       else {
         val whitelisted = (cwd.pathAsString :: config.whitelistedDirectories)
           .map(dir => s"--whitelist=$dir")
         val readOnly = config.readOnlyDirectories
           .map(dir => s"--read-only=$dir")
-        exec(Nel("firejail", whitelisted ++ readOnly) ::: command, cwd, envVars: _*)
+        exec(Nel("firejail", whitelisted ++ readOnly) ::: command, cwd)
       }
-    }
   }
 
   def create[F[_]](blocker: Blocker)(implicit
@@ -52,7 +49,8 @@ object ProcessAlg {
       logger: Logger[F],
       timer: Timer[F],
       F: Concurrent[F]
-  ): ProcessAlg[F] =
+  ): ProcessAlg[F] = {
+    val envVars = config.envVars.map(v => (v.name, v.value))
     new UsingFirejail[F](config) {
       override def exec(
           command: Nel[String],
@@ -63,10 +61,11 @@ object ProcessAlg {
           process.slurp[F](
             command,
             Some(cwd.toJava),
-            extraEnv.toMap,
+            (extraEnv ++ envVars).toMap,
             config.processTimeout,
             logger.trace(_),
             blocker
           )
     }
+  }
 }


Test patch:
diff --git a/modules/core/src/test/scala/org/scalasteward/core/buildtool/BuildToolDispatcherTest.scala b/modules/core/src/test/scala/org/scalasteward/core/buildtool/BuildToolDispatcherTest.scala
index 39851962..1eb94639 100644
--- a/modules/core/src/test/scala/org/scalasteward/core/buildtool/BuildToolDispatcherTest.scala
+++ b/modules/core/src/test/scala/org/scalasteward/core/buildtool/BuildToolDispatcherTest.scala
@@ -27,8 +27,8 @@ class BuildToolDispatcherTest extends AnyFunSuite with Matchers {
         List("test", "-f", s"$repoDir/build.sc"),
         List("test", "-f", s"$repoDir/build.sbt"),
         List(
-          "TEST_VAR=GREAT",
-          "ANOTHER_TEST_VAR=ALSO_GREAT",
+          "VAR1=val1",
+          "VAR2=val2",
           repoDir.toString,
           "firejail",
           s"--whitelist=$repoDir",
diff --git a/modules/core/src/test/scala/org/scalasteward/core/buildtool/maven/MavenAlgTest.scala b/modules/core/src/test/scala/org/scalasteward/core/buildtool/maven/MavenAlgTest.scala
index 70f26df7..630526a6 100644
--- a/modules/core/src/test/scala/org/scalasteward/core/buildtool/maven/MavenAlgTest.scala
+++ b/modules/core/src/test/scala/org/scalasteward/core/buildtool/maven/MavenAlgTest.scala
@@ -8,9 +8,6 @@ import org.scalatest.funsuite.AnyFunSuite
 import org.scalatest.matchers.should.Matchers
 
 class MavenAlgTest extends AnyFunSuite with Matchers {
-  val var1 = "TEST_VAR=GREAT"
-  val var2 = "ANOTHER_TEST_VAR=ALSO_GREAT"
-
   test("getDependencies") {
     val repo = Repo("namespace", "repo-name")
     val repoDir = config.workspace / repo.show
@@ -24,8 +21,8 @@ class MavenAlgTest extends AnyFunSuite with Matchers {
       logs = Vector.empty,
       commands = Vector(
         List(
-          var1,
-          var2,
+          "VAR1=val1",
+          "VAR2=val2",
           repoDir.toString,
           "firejail",
           s"--whitelist=$repoDir",
@@ -34,8 +31,8 @@ class MavenAlgTest extends AnyFunSuite with Matchers {
           command.listDependencies
         ),
         List(
-          var1,
-          var2,
+          "VAR1=val1",
+          "VAR2=val2",
           repoDir.toString,
           "firejail",
           s"--whitelist=$repoDir",
diff --git a/modules/core/src/test/scala/org/scalasteward/core/buildtool/sbt/SbtAlgTest.scala b/modules/core/src/test/scala/org/scalasteward/core/buildtool/sbt/SbtAlgTest.scala
index ee150e9a..5983e90c 100644
--- a/modules/core/src/test/scala/org/scalasteward/core/buildtool/sbt/SbtAlgTest.scala
+++ b/modules/core/src/test/scala/org/scalasteward/core/buildtool/sbt/SbtAlgTest.scala
@@ -39,8 +39,8 @@ class SbtAlgTest extends AnyFunSuite with Matchers {
     state shouldBe initial.copy(
       commands = Vector(
         List(
-          "TEST_VAR=GREAT",
-          "ANOTHER_TEST_VAR=ALSO_GREAT",
+          "VAR1=val1",
+          "VAR2=val2",
           repoDir.toString,
           "firejail",
           s"--whitelist=$repoDir",
@@ -74,8 +74,8 @@ class SbtAlgTest extends AnyFunSuite with Matchers {
         List("write", "/tmp/steward/.sbt/0.13/plugins/scala-steward-scalafix.sbt"),
         List("write", "/tmp/steward/.sbt/1.0/plugins/scala-steward-scalafix.sbt"),
         List(
-          "TEST_VAR=GREAT",
-          "ANOTHER_TEST_VAR=ALSO_GREAT",
+          "VAR1=val1",
+          "VAR2=val2",
           repoDir.toString,
           "firejail",
           s"--whitelist=$repoDir",
@@ -111,8 +111,8 @@ class SbtAlgTest extends AnyFunSuite with Matchers {
         List("write", "/tmp/steward/.sbt/1.0/plugins/scala-steward-scalafix.sbt"),
         List("write", s"$repoDir/scala-steward-scalafix-options.sbt"),
         List(
-          "TEST_VAR=GREAT",
-          "ANOTHER_TEST_VAR=ALSO_GREAT",
+          "VAR1=val1",
+          "VAR2=val2",
           repoDir.toString,
           "firejail",
           s"--whitelist=$repoDir",
diff --git a/modules/core/src/test/scala/org/scalasteward/core/git/GitAlgTest.scala b/modules/core/src/test/scala/org/scalasteward/core/git/GitAlgTest.scala
index dab23593..4e61f020 100644
--- a/modules/core/src/test/scala/org/scalasteward/core/git/GitAlgTest.scala
+++ b/modules/core/src/test/scala/org/scalasteward/core/git/GitAlgTest.scala
@@ -20,9 +20,10 @@ class GitAlgTest extends AnyFunSuite with Matchers {
   implicit val workspaceAlg: WorkspaceAlg[IO] = WorkspaceAlg.create[IO]
   val ioGitAlg: GitAlg[IO] = GitAlg.create[IO]
 
-  val repo = Repo("fthomas", "datapackage")
+  val repo: Repo = Repo("fthomas", "datapackage")
   val repoDir: String = (config.workspace / "fthomas/datapackage").toString
   val askPass = s"GIT_ASKPASS=${config.gitAskPass}"
+  val envVars = List(askPass, "VAR1=val1", "VAR2=val2")
 
   test("clone") {
     val url = uri"https://scala-steward@github.com/fthomas/datapackage"
@@ -30,8 +31,7 @@ class GitAlgTest extends AnyFunSuite with Matchers {
 
     state shouldBe MockState.empty.copy(
       commands = Vector(
-        List(
-          askPass,
+        envVars ++ List(
           config.workspace.toString,
           "git",
           "clone",
@@ -51,7 +51,7 @@ class GitAlgTest extends AnyFunSuite with Matchers {
 
     state shouldBe MockState.empty.copy(
       commands = Vector(
-        List(askPass, repoDir, "git", "log", "--pretty=format:'%an'", "master..update/cats-1.0.0")
+        envVars ++ List(repoDir, "git", "log", "--pretty=format:'%an'", "master..update/cats-1.0.0")
       )
     )
   }
@@ -64,7 +64,7 @@ class GitAlgTest extends AnyFunSuite with Matchers {
 
     state shouldBe MockState.empty.copy(
       commands = Vector(
-        List(askPass, repoDir, "git", "commit", "--all", "--no-gpg-sign", "-m", "Initial commit")
+        envVars ++ List(repoDir, "git", "commit", "--all", "--no-gpg-sign", "-m", "Initial commit")
       )
     )
   }
@@ -80,8 +80,7 @@ class GitAlgTest extends AnyFunSuite with Matchers {
 
     state shouldBe MockState.empty.copy(
       commands = Vector(
-        List(
-          askPass,
+        envVars ++ List(
           repoDir,
           "git",
           "remote",
@@ -89,10 +88,10 @@ class GitAlgTest extends AnyFunSuite with Matchers {
           "upstream",
           "http://github.com/fthomas/datapackage"
         ),
-        List(askPass, repoDir, "git", "fetch", "--tags", "upstream", "master"),
-        List(askPass, repoDir, "git", "checkout", "-B", "master", "--track", "upstream/master"),
-        List(askPass, repoDir, "git", "merge", "upstream/master"),
-        List(askPass, repoDir, "git", "push", "--force", "--set-upstream", "origin", "master")
+        envVars ++ List(repoDir, "git", "fetch", "--tags", "upstream", "master"),
+        envVars ++ List(repoDir, "git", "checkout", "-B", "master", "--track", "upstream/master"),
+        envVars ++ List(repoDir, "git", "merge", "upstream/master"),
+        envVars ++ List(repoDir, "git", "push", "--force", "--set-upstream", "origin", "master")
       )
     )
   }
diff --git a/modules/core/src/test/scala/org/scalasteward/core/io/MockProcessAlg.scala b/modules/core/src/test/scala/org/scalasteward/core/io/MockProcessAlg.scala
index 14b3ea04..9375a1ca 100644
--- a/modules/core/src/test/scala/org/scalasteward/core/io/MockProcessAlg.scala
+++ b/modules/core/src/test/scala/org/scalasteward/core/io/MockProcessAlg.scala
@@ -5,11 +5,15 @@ import org.scalasteward.core.application.Config
 import org.scalasteward.core.mock.{applyPure, MockEff}
 import org.scalasteward.core.util.Nel
 
-class MockProcessAlg(implicit config: Config) extends ProcessAlg.UsingFirejail[MockEff](config) {
+class MockProcessAlg(config: Config) extends ProcessAlg.UsingFirejail[MockEff](config) {
+  private val envVars = config.envVars.map(v => (v.name, v.value))
+
   override def exec(
       command: Nel[String],
       cwd: File,
       extraEnv: (String, String)*
   ): MockEff[List[String]] =
-    applyPure(s => (s.exec(cwd.toString :: command.toList, extraEnv: _*), List.empty[String]))
+    applyPure { s =>
+      (s.exec(cwd.toString :: command.toList, extraEnv ++ envVars: _*), List.empty[String])
+    }
 }
diff --git a/modules/core/src/test/scala/org/scalasteward/core/io/ProcessAlgTest.scala b/modules/core/src/test/scala/org/scalasteward/core/io/ProcessAlgTest.scala
index eda63c7d..4c51678b 100644
--- a/modules/core/src/test/scala/org/scalasteward/core/io/ProcessAlgTest.scala
+++ b/modules/core/src/test/scala/org/scalasteward/core/io/ProcessAlgTest.scala
@@ -28,7 +28,7 @@ class ProcessAlgTest extends AnyFunSuite with Matchers {
 
   test("respect the disableSandbox setting") {
     val cfg = config.copy(disableSandbox = true)
-    val processAlg = new MockProcessAlg()(cfg)
+    val processAlg = new MockProcessAlg(cfg)
 
     val state = processAlg
       .execSandboxed(Nel.of("echo", "hello"), File.temp)
@@ -36,9 +36,7 @@ class ProcessAlgTest extends AnyFunSuite with Matchers {
       .unsafeRunSync()
 
     state shouldBe MockState.empty.copy(
-      commands = Vector(
-        List("TEST_VAR=GREAT", "ANOTHER_TEST_VAR=ALSO_GREAT", File.temp.toString, "echo", "hello")
-      )
+      commands = Vector(List("VAR1=val1", "VAR2=val2", File.temp.toString, "echo", "hello"))
     )
   }
 
@@ -51,8 +49,8 @@ class ProcessAlgTest extends AnyFunSuite with Matchers {
     state shouldBe MockState.empty.copy(
       commands = Vector(
         List(
-          "TEST_VAR=GREAT",
-          "ANOTHER_TEST_VAR=ALSO_GREAT",
+          "VAR1=val1",
+          "VAR2=val2",
           File.temp.toString,
           "firejail",
           s"--whitelist=${File.temp}",
diff --git a/modules/core/src/test/scala/org/scalasteward/core/mock/MockContext.scala b/modules/core/src/test/scala/org/scalasteward/core/mock/MockContext.scala
index c654fe92..d013ded9 100644
--- a/modules/core/src/test/scala/org/scalasteward/core/mock/MockContext.scala
+++ b/modules/core/src/test/scala/org/scalasteward/core/mock/MockContext.scala
@@ -45,8 +45,8 @@ object MockContext {
     doNotFork = false,
     ignoreOptsFiles = false,
     envVars = List(
-      EnvVar("TEST_VAR", "GREAT"),
-      EnvVar("ANOTHER_TEST_VAR", "ALSO_GREAT")
+      EnvVar("VAR1", "val1"),
+      EnvVar("VAR2", "val2")
     ),
     processTimeout = 10.minutes,
     scalafix = Config.Scalafix(Nil, disableDefaults = false),
@@ -62,7 +62,7 @@ object MockContext {
 
   implicit val fileAlg: MockFileAlg = new MockFileAlg
   implicit val mockLogger: MockLogger = new MockLogger
-  implicit val processAlg: MockProcessAlg = new MockProcessAlg
+  implicit val processAlg: MockProcessAlg = new MockProcessAlg(config)
   implicit val workspaceAlg: MockWorkspaceAlg = new MockWorkspaceAlg
 
   implicit val coursierAlg: CoursierAlg[MockEff] = CoursierAlg.create
diff --git a/modules/core/src/test/scala/org/scalasteward/core/vcs/VCSRepoAlgTest.scala b/modules/core/src/test/scala/org/scalasteward/core/vcs/VCSRepoAlgTest.scala
index da704c32..819e2b7a 100644
--- a/modules/core/src/test/scala/org/scalasteward/core/vcs/VCSRepoAlgTest.scala
+++ b/modules/core/src/test/scala/org/scalasteward/core/vcs/VCSRepoAlgTest.scala
@@ -12,6 +12,7 @@ class VCSRepoAlgTest extends AnyFunSuite with Matchers {
   val repo: Repo = Repo("fthomas", "datapackage")
   val repoDir: String = (config.workspace / "fthomas/datapackage").toString
   val askPass = s"GIT_ASKPASS=${config.gitAskPass}"
+  val envVars = List(askPass, "VAR1=val1", "VAR2=val2")
 
   val parentRepoOut: RepoOut = RepoOut(
     "datapackage",
@@ -33,17 +34,16 @@ class VCSRepoAlgTest extends AnyFunSuite with Matchers {
     val state = vcsRepoAlg.clone(repo, forkRepoOut).runS(MockState.empty).unsafeRunSync()
     state shouldBe MockState.empty.copy(
       commands = Vector(
-        List(
-          askPass,
+        envVars ++ List(
           config.workspace.toString,
           "git",
           "clone",
           "--recursive",
           s"https://${config.vcsLogin}@github.com/scala-steward/datapackage",
-          repoDir.toString
+          repoDir
         ),
-        List(askPass, repoDir, "git", "config", "user.email", "bot@example.org"),
-        List(askPass, repoDir, "git", "config", "user.name", "Bot Doe")
+        envVars ++ List(repoDir, "git", "config", "user.email", "bot@example.org"),
+        envVars ++ List(repoDir, "git", "config", "user.name", "Bot Doe")
       )
     )
   }
@@ -61,8 +61,7 @@ class VCSRepoAlgTest extends AnyFunSuite with Matchers {
     val state = vcsRepoAlg.syncFork(repo, forkRepoOut).runS(MockState.empty).unsafeRunSync()
     state shouldBe MockState.empty.copy(
       commands = Vector(
-        List(
-          askPass,
+        envVars ++ List(
           repoDir,
           "git",
           "remote",
@@ -70,10 +69,10 @@ class VCSRepoAlgTest extends AnyFunSuite with Matchers {
           "upstream",
           s"https://${config.vcsLogin}@github.com/fthomas/datapackage"
         ),
-        List(askPass, repoDir, "git", "fetch", "--tags", "upstream", "master"),
-        List(askPass, repoDir, "git", "checkout", "-B", "master", "--track", "upstream/master"),
-        List(askPass, repoDir, "git", "merge", "upstream/master"),
-        List(askPass, repoDir, "git", "push", "--force", "--set-upstream", "origin", "master")
+        envVars ++ List(repoDir, "git", "fetch", "--tags", "upstream", "master"),
+        envVars ++ List(repoDir, "git", "checkout", "-B", "master", "--track", "upstream/master"),
+        envVars ++ List(repoDir, "git", "merge", "upstream/master"),
+        envVars ++ List(repoDir, "git", "push", "--force", "--set-upstream", "origin", "master")
       )
     )
   }