This commit changes how the Spring Boot Gradle plugin identifies
other plugins to react to. Before, it was using the withType
method, which requires to know the plugin class. There are a few
drawbacks to this approach:
- one has to figure out what the "main plugin class" is
- the plugin class might not even be on classpath, if the plugin isn't bundled by Gradle
- the plugin class might not be available anymore, for example
Gradle 7 removes the
MavenPlugin
class
Therefore, the code has been updated to use the withPlugin
method which instead reacts on a plugin id. By using the id
we avoid eagerly loading classes and failing if they are not
available.
This should fix the compatibility with Gradle 7 even if this plugin is still built with an older version of Gradle.
Comment From: wilkinsona
Thanks, @melix. I've been mulling this one over a bit since we chatted about it yesterday and I'm not sure about using the string ID. It feels a bit less safe than using the class as we're now hardcoding knowledge of an ID that isn't as obviously tied to a specific plugin. Is there not also a chance that someone creates their own plugin with the same ID as we're looking for and we'd then incorrectly react to it being applied?
As an alternative solution, I'm wondering about making MavenPluginAction
return null
when org.gradle.api.plugins.MavenPlugin
can't be loaded as KotlinPluginAction
currently does for org.jetbrains.kotlin.gradle.plugin.KotlinPluginWrapper
.
Comment From: melix
It feels a bit less safe than using the class as we're now hardcoding knowledge of an ID that isn't as obviously tied to a specific plugin.
It is certainly less type-safe. However I wouldn't consider this unsafe.
Is there not also a chance that someone creates their own plugin with the same ID as we're looking for and we'd then incorrectly react to it being applied?
Yes, there is this risk. However, a plugin id is, as its name implies, an id which should be unique. It's quite unlikely that one would write a plugin which clashes with the ids you're looking for. If they do, then it would be probably to pretend to be the same plugin.
As an alternative solution, I'm wondering about making MavenPluginAction return null when org.gradle.api.plugins.MavenPlugin can't be loaded
Yes, this is another option. In general I'm not super fan of try/catch exception flows but it's your call, both solutions work and have their own downsides :)
Comment From: wilkinsona
Thanks again, @melix. I'd prefer to continue with an approach that's similar to the one already taken in KotlinPluginAction
. I've opened https://github.com/spring-projects/spring-boot/issues/24526 to do that.