Don't use evaluationDependsOnChildren
oehme opened this issue · 18 comments
I can't see why this plugin would need it and it will be highly confusing to users that the plugin reverses the normal evaluation order.
@oehme Thanks. I wonder if that explains jaredsburrows/gradle-license-plugin#25 because of evaluationDependsOnChildren here: https://github.com/jaredsburrows/gradle-license-plugin/blob/master/src/main/groovy/com/jaredsburrows/license/LicensePlugin.groovy#L11.
Not sure, but the checks for other plugins shouldn't be done imperatively, but with a callback. I.e. don't do this:
if (project.plugins.hasPlugin('somePlugin') {
}
instead use
project.plugins.withId('somePlugin') {
}
That way you no longer depend on the order the plugins are applied.
Thank you. I use hasPlugin in order to determine whether or not to through an exception.
Why throw an exception? Just do nothing :)
A plugin really shouldn't depend on order. It should either apply the other plugins it needs or it should react to others via a callback.
That is the thing. This plugin in particular is for java or android projects only. Android has 3 main plugins: "com.android.applicaiton", "com.android.library" and "com.android.test". I believe there are others as well for instant apps and the architecture components. When my plugins access the Android Gradle Plugin's DSL via android { }, it will throw an different exception. Or are you saying, simply do not "configure" the plugin if it is not compatible?
Exactly, only react to the plugins you care about and be a noop otherwise. Or if there is a plugin you are definitely incompatible with (and want people to know explicitly) you can react to it and throw an exception in the callback.
Right now your plugin only works if applied after the others. Plugins should not depend on order.
Ok. I removed evaluationDependsOnChildren. I will look into throwing an exception later down the line.
@oehme Where would you suggest a throw an exception in this checker framework plugin?
I am guessing for this plugin, it will just fail if Java plugin is not applied. For Android projects, it will just not work and the AGP plugin will throw an exception if it not setup properly. I may just be able to remove the checks entirely from this plugin.
I'm not sure what you mean. If you just do
plugins.withId('java') { //this covers groovy and java-library, they both apply the java plugin
//do Java specific config
}
plugins.withId('android-application') {
//do Android specific config
}
plugins.withId('android-library') {
//do Android specific config
}
everything will work as expected.
@oehme What is both android-application and android-library configurations are the same? See : https://github.com/jaredsburrows/gradle-license-plugin/blob/master/src/main/groovy/com/jaredsburrows/license/LicensePlugin.groovy#L12.
Since withId returns void, I would have to do the following:
plugins.withId('java') {
configureJava(project)
}
plugins.withId('android-application') {
configureAndroid(project)
}
plugins.withId('android-library') {
configureAndroid(project)
}
Yeah or have the closure be a variable and pass it to both.
Btw I don't see anything Android specific in the checker plugin. The only line wrapped in if (android) is correct for the Java plugin too. In fact the code could be applied regardless of any plugin and would be correct since you are only reconfiguring Java compile tasks to have a bigger bootclasspath and processor path.
@oehme Something like?
private def configureAndroid(def project) {
configureAndroidProject(project)
}
@Override void apply(Project project) {
project.plugins.withId("com.android.application", configureAndroid(project))
project.plugins.withId("java") {
configureJavaProject(project)
}
}
@oehme I was able to find a better way. The file com.android.base.properties in com.android.build.tools:gradle-core which is apart of com.android.build.tools:gradle.
package com.android.build.gradle.api;
import org.gradle.api.Plugin;
import org.gradle.api.Project;
/**
* Common plugin applied by all plugins.
*
* <p>The purpose of this no-op plugin is to allow other plugin authors to determine if an Android
* plugin was applied.
*/
public class AndroidBasePlugin implements Plugin<Project> {
@Override
public void apply(Project project) {}
}
I can now simplify this to:
@Override void apply(Project project) {
project.plugins.withId("com.android.base") { configureAndroidProject(project) }
project.plugins.withId("java") { configureJavaProject(project) }
}
Even better! :)
Sorry to hijack this, I think this was a very fruitful discussion. @oehme Is there a way to negate the withId check, i.e. run code only if a subproject does NOT have a specific plugin? Currently I need to use evaluationDependsOnChildren() in the root project because of this.
project.plugins.withoutId("java-test-fixtures") { // note: withOUT
// do something after the subprojects build.gradle has been evaluated
}