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

0.8.19 exposes Guava in a Groovy API but has it as an implementation dependency #572

Closed
tinder-dthomson opened this issue Jun 30, 2022 · 6 comments · Fixed by #609
Closed

Comments

@tinder-dthomson
Copy link

tinder-dthomson commented Jun 30, 2022

Hello @ejona86 and team!

Using Gradle version 7.4.1.

We have a custom Gradle plugin written in Groovy that uses this plugin as a dependency (essentially it wraps this plugin with some additional tasks).

Small sample of the code:

import com.google.protobuf.gradle.GenerateProtoTask

@CompileStatic
class ProtobufPlugin implements Plugin<Project> {
// plugin body
}

Up until 0.8.19, we were able to annotate the Plugin implementation class with @CompileStatic as shown above. As of 0.8.19, this is no longer working, with the following exception:

> Task :compileGroovy FAILED
startup failed:
General error during canonicalization: java.lang.NoClassDefFoundError: com.google.common.collect.ImmutableList

java.lang.RuntimeException: java.lang.NoClassDefFoundError: com.google.common.collect.ImmutableList
        at org.codehaus.groovy.control.CompilationUnit$IPrimaryClassNodeOperation.doPhaseOperation(CompilationUnit.java:976)
        at org.codehaus.groovy.control.CompilationUnit.processPhaseOperations(CompilationUnit.java:671)
        at org.codehaus.groovy.control.CompilationUnit.compile(CompilationUnit.java:635)
        at org.codehaus.groovy.control.CompilationUnit.compile(CompilationUnit.java:610)
        at org.gradle.api.internal.tasks.compile.ApiGroovyCompiler.execute(ApiGroovyCompiler.java:270)
        at org.gradle.api.internal.tasks.compile.ApiGroovyCompiler.execute(ApiGroovyCompiler.java:64)
        at org.gradle.api.internal.tasks.compile.GroovyCompilerFactory$DaemonSideCompiler.execute(GroovyCompilerFactory.java:97)
        at org.gradle.api.internal.tasks.compile.GroovyCompilerFactory$DaemonSideCompiler.execute(GroovyCompilerFactory.java:76)
        at org.gradle.api.internal.tasks.compile.daemon.AbstractDaemonCompiler$CompilerWorkAction.execute(AbstractDaemonCompiler.java:135)
        at org.gradle.workers.internal.DefaultWorkerServer.execute(DefaultWorkerServer.java:63)
        at org.gradle.workers.internal.AbstractClassLoaderWorker$1.create(AbstractClassLoaderWorker.java:49)
        at org.gradle.workers.internal.AbstractClassLoaderWorker$1.create(AbstractClassLoaderWorker.java:43)
        at org.gradle.internal.classloader.ClassLoaderUtils.executeInClassloader(ClassLoaderUtils.java:97)
        at org.gradle.workers.internal.AbstractClassLoaderWorker.executeInClassLoader(AbstractClassLoaderWorker.java:43)
        at org.gradle.workers.internal.IsolatedClassloaderWorker.run(IsolatedClassloaderWorker.java:49)
        at org.gradle.workers.internal.IsolatedClassloaderWorker.run(IsolatedClassloaderWorker.java:30)
        at org.gradle.workers.internal.WorkerDaemonServer.run(WorkerDaemonServer.java:87)
        at org.gradle.workers.internal.WorkerDaemonServer.run(WorkerDaemonServer.java:56)
        at org.gradle.process.internal.worker.request.WorkerAction$1.call(WorkerAction.java:138)
        at org.gradle.process.internal.worker.child.WorkerLogEventListener.withWorkerLoggingProtocol(WorkerLogEventListener.java:41)
        at org.gradle.process.internal.worker.request.WorkerAction.run(WorkerAction.java:135)
        at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
        at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.lang.reflect.Method.invoke(Method.java:498)
        at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:36)
        at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
        at org.gradle.internal.remote.internal.hub.MessageHubBackedObjectConnection$DispatchWrapper.dispatch(MessageHubBackedObjectConnection.java:182)
        at org.gradle.internal.remote.internal.hub.MessageHubBackedObjectConnection$DispatchWrapper.dispatch(MessageHubBackedObjectConnection.java:164)
        at org.gradle.internal.remote.internal.hub.MessageHub$Handler.run(MessageHub.java:414)
        at org.gradle.internal.concurrent.ExecutorPolicy$CatchAndRecordFailures.onExecute(ExecutorPolicy.java:64)
        at org.gradle.internal.concurrent.ManagedExecutorImpl$1.run(ManagedExecutorImpl.java:48)
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
        at java.lang.Thread.run(Thread.java:748)
Caused by: java.lang.NoClassDefFoundError: com.google.common.collect.ImmutableList
        at org.codehaus.groovy.ast.decompiled.AsmReferenceResolver.resolveClass(AsmReferenceResolver.java:46)
        at org.codehaus.groovy.ast.decompiled.AsmReferenceResolver.resolveNonArrayType(AsmReferenceResolver.java:79)
        at org.codehaus.groovy.ast.decompiled.AsmReferenceResolver.resolveType(AsmReferenceResolver.java:70)
        at org.codehaus.groovy.ast.decompiled.MemberSignatureParser.createMethodNode(MemberSignatureParser.java:50)
        at org.codehaus.groovy.ast.decompiled.DecompiledClassNode.lambda$createMethodNode$1(DecompiledClassNode.java:230)
        at org.codehaus.groovy.ast.decompiled.DecompiledClassNode.createMethodNode(DecompiledClassNode.java:236)
        at org.codehaus.groovy.ast.decompiled.DecompiledClassNode.lazyInitMembers(DecompiledClassNode.java:203)
        at org.codehaus.groovy.ast.decompiled.DecompiledClassNode.getDeclaredMethods(DecompiledClassNode.java:122)
        at org.codehaus.groovy.ast.ClassNode.getDeclaredMethods(ClassNode.java:850)
        at org.codehaus.groovy.ast.ClassNode.getMethods(ClassNode.java:865)
        at org.codehaus.groovy.transform.stc.StaticTypeCheckingVisitor.findMethodsWithGenerated(StaticTypeCheckingVisitor.java:4650)
        at org.codehaus.groovy.transform.stc.StaticTypeCheckingVisitor.findMethod(StaticTypeCheckingVisitor.java:4741)
        at org.codehaus.groovy.transform.stc.StaticTypeCheckingVisitor.visitMethodCallExpression(StaticTypeCheckingVisitor.java:3439)
        at org.codehaus.groovy.transform.sc.StaticCompilationVisitor.visitMethodCallExpression(StaticCompilationVisitor.java:414)
        at org.codehaus.groovy.ast.expr.MethodCallExpression.visit(MethodCallExpression.java:76)
        at org.codehaus.groovy.ast.CodeVisitorSupport.visitExpressionStatement(CodeVisitorSupport.java:117)
        at org.codehaus.groovy.ast.ClassCodeVisitorSupport.visitExpressionStatement(ClassCodeVisitorSupport.java:200)
        at org.codehaus.groovy.transform.stc.StaticTypeCheckingVisitor.visitExpressionStatement(StaticTypeCheckingVisitor.java:2148)
        at org.codehaus.groovy.ast.stmt.ExpressionStatement.visit(ExpressionStatement.java:40)
        at org.codehaus.groovy.ast.CodeVisitorSupport.visitBlockStatement(CodeVisitorSupport.java:86)
        at org.codehaus.groovy.ast.ClassCodeVisitorSupport.visitBlockStatement(ClassCodeVisitorSupport.java:164)
        at org.codehaus.groovy.transform.stc.StaticTypeCheckingVisitor.visitBlockStatement(StaticTypeCheckingVisitor.java:3943)
        at org.codehaus.groovy.ast.stmt.BlockStatement.visit(BlockStatement.java:69)
        at org.codehaus.groovy.ast.ClassCodeVisitorSupport.visitClassCodeContainer(ClassCodeVisitorSupport.java:138)
        at org.codehaus.groovy.ast.ClassCodeVisitorSupport.visitConstructorOrMethod(ClassCodeVisitorSupport.java:111)
        at org.codehaus.groovy.transform.stc.StaticTypeCheckingVisitor.visitConstructorOrMethod(StaticTypeCheckingVisitor.java:2137)
        at org.codehaus.groovy.ast.ClassCodeVisitorSupport.visitMethod(ClassCodeVisitorSupport.java:106)
        at org.codehaus.groovy.transform.stc.StaticTypeCheckingVisitor.startMethodInference(StaticTypeCheckingVisitor.java:2567)
        at org.codehaus.groovy.transform.stc.StaticTypeCheckingVisitor.visitMethod(StaticTypeCheckingVisitor.java:2530)
        at org.codehaus.groovy.transform.sc.StaticCompilationVisitor.visitMethod(StaticCompilationVisitor.java:239)
        at org.codehaus.groovy.ast.ClassNode.visitMethods(ClassNode.java:1099)
        at org.codehaus.groovy.ast.ClassNode.visitContents(ClassNode.java:1092)
        at org.codehaus.groovy.ast.ClassCodeVisitorSupport.visitClass(ClassCodeVisitorSupport.java:52)
        at org.codehaus.groovy.transform.stc.StaticTypeCheckingVisitor.visitClass(StaticTypeCheckingVisitor.java:410)
        at org.codehaus.groovy.transform.sc.StaticCompilationVisitor.visitClass(StaticCompilationVisitor.java:197)
        at org.codehaus.groovy.transform.sc.StaticCompileTransformation.visit(StaticCompileTransformation.java:67)
        at org.codehaus.groovy.transform.ASTTransformationVisitor.visitClass(ASTTransformationVisitor.java:143)
        at org.codehaus.groovy.transform.ASTTransformationVisitor.lambda$addPhaseOperations$2(ASTTransformationVisitor.java:220)
        at org.codehaus.groovy.control.CompilationUnit$IPrimaryClassNodeOperation.doPhaseOperation(CompilationUnit.java:942)
        ... 34 more

It looks like some issue related to guava's ImmutableList, but I don't understand why it's impacting the compilation of our plugin since ImmutableList is not in this plugin's public API (only exposed as private or package-private).

Removing @CompileStatic resolves the issue, however we'd rather not lose the static compilation performance benefit.

@ejona86
Copy link
Collaborator

ejona86 commented Jun 30, 2022

0.8.19 swapped to using maven-publish which "hides" implementation dependencies by making them "runtime" scope instead of "compile" scope. Only API dependencies are available to consumers without adding a direct dependency.

I expect this problem can be fixed by adding an explicit dependency on Guava. But I'm not sure if that is a fix or a workaround. If you use Guava in your plugin and don't have an explicit dependency on it then that would be the proper fix. If you don't depend on Guava, then it is a workaround. I'd normally have expected a different error message, although we're also dealing with Groovy here so harder to say.

It might be that ImmutableList is on our API surface without us realizing. I see GenerateProtoTask has a package-private API that uses ImmutableList. In Java-land that may not cause any trouble, but it seems Groovy may be expecting the class to be available.

@tinder-dthomson
Copy link
Author

I figured this might be a Groovy-specific problem. We only wrote our plugin in Groovy for a more seamless integration with this plugin to be honest.

We don't depend on Guava, but considering it is part of this plugin's dependencies already it won't be a harmful workaround.

Still, we'd rather not add dependencies that we don't actually use in our plugin's implementation. This can/will cause confusion for anyone who doesn't know about this issue.

Any chance we can look at either making guava an api dependency, or perhaps fix the Groovy scope issue?

@tinder-dthomson
Copy link
Author

tinder-dthomson commented Jun 30, 2022

@ejona86 I just did a wee bit of Groovy research, and it looks like annotating the package-private methods with @PackageScope may resolve the issue. It appears Groovy will automatically convert package-private methods to public.

https://docs.groovy-lang.org/3.0.9/html/gapi/groovy/transform/PackageScope.html

@ejona86
Copy link
Collaborator

ejona86 commented Jun 30, 2022

We don't depend on Guava

Okay, then it is a Groovy-ism. We can either swapping that internal API to List or make Guava an API dependency.

It isn't outlandish to remove the ImmutableLists entirely. We only use some immutable collections and preconditions from Guava, and we have considered dropping the Guava dependency before because Gradle doesn't put plugins in their own classloaders.


@PackageScope? Wat. Oooh. Yeah, that was the intention. We're really not Groovy programmers :-). That's an easy solution.

@ejona86
Copy link
Collaborator

ejona86 commented Jun 30, 2022

In case it wasn't clear, I suggest adding the Guava dependency for now and the next release can fix this up.

@tinder-dthomson
Copy link
Author

Appreciate, and thanks for the responsiveness! We'll go with this workaround and wait for 0.8.20.

@ejona86 ejona86 changed the title Custom Plugin using @CompileStatic fails to compile when using version 0.8.19 as a dependency 0.8.19 exposes Guava in a Groovy API but has it as an implementation dependency Jun 30, 2022
ejona86 added a commit to ejona86/protobuf-gradle-plugin that referenced this issue Sep 26, 2022
We like Guava, but the plugin has little need for it. As a side-benefit
dropping it avoids systems-not-handling-classpaths-correctly issues
like google#247. We wouldn't drop Guava just for that reason (as the ecosystem
should fix its bugs), but instead of just removing it from the API
surface, let's remove it entirely. Similarly, we could continue using it
in our tests, but we really don't need it.

Fixes google#572
ejona86 added a commit that referenced this issue Oct 2, 2022
We like Guava, but the plugin has little need for it. As a side-benefit
dropping it avoids systems-not-handling-classpaths-correctly issues
like #247. We wouldn't drop Guava just for that reason (as the ecosystem
should fix its bugs), but instead of just removing it from the API
surface, let's remove it entirely. Similarly, we could continue using it
in our tests, but we really don't need it.

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

Successfully merging a pull request may close this issue.

2 participants