gradle / gradle-native

The home of Gradle's support for natively compiled languages
https://blog.gradle.org/introducing-the-new-cpp-plugins
Apache License 2.0
92 stars 8 forks source link

Allow plugin authors to specify default values for Property #918

Closed big-guy closed 5 years ago

big-guy commented 5 years ago

Plugins often want to supply a default value for a property, but allow the user to override it.
Other plugins may want to supply a convention value for a property that overrides the original plugin's default but still allow the user to override it.

Both of these use cases can be very sensitive to ordering problems.

Simple default

Here's a simple example of a plugin supplying a task, configuring some defaults and a build script trying to override it.

myplugin code:

tasks.register("mytask", MyTask) {
    prop.set("plugin value")
}

Build script code:

tasks.withType(MyTask).configureEach {
    prop.set("build script value")
}
apply plugin: 'myplugin'

mytask.prop ends up with the value "plugin value", which isn't what the build script author wanted or the plugin author intended. This leads some people to try to defer setting things:

Possible fix #1 in build script code:

apply plugin: 'myplugin' // OK, as long as this is never moved after the withType below.
tasks.withType(MyTask).configureEach {
    prop.set("build script value")
}

Possible fix #2 in build script code, just slap a afterEvaluate in there:

afterEvaluate {
    tasks.withType(MyTask).configureEach {
        prop.set("build script value")
    }
}
apply plugin: 'myplugin'

Possible fix #3 in build script code, overkill/reconfigure the task in the execution phase:

apply plugin: 'myplugin' 
task configureMyTask { 
    doLast {
        tasks.mytask.prop.set("build script value")
    }
}
tasks.withType(MyTask).configureEach {
    dependsOn configureMyTask
}

Possible fix #1 plugin code:

tasks.register("mytask", MyTask) {
    // OK, as long as prop isn't expensive to determine or require other things to be set first
    if (prop.getOrNull() == null) {
        prop.set("plugin value")
    }
}

There may be some other clever things you could do in the plugin to ensure a particular "configure first" action happened before all others.

Plugin default + convention default

myplugin code:

tasks.register("mytask", MyTask) {
    prop.set("plugin value")
}

convention code:

apply plugin: 'myplugin'
tasks.withType(MyTask).configureEach {
    prop.set("convention value")
}

Build script code:

apply plugin: 'convention'
tasks.withType(MyTask).configureEach {
    prop.set("build script value")
}

This works because the appropriate ordering is maintained, but has the same kind of ordering problems as the simple default. Depending on the ordering between myplugin, convention and the build script, you can get mytask.prop to have either "convention value" or "build script value". A poorly written convention plugin could force "convention value" or even let "plugin value" slip through as the default. If you imagine this combined with the possible fixes from above, it's easy to make a property that is difficult to configure properly.

defaults API

All of the sets from plugins above would be replaced with defaults (or better name):

myplugin code:

tasks.register("mytask", MyTask) {
    prop.defaults("plugin value")
}

convention code:

apply plugin: 'myplugin'
tasks.withType(MyTask).configureEach {
    prop.defaults("convention value")
}

Build script code:

apply plugin: 'convention'
tasks.withType(MyTask).configureEach {
    prop.set("build script value")
}

TBD fully flesh this out.

big-guy commented 5 years ago

This is .convention() and is in 5.1