Scala/Java mixed projects fail to compile using processorpath bloop + metals

[gersonsosa]

Hello folks, hope everything is good on your end.

I'm facing an issue with bloop + metals since a few months ago, here is the overall description
After issuing bloopInstall in my gradle project and starting metals, bloop fails to compile for scala/java mixed projects that include a "-processorpath" with an error.

bloop compile project-name --verbose
[D] Loading workspace settings from bloop.settings.json
[D] Detected connected cli clients, starting CLI with unique dirs...
[D] Loading workspace settings from bloop.settings.json
[D] Computing sources and classpath hashes for project-name
[D] Scheduling compilation for project-name...
... [redacted]
[D] Attempting to call com.sun.tools.javac.api.JavacTool@421afe44 directly...
[D] Invoking javac with -Xplugin:semanticdb -sourceroot:/path/to/project-name -targetroot:javac-classes-directory -source 11 -target 11 -encoding UTF-8 -h /path/to/project/project-name/build/generated/sources/headers/java/main -g -sourcepath  -processorpath .../.gradle/caches/modules-2/files-2.1/org.immutables/value/2.6.1/d25b09df0cefc2249bb0e0354f6a980327c91939/value-2.6.1.jar -proc:none -XDuseUnsharedTable=true -Xlint:all -Xlint:-fallthrough -Xlint:-serial -Xlint:-processing -Xlint:-deprecation -Werror -classpath [reacted]
[E] [E-1] plug-in not found: semanticdb
Compiled project-name (676ms)
[E] Failed to compile 'project-name'
[D] Elapsed: 719.870083 ms

I would expect that the build succeeds after starting metals, in fact it does before I start metals, the reason that this happens is that metals creates the .bloop/bloop-settings.json file with semanticdb configuration for java.

{
    "javaSemanticDBVersion": "0.7.4",
     "semanticDBVersion": "4.6.0",
     ...

However after trying a lot of things with it, I found a way to workaround the issue by appending the semanticdb plugin path to the already existing -processorpath entry in the corresponding bloop config file for the project.
So it goes from this

...
"java": {
            "options": [
                ...
                "-sourcepath",
                "",
                "-processorpath",
                "../.gradle/caches/modules-2/files-2.1/org.immutables/value/2.6.1/d25b09df0cefc2249bb0e0354f6a980327c91939/value-2.6.1.jar",
                "-s",
...

to this

...
"java": {
            "options": [
                ...
                "-sourcepath",
                "",
                "-processorpath",
                "../.gradle/caches/modules-2/files-2.1/org.immutables/value/2.6.1/d25b09df0cefc2249bb0e0354f6a980327c91939/value-2.6.1.jar:.../Library/Caches/bloop/semanticdb/com.sourcegraph.semanticdb-javac.0.7.4/semanticdb-javac-0.7.4.jar",
                "-s",
...

And with that it succeeds, apparently java compiler only searches for a compiler plugin processor in the classpath if you leave out the "-processorpath" option.

So I bug into the code and I see some places where semanticbd options are attached in memory, but I wanted to have some guidance on where to start to create a minimal repo to reproduce the issue, including a failing test in the project.

---

I'm running on macOS
Gradle version 7.5.1
Gradle bloop version: ch.epfl.scala:gradle-bloop_2.12:1.5.3, also tested with ch.epfl.scala:gradle-bloop_2.13:1.6.0
Bloop version: 1.5.4, also tested with 1.5.6

Thanks for reading until here!
Cheers

[Arthurm1]

Hi @gersonsosa

Not sure if you were saying that you'd like to look at or fix this yourself. If so, the code responsible for adding the semanticdb plugin is here...

bloop/frontend/src/main/scala/bloop/data/Project.scala

Lines 477 to 504 in 7a7a913

	def enableJavaSemanticdbOptions(options: List[String]): List[String] = {
	if (hasJavaSemanticDBEnabledInCompilerOptions(options)) options
	else {
	val semanticdbOptions =
	s"-Xplugin:semanticdb -sourceroot:${workspaceDir} -targetroot:javac-classes-directory" :: options
	if (project.javaVersionAtLeast("17", logger))
	List(
	"-J--add-exports",
	"-Jjdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED",
	"-J--add-exports",
	"-Jjdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED",
	"-J--add-exports",
	"-Jjdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED",
	"-J--add-exports",
	"-Jjdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED",
	"-J--add-exports",
	"-Jjdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED"
	) ::: semanticdbOptions
	else semanticdbOptions
	}
	}
	def enableJavaSemanticdbClasspath(
	pluginPath: AbsolutePath,
	classpath: List[AbsolutePath]
	): List[AbsolutePath] = {
	if (classpath.contains(pluginPath)) classpath else pluginPath :: classpath
	}

Currently it adds the semanticdb plugin to the classpath and adds various javac options. I guess it could test to see if -processorpath exists and then classpath can be left unchanged and -processorpath could be altered to include the semanticdb plugin path.

I note here that it's recommended to use the classpath unless you're using annotation processors so I don't know if there would be an issue moving to -processorpath in all cases, maybe a performance hit?

It would also need to be changed in Metals here...

https://github.com/scalameta/metals/blob/cce28f192c1625a0b77c09fef63ea63cd0b83bc0/metals/src/main/scala/scala/meta/internal/metals/JavaInteractiveSemanticdb.scala#L59-L72

and possibly here but I don't know this area...

https://github.com/scalameta/metals/blob/cce28f192c1625a0b77c09fef63ea63cd0b83bc0/sbt-metals/src/main/scala/scala/meta/metals/MetalsPlugin.scala#L109-L130

[tgodzik]

It looks correct what @Arthurm1 wrote. Let us know if you are interesting in helping out!

If you can't, it would be awesome if you created a small repro that we can take a look like. So probably a small project that uses a preprocessor.

[gersonsosa]

@Arthurm1 @tgodzik thanks for your response, sure I’m in for helping out, while playing with the code it seems that with the changes in bloop

1. here (by altering the processorpath string including semanticdb path)

   bloop/frontend/src/main/scala/bloop/data/Project.scala

   Lines 477 to 504 in 7a7a913

   	def enableJavaSemanticdbOptions(options: List[String]): List[String] = {
   	if (hasJavaSemanticDBEnabledInCompilerOptions(options)) options
   	else {
   	val semanticdbOptions =
   	s"-Xplugin:semanticdb -sourceroot:${workspaceDir} -targetroot:javac-classes-directory" :: options
   	if (project.javaVersionAtLeast("17", logger))
   	List(
   	"-J--add-exports",
   	"-Jjdk.compiler/com.sun.tools.javac.api=ALL-UNNAMED",
   	"-J--add-exports",
   	"-Jjdk.compiler/com.sun.tools.javac.code=ALL-UNNAMED",
   	"-J--add-exports",
   	"-Jjdk.compiler/com.sun.tools.javac.model=ALL-UNNAMED",
   	"-J--add-exports",
   	"-Jjdk.compiler/com.sun.tools.javac.tree=ALL-UNNAMED",
   	"-J--add-exports",
   	"-Jjdk.compiler/com.sun.tools.javac.util=ALL-UNNAMED"
   	) ::: semanticdbOptions
   	else semanticdbOptions
   	}
   	}
   	def enableJavaSemanticdbClasspath(
   	pluginPath: AbsolutePath,
   	classpath: List[AbsolutePath]
   	): List[AbsolutePath] = {
   	if (classpath.contains(pluginPath)) classpath else pluginPath :: classpath
   	}

2. also looking at whether it’s necessary to include the classpath if the processorpath is there, but this change seems optional

   bloop/frontend/src/main/scala/bloop/data/Project.scala

   Lines 530 to 541 in 7a7a913

   	javaSemanticDBPlugin match {
   	case None =>
   	scalaProjectWithRangePositions
   	case Some(pluginPath) =>
   	val javacOptionsWithSemanticDB = enableJavaSemanticdbOptions(javacOptions)
   	val classpathWithSemanticDB =
   	enableJavaSemanticdbClasspath(pluginPath, scalaProjectWithRangePositions.rawClasspath)
   	scalaProjectWithRangePositions.copy(
   	javacOptions = javacOptionsWithSemanticDB,
   	rawClasspath = classpathWithSemanticDB
   	)
   	}

things run smooth once again.

What I would like some guidance is if I can include a sample project for example here as a test in the code or it would be best to just test the parts I change independently and where should I put those tests

additionally I haven’t checked metals code but I can give a try after having a PR for bloop.

[tgodzik]

What I would like some guidance is if I can include a sample project for example here as a test in the code or it would be best to just test the parts I change independently and where should I put those tests

I think using a sample project is perfectly fine, we already have some tests that use it. For example in the BspProtocolSuite. We have a method that loads up those test project (loadBspBuildFromResources)

[gersonsosa]

@tgodzik @Arthurm1 I finally got some time to work on this, I made a small PR to fix this, I included small project in the test resources

I'll now look into metals now, I'm not sure why we have to change it there though

[gersonsosa]

To elaborate on why I'm not sure I have to change metals usages of javac

I think the parts where metals compile java sources using javac do not rely on the build tool to generate the classpath of the command, therefore they will never have the -processorpath option when executing (please correct me if I'm wrong)

1. for on the fly generation https://github.com/scalameta/metals/blob/8283260dc275a82dea0261ad3aaec632892014b2/metals/src/main/scala/scala/meta/internal/metals/JavaInteractiveSemanticdb.scala#L63
2. for the project settings https://github.com/scalameta/metals/blob/8283260dc275a82dea0261ad3aaec632892014b2/sbt-metals/src/main/scala/scala/meta/metals/MetalsPlugin.scala#L67

I added a small test to a branch here it seems to work as expected https://github.com/gersonsosa/metals/tree/fix-java-options-processorpath

[Arthurm1]

@gersonsosa I think you're right. It looks like the on-the-fly generation ignores the java options.

I'm looking to try to change how this code works so I'd recommend not bothering to change Metals.