Make partest work with spaces in the path (from...

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.

11 files changed, 50 insertions, 35 deletions

diff --git a/build.xml b/build.xml
index 0badd57afc..5f2a7080b9 100644
--- a/build.xml
+++ b/build.xml
@@ -1599,7 +1599,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" />
diff --git a/src/partest/scala/tools/partest/DirectTest.scala b/src/partest/scala/tools/partest/DirectTest.scala
index 6c210f1ac1..be8cac9147 100644
--- a/src/partest/scala/tools/partest/DirectTest.scala
+++ b/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
diff --git a/src/partest/scala/tools/partest/PartestTask.scala b/src/partest/scala/tools/partest/PartestTask.scala
index 76b736c904..588f87449d 100644
--- a/src/partest/scala/tools/partest/PartestTask.scala
+++ b/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)
diff --git a/src/partest/scala/tools/partest/nest/CompileManager.scala b/src/partest/scala/tools/partest/nest/CompileManager.scala
index 1b7e5cfff6..24eafaf456 100644
--- a/src/partest/scala/tools/partest/nest/CompileManager.scala
+++ b/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)
 
diff --git a/src/partest/scala/tools/partest/nest/ConsoleFileManager.scala b/src/partest/scala/tools/partest/nest/ConsoleFileManager.scala
index e2e4ba648e..b9b371d6cb 100644
--- a/src/partest/scala/tools/partest/nest/ConsoleFileManager.scala
+++ b/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
diff --git a/src/partest/scala/tools/partest/nest/FileManager.scala b/src/partest/scala/tools/partest/nest/FileManager.scala
index a39aa27f32..780f7a35e5 100644
--- a/src/partest/scala/tools/partest/nest/FileManager.scala
+++ b/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?
diff --git a/src/partest/scala/tools/partest/nest/Worker.scala b/src/partest/scala/tools/partest/nest/Worker.scala
index 4cc1c87c82..fb1546798a 100644
--- a/src/partest/scala/tools/partest/nest/Worker.scala
+++ b/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)
diff --git a/test/files/jvm/inner.scala b/test/files/jvm/inner.scala
index 156d9e69a8..009d52e5bc 100644
--- a/test/files/jvm/inner.scala
+++ b/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()
diff --git a/test/scaladoc/scala/IndexScriptTest.scala b/test/scaladoc/scala/IndexScriptTest.scala
index 991491c376..e0372020fd 100644
--- a/test/scaladoc/scala/IndexScriptTest.scala
+++ b/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 = {
diff --git a/test/scaladoc/scala/IndexTest.scala b/test/scaladoc/scala/IndexTest.scala
index 2d731643da..c14fd98297 100644
--- a/test/scaladoc/scala/IndexTest.scala
+++ b/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)
   }
   
diff --git a/test/scaladoc/scala/html/HtmlFactoryTest.scala b/test/scaladoc/scala/html/HtmlFactoryTest.scala
index 9d40996799..916fe2e42f 100644
--- a/test/scaladoc/scala/html/HtmlFactoryTest.scala
+++ b/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)
   }