From 5455902d9e4d6b8ca712e4c62e9771640029927d Mon Sep 17 00:00:00 2001 From: Phil Date: Mon, 27 Sep 2021 13:17:34 -0600 Subject: [PATCH 1/4] undo windows wildcard classpath globbing - fix for #13618 --- .../src/dotty/tools/MainGenericRunner.scala | 11 +++--- .../scripting/unglobClasspath.sc | 8 ++++ .../tools/scripting/ClasspathTests.scala | 38 ++++++++++++++++--- .../tools/scripting/ScriptingTests.scala | 2 +- 4 files changed, 46 insertions(+), 13 deletions(-) create mode 100755 compiler/test-resources/scripting/unglobClasspath.sc diff --git a/compiler/src/dotty/tools/MainGenericRunner.scala b/compiler/src/dotty/tools/MainGenericRunner.scala index fa6faa7d570e..189030fdcd81 100644 --- a/compiler/src/dotty/tools/MainGenericRunner.scala +++ b/compiler/src/dotty/tools/MainGenericRunner.scala @@ -105,7 +105,7 @@ object MainGenericRunner { case "-run" :: fqName :: tail => process(tail, settings.withExecuteMode(ExecuteMode.Run).withTargetToRun(fqName)) case ("-cp" | "-classpath" | "--class-path") :: cp :: tail => - val globdir = cp.replaceAll("[\\/][^\\/]*$", "") // slash/backslash agnostic + val globdir = cp.replaceAll("[\\\\/][^\\\\/]*$", "") // slash/backslash agnostic val (tailargs, cpstr) = if globdir.nonEmpty && classpathSeparator != ";" || cp.contains(classpathSeparator) then (tail, cp) else @@ -204,16 +204,15 @@ object MainGenericRunner { 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 + val mainClass = if !precompiledJar.isFile then "" else Jar(targetJar).mainClass.getOrElse("") + val jarIsValid = 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) + if mainClass.nonEmpty then + ObjectRunner.runAndCatch(newClasspath :+ File(targetJar).toURI.toURL, mainClass, settings.scriptArgs) else Some(IllegalArgumentException(s"No main class defined in manifest in jar: $precompiledJar")) else diff --git a/compiler/test-resources/scripting/unglobClasspath.sc b/compiler/test-resources/scripting/unglobClasspath.sc new file mode 100755 index 000000000000..796697cdedf2 --- /dev/null +++ b/compiler/test-resources/scripting/unglobClasspath.sc @@ -0,0 +1,8 @@ +#!bin/scala -classpath 'dist/target/pack/lib/*' + +// won't compile unless the hashbang line sets classpath +import org.jline.terminal.Terminal + +def main(args: Array[String]) = + val cp = sys.props("java.class.path") + printf("unglobbed classpath: %s\n", cp) diff --git a/compiler/test/dotty/tools/scripting/ClasspathTests.scala b/compiler/test/dotty/tools/scripting/ClasspathTests.scala index 159605cd967c..db359d646d74 100755 --- a/compiler/test/dotty/tools/scripting/ClasspathTests.scala +++ b/compiler/test/dotty/tools/scripting/ClasspathTests.scala @@ -18,12 +18,6 @@ class ClasspathTests: val packBinDir = "dist/target/pack/bin" val packLibDir = "dist/target/pack/lib" - // only interested in classpath test scripts - val testScriptName = "classpathReport.sc" - val testScript = scripts("/scripting").find { _.getName.matches(testScriptName) } match - case None => sys.error(s"test script not found: ${testScriptName}") - case Some(file) => file - def exists(scriptPath: Path): Boolean = Files.exists(scriptPath) def packBinScalaExists:Boolean = exists(Paths.get(s"$packBinDir/scala")) @@ -31,6 +25,12 @@ class ClasspathTests: * verify classpath reported by called script. */ @Test def hashbangClasspathVerifyTest = { + // only interested in classpath test scripts + val testScriptName = "classpathReport.sc" + val testScript = scripts("/scripting").find { _.getName.matches(testScriptName) } match + case None => sys.error(s"test script not found: ${testScriptName}") + case Some(file) => file + val relpath = testScript.toPath.relpath.norm printf("===> hashbangClasspathVerifyTest for script [%s]\n", relpath) printf("bash is [%s]\n", bashExe) @@ -57,6 +57,32 @@ class ClasspathTests: assert(hashbangClasspathJars.size == packlibJars.size) } + /* + * verify classpath is unglobbed by MainGenericRunner. + */ + @Test def unglobClasspathVerifyTest = { + val testScriptName = "unglobClasspath.sc" + val testScript = scripts("/scripting").find { _.getName.matches(testScriptName) } match + case None => sys.error(s"test script not found: ${testScriptName}") + case Some(file) => file + + val relpath = testScript.toPath.relpath.norm + printf("===> unglobClasspathVerifyTest for script [%s]\n", relpath) + printf("bash is [%s]\n", bashExe) + + if packBinScalaExists then + val bashCmdline = s"SCALA_OPTS= $relpath" + val cmd = Array(bashExe, "-c", bashCmdline) + + cmd.foreach { printf("[%s]\n", _) } + + // test script reports the classpath it sees + val scriptOutput = exec(cmd:_*) + val scriptCp = findTaggedLine("unglobbed classpath", scriptOutput) + val classpathJars = scriptCp.split(psep).map { _.getName }.sorted.distinct + //classpathJars.foreach { printf("%s\n", _) } + assert(classpathJars.size > 1) + } //////////////// end of tests //////////////// diff --git a/compiler/test/dotty/tools/scripting/ScriptingTests.scala b/compiler/test/dotty/tools/scripting/ScriptingTests.scala index 25fdfe14f5c7..922564d4ff58 100644 --- a/compiler/test/dotty/tools/scripting/ScriptingTests.scala +++ b/compiler/test/dotty/tools/scripting/ScriptingTests.scala @@ -19,7 +19,7 @@ class ScriptingTests: f.getAbsolutePath.replace('\\', '/') // classpath tests managed by scripting.ClasspathTests.scala - def testFiles = scripts("/scripting").filter { ! _.getName.startsWith("classpath") } + def testFiles = scripts("/scripting").filter { ! _.getName.toLowerCase.contains("classpath") } def script2jar(scriptFile: File) = val jarName = s"${scriptFile.getName.dropExtension}.jar" From 275deea2cd702f1bb80a4e031bdb14cba68eb293 Mon Sep 17 00:00:00 2001 From: Phil Date: Tue, 28 Sep 2021 13:58:11 -0600 Subject: [PATCH 2/4] remove platform-specific behaviour; improve comments, invert negative conditional --- compiler/src/dotty/tools/MainGenericRunner.scala | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/compiler/src/dotty/tools/MainGenericRunner.scala b/compiler/src/dotty/tools/MainGenericRunner.scala index 189030fdcd81..bdf1b66e1692 100644 --- a/compiler/src/dotty/tools/MainGenericRunner.scala +++ b/compiler/src/dotty/tools/MainGenericRunner.scala @@ -106,14 +106,14 @@ object MainGenericRunner { process(tail, settings.withExecuteMode(ExecuteMode.Run).withTargetToRun(fqName)) case ("-cp" | "-classpath" | "--class-path") :: cp :: tail => 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 (tailargs, cpstr) = if globdir.nonEmpty && !cp.contains(classpathSeparator) then + // globdir is wildcard directory for globbed jar files, reconstruct the intended 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)) + else + (tail, cp) process(tailargs, settings.copy(classPath = settings.classPath ++ cpstr.split(classpathSeparator).filter(_.nonEmpty))) From eda3104630ad75d1e937dee4a2ef9ad37a03e5e1 Mon Sep 17 00:00:00 2001 From: Phil Date: Wed, 29 Sep 2021 15:03:41 -0600 Subject: [PATCH 3/4] refine globbed classpath test to distinguish jars from dirs --- .../src/dotty/tools/MainGenericRunner.scala | 39 ++++++++++++++----- .../tools/scripting/BashScriptsTests.scala | 10 +++-- 2 files changed, 36 insertions(+), 13 deletions(-) diff --git a/compiler/src/dotty/tools/MainGenericRunner.scala b/compiler/src/dotty/tools/MainGenericRunner.scala index bdf1b66e1692..546ca6b213fb 100644 --- a/compiler/src/dotty/tools/MainGenericRunner.scala +++ b/compiler/src/dotty/tools/MainGenericRunner.scala @@ -105,17 +105,20 @@ object MainGenericRunner { case "-run" :: fqName :: tail => process(tail, settings.withExecuteMode(ExecuteMode.Run).withTargetToRun(fqName)) case ("-cp" | "-classpath" | "--class-path") :: cp :: tail => - val globdir = cp.replaceAll("[\\\\/][^\\\\/]*$", "") // slash/backslash agnostic - val (tailargs, cpstr) = if globdir.nonEmpty && !cp.contains(classpathSeparator) then + val cpEntries = cp.split(classpathSeparator).toList + val singleEntryClasspath: Boolean = cpEntries.nonEmpty && cpEntries.drop(1).isEmpty + val globdir: String = if singleEntryClasspath then cp.replaceAll("[\\\\/][^\\\\/]*$", "") else "" // slash/backslash agnostic + def validGlobbedJar(s: String): Boolean = s.startsWith(globdir) && ((s.toLowerCase.endsWith(".jar") || s.toLowerCase.endsWith(".zip"))) + val (tailargs, newEntries) = if singleEntryClasspath && validGlobbedJar(cpEntries.head) then + // reassemble globbed wildcard classpath // globdir is wildcard directory for globbed jar files, reconstruct the intended 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)) + val cpJars = tail.takeWhile( f => validGlobbedJar(f) ) + val remainingArgs = tail.drop(cpJars.size) + (remainingArgs, cpEntries ++ cpJars) else - (tail, cp) - - process(tailargs, settings.copy(classPath = settings.classPath ++ cpstr.split(classpathSeparator).filter(_.nonEmpty))) + (tail, cpEntries) + + process(tailargs, settings.copy(classPath = settings.classPath ++ newEntries.filter(_.nonEmpty))) case ("-version" | "--version") :: _ => settings.copy( @@ -240,4 +243,22 @@ object MainGenericRunner { e.foreach(_.printStackTrace()) !isFailure } + + def display(settings: Settings)= Seq( + s"verbose: ${settings.verbose}", + s"classPath: ${settings.classPath.mkString("\n ","\n ","")}", + s"executeMode: ${settings.executeMode}", + s"exitCode: ${settings.exitCode}", + s"javaArgs: ${settings.javaArgs}", + s"scalaArgs: ${settings.scalaArgs}", + s"residualArgs: ${settings.residualArgs}", + s"possibleEntryPaths: ${settings.possibleEntryPaths}", + s"scriptArgs: ${settings.scriptArgs}", + s"targetScript: ${settings.targetScript}", + s"targetToRun: ${settings.targetToRun}", + s"save: ${settings.save}", + s"modeShouldBePossibleRun: ${settings.modeShouldBePossibleRun}", + s"modeShouldBeRun: ${settings.modeShouldBeRun}", + s"compiler: ${settings.compiler}", + ) } diff --git a/compiler/test/dotty/tools/scripting/BashScriptsTests.scala b/compiler/test/dotty/tools/scripting/BashScriptsTests.scala index 8a6c6aad1ec1..061350e86e33 100644 --- a/compiler/test/dotty/tools/scripting/BashScriptsTests.scala +++ b/compiler/test/dotty/tools/scripting/BashScriptsTests.scala @@ -97,11 +97,13 @@ class BashScriptsTests: printf("===> verify SCALA_OPTS='@argsfile' is properly handled by `dist/bin/scala`\n") val envPairs = List(("SCALA_OPTS", s"@$argsfile")) val (validTest, exitCode, stdout, stderr) = bashCommand(scriptFile.absPath, envPairs) + printf("stdout: %s\n", stdout.mkString("\n","\n","")) 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) + val expected = s"${workingDirectory.norm}" + val output = stdout.find( _.trim.startsWith("cwd") ).getOrElse("").dropWhile(_!=' ').trim + printf("output [%s]\n", output) + printf("expected[%s]\n", expected) + val valid = output.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") From bf753c7c2cc07c6f2dbaa0c6b40981d7a68a900e Mon Sep 17 00:00:00 2001 From: Phil Date: Thu, 30 Sep 2021 08:40:30 -0600 Subject: [PATCH 4/4] remove debug-assist method; cleanup solitary jar test --- .../src/dotty/tools/MainGenericRunner.scala | 20 +------------------ .../tools/scripting/ClasspathTests.scala | 6 ++++-- 2 files changed, 5 insertions(+), 21 deletions(-) diff --git a/compiler/src/dotty/tools/MainGenericRunner.scala b/compiler/src/dotty/tools/MainGenericRunner.scala index 546ca6b213fb..dfc56a8c11d2 100644 --- a/compiler/src/dotty/tools/MainGenericRunner.scala +++ b/compiler/src/dotty/tools/MainGenericRunner.scala @@ -106,7 +106,7 @@ object MainGenericRunner { process(tail, settings.withExecuteMode(ExecuteMode.Run).withTargetToRun(fqName)) case ("-cp" | "-classpath" | "--class-path") :: cp :: tail => val cpEntries = cp.split(classpathSeparator).toList - val singleEntryClasspath: Boolean = cpEntries.nonEmpty && cpEntries.drop(1).isEmpty + val singleEntryClasspath: Boolean = cpEntries.take(2).size == 1 val globdir: String = if singleEntryClasspath then cp.replaceAll("[\\\\/][^\\\\/]*$", "") else "" // slash/backslash agnostic def validGlobbedJar(s: String): Boolean = s.startsWith(globdir) && ((s.toLowerCase.endsWith(".jar") || s.toLowerCase.endsWith(".zip"))) val (tailargs, newEntries) = if singleEntryClasspath && validGlobbedJar(cpEntries.head) then @@ -243,22 +243,4 @@ object MainGenericRunner { e.foreach(_.printStackTrace()) !isFailure } - - def display(settings: Settings)= Seq( - s"verbose: ${settings.verbose}", - s"classPath: ${settings.classPath.mkString("\n ","\n ","")}", - s"executeMode: ${settings.executeMode}", - s"exitCode: ${settings.exitCode}", - s"javaArgs: ${settings.javaArgs}", - s"scalaArgs: ${settings.scalaArgs}", - s"residualArgs: ${settings.residualArgs}", - s"possibleEntryPaths: ${settings.possibleEntryPaths}", - s"scriptArgs: ${settings.scriptArgs}", - s"targetScript: ${settings.targetScript}", - s"targetToRun: ${settings.targetToRun}", - s"save: ${settings.save}", - s"modeShouldBePossibleRun: ${settings.modeShouldBePossibleRun}", - s"modeShouldBeRun: ${settings.modeShouldBeRun}", - s"compiler: ${settings.compiler}", - ) } diff --git a/compiler/test/dotty/tools/scripting/ClasspathTests.scala b/compiler/test/dotty/tools/scripting/ClasspathTests.scala index db359d646d74..82b6affe0c02 100755 --- a/compiler/test/dotty/tools/scripting/ClasspathTests.scala +++ b/compiler/test/dotty/tools/scripting/ClasspathTests.scala @@ -50,10 +50,12 @@ class ClasspathTests: val hashbangClasspathJars = scriptCp.split(psep).map { _.getName }.sorted.distinct val packlibJars = listJars(s"$scriptCwd/$packLibDir").sorted.distinct + printf("%d jar files in dist/target/pack/lib\n", packlibJars.size) + printf("%d test script jars in classpath\n", hashbangClasspathJars.size) + // verify that the classpath set in the hashbang line is effective if hashbangClasspathJars.size != packlibJars.size then - printf("%d test script jars in classpath\n", hashbangClasspathJars.size) - printf("%d jar files in dist/target/pack/lib\n", packlibJars.size) + printf("hashbangClasspathJars: %s\n", hashbangClasspathJars.mkString("\n ", "\n ", "")) assert(hashbangClasspathJars.size == packlibJars.size) }