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

gradle-scala plugin overrides isolated gradle provisioner scala version in spotless #21502

Closed
mdedetrich opened this issue Aug 11, 2022 · 6 comments

Comments

@mdedetrich
Copy link

mdedetrich commented Aug 11, 2022

Expected Behavior

When using gradle + gradle-scala and specifying the scala version with -PscalaVersion=2.12 along with spotless-scala (https://github.com/diffplug/spotless) with scalafmt 3.5.8, there shouldn't be any problems and it should check/format the code fine.

Current Behavior

Currently if you use spotless scala along with scalafmt 3.5.8 + scala 2.12 to see if the code is formatted, i.e.

./gradlew -PscalaVersion=2.12 :spotlessScalaCheck

You get the following stack trace

> Configure project :
Starting build with version 3.4.0-SNAPSHOT (commit id de2ccf1f) using Gradle 7.5.1, Java 18 and Scala 2.12.15
Build properties: maxParallelForks=10, maxScalacThreads=8, maxTestRetries=0

> Task :spotlessScala FAILED
Step 'scalafmt' found problem in 'streams/streams-scala/src/main/scala/org/apache/kafka/streams/scala/FunctionsCompatConversions.scala':
null
java.lang.reflect.InvocationTargetException
        at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:119)
        at com.diffplug.spotless.scala.ScalaFmtStep$State.createFormat(ScalaFmtStep.java:98)
        at com.diffplug.spotless.FormatterStepImpl$Standard.format(FormatterStepImpl.java:80)
        at com.diffplug.spotless.FormatterStep$Strict.format(FormatterStep.java:88)
        at com.diffplug.spotless.Formatter.compute(Formatter.java:230)
        at com.diffplug.spotless.PaddedCell.calculateDirtyState(PaddedCell.java:203)
        at com.diffplug.spotless.PaddedCell.calculateDirtyState(PaddedCell.java:190)
        at com.diffplug.gradle.spotless.SpotlessTaskImpl.processInputFile(SpotlessTaskImpl.java:102)
        at com.diffplug.gradle.spotless.SpotlessTaskImpl.performAction(SpotlessTaskImpl.java:88)
        at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
        at org.gradle.internal.reflect.JavaMethod.invoke(JavaMethod.java:125)
        at org.gradle.api.internal.project.taskfactory.IncrementalInputsTaskAction.doExecute(IncrementalInputsTaskAction.java:32)
        at org.gradle.api.internal.project.taskfactory.StandardTaskAction.execute(StandardTaskAction.java:51)
        at org.gradle.api.internal.project.taskfactory.AbstractIncrementalTaskAction.execute(AbstractIncrementalTaskAction.java:25)
        at org.gradle.api.internal.project.taskfactory.StandardTaskAction.execute(StandardTaskAction.java:29)
        at org.gradle.api.internal.tasks.execution.TaskExecution$3.run(TaskExecution.java:236)
        at org.gradle.internal.operations.DefaultBuildOperationRunner$1.execute(DefaultBuildOperationRunner.java:29)
        at org.gradle.internal.operations.DefaultBuildOperationRunner$1.execute(DefaultBuildOperationRunner.java:26)
        at org.gradle.internal.operations.DefaultBuildOperationRunner$2.execute(DefaultBuildOperationRunner.java:66)
        at org.gradle.internal.operations.DefaultBuildOperationRunner$2.execute(DefaultBuildOperationRunner.java:59)
        at org.gradle.internal.operations.DefaultBuildOperationRunner.execute(DefaultBuildOperationRunner.java:157)
        at org.gradle.internal.operations.DefaultBuildOperationRunner.execute(DefaultBuildOperationRunner.java:59)
        at org.gradle.internal.operations.DefaultBuildOperationRunner.run(DefaultBuildOperationRunner.java:47)
        at org.gradle.internal.operations.DefaultBuildOperationExecutor.run(DefaultBuildOperationExecutor.java:68)
        at org.gradle.api.internal.tasks.execution.TaskExecution.executeAction(TaskExecution.java:221)
        at org.gradle.api.internal.tasks.execution.TaskExecution.executeActions(TaskExecution.java:204)
        at org.gradle.api.internal.tasks.execution.TaskExecution.executeWithPreviousOutputFiles(TaskExecution.java:187)
        at org.gradle.api.internal.tasks.execution.TaskExecution.execute(TaskExecution.java:165)
        at org.gradle.internal.execution.steps.ExecuteStep.executeInternal(ExecuteStep.java:89)
        at org.gradle.internal.execution.steps.ExecuteStep.access$000(ExecuteStep.java:40)
        at org.gradle.internal.execution.steps.ExecuteStep$1.call(ExecuteStep.java:53)
        at org.gradle.internal.execution.steps.ExecuteStep$1.call(ExecuteStep.java:50)
        at org.gradle.internal.operations.DefaultBuildOperationRunner$CallableBuildOperationWorker.execute(DefaultBuildOperationRunner.java:204)
        at org.gradle.internal.operations.DefaultBuildOperationRunner$CallableBuildOperationWorker.execute(DefaultBuildOperationRunner.java:199)
        at org.gradle.internal.operations.DefaultBuildOperationRunner$2.execute(DefaultBuildOperationRunner.java:66)
        at org.gradle.internal.operations.DefaultBuildOperationRunner$2.execute(DefaultBuildOperationRunner.java:59)
        at org.gradle.internal.operations.DefaultBuildOperationRunner.execute(DefaultBuildOperationRunner.java:157)
        at org.gradle.internal.operations.DefaultBuildOperationRunner.execute(DefaultBuildOperationRunner.java:59)
        at org.gradle.internal.operations.DefaultBuildOperationRunner.call(DefaultBuildOperationRunner.java:53)
        at org.gradle.internal.operations.DefaultBuildOperationExecutor.call(DefaultBuildOperationExecutor.java:73)
        at org.gradle.internal.execution.steps.ExecuteStep.execute(ExecuteStep.java:50)
        at org.gradle.internal.execution.steps.ExecuteStep.execute(ExecuteStep.java:40)
        at org.gradle.internal.execution.steps.RemovePreviousOutputsStep.execute(RemovePreviousOutputsStep.java:68)
        at org.gradle.internal.execution.steps.RemovePreviousOutputsStep.execute(RemovePreviousOutputsStep.java:38)
        at org.gradle.internal.execution.steps.CancelExecutionStep.execute(CancelExecutionStep.java:41)
        at org.gradle.internal.execution.steps.TimeoutStep.executeWithoutTimeout(TimeoutStep.java:74)
        at org.gradle.internal.execution.steps.TimeoutStep.execute(TimeoutStep.java:55)
        at org.gradle.internal.execution.steps.CreateOutputsStep.execute(CreateOutputsStep.java:51)
        at org.gradle.internal.execution.steps.CreateOutputsStep.execute(CreateOutputsStep.java:29)
        at org.gradle.internal.execution.steps.CaptureStateAfterExecutionStep.executeDelegateBroadcastingChanges(CaptureStateAfterExecutionStep.java:124)
        at org.gradle.internal.execution.steps.CaptureStateAfterExecutionStep.execute(CaptureStateAfterExecutionStep.java:80)
        at org.gradle.internal.execution.steps.CaptureStateAfterExecutionStep.execute(CaptureStateAfterExecutionStep.java:58)
        at org.gradle.internal.execution.steps.ResolveInputChangesStep.execute(ResolveInputChangesStep.java:48)
        at org.gradle.internal.execution.steps.ResolveInputChangesStep.execute(ResolveInputChangesStep.java:36)
        at org.gradle.internal.execution.steps.BuildCacheStep.executeWithoutCache(BuildCacheStep.java:181)
        at org.gradle.internal.execution.steps.BuildCacheStep.lambda$execute$1(BuildCacheStep.java:71)
        at org.gradle.internal.Either$Right.fold(Either.java:175)
        at org.gradle.internal.execution.caching.CachingState.fold(CachingState.java:59)
        at org.gradle.internal.execution.steps.BuildCacheStep.execute(BuildCacheStep.java:69)
        at org.gradle.internal.execution.steps.BuildCacheStep.execute(BuildCacheStep.java:47)
        at org.gradle.internal.execution.steps.StoreExecutionStateStep.execute(StoreExecutionStateStep.java:36)
        at org.gradle.internal.execution.steps.StoreExecutionStateStep.execute(StoreExecutionStateStep.java:25)
        at org.gradle.internal.execution.steps.RecordOutputsStep.execute(RecordOutputsStep.java:36)
        at org.gradle.internal.execution.steps.RecordOutputsStep.execute(RecordOutputsStep.java:22)
        at org.gradle.internal.execution.steps.SkipUpToDateStep.executeBecause(SkipUpToDateStep.java:110)
        at org.gradle.internal.execution.steps.SkipUpToDateStep.lambda$execute$2(SkipUpToDateStep.java:56)
        at org.gradle.internal.execution.steps.SkipUpToDateStep.execute(SkipUpToDateStep.java:56)
        at org.gradle.internal.execution.steps.SkipUpToDateStep.execute(SkipUpToDateStep.java:38)
        at org.gradle.internal.execution.steps.ResolveChangesStep.execute(ResolveChangesStep.java:73)
        at org.gradle.internal.execution.steps.ResolveChangesStep.execute(ResolveChangesStep.java:44)
        at org.gradle.internal.execution.steps.legacy.MarkSnapshottingInputsFinishedStep.execute(MarkSnapshottingInputsFinishedStep.java:37)
        at org.gradle.internal.execution.steps.legacy.MarkSnapshottingInputsFinishedStep.execute(MarkSnapshottingInputsFinishedStep.java:27)
        at org.gradle.internal.execution.steps.ResolveCachingStateStep.execute(ResolveCachingStateStep.java:89)
        at org.gradle.internal.execution.steps.ResolveCachingStateStep.execute(ResolveCachingStateStep.java:50)
        at org.gradle.internal.execution.steps.ValidateStep.execute(ValidateStep.java:114)
        at org.gradle.internal.execution.steps.ValidateStep.execute(ValidateStep.java:57)
        at org.gradle.internal.execution.steps.CaptureStateBeforeExecutionStep.execute(CaptureStateBeforeExecutionStep.java:76)
        at org.gradle.internal.execution.steps.CaptureStateBeforeExecutionStep.execute(CaptureStateBeforeExecutionStep.java:50)
        at org.gradle.internal.execution.steps.SkipEmptyWorkStep.executeWithNoEmptySources(SkipEmptyWorkStep.java:254)
        at org.gradle.internal.execution.steps.SkipEmptyWorkStep.execute(SkipEmptyWorkStep.java:91)
        at org.gradle.internal.execution.steps.SkipEmptyWorkStep.execute(SkipEmptyWorkStep.java:56)
        at org.gradle.internal.execution.steps.RemoveUntrackedExecutionStateStep.execute(RemoveUntrackedExecutionStateStep.java:32)
        at org.gradle.internal.execution.steps.RemoveUntrackedExecutionStateStep.execute(RemoveUntrackedExecutionStateStep.java:21)
        at org.gradle.internal.execution.steps.legacy.MarkSnapshottingInputsStartedStep.execute(MarkSnapshottingInputsStartedStep.java:38)
        at org.gradle.internal.execution.steps.LoadPreviousExecutionStateStep.execute(LoadPreviousExecutionStateStep.java:43)
        at org.gradle.internal.execution.steps.LoadPreviousExecutionStateStep.execute(LoadPreviousExecutionStateStep.java:31)
        at org.gradle.internal.execution.steps.AssignWorkspaceStep.lambda$execute$0(AssignWorkspaceStep.java:40)
        at org.gradle.api.internal.tasks.execution.TaskExecution$4.withWorkspace(TaskExecution.java:281)
        at org.gradle.internal.execution.steps.AssignWorkspaceStep.execute(AssignWorkspaceStep.java:40)
        at org.gradle.internal.execution.steps.AssignWorkspaceStep.execute(AssignWorkspaceStep.java:30)
        at org.gradle.internal.execution.steps.IdentityCacheStep.execute(IdentityCacheStep.java:37)
        at org.gradle.internal.execution.steps.IdentityCacheStep.execute(IdentityCacheStep.java:27)
        at org.gradle.internal.execution.steps.IdentifyStep.execute(IdentifyStep.java:44)
        at org.gradle.internal.execution.steps.IdentifyStep.execute(IdentifyStep.java:33)
        at org.gradle.internal.execution.impl.DefaultExecutionEngine$1.execute(DefaultExecutionEngine.java:76)
        at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.executeIfValid(ExecuteActionsTaskExecuter.java:139)
        at org.gradle.api.internal.tasks.execution.ExecuteActionsTaskExecuter.execute(ExecuteActionsTaskExecuter.java:128)
        at org.gradle.api.internal.tasks.execution.CleanupStaleOutputsExecuter.execute(CleanupStaleOutputsExecuter.java:77)
        at org.gradle.api.internal.tasks.execution.FinalizePropertiesTaskExecuter.execute(FinalizePropertiesTaskExecuter.java:46)
        at org.gradle.api.internal.tasks.execution.ResolveTaskExecutionModeExecuter.execute(ResolveTaskExecutionModeExecuter.java:51)
        at org.gradle.api.internal.tasks.execution.SkipTaskWithNoActionsExecuter.execute(SkipTaskWithNoActionsExecuter.java:57)
        at org.gradle.api.internal.tasks.execution.SkipOnlyIfTaskExecuter.execute(SkipOnlyIfTaskExecuter.java:56)
        at org.gradle.api.internal.tasks.execution.CatchExceptionTaskExecuter.execute(CatchExceptionTaskExecuter.java:36)
        at org.gradle.api.internal.tasks.execution.EventFiringTaskExecuter$1.executeTask(EventFiringTaskExecuter.java:77)
        at org.gradle.api.internal.tasks.execution.EventFiringTaskExecuter$1.call(EventFiringTaskExecuter.java:55)
        at org.gradle.api.internal.tasks.execution.EventFiringTaskExecuter$1.call(EventFiringTaskExecuter.java:52)
        at org.gradle.internal.operations.DefaultBuildOperationRunner$CallableBuildOperationWorker.execute(DefaultBuildOperationRunner.java:204)
        at org.gradle.internal.operations.DefaultBuildOperationRunner$CallableBuildOperationWorker.execute(DefaultBuildOperationRunner.java:199)
        at org.gradle.internal.operations.DefaultBuildOperationRunner$2.execute(DefaultBuildOperationRunner.java:66)
        at org.gradle.internal.operations.DefaultBuildOperationRunner$2.execute(DefaultBuildOperationRunner.java:59)
        at org.gradle.internal.operations.DefaultBuildOperationRunner.execute(DefaultBuildOperationRunner.java:157)
        at org.gradle.internal.operations.DefaultBuildOperationRunner.execute(DefaultBuildOperationRunner.java:59)
        at org.gradle.internal.operations.DefaultBuildOperationRunner.call(DefaultBuildOperationRunner.java:53)
        at org.gradle.internal.operations.DefaultBuildOperationExecutor.call(DefaultBuildOperationExecutor.java:73)
        at org.gradle.api.internal.tasks.execution.EventFiringTaskExecuter.execute(EventFiringTaskExecuter.java:52)
        at org.gradle.execution.plan.LocalTaskNodeExecutor.execute(LocalTaskNodeExecutor.java:69)
        at org.gradle.execution.taskgraph.DefaultTaskExecutionGraph$InvokeNodeExecutorsAction.execute(DefaultTaskExecutionGraph.java:327)
        at org.gradle.execution.taskgraph.DefaultTaskExecutionGraph$InvokeNodeExecutorsAction.execute(DefaultTaskExecutionGraph.java:314)
        at org.gradle.execution.taskgraph.DefaultTaskExecutionGraph$BuildOperationAwareExecutionAction.execute(DefaultTaskExecutionGraph.java:307)
        at org.gradle.execution.taskgraph.DefaultTaskExecutionGraph$BuildOperationAwareExecutionAction.execute(DefaultTaskExecutionGraph.java:293)
        at org.gradle.execution.plan.DefaultPlanExecutor$ExecutorWorker.execute(DefaultPlanExecutor.java:420)
        at org.gradle.execution.plan.DefaultPlanExecutor$ExecutorWorker.run(DefaultPlanExecutor.java:342)
        at org.gradle.internal.concurrent.ExecutorPolicy$CatchAndRecordFailures.onExecute(ExecutorPolicy.java:64)
        at org.gradle.internal.concurrent.ManagedExecutorImpl$1.run(ManagedExecutorImpl.java:48)
Caused by: java.lang.NoClassDefFoundError: scala/collection/IterableOnce
        at org.scalafmt.Scalafmt$.format$default$2(Scalafmt.scala:171)
        at org.scalafmt.Scalafmt.format$default$2(Scalafmt.scala)
        at java.base/jdk.internal.reflect.DirectMethodHandleAccessor.invoke(DirectMethodHandleAccessor.java:104)
        ... 123 more
Caused by: java.lang.ClassNotFoundException: scala.collection.IterableOnce
        at com.diffplug.spotless.FeatureClassLoader.findClass(FeatureClassLoader.java:81)
        ... 126 more

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':spotlessScala'.
> java.lang.reflect.InvocationTargetException

* Try:
> Run with --stacktrace option to get the stack trace.
> Run with --info or --debug option to get more log output.
> Run with --scan to get full insights.

* Get more help at https://help.gradle.org

BUILD FAILED in 9s
2 actionable tasks: 1 executed, 1 up-to-date

Context

Relevant issue in spotless: diffplug/spotless#1273
Relevant PR in kafka: apache/kafka#12475

The reason why spotless-scala with scalafmt is failing is because the usage of -PscalaVersion=2.12 causes gradle-scala plugin to override the scala version. While this is expected/fine if you happen to be building your project for a different version of Scala, in the case of spotless-scala (that uses scalafmt underneath) the version of scala should not be overridden because regardless of what Scala version you are building the project, scalafmt 3.5.8 only works with Scala 2.13.x. Specifically if you look at the stacktrace its complaining about classes that only exist in Scala 2.13.x stdlib but not in Scala 2.12.x stdlib. This is why if you run ./gradlew -PscalaVersion=2.13 :spotlessScalaCheck there is no issue (since scalafmt 3.5.8 is compild

Spotless is already trying to solve this problem by isolating it (see diffplug/spotless#1273 (comment)) but for some reason the -PscalaVersion=2.12 bypasses this isolation. My immediate suspicion is that guild-scala is providing the scala runtime/stdlib globally within gradle which is being picked up by spotless-scala? If this is the case I imagine the way to solve this issue is to use an isolated classloader (no idea if Gradle is already doing this?)

Steps to Reproduce

Checkout the following branch/PR apache/kafka#12475 and run ./gradlew -PscalaVersion=2.12 :spotlessScalaCheck.

Your Environment

My environment is MacOS Montery 12.5 M1 however this problem occurs on any platform (this is evidenced by the fact it fails in Kafka's CI/CD, see https://ci-builds.apache.org/job/Kafka/job/kafka-pr/job/PR-12475/)
Build scan URL: https://scans.gradle.com/s/bycldhmiy5imy

@mdedetrich
Copy link
Author

@tgodzik Since you have worked on the gradle-scala plugin in the past and also with scalafmt I believe you may have some idea/context on this.

@tgodzik
Copy link
Contributor

tgodzik commented Aug 12, 2022

I am not familiar with Spotless, though the issue here seems that it's using a specific Scalafmt version released for 2.12 instead of using the one for the current Scala version. I also relies quite heavily on reflection, which might also cause other problems.

Scalafmt should work well both with Scala 2.13 and 2.12 (probably any Scala version) with the API that we already use in Metals:
https://github.com/scalameta/metals/blob/main/metals/src/main/scala/scala/meta/internal/metals/FormattingProvider.scala

I wonder if it would be possible to replace the current implementation with API, which is run in it's own classloader as far as I can remember.

The other option would be to dynamically download scalafmt version base don which is used in the project currently.

I think it's not possible currently to have two different Scala versions on the classpath when using Gradle. Gradle Scala plugin needs a specific version and I don't think it's isolated behind a separate classloader. As long as both spotless and Scala plugin use the same classpath it will not work (unless I am mistaken and it actually does separate classloaders 🤔 )

@mdedetrich
Copy link
Author

mdedetrich commented Aug 12, 2022

@tgodzik Thanks for the reply

I am not familiar with Spotless, though the issue here seems that it's using a specific Scalafmt version released for 2.12 instead of using the one for the current Scala version. I also relies quite heavily on reflection, which might also cause other problems.

So this seems to be the case, if you look at https://github.com/diffplug/spotless/blob/0fd20bb80c6c426d20e0a3157c3c2b89317032da/lib/src/main/java/com/diffplug/spotless/scala/ScalaFmtStep.java the Scala version happens to be hardcoded depending on what version of Scalafmt you are using, so if you happen to use Scalafmt 3.x it will just always pick Scala 2.13.x

I wonder if it would be possible to replace the current implementation with API, which is run in it's own classloader as far as I can remember.

Design wise this seems like the best solution as it will always work and it makes sense for a CLI tool to run on its own completely isolated.

The other option would be to dynamically download scalafmt version base don which is used in the project currently.

As you say, if Gradle cannot support proper classload isolation for Scala then yes it appears that our next best solution is to dynamically figure out if there is a gradle-scala plugin loaded and if so then we get the major scala version from the gradle-scala plugin and use that instead.

@ljacomet
Copy link
Member

A full reproducer would help if anything needs to change in Gradle itself.

@mdedetrich
Copy link
Author

I had another look at this during the weekend and I think that it makes more sense to wait until Gradle provides classloader isolation in their plugins by default, i.e.

https://docs.gradle.org/current/userguide/designing_gradle_plugins.html

It’s important to understand that a Gradle plugin does not run in its own, isolated classloader. In turn those dependencies might conflict with other versions of the same library being resolved from other plugins and might lead to unexpected runtime behavior. When writing Gradle plugins consider if you really need a specific library or if you could just implement a simple method yourself. A future version of Gradle will introduce proper classpath isolation for plugins.

Another solution for the gradle-scala plugin itself would be to manually implement classloader isolation but I assume that would be a large amount of work. @ljacomet Do you know if there is any movement in gradle to implement classloader isolation on the plugin level or is this still something that is far off?

@mdedetrich
Copy link
Author

Managed to solve this issue via spotless (see diffplug/spotless#1283)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants