-
Notifications
You must be signed in to change notification settings - Fork 274
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
Comments
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. |
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 |
@ejona86 I just did a wee bit of Groovy research, and it looks like annotating the package-private methods with https://docs.groovy-lang.org/3.0.9/html/gapi/groovy/transform/PackageScope.html |
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.
|
In case it wasn't clear, I suggest adding the Guava dependency for now and the next release can fix this up. |
Appreciate, and thanks for the responsiveness! We'll go with this workaround and wait for |
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
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
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:
Up until
0.8.19
, we were able to annotate thePlugin
implementation class with@CompileStatic
as shown above. As of0.8.19
, this is no longer working, with the following exception:It looks like some issue related to guava's
ImmutableList
, but I don't understand why it's impacting the compilation of our plugin sinceImmutableList
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.The text was updated successfully, but these errors were encountered: