Skip to content

Commit

Permalink
Merge pull request #13562 from philwalk/argument-file-fix-13552
Browse files Browse the repository at this point in the history
insert compiler libraries head of user-specified classpath - fix for 13552
  • Loading branch information
BarkingBad committed Sep 27, 2021
2 parents ee8d2ab + c9f21b8 commit 411b9f0
Show file tree
Hide file tree
Showing 11 changed files with 120 additions and 132 deletions.
70 changes: 50 additions & 20 deletions compiler/src/dotty/tools/MainGenericRunner.scala
Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand All @@ -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 =>
Expand All @@ -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 =
Expand All @@ -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 =>
Expand All @@ -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
Expand All @@ -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))
Expand Down
19 changes: 2 additions & 17 deletions compiler/src/dotty/tools/dotc/config/CliCommand.scala
Expand Up @@ -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:

Expand Down Expand Up @@ -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)
}

Expand Down
17 changes: 17 additions & 0 deletions compiler/src/dotty/tools/dotc/config/CommandLineParser.scala
Expand Up @@ -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.
*/
Expand Down Expand Up @@ -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)
Expand Up @@ -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] =
Expand Down
1 change: 1 addition & 0 deletions compiler/test-coursier/run/myargs.txt
@@ -0,0 +1 @@
-classpath .
9 changes: 9 additions & 0 deletions 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"))

9 changes: 6 additions & 3 deletions 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('\\', '/')

1 change: 1 addition & 0 deletions compiler/test-resources/scripting/cpArgumentsFile.txt
@@ -0,0 +1 @@
-classpath dist/target/pack/lib/*
5 changes: 4 additions & 1 deletion 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) }
Expand All @@ -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('\\', '/')
19 changes: 10 additions & 9 deletions compiler/test/dotty/tools/scripting/BashScriptsTests.scala
Expand Up @@ -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
Expand All @@ -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)
Expand All @@ -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

Expand All @@ -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) {
Expand All @@ -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
Expand All @@ -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
Expand All @@ -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 = ""
Expand All @@ -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)
Expand Down

0 comments on commit 411b9f0

Please sign in to comment.