[sbt-dotty] Use sbt loader as parent of scala instance loader

[adpi2]

This PR is an attempt to get rid of the CompilerClassLoader hack in sbt-bridge (see origin of the problem). The underlying purpose being to implement the new Zinc CompilerInterface2 so that we can fix a few bugs in sbt-dotty and benefit from the recent Zinc improvement in Scala 3.

In short the problem is that the ScalaInstance.loader is incompatible with the sbt loader because it loads its own xsbti.* classes.

This PR fix the problem by using the sbt ClassLoader as a parent of the ScalaInstance.loader. But we filter out all the classes that are not binary compatible with the ScalaInstance, among them the scala-library-2.12 and compiler-bridge-2.12 classes.

[test_sbt]

[adpi2]

The new sbt-dotty works well without the CompilerClassLoader.

However Mill won't work until we apply the same logic inside it: Mill should create a ScalaInstance whose loader is compatible with the Zinc loader.

So, before merging this PR, I will have to:

• apply the ScalaInstance loader fix in Mill
• release Mill
• update the community build projects that rely on Mill

@sjrd and @smarter: Do you confirm this is the right direction?

[adpi2]

As a side note, here is the code in Mill that create the ScalaInstance loader:
https://github.com/lihaoyi/mill/blob/aa3240a52a72cfa5fe76c4d4c747f7f61eef2f1d/scalalib/worker/src/ZincWorkerImpl.scala#L309-L321

[smarter]

This PR fix the problem by using the sbt ClassLoader as a parent of the ScalaInstance.loader.

If this works well enough, could this be done upstream in sbt for Scala 2 too so we don't need a special case just for Scala 3?

[adpi2]

@smarter

If this works well enough, could this be done upstream in sbt for Scala 2 too so we don't need a special case just for Scala 3?

Yes absolutely I test it first in sbt-dotty then I will generalize in sbt for all Scala versions.

[smarter]

OK, I'd be interested in hearing the opinion of the sbt maintainers on this direction.

[adpi2]

@eed3si9n @eatkins

My plan is to use a filtered version of appConfiguration.provider.loader as a parent of scalaInstance.loader so that the Scala 3 compiler can load the xsbti.* classes from the same loader than sbt. A similar thing is already done inside sbt to load the jline Terminal provided by sbt in the Scala REPL (code here)

Right now I am testing this in sbt-dotty but my plan is to implement it in sbt by reusing the scalaInstanceTopLoader key. This thing will allow me to get rid of the messy CompilerClassLoader and then implement the Zinc CompilerInterface2 in the scala 3 compiler bridge.

Can you confirm that this is the right direction?

[adpi2]

I found a similar implementation in Mill that is looking better to me:
https://github.com/lihaoyi/mill/blob/master/main/api/src/mill/api/ClassLoader.scala

In our case, sharedPrefixes would contain "org.jline." and "xsbti.".

I'll give a try.

[eatkins]

This approach sounds correct to me without thinking too much about it. It may be helpful to look at the code that constructs the sbt metabuild loader as well: https://github.com/sbt/sbt/blob/develop/main/src/main/java/sbt/internal/MetaBuildLoader.java. It shows how the sbt meta build classloader is constructed. You may end up wanting to make a change there as well.

[smarter]

Listing the packages to include rather than the packages to exclude like you did in the first commit seems safer to me (for example, you're not excluding "dotty." which could be loaded by a future version of sbt). However in that commit you had to include all the JDK prefixes ("java.", "sun.", ...). Perhaps this can be avoided by replicating what CompilerClassLoader did before this PR: use null as a parent for FilteringClassLoader so super.loadClass will delegate to the system JDK classloader, and call loadClass on the sbt loader explicitly when needed instead.

[adpi2]

Related PR in Mill: com-lihaoyi/mill#1019