Permalink
Browse files

SI-4841 Plugins get a class path

Let -Xplugin specify a class path (or multiple of them).

Each entry can be a jar or dir, and the first entry to supply
a plugin descriptor defines the plugin to load.

If no plugin is found on the path, then issue a warning if `-Xdev`.
This honors the legacy silent treatment (which scala-ide tests for).

In the proposed scheme, each plugin gets a class loader so that
plugins are isolated from each other.

Presumably, if compiler plugins were a rich ecosystem, in which
shared dependencies were required in incompatible versions, this
would have become a requirement by now.

(Updated with a `DirectTest` that uses two plugins, but keeping
the following as documentation.)

Partest can't do multiple plugins yet, but this is what it looks like:

```
skalac -Xplugin:sample.jar:useful.jar:util,needful.jar:another.jar:util,needful.jar:util:exploded -Xplugin-require:sample,another,other foo.scala

skalac -Xplugin:sample.jar:useful.jar:util,needful.jar:another.jar:util,needful.jar:util:exploded -Xplugin-require:sample,other -Xplugin-disable:another foo.scala

skalac -Xplugin:sample.jar:useful.jar:util,sample.jar:useful.jar:util -Xplugin-require:sample foo.scala
```
The manual test shows three plugins with various permutations of jars and dirs.

The manual test demonstrates that plugins only see classes on their class path:

```
Initializing test.plugins.SamplePlugin
needful.Needful? Failure(java.lang.ClassNotFoundException: needful.Needful)
useful.Useful? Success(class useful.Useful)
Initializing more.plugins.AnotherPlugin
needful.Needful? Success(class needful.Needful)
useful.Useful? Failure(java.lang.ClassNotFoundException: useful.Useful)
Initializing other.plugins.OtherPlugin
```

Disabling a plugin results in a message instead of silent suppression:

```
Disabling plugin another
```

The duplicate plugin class test must still be honored:

```
Ignoring duplicate plugin sample (test.plugins.SamplePlugin)
Initializing test.plugins.SamplePlugin
needful.Needful? Failure(java.lang.ClassNotFoundException: needful.Needful)
useful.Useful? Success(class useful.Useful)
```

If the path is bad, then missing classes will report which plugin induced
the error:

```
Error: class not found: util/Probe required by test.plugins.SamplePlugin
Error: class not found: util/Probe required by more.plugins.AnotherPlugin
Initializing other.plugins.OtherPlugin
needful.Needful? Success(class needful.Needful)
useful.Useful? Failure(java.lang.ClassNotFoundException: useful.Useful)
error: Missing required plugin: sample
error: Missing required plugin: another
two errors found
```
  • Loading branch information...
1 parent 7d28c49 commit 1d30ea86690db1b3b074190094d1f62d12e4efe1 @som-snytt som-snytt committed Nov 20, 2013
@@ -12,7 +12,8 @@ import scala.reflect.io.{ Directory, File, Path }
import java.io.InputStream
import java.util.zip.ZipException
-import scala.collection.mutable.ListBuffer
+import scala.collection.mutable
+import mutable.ListBuffer
import scala.util.{ Try, Success, Failure }
/** Information about a plugin loaded from a jar file.
@@ -99,7 +100,7 @@ object Plugin {
private def loadDescriptionFromJar(jarp: Path): Try[PluginDescription] = {
// XXX Return to this once we have more ARM support
def read(is: Option[InputStream]) = is match {
- case None => throw new RuntimeException(s"Missing $PluginXML in $jarp")
+ case None => throw new PluginLoadException(jarp.path, s"Missing $PluginXML in $jarp")
case Some(is) => PluginDescription.fromXML(is)
}
Try(new Jar(jarp.jfile).withEntryStream(PluginXML)(read))
@@ -113,11 +114,14 @@ object Plugin {
/** Use a class loader to load the plugin class.
*/
def load(classname: String, loader: ClassLoader): Try[AnyClass] = {
- Try[AnyClass] {
- loader loadClass classname
- } recoverWith {
- case _: Exception =>
- Failure(new RuntimeException(s"Warning: class not found: ${classname}"))
+ import scala.util.control.NonFatal
+ try {
+ Success[AnyClass](loader loadClass classname)
+ } catch {
+ case NonFatal(e) =>
+ Failure(new PluginLoadException(classname, s"Error: unable to load class: $classname"))
+ case e: NoClassDefFoundError =>
+ Failure(new PluginLoadException(classname, s"Error: class not found: ${e.getMessage} required by $classname"))
}
}
@@ -128,33 +132,54 @@ object Plugin {
* A single classloader is created and used to load all of them.
*/
def loadAllFrom(
- jars: List[Path],
+ paths: List[List[Path]],
dirs: List[Path],
ignoring: List[String]): List[Try[AnyClass]] =
{
- // List[(jar, Success(descriptor))] in dir
- def scan(d: Directory) = for {
- f <- d.files.toList sortBy (_.name)
- if Jar isJarOrZip f
- pd = loadDescriptionFromJar(f)
- if pd.isSuccess
- } yield (f, pd)
- // (dir, Try(descriptor))
- def explode(d: Directory) = d -> loadDescriptionFromFile(d / PluginXML)
- // (j, Try(descriptor))
- def required(j: Path) = j -> loadDescriptionFromJar(j)
-
- type Paired = Tuple2[Path, Try[PluginDescription]]
- val included: List[Paired] = (dirs flatMap (_ ifDirectory scan)).flatten
- val exploded: List[Paired] = jars flatMap (_ ifDirectory explode)
- val explicit: List[Paired] = jars flatMap (_ ifFile required)
- def ignored(p: Paired) = p match {
- case (path, Success(pd)) => ignoring contains pd.name
- case _ => false
+ // List[(jar, Try(descriptor))] in dir
+ def scan(d: Directory) =
+ d.files.toList sortBy (_.name) filter (Jar isJarOrZip _) map (j => (j, loadDescriptionFromJar(j)))
+
+ type PDResults = List[Try[(PluginDescription, ScalaClassLoader)]]
+
+ // scan plugin dirs for jars containing plugins, ignoring dirs with none and other jars
+ val fromDirs: PDResults = dirs filter (_.isDirectory) flatMap { d =>
+ scan(d.toDirectory) collect {
+ case (j, Success(pd)) => Success((pd, loaderFor(Seq(j))))
+ }
+ }
+
+ // scan jar paths for plugins, taking the first plugin you find.
+ // a path element can be either a plugin.jar or an exploded dir.
+ def findDescriptor(ps: List[Path]) = {
+ def loop(qs: List[Path]): Try[PluginDescription] = qs match {
+ case Nil => Failure(new MissingPluginException(ps))
+ case p :: rest =>
+ if (p.isDirectory) loadDescriptionFromFile(p.toDirectory / PluginXML)
+ else if (p.isFile) loadDescriptionFromJar(p.toFile)
+ else loop(rest)
+ }
+ loop(ps)
+ }
+ val fromPaths: PDResults = paths map (p => (p, findDescriptor(p))) map {
+ case (p, Success(pd)) => Success((pd, loaderFor(p)))
+ case (_, Failure(e)) => Failure(e)
}
- val (locs, pds) = ((explicit ::: exploded ::: included) filterNot ignored).unzip
- val loader = loaderFor(locs.distinct)
- (pds filter (_.isSuccess) map (_.get.classname)).distinct map (Plugin load (_, loader))
+
+ val seen = mutable.HashSet[String]()
+ val enabled = (fromPaths ::: fromDirs) map {
+ case Success((pd, loader)) if seen(pd.classname) =>
+ // a nod to SI-7494, take the plugin classes distinctly
+ Failure(new PluginLoadException(pd.name, s"Ignoring duplicate plugin ${pd.name} (${pd.classname})"))
+ case Success((pd, loader)) if ignoring contains pd.name =>
+ Failure(new PluginLoadException(pd.name, s"Disabling plugin ${pd.name}"))
+ case Success((pd, loader)) =>
+ seen += pd.classname
+ Plugin.load(pd.classname, loader)
+ case Failure(e) =>
+ Failure(e)
+ }
+ enabled // distinct and not disabled
}
/** Instantiate a plugin class, given the class and
@@ -164,3 +189,11 @@ object Plugin {
(clazz getConstructor classOf[Global] newInstance global).asInstanceOf[Plugin]
}
}
+
+class PluginLoadException(val path: String, message: String, cause: Exception) extends Exception(message, cause) {
+ def this(path: String, message: String) = this(path, message, null)
+}
+
+class MissingPluginException(path: String) extends PluginLoadException(path, s"No plugin in path $path") {
+ def this(paths: List[Path]) = this(paths mkString File.pathSeparator)
+}
@@ -1,15 +0,0 @@
-/* NSC -- new Scala compiler
- * Copyright 2007-2013 LAMP/EPFL
- * @author Lex Spoon
- */
-
-package scala.tools.nsc
-package plugins
-
-/** ...
- *
- * @author Lex Spoon
- * @version 1.0, 2007-5-21
- */
-class PluginLoadException(filename: String, cause: Exception)
-extends Exception(cause)
@@ -8,6 +8,7 @@ package scala.tools.nsc
package plugins
import scala.reflect.io.{ File, Path }
+import scala.tools.nsc.util.ClassPath
import scala.tools.util.PathResolver.Defaults
/** Support for run-time loading of compiler plugins.
@@ -16,22 +17,28 @@ import scala.tools.util.PathResolver.Defaults
* @version 1.1, 2009/1/2
* Updated 2009/1/2 by Anders Bach Nielsen: Added features to implement SIP 00002
*/
-trait Plugins {
- self: Global =>
+trait Plugins { global: Global =>
/** Load a rough list of the plugins. For speed, it
* does not instantiate a compiler run. Therefore it cannot
* test for same-named phases or other problems that are
* filtered from the final list of plugins.
*/
protected def loadRoughPluginsList(): List[Plugin] = {
- val jars = settings.plugin.value map Path.apply
- def injectDefault(s: String) = if (s.isEmpty) Defaults.scalaPluginPath else s
- val dirs = (settings.pluginsDir.value split File.pathSeparator).toList map injectDefault map Path.apply
- val maybes = Plugin.loadAllFrom(jars, dirs, settings.disable.value)
+ def asPath(p: String) = ClassPath split p
+ val paths = settings.plugin.value filter (_ != "") map (s => asPath(s) map Path.apply)
+ val dirs = {
+ def injectDefault(s: String) = if (s.isEmpty) Defaults.scalaPluginPath else s
+ asPath(settings.pluginsDir.value) map injectDefault map Path.apply
+ }
+ val maybes = Plugin.loadAllFrom(paths, dirs, settings.disable.value)
val (goods, errors) = maybes partition (_.isSuccess)
// Explicit parameterization of recover to suppress -Xlint warning about inferred Any
- errors foreach (_.recover[Any] { case e: Exception => inform(e.getMessage) })
+ errors foreach (_.recover[Any] {
+ // legacy behavior ignores altogether, so at least warn devs
+ case e: MissingPluginException => if (global.isDeveloper) warning(e.getMessage)
+ case e: Exception => inform(e.getMessage)
+ })
val classes = goods map (_.get) // flatten
// Each plugin must only be instantiated once. A common pattern
@@ -1,4 +1,4 @@
-Warning: class not found: t6446.Ploogin
+Error: unable to load class: t6446.Ploogin
phase name id description
---------- -- -----------
parser 1 parse source into ASTs, perform simple desugaring
@@ -0,0 +1,2 @@
+My phase name is ploogin1_1
+My phase name is ploogin1_2
@@ -0,0 +1,30 @@
+
+package t4841
+
+import scala.tools.nsc.{ Global, Phase }
+import scala.tools.nsc.plugins.{ Plugin, PluginComponent }
+import scala.reflect.io.Path
+import scala.reflect.io.File
+
+/** A test plugin. */
+class Ploogin(val global: Global, val name: String = "ploogin") extends Plugin {
+ import global._
+
+ val description = "A sample plugin for testing."
+ val components = List[PluginComponent](TestComponent)
+
+ private object TestComponent extends PluginComponent {
+ val global: Ploogin.this.global.type = Ploogin.this.global
+ //override val runsBefore = List("refchecks")
+ val runsAfter = List("jvm")
+ val phaseName = Ploogin.this.name
+ override def description = "A sample phase that does so many things it's kind of hard to describe briefly."
+ def newPhase(prev: Phase) = new TestPhase(prev)
+ class TestPhase(prev: Phase) extends StdPhase(prev) {
+ override def description = TestComponent.this.description
+ def apply(unit: CompilationUnit) {
+ if (settings.developer) inform(s"My phase name is $phaseName")
+ }
+ }
+ }
+}
@@ -0,0 +1,39 @@
+
+import tools.nsc.plugins.PluginDescription
+import tools.partest.DirectTest
+
+import java.io.File
+
+// show that plugins are on isolated class loaders
+object Test extends DirectTest {
+ override def code = "class Code"
+
+ override def extraSettings = s"-usejavacp"
+
+ // plugin named ploogin1_1 or ploogin1_2, but not ploogin2_x
+ // Although the samples are in different classloaders, the plugin
+ // loader checks for distinctness by class name, so the names must differ.
+ def pluginCode(index: Int) = s"""
+ |package t4841 {
+ | class SamplePloogin$index(global: scala.tools.nsc.Global) extends Ploogin(global, s"$${PlooginCounter.named}_$index")
+ | object PlooginCounter {
+ | val count = new java.util.concurrent.atomic.AtomicInteger
+ | def named = s"ploogin$${count.incrementAndGet}"
+ | }
+ |}""".stripMargin.trim
+
+ def compilePlugin(i: Int) = {
+ val out = (testOutput / s"p$i").createDirectory()
+ val args = Seq("-usejavacp", "-d", out.path)
+ compileString(newCompiler(args: _*))(pluginCode(i))
+ val xml = PluginDescription(s"p$i", s"t4841.SamplePloogin$i").toXML
+ (out / "scalac-plugin.xml").toFile writeAll xml
+ out
+ }
+
+ override def show() = {
+ val dirs = 1 to 2 map (compilePlugin(_))
+ compile("-Xdev", s"-Xplugin:${dirs mkString ","}", "-usejavacp", "-d", testOutput.path)
+ }
+}
+
@@ -0,0 +1 @@
+warning: No plugin in path t4841-no-plugin-run.obj/plugins.partest
@@ -0,0 +1,17 @@
+
+import tools.partest.DirectTest
+
+import java.io.File
+
+// warn only if no plugin on Xplugin path
+object Test extends DirectTest {
+ override def code = "class Code"
+
+ override def extraSettings = s"-usejavacp -d ${testOutput.path}"
+
+ override def show() = {
+ val tmp = new File(testOutput.jfile, "plugins.partest").getAbsolutePath
+ compile("-Xdev", s"-Xplugin:$tmp", "-Xpluginsdir", tmp)
+ }
+}
+

0 comments on commit 1d30ea8

Please sign in to comment.