Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Spurious "Ignoring duplicate plugin" info from scalac 2.12.9 and 2.13.0 #11666

Open
robby-phd opened this issue Aug 6, 2019 · 9 comments

Comments

@robby-phd
Copy link

commented Aug 6, 2019

In some cases, scalac 2.12.9 and 2.13.0 display spurious information that there are plugin duplicates (2.12.8 does not have the issue).

This might be due to the new Plugins.findPluginClassLoader added in the following commit:

scala/scala@cfbb2a2?diff=split#diff-27801265674d0daded4e1af60661525bR71-R98

where the ClassLoader returned by findPluginClassLoader is somehow not "isolated" for detecting plugins available through that classpath.

The following illustrates the issue:

git clone https://github.com/sireum/runtime
runtime/bin/build.cmd compile

For each mill js module, it produces 5 spurious info, but when inspecting one of the modules' plugin classpath:

cd runtime && bin/mill show runtime.test.js.scalacPluginClasspath

Then, there are 6 entries, but only one actually contains the plugin referenced in the spurious information.

@robby-phd robby-phd changed the title Spurious "Ignoring duplicate plugin" in 2.12.9 and 2.13.0 Spurious "Ignoring duplicate plugin" info from scalac 2.12.9 and 2.13.0 Aug 6, 2019

@SethTisue SethTisue added this to the 2.12.10 milestone Aug 6, 2019

@robby-phd

This comment has been minimized.

Copy link
Author

commented Aug 6, 2019

This might actually be more severe than just displaying spurious info as it might load the wrong plugin based on the resolution of PluginXML ("scalac-plugin.xml) in the returned ClassLoader.

Using the same repo, the js test suite fails in 2.12.9 while it works in 2.12.8 (perhaps because the Scala.js plugin is "shadowed"):

runtime/bin/build.cmd test-js
@SethTisue

This comment has been minimized.

Copy link
Member

commented Aug 7, 2019

@retronym looks like your wheelhouse

@robby-phd

This comment has been minimized.

Copy link
Author

commented Aug 7, 2019

Unfortunately, I don't have more cycles to spend to simplify this further. My guess is that the issue requires a situation where a plugin is somehow available through the compiler class loader (classOf[Plugin].getClassLoader), which then get prioritized regardless of what is available through the findPluginClassLoader 's classpath parameter.

At any rate, the build script should work in Linux as well; on some non-POSIX shells (e.g., fish), sh should be used to run the build script and mill:

git clone https://github.com/sireum/runtime
sh runtime/bin/build.cmd test-js
cd runtime
sh bin/mill.bat show runtime.test.js.scalacPluginClasspath

Here's is a CI run showing the issue in 2.12.9 running inside a Ubuntu 18.04 docker container (with bash as the default shell):

https://app.shippable.com/github/sireum/runtime/runs/654/1/console

and here's showing that it works in 2.12.8 using the same container:

https://app.shippable.com/github/sireum/runtime/runs/653/1/console

@retronym

This comment has been minimized.

Copy link
Member

commented Aug 8, 2019

@robby-phd I had the build script running locally once to reproduce the issue after defining SLASHDIR=$PWD/bin int he environment to avoid an NPE.

But after building a candidate fix in the compiler (https://github.com/scala/scala/pull/8322/files) I wasn't able to get lucky enough to run the build again, I'm hitting:

1 targets failed
runtime.library.js.tests.test org.scalajs.testcommon.RPCCore$RPCException: java.lang.RuntimeException: Cannot load suite class: org.sireum.GraphTest
java.lang.RuntimeException: Cannot load suite class: org.sireum.GraphTest
Error encountered when running: sh /Users/jz/code/runtime/bin/mill.bat runtime.library.js.tests, exit code: 1

I wonder if you could try out my fix instead?

You can obtain the build with:

CP=$(coursier fetch --keep-optional -q -p -r https://scala-ci.typesafe.com/artifactory/scala-pr-validation-snapshots -r https://scala-ci.typesafe.com/artifactory/scala-integration org.scala-lang:scala-compiler:2.12.10-bin-1053fda-SNAPSHOT)
java -classpath $CP  scala.tools.nsc.MainGenericRunner -usejavacp
@robby-phd

This comment has been minimized.

Copy link
Author

commented Aug 8, 2019

@retronym Thanks for trying to reduplicate the issue despite the NPE. It's interesting that SLASH_DIR didn't get propagated by the script runner somehow.

Anyway, scala/scala#8322 fixes the broken js test suites. However, it still displays some spurious info that the Scala.js plugin is duplicated multiple times (4 times instead of 5 times).

I created a PR using a different approach: scala/scala#8324, which fixes both the spurious info and the js test suite.

@DavidGregory084

This comment has been minimized.

Copy link

commented Aug 13, 2019

FWIW I am experiencing the same issue on this PR: https://github.com/DavidGregory084/inc/pull/15/files

It manifests via lots of "Either does not implement withFilter" messsages as the better-monadic-for plugin does not get applied.

It might be a little bit more self-contained - I have noticed that uncommenting the semanticdb-scalac plugin in build.sc seems to trigger the issue although it is not consistent.

[info] Compiling 11 Scala sources to /Users/davidgregory/Repos/inc/out/proto/compile/dest/classes ...
[info] Ignoring duplicate plugin bm4 (com.olegpy.bm4.BetterMonadicFor)
[info] Ignoring duplicate plugin bm4 (com.olegpy.bm4.BetterMonadicFor)
[info] Ignoring duplicate plugin bm4 (com.olegpy.bm4.BetterMonadicFor)
[info] Ignoring duplicate plugin bm4 (com.olegpy.bm4.BetterMonadicFor)
@retronym

This comment has been minimized.

Copy link
Member

commented Aug 15, 2019

@DavidGregory084 Thanks for that.

I tried to run your build against the fixed version:

diff --git a/build.sc b/build.sc
index aa13a25..b0c03cd 100644
--- a/build.sc
+++ b/build.sc
@@ -38,13 +38,20 @@ trait PublishSettingsModule extends PublishModule {
 }

 trait ScalaSettingsModule extends TpolecatModule with PublishSettingsModule {
-  def scalaVersion = "2.12.8"
+  def scalaVersion = "2.12.10-bin-f552221-SNAPSHOT"

   def scalacPluginIvyDeps = Agg(
     ivy"com.olegpy::better-monadic-for:0.3.1",
-    ivy"org.scalameta:semanticdb-scalac_${scalaVersion()}:4.2.0"
+    ivy"org.scalameta:semanticdb-scalac_2.12.8:4.2.0"
   )

+  import coursier.maven.MavenRepository
+
+  def repositories = super.repositories ++ Seq(
+    MavenRepository("https://scala-ci.typesafe.com/artifactory/scala-pr-validation-snapshots/")
+  )
+
+
   def scalacOptions = T {
     super.scalacOptions() ++ Seq(
       "-Ypartial-unification",
@@ -71,7 +78,7 @@ trait ScalaSettingsModule extends TpolecatModule with PublishSettingsModule {

 object decompiled extends JavaModule

-object proto extends ScalaPBModule with ScalaSettingsModule {
+object proto extends ScalaSettingsModule {
   def scalaPBVersion = "0.9.0"
   def scalaPBFlatPackage = true
   def scalaPBGrpc = false
(END)

But ran into problems when the build failed to resolve compiler plugins suffixed with _2.12.10-bin-f552221-SNAPSHOT. I'm not familiar with Mill so I don't know now to tell it to just 2.12.8 as the compiler plugin version suffix.

@DavidGregory084

This comment has been minimized.

Copy link

commented Aug 15, 2019

Ah awesome, thanks @retronym! I'll give this a try and let you know how it goes.

FWIW the syntax in Mill is just like sbt, I just need to drop the second : in the dependency coordinate to supply the version suffix manually.

@DavidGregory084

This comment has been minimized.

Copy link

commented Aug 21, 2019

Sorry @retronym I ran into a wall trying to get all the various bits of mill to recognise the major+minor version of that PR build - I will get back to this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.