SI-4841 Plugins get a class path#3169
Conversation
|
There is some cognitive dissonance in the plugin feature, in that a missing plugin is a warning unless you specify -Xplugin-require. While the "extension directory" mechanism -Xpluginsdir does suggest I might be compiling with some unknown set of installed plugins (perhaps mandated by the team, for instance), if I ask for a plugin to run with -Xplugin, probably I really mean that I don't want to compile without it. |
|
|
|
our self-deprecating sense of humo(u)r -- part of the maven task that goes looking for an artifact. "Build failure" just means "artifact not found, kthx!" |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Rebased around the removal of ...and a partridge in a |
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 ```
|
Updated not to bail on |
There was a problem hiding this comment.
Still throws on error instead of reporting.
|
Review by @adriaanm perhaps, who touched plugins code once has already commented in an aside. Or, he is also good at delegating. |
|
I see we were just bumped to critical. Possibly, any issues under Si-5000 are grandfathered into criticality. But Adriaan may be too busy delegating to delegate the review. Plus I think I noticed he has an upcoming meetup. |
|
I know @retronym enjoys a good test. He has seen everything under the sun, no doubt; but this is more like strolling into a carnival to watch an amateur, not even a magician or high wire act, but some guy who bends over backward to walk on his hands. That's kind of entertaining, right? |
|
Criticality was my JIRA bulk edit device. Trying to machete my way through without turning into the hulk editor. |
There was a problem hiding this comment.
PluginLoadException should wrap the underlying exception as its cause.
There was a problem hiding this comment.
I was responsible for the previous iteration, too. I don't remember before that (fail silently?) but I do remember wondering if people want to see a stack trace about a missing class, that says "ClassNotFound". By people, I mean the guy who takes up support for continuations plugin and can't get the class path right in sbt, or whatever. In Plugins, it recover[Any]s and just uses the message, which would include the causative message IIUC.
|
Highly entraining feats of contortionism. My only comment was really about old code, so: |
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.
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, this would
have become a requirement by now.
Partest can't do multiple plugins yet, but this is what it looks like:
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:
Disabling a plugin results in a message instead of silent suppression:
The duplicate plugin class test must still be honored:
If the path is bad, then missing classes will report which plugin induced
the error:
fixes scala/bug#4841