Make partest work with spaces in the path (from batch script and ant task).

- The 'partest' ant task gets a new 'compilerargs' element for scalac options (like in scalacfork and javac).
- Fixed argument list handling in partest task.
- Further improvements to argument list handling for all ant tasks.
- Fixed argument list handling in DirectTest (used by partest shell scripts)
- Fixed path handling in several test cases.

Closes SI-622. Review by phaller.

git-svn-id: http://lampsvn.epfl.ch/svn-repos/scala/scala/trunk@26047 5e8d7ff9-d8ef-0310-90f0-a4852d11357a

build.xml

@@ -1600,7 +1600,9 @@ BOOTRAPING TEST AND TEST SUITE
     <partest showlog="yes" erroronfailed="yes" javacmd="${java.home}/bin/java"
              timeout="2400000"
              srcdir="${partest.srcdir}"
-             scalacopts="${scalac.args.optimise} -Xpluginsdir ${build-quick.dir}/misc/scala-devel/plugins -Xplugin-require:continuations -P:continuations:enable">
+             scalacopts="${scalac.args.optimise} -Xplugin-require:continuations -P:continuations:enable">
+      <compilerarg value="-Xpluginsdir"/>
+      <compilerarg file="${build-quick.dir}/misc/scala-devel/plugins"/>
       <compilationpath>
         <path refid="pack.classpath"/>
         <fileset dir="${partest.dir}/files/lib" includes="*.jar" />

src/partest/scala/tools/partest/DirectTest.scala

@@ -7,7 +7,7 @@ package scala.tools.partest
 
 import scala.tools.nsc._
 import io.Directory
-import util.BatchSourceFile
+import util.{BatchSourceFile, CommandLineParser}
 
 /** A class for testing code which is embedded as a string.
  *  It allows for more complete control over settings, compiler
@@ -24,20 +24,20 @@ abstract class DirectTest extends App {
   def testOutput = io.Directory(sys.props("partest.output"))
 
   // override to add additional settings with strings
-  def extraSettings = ""
+  def extraSettings: String = ""
   // a default Settings object
-  def settings: Settings = newSettings(extraSettings)
+  def settings: Settings = newSettings(CommandLineParser tokenize extraSettings)
   // a custom Settings object
-  def newSettings(argString: String) = {
+  def newSettings(args: List[String]) = {
     val s = new Settings
-    val args = argString + " " + debugSettings
-    log("newSettings: args = '" + args + "'")
-    s processArgumentString args
+    val allArgs = args ++ (CommandLineParser tokenize debugSettings)
+    log("newSettings: allArgs = " + allArgs)
+    s processArguments (allArgs, true)
     s
   }
   // compile the code, optionally first adding to the settings
   def compile(args: String*) = {
-    val settings = newSettings(extraSettings +: args mkString " ")
+    val settings = newSettings((CommandLineParser tokenize extraSettings) ++ args.toList)
     val global   = new Global(settings)
     new global.Run compileSources List(new BatchSourceFile("<partest>", code))
     !global.reporter.hasErrors

src/partest/scala/tools/partest/PartestTask.scala

@@ -23,6 +23,7 @@ import java.lang.reflect.Method
 
 import org.apache.tools.ant.Task
 import org.apache.tools.ant.types.{Path, Reference, FileSet}
+import org.apache.tools.ant.types.Commandline.Argument
 
 class PartestTask extends Task with CompilationPathProperty {
 
@@ -115,8 +116,15 @@ class PartestTask extends Task with CompilationPathProperty {
     javaccmd = Some(input)
   }
 
-  def setScalacOpts(opts: String) {
-    scalacOpts = Some(opts)
+  def setScalacOpts(input: String) {
+    val s = input.split(' ').map { s => val a = new Argument; a.setValue(s); a }
+    scalacArgs = Some(scalacArgs.getOrElse(Seq()) ++ s)
+  }
+
+  def createCompilerArg(): Argument = {
+    val a = new Argument
+    scalacArgs = Some(scalacArgs.getOrElse(Seq()) :+ a)
+    a
   }
 
   def setTimeout(delay: String) {
@@ -151,7 +159,7 @@ class PartestTask extends Task with CompilationPathProperty {
   private var specializedFiles: Option[FileSet] = None
   private var presentationFiles: Option[FileSet] = None
   private var errorOnFailed: Boolean = false
-  private var scalacOpts: Option[String] = None
+  private var scalacArgs: Option[Seq[Argument]] = None
   private var timeout: Option[String] = None
   private var jUnitReportDir: Option[File] = None
   private var debug = false
@@ -236,6 +244,11 @@ class PartestTask extends Task with CompilationPathProperty {
       }
     } getOrElse sys.error("Provided classpath does not contain a Scala library.") 
     
+    def scalacArgsFlat: Option[Seq[String]] = scalacArgs map (_ flatMap { a =>
+      val parts = a.getParts
+      if(parts eq null) Seq[String]() else parts.toSeq
+    })
+
     val antRunner = new scala.tools.partest.nest.AntRunner
     val antFileManager = antRunner.fileManager
     
@@ -247,7 +260,7 @@ class PartestTask extends Task with CompilationPathProperty {
     
     javacmd foreach (x => antFileManager.JAVACMD = x.getAbsolutePath)
     javaccmd foreach (x => antFileManager.JAVAC_CMD = x.getAbsolutePath)
-    scalacOpts foreach (antFileManager.SCALAC_OPTS = _)
+    scalacArgsFlat foreach (antFileManager.SCALAC_OPTS = _)
     timeout foreach (antFileManager.timeout = _)
     
     type TFSet = (Array[File], String, String)

src/partest/scala/tools/partest/nest/CompileManager.scala

@@ -77,8 +77,8 @@ class DirectCompiler(val fileManager: FileManager) extends SimpleCompiler {
     // check whether there is a ".flags" file
     val flagsFileName = "%s.flags" format (basename(log.getName) dropRight 4) // 4 is "-run" or similar
     val argString = (io.File(log).parent / flagsFileName) ifFile (x => updatePluginPath(x.slurp())) getOrElse ""
-    val allOpts = fileManager.SCALAC_OPTS+" "+argString
-    val args = (allOpts split "\\s").toList
+    val allOpts = fileManager.SCALAC_OPTS.toList ::: argString.split(' ').toList.filter(_.length > 0)
+    val args = allOpts.toList
     
     NestUI.verbose("scalac options: "+allOpts)
     

src/partest/scala/tools/partest/nest/ConsoleFileManager.scala

@@ -44,7 +44,7 @@ class ConsoleFileManager extends FileManager {
 
   def this(buildPath: String, rawClasses: Boolean, moreOpts: String) = {
     this(buildPath, rawClasses)
-    SCALAC_OPTS = SCALAC_OPTS+" "+moreOpts
+    SCALAC_OPTS = SCALAC_OPTS ++ moreOpts.split(' ').toSeq.filter(_.length > 0)
   }
 
   lazy val srcDir        = PathSettings.srcDir

src/partest/scala/tools/partest/nest/FileManager.scala

@@ -66,7 +66,7 @@ trait FileManager extends FileUtil {
   var showLog = false
   var failed = false
 
-  var SCALAC_OPTS = PartestDefaults.scalacOpts
+  var SCALAC_OPTS = PartestDefaults.scalacOpts.split(' ').toSeq
   var JAVA_OPTS   = PartestDefaults.javaOpts
   var timeout     = PartestDefaults.timeout
   // how can 15 minutes not be enough? What are you doing, run/lisp.scala?

src/partest/scala/tools/partest/nest/Worker.scala

@@ -328,7 +328,8 @@ class Worker(val fileManager: FileManager, params: TestRunParams) extends Actor
       "-Dpartest.testname="+fileBase,
       "-Djavacmd="+javaCmd,
       "-Djavaccmd="+javacCmd,
-      "-Duser.language=en -Duser.country=US"
+      "-Duser.language=en",
+      "-Duser.country=US"
     ) ++ extras
 
     val classpath = if (classpathPrefix != "") join(classpathPrefix, CLASSPATH) else CLASSPATH
@@ -742,11 +743,10 @@ class Worker(val fileManager: FileManager, params: TestRunParams) extends Actor
             val sourcepath = sourcedir.getAbsolutePath+File.separator
             NestUI.verbose("sourcepath: "+sourcepath)
 
-            val argString =
-              "-d "+outDir.getAbsoluteFile.getPath+
-              " -Xresident"+
-              " -sourcepath "+sourcepath
-            val argList = argString split ' ' toList
+            val argList = List(
+              "-d", outDir.getAbsoluteFile.getPath,
+              "-Xresident",
+              "-sourcepath", sourcepath)
 
             // configure input/output files
             val logOut    = new FileOutputStream(logFile)

test/files/jvm/inner.scala

@@ -71,15 +71,15 @@ object Scalatest {
     val tmpfile = new FileWriter(tmpfilename)
     tmpfile.write(src)
     tmpfile.close
-    exec(javac + " -d " + outputdir + " -classpath " + classpath + " " + tmpfilename)
+    exec(javac, "-d", outputdir, "-classpath", classpath, tmpfilename)
   }
 
   def java(cname: String) =
-    exec(javacmd + " -cp " + classpath + " " + cname)
+    exec(javacmd, "-cp", classpath, cname)
 
   /** Execute cmd, wait for the process to end and pipe it's output to stdout */
-  private def exec(cmd: String) {
-    val proc = Runtime.getRuntime().exec(cmd)
+  private def exec(args: String*) {
+    val proc = Runtime.getRuntime().exec(args.toArray)
     val inp = new BufferedReader(new InputStreamReader(proc.getInputStream))
     val errp = new BufferedReader(new InputStreamReader(proc.getErrorStream))
     proc.waitFor()

test/scaladoc/scala/IndexScriptTest.scala

@@ -3,7 +3,7 @@ import org.scalacheck.Prop._
 
 import scala.tools.nsc.doc
 import scala.tools.nsc.doc.html.page.IndexScript
-import java.net.URLClassLoader
+import java.net.{URLClassLoader, URLDecoder}
 
 object Test extends Properties("IndexScript") {
 
@@ -11,7 +11,7 @@ object Test extends Properties("IndexScript") {
     val loader = Thread.currentThread.getContextClassLoader
     val paths = loader.asInstanceOf[URLClassLoader].getURLs
     val morepaths = loader.getParent.asInstanceOf[URLClassLoader].getURLs
-    (paths ++ morepaths).map(_.getPath).mkString(java.io.File.pathSeparator)
+    (paths ++ morepaths).map(u => URLDecoder.decode(u.getPath)).mkString(java.io.File.pathSeparator)
   }
 
   val docFactory = {

test/scaladoc/scala/IndexTest.scala

@@ -3,7 +3,7 @@ import org.scalacheck.Prop._
 
 import scala.tools.nsc.doc
 import scala.tools.nsc.doc.html.page.Index
-import java.net.URLClassLoader
+import java.net.{URLClassLoader, URLDecoder}
 
 object Test extends Properties("Index") {
 
@@ -12,8 +12,8 @@ object Test extends Properties("Index") {
     // this test previously relied on the assumption that the current thread's classloader is an url classloader and contains all the classpaths
     // does partest actually guarantee this? to quote Leonard Nimoy: The answer, of course, is no.
     // this test _will_ fail again some time in the future.
-    val paths = Thread.currentThread.getContextClassLoader.asInstanceOf[URLClassLoader].getURLs.map(_.getPath)
-    val morepaths = Thread.currentThread.getContextClassLoader.getParent.asInstanceOf[URLClassLoader].getURLs.map(_.getPath)
+    val paths = Thread.currentThread.getContextClassLoader.asInstanceOf[URLClassLoader].getURLs.map(u => URLDecoder.decode(u.getPath))
+    val morepaths = Thread.currentThread.getContextClassLoader.getParent.asInstanceOf[URLClassLoader].getURLs.map(u => URLDecoder.decode(u.getPath))
     (paths ++ morepaths).mkString(java.io.File.pathSeparator)
   }
   

test/scaladoc/scala/html/HtmlFactoryTest.scala

@@ -1,7 +1,7 @@
 import org.scalacheck._
 import org.scalacheck.Prop._
 
-import java.net.URLClassLoader
+import java.net.{URLClassLoader, URLDecoder}
 
 object XMLUtil {
   import scala.xml._
@@ -31,8 +31,8 @@ object Test extends Properties("HtmlFactory") {
     // this test previously relied on the assumption that the current thread's classloader is an url classloader and contains all the classpaths
     // does partest actually guarantee this? to quote Leonard Nimoy: The answer, of course, is no.
     // this test _will_ fail again some time in the future.
-    val paths = Thread.currentThread.getContextClassLoader.asInstanceOf[URLClassLoader].getURLs.map(_.getPath)
-    val morepaths = Thread.currentThread.getContextClassLoader.getParent.asInstanceOf[URLClassLoader].getURLs.map(_.getPath)
+    val paths = Thread.currentThread.getContextClassLoader.asInstanceOf[URLClassLoader].getURLs.map(u => URLDecoder.decode(u.getPath))
+    val morepaths = Thread.currentThread.getContextClassLoader.getParent.asInstanceOf[URLClassLoader].getURLs.map(u => URLDecoder.decode(u.getPath))
     (paths ++ morepaths).mkString(java.io.File.pathSeparator)
   }