Skip to content

Commit

Permalink
include only scala-library into the boot classpath during run
Browse files Browse the repository at this point in the history
Fixes sbt/sbt#3405
Ref scala/scala-xml#195

sbt's `run` is emulated using a classloader trick that includes ScalaInstance as the parent classloader under the classpath. The problem is the ScalaInstance classloader currently contains both compiler, library, and their transitive JARs:

```scala
res0: Array[java.io.File] = Array(scala-library.jar, scala-compiler.jar, jline.jar, scala-reflect.jar, scala-xml_2.12.jar)
```

This could have been causing various issues, but most recently it showed up as wrong version of scala-xml getting prioritized over what's passed by the user.

1. new field loaderLibraryOnly is added to xsbti.ScalaInstance.
2. it is initialized to the library loader if the launcher creates it, otherwise create layered loader here.

This aims to isolate the library loader, and retain the perf.
  • Loading branch information
eed3si9n committed Mar 19, 2018
1 parent ef5b5fc commit 9397b6a
Show file tree
Hide file tree
Showing 5 changed files with 86 additions and 23 deletions.
1 change: 1 addition & 0 deletions build.sbt
Original file line number Diff line number Diff line change
Expand Up @@ -337,6 +337,7 @@ lazy val compilerInterface = (project in internalPath / "compiler-interface")
import com.typesafe.tools.mima.core.ProblemFilters._
Seq(
exclude[ReversedMissingMethodProblem]("xsbti.compile.ExternalHooks#Lookup.hashClasspath"),
exclude[ReversedMissingMethodProblem]("xsbti.compile.ScalaInstance.loaderLibraryOnly"),
)
},
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,13 @@ public interface ScalaInstance {
/** A class loader providing access to the classes and resources in the library and compiler jars. */
ClassLoader loader();

/** @deprecated Only `jars` can be reliably provided for modularized Scala (since 0.13.0). */
@Deprecated
/** A class loader providing access to the classes and resources in the library. */
ClassLoader loaderLibraryOnly();

/** Only `jars` can be reliably provided for modularized Scala. */
File libraryJar();

/** @deprecated Only `jars` can be reliably provided for modularized Scala (since 0.13.0). */
@Deprecated
/** Only `jars` can be reliably provided for modularized Scala. */
File compilerJar();

/** @deprecated Only `jars` can be reliably provided for modularized Scala (since 0.13.0). */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ package inc
import java.io.File
import xsbti.ArtifactInfo.ScalaOrganization
import sbt.io.IO
import scala.language.reflectiveCalls
import sbt.internal.inc.classpath.ClasspathUtilities

/**
* A Scala instance encapsulates all the information that is bound to a concrete
Expand All @@ -35,12 +37,33 @@ import sbt.io.IO
final class ScalaInstance(
val version: String,
val loader: ClassLoader,
val loaderLibraryOnly: ClassLoader,
val libraryJar: File,
val compilerJar: File,
val allJars: Array[File],
val explicitActual: Option[String]
) extends xsbti.compile.ScalaInstance {

@deprecated("Use constructor with loaderLibraryOnly", "1.1.2")
def this(
version: String,
loader: ClassLoader,
libraryJar: File,
compilerJar: File,
allJars: Array[File],
explicitActual: Option[String]
) {
this(
version,
loader,
ClasspathUtilities.rootLoader,
libraryJar,
compilerJar,
allJars,
explicitActual
)
}

/**
* Check whether `scalaInstance` comes from a managed (i.e. ivy-resolved)
* scala **or** if it's a free-floating `ScalaInstance`, in which case we
Expand Down Expand Up @@ -75,7 +98,13 @@ final class ScalaInstance(
override def toString: String =
s"Scala instance { version label $version, actual version $actualVersion, $jarStrings }"
}

object ScalaInstance {
/*
* Structural extention for the ScalaProvider post 1.0.3 launcher.
* See https://github.com/sbt/zinc/pull/505.
*/
private type ScalaProvider2 = { def loaderLibraryOnly: ClassLoader }

/** Name of scala organisation to be used for artifact resolution. */
val ScalaOrg = ScalaOrganization
Expand Down Expand Up @@ -123,30 +152,47 @@ object ScalaInstance {
}
}
val jars = provider.jars
val loader = provider.loader
val libraryJar = findOrCrash(jars, "scala-library.jar")
val compilerJar = findOrCrash(jars, "scala-compiler.jar")
new ScalaInstance(version, loader, libraryJar, compilerJar, jars, None)
def fallbackClassLoaders = {
val l = ClasspathUtilities.toLoader(Vector(libraryJar))
val c = scalaLoader(l)(jars.toVector filterNot { _ == libraryJar })
(c, l)
}
// sbt launcher 1.0.3 will construct layered classloader. Use them if we find them.
// otherwise, construct layered loaders manually.
val (loader, loaderLibraryOnly) = {
(try {
provider match {
case p: ScalaProvider2 @unchecked => Option((provider.loader, p.loaderLibraryOnly))
}
} catch {
case _: NoSuchMethodError => None
}) getOrElse fallbackClassLoaders
}
new ScalaInstance(version, loader, loaderLibraryOnly, libraryJar, compilerJar, jars, None)
}

def apply(scalaHome: File, launcher: xsbti.Launcher): ScalaInstance =
apply(scalaHome)(scalaLoader(launcher))
apply(scalaHome)(scalaLibraryLoader(launcher))

def apply(scalaHome: File)(classLoader: List[File] => ClassLoader): ScalaInstance = {
val all = allJars(scalaHome)
val loader = classLoader(all.toList)
val library = libraryJar(scalaHome)
val loaderLibraryOnly = classLoader(List(library))
val loader = scalaLoader(loaderLibraryOnly)(all.toVector filterNot { _ == library })
val version = actualVersion(loader)(" (library jar " + library.getAbsolutePath + ")")
val compiler = compilerJar(scalaHome)
new ScalaInstance(version, loader, library, compiler, all.toArray, None)
new ScalaInstance(version, loader, loaderLibraryOnly, library, compiler, all.toArray, None)
}

def apply(version: String, scalaHome: File, launcher: xsbti.Launcher): ScalaInstance = {
val all = allJars(scalaHome)
val loader = scalaLoader(launcher)(all.toList)
val library = libraryJar(scalaHome)
val loaderLibraryOnly = scalaLibraryLoader(launcher)(List(library))
val loader = scalaLoader(loaderLibraryOnly)(all.toVector)
val compiler = compilerJar(scalaHome)
new ScalaInstance(version, loader, library, compiler, all.toArray, None)
new ScalaInstance(version, loader, loaderLibraryOnly, library, compiler, all.toArray, None)
}

/** Return all the required Scala jars from a path `scalaHome`. */
Expand Down Expand Up @@ -209,12 +255,12 @@ object ScalaInstance {
} finally stream.close()
}

private def scalaLoader(launcher: xsbti.Launcher): Seq[File] => ClassLoader = { jars =>
import java.net.{ URL, URLClassLoader }
new URLClassLoader(
jars.map(_.toURI.toURL).toArray[URL],
launcher.topLoader
)
private def scalaLibraryLoader(launcher: xsbti.Launcher): Seq[File] => ClassLoader = { jars =>
ClasspathUtilities.toLoader(jars, launcher.topLoader)
}

private def scalaLoader(parent: ClassLoader): Seq[File] => ClassLoader = { jars =>
ClasspathUtilities.toLoader(jars, parent)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,10 @@ object ClasspathUtilities {
final val AppClassPath = "app.class.path"
final val BootClassPath = "boot.class.path"

def createClasspathResources(classpath: Seq[File], instance: ScalaInstance): Map[String, String] =
createClasspathResources(classpath, instance.allJars)
def createClasspathResources(classpath: Seq[File],
instance: ScalaInstance): Map[String, String] = {
createClasspathResources(classpath, Array(instance.libraryJar))
}

def createClasspathResources(appPaths: Seq[File], bootPaths: Seq[File]): Map[String, String] = {
def make(name: String, paths: Seq[File]) = name -> Path.makeString(paths)
Expand All @@ -74,11 +76,16 @@ object ClasspathUtilities {
private[sbt] def filterByClasspath(classpath: Seq[File], loader: ClassLoader): ClassLoader =
new ClasspathFilter(loader, xsbtiLoader, classpath.toSet)

/**
* Creates a ClassLoader that contains the classpath and the scala-library from
* the given instance.
*/
def makeLoader(classpath: Seq[File], instance: ScalaInstance): ClassLoader =
filterByClasspath(classpath, makeLoader(classpath, instance.loader, instance))
filterByClasspath(classpath, makeLoader(classpath, instance.loaderLibraryOnly, instance))

def makeLoader(classpath: Seq[File], instance: ScalaInstance, nativeTemp: File): ClassLoader =
filterByClasspath(classpath, makeLoader(classpath, instance.loader, instance, nativeTemp))
filterByClasspath(classpath,
makeLoader(classpath, instance.loaderLibraryOnly, instance, nativeTemp))

def makeLoader(classpath: Seq[File], parent: ClassLoader, instance: ScalaInstance): ClassLoader =
toLoader(classpath, parent, createClasspathResources(classpath, instance))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,11 +140,19 @@ private[sbt] object ZincComponentCompiler {
val scalaLibrary = scalaArtifacts.library
val jarsToLoad = (scalaCompiler +: scalaLibrary +: scalaArtifacts.others).toArray
assert(jarsToLoad.forall(_.exists), "One or more jar(s) in the Scala instance do not exist.")
val loader = new URLClassLoader(toURLs(jarsToLoad), ClasspathUtilities.rootLoader)
val loaderLibraryOnly = ClasspathUtilities.toLoader(Vector(scalaLibrary))
val loader = ClasspathUtilities.toLoader(jarsToLoad.toVector filterNot { _ == scalaLibrary },
loaderLibraryOnly)
val properties = ResourceLoader.getSafePropertiesFor("compiler.properties", loader)
val loaderVersion = Option(properties.getProperty("version.number"))
val scalaV = loaderVersion.getOrElse("unknown")
new ScalaInstance(scalaV, loader, scalaLibrary, scalaCompiler, jarsToLoad, loaderVersion)
new ScalaInstance(scalaV,
loader,
loaderLibraryOnly,
scalaLibrary,
scalaCompiler,
jarsToLoad,
loaderVersion)
}
}

Expand Down

0 comments on commit 9397b6a

Please sign in to comment.