drive-relative paths in Windows SHELL environments (#2516)

* fix for #2359, drive-relative paths accepted in Windows SHELL environments * added anti-regression test handling of drive-relative for os.Path * fix classpath-split regex String to accept either forward slash or backslash * added integration tests for JAVA_HOME setting to RunScriptTestDefinitions.scala * apply suggested changes * fix for cli.base-image.nativeImage in Windows * fix JAVA_HOME test; tweak mill script * Apply suggestions from code review --------- Co-authored-by: Piotr Chabelski <[email protected]>
VirtusLab · Nov 24, 2023 · 8df7956 · 8df7956
1 parent 5c243b5
commit 8df7956
Show file tree

Hide file tree

Showing 11 changed files with 73 additions and 11 deletions.
diff --git a/.github/scripts/generate-os-packages.sh b/.github/scripts/generate-os-packages.sh
@@ -14,7 +14,7 @@ fi
 ARTIFACTS_DIR="artifacts/"
 mkdir -p "$ARTIFACTS_DIR"
 
-if [[ "$OSTYPE" == "msys" ]]; then
+if [[ -z "$OSTYPE" ]]; then
   mill="./mill.bat"
 else
   mill="./mill"
@@ -28,7 +28,7 @@ launcher() {
   local launcherMillCommand="cli.nativeImage"
   local launcherName
 
-  if [[ "$OSTYPE" == "msys" ]]; then
+  if [[ "${OS-}" == "Windows_NT" ]]; then
     launcherName="scala.exe"
   else
     launcherName="scala"

diff --git a/.gitignore b/.gitignore
@@ -8,3 +8,6 @@ out/
 .scala-build
 dest/
 target/
+
+# ignore vim backup files
+*.sw[op]
diff --git a/mill b/mill
@@ -53,7 +53,21 @@ elif [[ "$cache_dest" == *.zip ]]; then
   fi
 fi
 
-eval "$("$cs" java --env --jvm temurin:17 || "$cs" java --env --jvm openjdk:1.17.0)"
+function to_bash_syntax {
+  local S
+  for S in "$@" ; do
+    echo "$S" | sed -E -e 's#^set #export #' -e 's#%([A-Z_][A-Z_0-9]*)%#${\1}#g' | tr '\\' '/'
+  done
+}
+# necessary for Windows various shell environments
+if [[ `uname | grep -E 'CYG*|MSYS*|MING*|UCRT*|ClANG*|GIT*'` ]]; then
+  # needed for coursier version < 2.1.8, harmless otherwise
+  IFS=$'\n'
+  eval "$(to_bash_syntax `"$cs" java --env --jvm temurin:17` || to_bash_syntax `"$cs" java --env --jvm openjdk:1.17.0`)"
+  unset IFS
+else
+  eval "$("$cs" java --env --jvm temurin:17 || "$cs" java --env --jvm openjdk:1.17.0)"
+fi
 
 # temporary, until we pass JPMS options to native-image,
 # see https://www.graalvm.org/release-notes/22_2/#native-image

diff --git a/mill.bat b/mill.bat
diff --git a/modules/build/src/test/scala/scala/build/tests/ActionableDiagnosticTests.scala b/modules/build/src/test/scala/scala/build/tests/ActionableDiagnosticTests.scala
@@ -221,7 +221,9 @@ class ActionableDiagnosticTests extends TestUtil.ScalaCliBuildSuite {
     )
     val withRepoBuildOptions = baseOptions.copy(
       classPathOptions =
-        baseOptions.classPathOptions.copy(extraRepositories = Seq(s"file:${repoTmpDir.toString}"))
+        baseOptions.classPathOptions.copy(extraRepositories =
+          Seq(s"file:${repoTmpDir.toString.replace('\\', '/')}")
+        )
     )
     testInputs.withBuild(withRepoBuildOptions, buildThreads, None, actionableDiagnostics = true) {
       (_, _, maybeBuild) =>

diff --git a/modules/cli/src/main/scala/scala/cli/commands/CommandUtils.scala b/modules/cli/src/main/scala/scala/cli/commands/CommandUtils.scala
@@ -18,7 +18,7 @@ object CommandUtils {
 
   // Ensure the path to the CLI is absolute
   def getAbsolutePathToScalaCli(programName: String): String =
-    if (programName.contains(File.separator))
+    if (programName.replace('\\', '/').contains("/"))
       os.Path(programName, Os.pwd).toString
     else
       /*

diff --git a/modules/cli/src/main/scala/scala/cli/commands/installcompletions/InstallCompletions.scala b/modules/cli/src/main/scala/scala/cli/commands/installcompletions/InstallCompletions.scala
@@ -114,7 +114,7 @@ object InstallCompletions extends ScalaCommand[InstallCompletionsOptions] {
   def getFormat(format: Option[String]): Option[String] =
     format.map(_.trim).filter(_.nonEmpty)
       .orElse {
-        Option(System.getenv("SHELL")).map(_.split(File.separator).last).map {
+        Option(System.getenv("SHELL")).map(_.split("[\\/]+").last).map {
           case "bash" => Bash.id
           case "zsh"  => Zsh.id
           case other  => other

diff --git a/modules/integration/src/test/scala/scala/cli/integration/CompileTestDefinitions.scala b/modules/integration/src/test/scala/scala/cli/integration/CompileTestDefinitions.scala
@@ -3,7 +3,6 @@ package scala.cli.integration
 import com.eed3si9n.expecty.Expecty.expect
 
 import java.io.File
-import java.util.regex.Pattern
 
 import scala.cli.integration.util.BloopUtil
 
@@ -585,7 +584,7 @@ abstract class CompileTestDefinitions(val scalaVersionOpt: Option[String])
           "."
         ).call(cwd = root)
       val classPath          = res.out.trim().split(File.pathSeparator)
-      val classPathFileNames = classPath.map(_.split(Pattern.quote(File.separator)).last)
+      val classPathFileNames = classPath.map(_.split("[\\\\/]+").last)
       expect(classPathFileNames.exists(_.startsWith("spark-core_")))
       // usually a duplicate is there if we don't call .distrinct when necessary here or there
       expect(classPathFileNames.exists(_.startsWith("snappy-java")))

diff --git a/...ntegration/src/test/scala/scala/cli/integration/InstallAndUninstallCompletionsTests.scala b/...ntegration/src/test/scala/scala/cli/integration/InstallAndUninstallCompletionsTests.scala
@@ -37,7 +37,8 @@ class InstallAndUninstallCompletionsTests extends ScalaCliSuite {
     }
   }
 
-  if (!Properties.isWin)
+  def isWinShell: Boolean = Option(System.getenv("OSTYPE")).nonEmpty
+  if (!Properties.isWin || isWinShell)
     test("installing and uninstalling completions") {
       runInstallAndUninstallCompletions()
     }

diff --git a/modules/integration/src/test/scala/scala/cli/integration/PackageTestDefinitions.scala b/modules/integration/src/test/scala/scala/cli/integration/PackageTestDefinitions.scala
@@ -6,7 +6,6 @@ import java.io.{File, InputStream}
 import java.nio.charset.StandardCharsets
 import java.nio.file.Files
 import java.util
-import java.util.regex.Pattern
 import java.util.zip.ZipFile
 
 import scala.cli.integration.TestUtil.removeAnsiColors
@@ -731,7 +730,7 @@ abstract class PackageTestDefinitions(val scalaVersionOpt: Option[String])
         if (actualScalaVersion.startsWith("2.")) actualScalaVersion
         else {
           val scalaLibJarName = scalaLibCp.split(File.pathSeparator)
-            .map(_.split(Pattern.quote(File.separator)).last).find(_.startsWith("scala-library-"))
+            .map(_.split("[\\\\/]+").last).find(_.startsWith("scala-library-"))
             .getOrElse {
               sys.error(s"scala-library not found in provided class path $scalaLibCp")
             }

diff --git a/modules/integration/src/test/scala/scala/cli/integration/RunScriptTestDefinitions.scala b/modules/integration/src/test/scala/scala/cli/integration/RunScriptTestDefinitions.scala
@@ -2,6 +2,8 @@ package scala.cli.integration
 
 import com.eed3si9n.expecty.Expecty.expect
 
+import java.io.File
+
 import scala.cli.integration.TestUtil.normalizeConsoleOutput
 import scala.util.Properties
 
@@ -595,4 +597,46 @@ trait RunScriptTestDefinitions { _: RunTestDefinitions =>
       expect(p.out.trim() == "42")
     }
   }
+
+  test("verify drive-relative JAVA_HOME works") {
+    val java8Home =
+      os.Path(os.proc(TestUtil.cs, "java-home", "--jvm", "zulu:8").call().out.trim(), os.pwd)
+
+    val dr = os.Path.driveRoot
+
+    // forward slash is legal in `Windows`
+    val javaHome = java8Home.toString.replace('\\', '/')
+    expect(javaHome.drop(dr.length).startsWith("/"))
+
+    val sysPath: String = System.getenv("PATH").replace('\\', '/')
+    val newPath: String = s"$javaHome/bin" + File.pathSeparator + sysPath
+
+    val extraEnv = Map(
+      "JAVA_HOME" -> java8Home.toString,
+      "PATH"      -> newPath
+    )
+
+    val inputs = TestInputs(
+      os.rel / "script-with-shebang" ->
+        s"""|#!/usr/bin/env -S ${TestUtil.cli.mkString(" ")} shebang -S 2.13
+            |//> using scala "$actualScalaVersion"
+            |println(args.toList)""".stripMargin
+    )
+    inputs.fromRoot { root =>
+      printf("TestUtil.cli: [%s]\njavaHome: [%s]\nnewPath: [%s]\n", TestUtil.cli, javaHome, newPath)
+      val proc = if (!Properties.isWin) {
+        os.perms.set(root / "script-with-shebang", os.PermSet.fromString("rwx------"))
+        os.proc("./script-with-shebang", "1", "2", "3", "-v")
+      }
+      else
+        os.proc(TestUtil.cli, "shebang", "script-with-shebang", "1", "2", "3", "-v")
+
+      val output = proc.call(cwd = root, env = extraEnv).out.trim()
+
+      val expectedOutput = "List(1, 2, 3, -v)"
+
+      expect(output == expectedOutput)
+    }
+  }
+
 }