From 7b1b33758cc4f67273b569516e422ef845a423df Mon Sep 17 00:00:00 2001 From: Mark Harrah Date: Wed, 11 Sep 2013 18:22:13 -0400 Subject: [PATCH] Convert -classpath to CLASSPATH when forking on Windows and length exceeds a heuristic maximum. Fixes #755. --- project/Sbt.scala | 2 +- run/src/main/scala/sbt/Fork.scala | 31 ++++++++++++++- run/src/test/scala/sbt/ForkTest.scala | 55 +++++++++++++++++++++++++++ 3 files changed, 86 insertions(+), 2 deletions(-) create mode 100644 run/src/test/scala/sbt/ForkTest.scala diff --git a/project/Sbt.scala b/project/Sbt.scala index f4a466902..2c60bd1c0 100644 --- a/project/Sbt.scala +++ b/project/Sbt.scala @@ -92,7 +92,7 @@ object Sbt extends Build // Builds on cache to provide caching for filesystem-related operations lazy val trackingSub = baseProject(cachePath / "tracking", "Tracking") dependsOn(cacheSub, ioSub) // Embedded Scala code runner - lazy val runSub = baseProject(file("run"), "Run") dependsOn(ioSub, logSub, classpathSub, processSub) + lazy val runSub = testedBaseProject(file("run"), "Run") dependsOn(ioSub, logSub % "compile;test->test", classpathSub, processSub % "compile;test->test") // Compiler-side interface to compiler that is compiled against the compiler being used either in advance or on the fly. // Includes API and Analyzer phases that extract source API and relationships. diff --git a/run/src/main/scala/sbt/Fork.scala b/run/src/main/scala/sbt/Fork.scala index a325ba2c2..b7e4c8354 100644 --- a/run/src/main/scala/sbt/Fork.scala +++ b/run/src/main/scala/sbt/Fork.scala @@ -81,13 +81,17 @@ sealed class Fork(val commandName: String, val runnerClass: Option[String]) { import config.{envVars => env, _} val executable = Fork.javaCommand(javaHome, commandName).getAbsolutePath - val options = makeOptions(runJVMOptions, bootJars, arguments) + val preOptions = makeOptions(runJVMOptions, bootJars, arguments) + val (classpathEnv, options) = Fork.fitClasspath(preOptions) val command = (executable +: options).toArray val builder = new JProcessBuilder(command : _*) workingDirectory.foreach(wd => builder.directory(wd)) val environment = builder.environment for( (key, value) <- env ) environment.put(key, value) + for( cpenv <- classpathEnv ) + // overriding, not appending, is correct due to the specified priorities of -classpath and CLASSPATH + environment.put(Fork.ClasspathEnvKey, cpenv) outputStrategy.getOrElse(StdoutOutput) match { case StdoutOutput => Process(builder).run(connectInput) case BufferedOutput(logger) => Process(builder).runBuffered(logger, connectInput) @@ -112,6 +116,31 @@ object Fork val scala = new Fork(JavaCommandName, Some(ScalaMainClass)) val scalac = new Fork(JavaCommandName, Some(ScalacMainClass)) + private val ClasspathEnvKey = "CLASSPATH" + private[this] val ClasspathOptionLong = "-classpath" + private[this] val ClasspathOptionShort = "-cp" + private[this] def isClasspathOption(s: String) = s == ClasspathOptionLong || s == ClasspathOptionShort + /** Maximum length of classpath string before passing the classpath in an environment variable instead of an option. */ + private[this] val MaxConcatenatedOptionLength = 5000 + + private def fitClasspath(options: Seq[String]): (Option[String], Seq[String]) = + if(isWindows && optionsTooLong(options)) + convertClasspathToEnv(options) + else + (None, options) + private[this] def optionsTooLong(options: Seq[String]): Boolean = + options.mkString(" ").length > MaxConcatenatedOptionLength + + private[this] val isWindows: Boolean = System.getProperty("os.name").toLowerCase.contains("windows") + private[this] def convertClasspathToEnv(options: Seq[String]): (Option[String], Seq[String]) = + { + val (preCP, cpAndPost) = options.span(opt => !isClasspathOption(opt)) + val postCP = cpAndPost.drop(2) + val classpathOption = cpAndPost.drop(1).headOption + val newOptions = if(classpathOption.isDefined) preCP ++ postCP else options + (classpathOption, newOptions) + } + private def javaCommand(javaHome: Option[File], name: String): File = { val home = javaHome.getOrElse(new File(System.getProperty("java.home"))) diff --git a/run/src/test/scala/sbt/ForkTest.scala b/run/src/test/scala/sbt/ForkTest.scala new file mode 100644 index 000000000..53c6f5666 --- /dev/null +++ b/run/src/test/scala/sbt/ForkTest.scala @@ -0,0 +1,55 @@ +package sbt + +import org.scalacheck._ +import Prop.{Exception => _, _} +import Gen.{alphaNumChar,frequency,listOf1,oneOf} +import java.io.File + +object ForkTest extends Properties("Fork") +{ + /** Heuristic for limiting the length of the classpath string. + * Longer than this will hit hard limits in the total space + * allowed for process initialization, which includes environment variables, at least on linux. */ + final val MaximumClasspathLength = 100000 + + lazy val genOptionName = frequency( ( 9, Some("-cp")), (9, Some("-classpath")), (1, None)) + lazy val pathElement = listOf1(alphaNumChar).map(_.mkString) + lazy val path = listOf1(pathElement).map(_.mkString(File.separator)) + lazy val genRelClasspath = listOf1(path) + + lazy val requiredEntries = + IO.classLocationFile[scala.Option[_]] :: + IO.classLocationFile[sbt.exit.type] :: + Nil + lazy val mainAndArgs = + "sbt.exit" :: + "0" :: + Nil + + property("Arbitrary length classpath successfully passed.") = forAllNoShrink(genOptionName, genRelClasspath) { (optionName: Option[String], relCP: List[String]) => + IO.withTemporaryDirectory { dir => TestLogger { log => + val withScala = requiredEntries ::: relCP.map(rel => new File(dir, rel)) + val absClasspath = trimClasspath(Path.makeString(withScala)) + val args = optionName.map(_ :: absClasspath :: Nil).toList.flatten ++ mainAndArgs + val config = ForkOptions(outputStrategy = Some(LoggedOutput(log))) + val exitCode = try Fork.java(config, args) catch { case e: Exception => e.printStackTrace; 1 } + val expectedCode = if(optionName.isEmpty) 1 else 0 + s"temporary directory: ${dir.getAbsolutePath}" |: + s"required classpath: ${requiredEntries.mkString("\n\t", "\n\t", "")}" |: + s"main and args: ${mainAndArgs.mkString(" ")}" |: + s"args length: ${args.mkString(" ").length}" |: + s"exitCode: $exitCode, expected: $expectedCode" |: + (exitCode == expectedCode) + }} + } + + private[this] def trimClasspath(cp: String): String = + if(cp.length > MaximumClasspathLength) { + val lastEntryI = cp.lastIndexOf(File.pathSeparatorChar, MaximumClasspathLength) + if(lastEntryI > 0) + cp.substring(0, lastEntryI) + else + cp + } else + cp +} \ No newline at end of file