Merge pull request #13562 from philwalk/argument-file-fix-13552

BarkingBad · web-flow · commit 411b9f01063f · 2021-09-27T12:02:43.000+02:00
insert compiler libraries head of user-specified classpath - fix for 13552
diff --git a/compiler/src/dotty/tools/MainGenericRunner.scala b/compiler/src/dotty/tools/MainGenericRunner.scala
@@ -16,6 +16,9 @@ import java.util.jar._
 import java.util.jar.Attributes.Name
 import dotty.tools.io.Jar
 import dotty.tools.runner.ScalaClassLoader
+import java.nio.file.{Files, Paths, Path}
+import scala.collection.JavaConverters._
+import dotty.tools.dotc.config.CommandLineParser
 
 enum ExecuteMode:
   case Guess
@@ -102,7 +105,18 @@ object MainGenericRunner {
     case "-run" :: fqName :: tail =>
       process(tail, settings.withExecuteMode(ExecuteMode.Run).withTargetToRun(fqName))
     case ("-cp" | "-classpath" | "--class-path") :: cp :: tail =>
-      process(tail, settings.copy(classPath = settings.classPath.appended(cp)))
+      val globdir = cp.replaceAll("[\\/][^\\/]*$", "") // slash/backslash agnostic
+      val (tailargs, cpstr) = if globdir.nonEmpty && classpathSeparator != ";" || cp.contains(classpathSeparator) then
+        (tail, cp)
+      else
+        // combine globbed classpath entries into a classpath
+        val jarfiles = cp :: tail
+        val cpfiles = jarfiles.takeWhile( f => f.startsWith(globdir) && ((f.toLowerCase.endsWith(".jar") || f.endsWith(".zip"))) )
+        val tailargs = jarfiles.drop(cpfiles.size)
+        (tailargs, cpfiles.mkString(classpathSeparator))
+        
+      process(tailargs, settings.copy(classPath = settings.classPath ++ cpstr.split(classpathSeparator).filter(_.nonEmpty)))
+
     case ("-version" | "--version") :: _ =>
       settings.copy(
         executeMode = ExecuteMode.Repl,
@@ -123,7 +137,8 @@ object MainGenericRunner {
     case (o @ javaOption(striped)) :: tail =>
       process(tail, settings.withJavaArgs(striped).withScalaArgs(o))
     case (o @ scalaOption(_*)) :: tail =>
-      process(tail, settings.withScalaArgs(o))
+      val remainingArgs = (CommandLineParser.expandArg(o) ++ tail).toList
+      process(remainingArgs, settings)
     case (o @ colorOption(_*)) :: tail =>
       process(tail, settings.withScalaArgs(o))
     case arg :: tail =>
@@ -143,6 +158,13 @@ object MainGenericRunner {
     val settings = process(allArgs.toList, Settings())
     if settings.exitCode != 0 then System.exit(settings.exitCode)
 
+    def removeCompiler(cp: Array[String]) =
+      if (!settings.compiler) then // Let's remove compiler from the classpath
+        val compilerLibs = Seq("scala3-compiler", "scala3-interfaces", "tasty-core", "scala-asm", "scala3-staging", "scala3-tasty-inspector")
+        cp.filterNot(c => compilerLibs.exists(c.contains))
+      else
+        cp
+
     def run(settings: Settings): Unit = settings.executeMode match
       case ExecuteMode.Repl =>
         val properArgs =
@@ -151,7 +173,7 @@ object MainGenericRunner {
         repl.Main.main(properArgs.toArray)
 
       case ExecuteMode.PossibleRun =>
-        val newClasspath = (settings.classPath :+ ".").map(File(_).toURI.toURL)
+        val newClasspath = (settings.classPath :+ ".").flatMap(_.split(classpathSeparator).filter(_.nonEmpty)).map(File(_).toURI.toURL)
         import dotty.tools.runner.RichClassLoader._
         val newClassLoader = ScalaClassLoader.fromURLsParallelCapable(newClasspath)
         val targetToRun = settings.possibleEntryPaths.to(LazyList).find { entryPath =>
@@ -166,15 +188,7 @@ object MainGenericRunner {
             run(settings.withExecuteMode(ExecuteMode.Repl))
       case ExecuteMode.Run =>
         val scalaClasspath = ClasspathFromClassloader(Thread.currentThread().getContextClassLoader).split(classpathSeparator)
-
-        def removeCompiler(cp: Array[String]) =
-          if (!settings.compiler) then // Let's remove compiler from the classpath
-            val compilerLibs = Seq("scala3-compiler", "scala3-interfaces", "tasty-core", "scala-asm", "scala3-staging", "scala3-tasty-inspector")
-            cp.filterNot(c => compilerLibs.exists(c.contains))
-          else
-            cp
-        val newClasspath = (settings.classPath ++ removeCompiler(scalaClasspath) :+ ".").map(File(_).toURI.toURL)
-
+        val newClasspath = (settings.classPath.flatMap(_.split(classpathSeparator).filter(_.nonEmpty)) ++ removeCompiler(scalaClasspath) :+ ".").map(File(_).toURI.toURL)
         val res = ObjectRunner.runAndCatch(newClasspath, settings.targetToRun, settings.residualArgs).flatMap {
           case ex: ClassNotFoundException if ex.getMessage == settings.targetToRun =>
             val file = settings.targetToRun
@@ -187,14 +201,30 @@ object MainGenericRunner {
         }
         errorFn("", res)
       case ExecuteMode.Script =>
-        val properArgs =
-          List("-classpath", settings.classPath.mkString(classpathSeparator)).filter(Function.const(settings.classPath.nonEmpty))
-            ++ settings.residualArgs
-            ++ (if settings.save then List("-save") else Nil)
-            ++ List("-script", settings.targetScript)
-            ++ settings.scalaArgs
-            ++ settings.scriptArgs
-        scripting.Main.main(properArgs.toArray)
+        val targetScript = Paths.get(settings.targetScript).toFile
+        val targetJar = settings.targetScript.replaceAll("[.][^\\/]*$", "")+".jar"
+        val precompiledJar = Paths.get(targetJar).toFile
+        def mainClass = Jar(targetJar).mainClass.getOrElse("") // throws exception if file not found
+        val jarIsValid = precompiledJar.isFile && mainClass.nonEmpty && precompiledJar.lastModified >= targetScript.lastModified
+        if jarIsValid then
+          // precompiledJar exists, is newer than targetScript, and manifest defines a mainClass
+          sys.props("script.path") = targetScript.toPath.toAbsolutePath.normalize.toString
+          val scalaClasspath = ClasspathFromClassloader(Thread.currentThread().getContextClassLoader).split(classpathSeparator)
+          val newClasspath = (settings.classPath.flatMap(_.split(classpathSeparator).filter(_.nonEmpty)) ++ removeCompiler(scalaClasspath) :+ ".").map(File(_).toURI.toURL)
+          val mc = mainClass
+          if mc.nonEmpty then
+            ObjectRunner.runAndCatch(newClasspath :+ File(targetJar).toURI.toURL, mc, settings.scriptArgs)
+          else
+            Some(IllegalArgumentException(s"No main class defined in manifest in jar: $precompiledJar"))
+        else
+          val properArgs =
+            List("-classpath", settings.classPath.mkString(classpathSeparator)).filter(Function.const(settings.classPath.nonEmpty))
+              ++ settings.residualArgs
+              ++ (if settings.save then List("-save") else Nil)
+              ++ settings.scalaArgs
+              ++ List("-script", settings.targetScript)
+              ++ settings.scriptArgs
+          scripting.Main.main(properArgs.toArray)
       case ExecuteMode.Guess =>
         if settings.modeShouldBePossibleRun then
           run(settings.withExecuteMode(ExecuteMode.PossibleRun))
diff --git a/compiler/src/dotty/tools/dotc/config/CliCommand.scala b/compiler/src/dotty/tools/dotc/config/CliCommand.scala
@@ -7,8 +7,7 @@ import Settings._
 import core.Contexts._
 import Properties._
 
- import scala.PartialFunction.cond
- import scala.collection.JavaConverters._
+import scala.PartialFunction.cond
 
 trait CliCommand:
 
@@ -42,24 +41,10 @@ trait CliCommand:
 
   /** Distill arguments into summary detailing settings, errors and files to main */
   def distill(args: Array[String], sg: Settings.SettingGroup)(ss: SettingsState = sg.defaultState)(using Context): ArgsSummary =
-    /**
-     * Expands all arguments starting with @ to the contents of the
-     * file named like each argument.
-     */
-    def expandArg(arg: String): List[String] =
-      def stripComment(s: String) = s takeWhile (_ != '#')
-      val path = Paths.get(arg stripPrefix "@")
-      if (!Files.exists(path))
-        report.error(s"Argument file ${path.getFileName} could not be found")
-        Nil
-      else
-        val lines = Files.readAllLines(path) // default to UTF-8 encoding
-        val params = lines.asScala map stripComment mkString " "
-        CommandLineParser.tokenize(params)
 
     // expand out @filename to the contents of that filename
     def expandedArguments = args.toList flatMap {
-      case x if x startsWith "@"  => expandArg(x)
+      case x if x startsWith "@"  => CommandLineParser.expandArg(x)
       case x                      => List(x)
     }
 
diff --git a/compiler/src/dotty/tools/dotc/config/CommandLineParser.scala b/compiler/src/dotty/tools/dotc/config/CommandLineParser.scala
@@ -3,6 +3,8 @@ package dotty.tools.dotc.config
 import scala.annotation.tailrec
 import scala.collection.mutable.ArrayBuffer
 import java.lang.Character.isWhitespace
+import java.nio.file.{Files, Paths}
+import scala.collection.JavaConverters._
 
 /** A simple enough command line parser.
  */
@@ -93,4 +95,19 @@ object CommandLineParser:
 
   def tokenize(line: String): List[String] = tokenize(line, x => throw new ParseException(x))
 
+  /**
+   * Expands all arguments starting with @ to the contents of the
+   * file named like each argument.
+   */
+  def expandArg(arg: String): List[String] =
+    def stripComment(s: String) = s takeWhile (_ != '#')
+    val path = Paths.get(arg stripPrefix "@")
+    if (!Files.exists(path))
+      System.err.println(s"Argument file ${path.getFileName} could not be found")
+      Nil
+    else
+      val lines = Files.readAllLines(path) // default to UTF-8 encoding
+      val params = lines.asScala map stripComment mkString " "
+      tokenize(params)
+
   class ParseException(msg: String) extends RuntimeException(msg)
diff --git a/compiler/test-coursier/dotty/tools/coursier/CoursierScalaTests.scala b/compiler/test-coursier/dotty/tools/coursier/CoursierScalaTests.scala
@@ -122,6 +122,16 @@ class CoursierScalaTests:
       assertTrue(output.mkString("\n").contains("Unable to create a system terminal")) // Scala attempted to create REPL so we can assume it is working
     replWithArgs()
 
+    def argumentFile() =
+      // verify that an arguments file is accepted
+      // verify that setting a user classpath does not remove compiler libraries from the classpath.
+      // arguments file contains "-classpath .", adding current directory to classpath.
+      val source = new File(getClass.getResource("/run/myfile.scala").getPath)
+      val argsFile = new File(getClass.getResource("/run/myargs.txt").getPath)
+      val output = CoursierScalaTests.csScalaCmd(s"@$argsFile", source.absPath)
+      assertEquals(output.mkString("\n"), "Hello")
+    argumentFile()
+
 object CoursierScalaTests:
 
   def execCmd(command: String, options: String*): List[String] =
diff --git a/compiler/test-coursier/run/myargs.txt b/compiler/test-coursier/run/myargs.txt
@@ -0,0 +1 @@
+-classpath .
diff --git a/compiler/test-resources/scripting/argfileClasspath.sc b/compiler/test-resources/scripting/argfileClasspath.sc
@@ -0,0 +1,9 @@
+#!dist/target/pack/bin/scala @compiler/test-resources/scripting/cpArgumentsFile.txt
+
+import java.nio.file.Paths
+
+def main(args: Array[String]): Unit =
+  val cwd = Paths.get(".").toAbsolutePath.toString.replace('\\', '/').replaceAll("/$", "")
+  printf("cwd: %s\n", cwd)
+  printf("classpath: %s\n", sys.props("java.class.path"))
+
diff --git a/compiler/test-resources/scripting/classpathReport.sc b/compiler/test-resources/scripting/classpathReport.sc
@@ -1,9 +1,12 @@
-#!dist/target/pack/bin/scala -classpath 'dist/target/pack/lib/*'
+#!dist/target/pack/bin/scala -classpath dist/target/pack/lib/*
 
 import java.nio.file.Paths
 
 def main(args: Array[String]): Unit =
-  val cwd = Paths.get(".").toAbsolutePath.toString.replace('\\', '/').replaceAll("/$", "")
+  val cwd = Paths.get(".").toAbsolutePath.normalize.toString.norm
   printf("cwd: %s\n", cwd)
-  printf("classpath: %s\n", sys.props("java.class.path"))
+  printf("classpath: %s\n", sys.props("java.class.path").norm)
+
+extension(s: String)
+  def norm: String = s.replace('\\', '/')
 
diff --git a/compiler/test-resources/scripting/cpArgumentsFile.txt b/compiler/test-resources/scripting/cpArgumentsFile.txt
@@ -0,0 +1 @@
+-classpath dist/target/pack/lib/*
diff --git a/compiler/test-resources/scripting/scriptPath.sc b/compiler/test-resources/scripting/scriptPath.sc
@@ -1,4 +1,4 @@
-#!/usr/bin/env scala
+#!dist/target/pack/bin/scala
 
   def main(args: Array[String]): Unit =
     args.zipWithIndex.foreach { case (arg,i) => printf("arg %d: [%s]\n",i,arg) }
@@ -17,3 +17,6 @@
       System.err.printf("sun.java.command: %s\n", sys.props("sun.java.command"))
       System.err.printf("first 5 PATH entries:\n%s\n",pathEntries.take(5).mkString("\n"))
     }
+
+  extension(s: String)
+    def norm: String = s.replace('\\', '/')
diff --git a/compiler/test/dotty/tools/scripting/BashScriptsTests.scala b/compiler/test/dotty/tools/scripting/BashScriptsTests.scala
@@ -58,7 +58,7 @@ class BashScriptsTests:
 
   /* verify `dist/bin/scala` non-interference with command line args following script name */
   @Test def verifyScalaArgs =
-    val commandline = (Seq(scalaPath, showArgsScript) ++ testScriptArgs).mkString(" ")
+    val commandline = (Seq("SCALA_OPTS= ", scalaPath, showArgsScript) ++ testScriptArgs).mkString(" ")
     val (validTest, exitCode, stdout, stderr) = bashCommand(commandline)
     if validTest then
       var fail = false
@@ -78,7 +78,7 @@ class BashScriptsTests:
    */
   @Test def verifyScriptPathProperty =
     val scriptFile = testFiles.find(_.getName == "scriptPath.sc").get
-    val expected = s"/${scriptFile.getName}"
+    val expected = s"${scriptFile.getName}"
     printf("===> verify valid system property script.path is reported by script [%s]\n", scriptFile.getName)
     printf("calling scriptFile: %s\n", scriptFile)
     val (validTest, exitCode, stdout, stderr) = bashCommand(scriptFile.absPath)
@@ -100,11 +100,12 @@ class BashScriptsTests:
     if validTest then
       val expected = s"${workingDirectory.toString}"
       val List(line1: String, line2: String) = stdout.take(2)
+      printf("line1 [%s]\n", line1)
       val valid = line2.dropWhile( _ != ' ').trim.startsWith(expected)
       if valid then printf(s"\n===> success: classpath begins with %s, as reported by [%s]\n", workingDirectory, scriptFile.getName)
       assert(valid, s"script ${scriptFile.absPath} did not report valid java.class.path first entry")
 
-  def existingPath: String = envOrElse("PATH","").norm
+  def existingPath: String = envOrElse("PATH", "").norm
   def adjustedPath = s"$javaHome/bin$psep$scalaHome/bin$psep$existingPath"
   def pathEntries = adjustedPath.split(psep).toList
 
@@ -114,12 +115,12 @@ class BashScriptsTests:
     val path = Files.createTempFile("scriptingTest", ".args")
     val text = s"-classpath ${workingDirectory.absPath}"
     Files.write(path, text.getBytes(utfCharset))
-    path.toFile.getAbsolutePath.replace('\\', '/')
+    path.toFile.getAbsolutePath.norm
 
   def fixHome(s: String): String =
     s.startsWith("~") match {
     case false => s
-    case true => s.replaceFirst("~",userHome)
+    case true => s.replaceFirst("~", userHome)
     }
 
   extension(s: String) {
@@ -145,7 +146,7 @@ class BashScriptsTests:
     def absPath: String = f.getAbsolutePath.norm
   }
 
-  lazy val psep: String = propOrElse("path.separator","")
+  lazy val psep: String = propOrElse("path.separator", "")
   lazy val osname = propOrElse("os.name", "").toLowerCase
 
   lazy val scalacPath = s"$workingDirectory/dist/target/pack/bin/scalac".norm
@@ -162,7 +163,7 @@ class BashScriptsTests:
   //    else, SCALA_HOME if defined
   //    else, not defined
   lazy val scalaHome =
-    if scalacPath.isFile then scalacPath.replaceAll("/bin/scalac","")
+    if scalacPath.isFile then scalacPath.replaceAll("/bin/scalac", "")
     else envOrElse("SCALA_HOME", "").norm
 
   lazy val javaHome = envOrElse("JAVA_HOME", "").norm
@@ -171,7 +172,7 @@ class BashScriptsTests:
     ("JAVA_HOME", javaHome),
     ("SCALA_HOME", scalaHome),
     ("PATH", adjustedPath),
-  ).filter { case (name,valu) => valu.nonEmpty }
+  ).filter { case (name, valu) => valu.nonEmpty }
 
   lazy val whichBash: String =
     var whichBash = ""
@@ -182,7 +183,7 @@ class BashScriptsTests:
 
     whichBash
 
-  def bashCommand(cmdstr: String, additionalEnvPairs:List[(String, String)] = Nil): (Boolean, Int, Seq[String], Seq[String]) = {
+  def bashCommand(cmdstr: String, additionalEnvPairs: List[(String, String)] = Nil): (Boolean, Int, Seq[String], Seq[String]) = {
     var (stdout, stderr) = (List.empty[String], List.empty[String])
     if bashExe.toFile.exists then
       val cmd = Seq(bashExe, "-c", cmdstr)
diff --git a/compiler/test/dotty/tools/scripting/ClasspathTests.scala b/compiler/test/dotty/tools/scripting/ClasspathTests.scala
