From 2aeeac5f167d854203b409dd0266a4a46343e47b Mon Sep 17 00:00:00 2001 From: Adrien Piquerez Date: Sat, 28 Nov 2020 21:59:26 +0100 Subject: [PATCH] [sbt-dotty] use sbt loader as parent of scala instance loader --- project/Build.scala | 3 +- sbt-bridge/src/xsbt/CompilerClassLoader.java | 122 ------------------ sbt-bridge/src/xsbt/CompilerInterface.java | 9 +- .../dotty/tools/sbtplugin/DottyPlugin.scala | 47 ++++++- 4 files changed, 46 insertions(+), 135 deletions(-) delete mode 100644 sbt-bridge/src/xsbt/CompilerClassLoader.java diff --git a/project/Build.scala b/project/Build.scala index 9666386b2100..e8937a1309fb 100644 --- a/project/Build.scala +++ b/project/Build.scala @@ -337,7 +337,8 @@ object Build { scalaLibrary, dottyLibrary, dottyCompiler, - allJars + allJars, + appConfiguration.value ) }, // sbt-dotty defines `scalaInstance in doc` so we need to override it manually diff --git a/sbt-bridge/src/xsbt/CompilerClassLoader.java b/sbt-bridge/src/xsbt/CompilerClassLoader.java deleted file mode 100644 index 1ce573148e47..000000000000 --- a/sbt-bridge/src/xsbt/CompilerClassLoader.java +++ /dev/null @@ -1,122 +0,0 @@ -package xsbt; - -import java.lang.reflect.Field; - -import java.net.URL; -import java.net.URLClassLoader; - -import java.util.WeakHashMap; - -/** - * A classloader to run the compiler - *

- * A CompilerClassLoader is constructed from a list of `urls` that need to be on - * the classpath to run the compiler and the classloader used by sbt. - *

- * To understand why a custom classloader is needed for the compiler, let us - * describe some alternatives that wouldn't work. - *

- *

- * Our solution is to implement a subclass of URLClassLoader with no parent, instead - * we override `loadClass` to load the `xsbti.*` interfaces from `sbtLoader`. - */ -public class CompilerClassLoader extends URLClassLoader { - private final ClassLoader sbtLoader; - - public CompilerClassLoader(URL[] urls, ClassLoader sbtLoader) { - super(urls, null); - this.sbtLoader = sbtLoader; - } - - @Override - public Class loadClass(String className, boolean resolve) throws ClassNotFoundException { - if (className.startsWith("xsbti.")) { - // We can't use the loadClass overload with two arguments because it's - // protected, but we can do the same by hand (the classloader instance - // from which we call resolveClass does not matter). - Class c = sbtLoader.loadClass(className); - if (resolve) - resolveClass(c); - return c; - } else { - return super.loadClass(className, resolve); - } - } - - /** - * Cache the result of `fixBridgeLoader`. - *

- * Reusing ClassLoaders is important for warm performance since otherwise the - * JIT code cache for the compiler will be discarded between every call to - * the sbt `compile` task. - */ - private static WeakHashMap fixedLoaderCache = new WeakHashMap<>(); - - /** - * Fix the compiler bridge ClassLoader - *

- * Soundtrack: https://www.youtube.com/watch?v=imamcajBEJs - *

- * The classloader that we get from sbt looks like: - *

- * URLClassLoader(bridgeURLs, - * DualLoader(scalaLoader, notXsbtiFilter, sbtLoader, xsbtiFilter)) - *

- * DualLoader will load the `xsbti.*` interfaces using `sbtLoader` and - * everything else with `scalaLoader`. Once we have loaded the dotty Main - * class using `scalaLoader`, subsequent classes in the dotty compiler will - * also be loaded by `scalaLoader` and _not_ by the DualLoader. But the sbt - * compiler phases are part of dotty and still need access to the `xsbti.*` - * interfaces in `sbtLoader`, therefore DualLoader does not work for us - * (this issue is not present with scalac because the sbt phases are - * currently defined in the compiler bridge itself, not in scalac). - *

- * CompilerClassLoader is a replacement for DualLoader. Until we can fix - * this in sbt proper, we need to use reflection to construct our own - * fixed classloader: - *

- * URLClassLoader(bridgeURLs, - * CompilerClassLoader(scalaLoader.getURLs, sbtLoader)) - * - * @param bridgeLoader The classloader that sbt uses to load the compiler bridge - * @return A fixed classloader that works with dotty - */ - synchronized public static ClassLoader fixBridgeLoader(ClassLoader bridgeLoader) { - return fixedLoaderCache.computeIfAbsent(bridgeLoader, k -> computeFixedLoader(k)); - } - - private static ClassLoader computeFixedLoader(ClassLoader bridgeLoader) { - URLClassLoader urlBridgeLoader = (URLClassLoader) bridgeLoader; - ClassLoader dualLoader = urlBridgeLoader.getParent(); - Class dualLoaderClass = dualLoader.getClass(); - - try { - // DualLoader.parentA and DualLoader.parentB are private - Field parentAField = dualLoaderClass.getDeclaredField("parentA"); - parentAField.setAccessible(true); - Field parentBField = dualLoaderClass.getDeclaredField("parentB"); - parentBField.setAccessible(true); - URLClassLoader scalaLoader = (URLClassLoader) parentAField.get(dualLoader); - ClassLoader sbtLoader = (ClassLoader) parentBField.get(dualLoader); - - URL[] bridgeURLs = urlBridgeLoader.getURLs(); - return new URLClassLoader(bridgeURLs, - new CompilerClassLoader(scalaLoader.getURLs(), sbtLoader)); - } catch (NoSuchFieldException | IllegalAccessException e) { - throw new RuntimeException(e); - } - } -} diff --git a/sbt-bridge/src/xsbt/CompilerInterface.java b/sbt-bridge/src/xsbt/CompilerInterface.java index 10b90adbb0e3..44349b4a879b 100644 --- a/sbt-bridge/src/xsbt/CompilerInterface.java +++ b/sbt-bridge/src/xsbt/CompilerInterface.java @@ -20,16 +20,9 @@ public final class CompilerInterface { public CachedCompiler newCompiler(String[] options, Output output, Logger initialLog, Reporter initialDelegate) { - // The classloader that sbt uses to load the compiler bridge is broken - // (see CompilerClassLoader#fixBridgeLoader for details). To workaround - // this we construct our own ClassLoader and then run the following code - // with it: - // new CachedCompilerImpl(options, output) - try { ClassLoader bridgeLoader = this.getClass().getClassLoader(); - ClassLoader fixedLoader = CompilerClassLoader.fixBridgeLoader(bridgeLoader); - Class cciClass = fixedLoader.loadClass("xsbt.CachedCompilerImpl"); + Class cciClass = bridgeLoader.loadClass("xsbt.CachedCompilerImpl"); return (CachedCompiler) cciClass.getConstructors()[0].newInstance(options, output); } catch (ClassNotFoundException | InstantiationException | IllegalAccessException | InvocationTargetException e) { throw new RuntimeException(e); diff --git a/sbt-dotty/src/dotty/tools/sbtplugin/DottyPlugin.scala b/sbt-dotty/src/dotty/tools/sbtplugin/DottyPlugin.scala index ea87b5473028..c946274232a8 100644 --- a/sbt-dotty/src/dotty/tools/sbtplugin/DottyPlugin.scala +++ b/sbt-dotty/src/dotty/tools/sbtplugin/DottyPlugin.scala @@ -8,11 +8,16 @@ import sbt.librarymanagement.{ VersionNumber } import sbt.internal.inc.ScalaInstance +import sbt.internal.inc.classpath.ClassLoaderCache import xsbti.compile._ +import xsbti.AppConfiguration import java.net.URLClassLoader import java.util.Optional +import java.util.{Enumeration, Collections} +import java.net.URL import scala.util.Properties.isJavaAtLeast + object DottyPlugin extends AutoPlugin { object autoImport { val isDotty = settingKey[Boolean]("Is this project compiled with Dotty?") @@ -521,15 +526,34 @@ object DottyPlugin extends AutoPlugin { scalaLibraryJar, dottyLibraryJar, compilerJar, - allJars + allJars, + appConfiguration.value ) } // Adapted from private mkScalaInstance in sbt def makeScalaInstance( - state: State, dottyVersion: String, scalaLibrary: File, dottyLibrary: File, compiler: File, all: Seq[File] + state: State, dottyVersion: String, scalaLibrary: File, dottyLibrary: File, compiler: File, all: Seq[File], appConfiguration: AppConfiguration ): ScalaInstance = { - val libraryLoader = state.classLoaderCache(List(dottyLibrary, scalaLibrary)) + /** + * The compiler bridge must load the xsbti classes from the sbt + * classloader, and similarly the Scala repl must load the sbt provided + * jline terminal. To do so we add the `appConfiguration` loader in + * the parent hierarchy of the scala 3 instance loader. + * + * The [[TopClassLoader]] ensures that the xsbti and jline classes + * only are loaded from the sbt loader. That is necessary because + * the sbt class loader contains the Scala 2.12 library and compiler + * bridge. + */ + val topLoader = new TopClassLoader(appConfiguration.provider.loader) + + val libraryJars = Array(dottyLibrary, scalaLibrary) + val libraryLoader = state.classLoaderCache.cachedCustomClassloader( + libraryJars.toList, + () => new URLClassLoader(libraryJars.map(_.toURI.toURL), topLoader) + ) + class DottyLoader extends URLClassLoader(all.map(_.toURI.toURL).toArray, libraryLoader) val fullLoader = state.classLoaderCache.cachedCustomClassloader( @@ -540,10 +564,25 @@ object DottyPlugin extends AutoPlugin { dottyVersion, fullLoader, libraryLoader, - Array(dottyLibrary, scalaLibrary), + libraryJars, compiler, all.toArray, None) + } +} + +private class TopClassLoader(sbtLoader: ClassLoader) extends ClassLoader(null) { + private val sharedPrefixes = List( + "xsbti.", + "org.jline." + ) + override protected def loadClass(name: String, resolve: Boolean): Class[_] = { + if (sharedPrefixes.exists(name.startsWith(_))) { + val c = sbtLoader.loadClass(name) + if (resolve) resolveClass(c) + c + } + else super.loadClass(name, resolve) } }