bndtools / bnd

Bnd/Bndtools. Tooling to build OSGi bundles including Eclipse, Maven, and Gradle plugins.
https://bndtools.org
Other
527 stars 305 forks source link

Tasks extending from AbstractBndrun are not Configuration Cache compatible #4919

Closed leonard84 closed 2 years ago

leonard84 commented 2 years ago

Tasks extending from AbstractBndrun are not compatible with Gradle's Configuration Cache, as they access the project during execution (The link contains information how to fix this problem).

bjhargrave commented 2 years ago

This is basically a dup of #4682.

As stated in that issue, the Bnd Gradle plugins can't support the Gradle configuration cache.

The tasks extending from AbstractBndrun need the Gradle Project and Task objects to be able to evaluate ${project.xxx} and ${task.xxx} properties in the bndrun file at execution time. Since Bnd cannot know what ${project.xxx} and ${task.xxx} properties may need to be evaluated until execution time (since the bndrun file cannot be evaluated at configuration time), it cannot collect all such needed values at configuration time for later use at execution time (in order to avoid the need for the project and task objects at execution time).

But perhaps you may have some advice here, as a Gradle maintainer, on how this may be achieved while supporting the configuration cache.

leonard84 commented 2 years ago

I seem to have overlooked the closed issue, apologies for that.

The suggested way to deal with this, is to map all those properties to task inputs, which you can then query in the run. I know that it is a bit of boilerplate code, but it should be achievable. On execution you could construct a simple pojo to hold the project properties that your need, if you still want to refer to project.name instead of task.projectName.

abstract MyTask extends DefaultTask

  @Input
  Property<String> getProjectName();

  MyTask() {
    getProjectName().convention(project.getName());
  }
}
bjhargrave commented 2 years ago

On execution you could construct a simple pojo to hold the project properties that your need,

But the Bnd Gradle plugin cannot know, at configuration time, the properties that are needed at execution time when Bnd must resolve the macro references in the bndrun file like ${project.customUserDefinedProjectProperty}.

A custom project property example:

https://github.com/bndtools/bnd/blob/9a8938460018e1a74c7b9aa0fa1045aac893fad2/gradle-plugins/biz.aQute.bnd.gradle/testresources/exporttask1/build.gradle#L24-L26

https://github.com/bndtools/bnd/blob/9a8938460018e1a74c7b9aa0fa1045aac893fad2/gradle-plugins/biz.aQute.bnd.gradle/testresources/exporttask1/export.bndrun#L3

This support is expected and used by Bnd gradle plugin users so they can centralize configuration in the build.gradle file.

rotty3000 commented 2 years ago

It would be annoying and error prone to have to duplicate all this information in two places; once in the bnd macros and again in the plugin configuration.

leonard84 commented 2 years ago

You could do the following to keep the old behavior, while still allowing those who want to use configuration caching and proper up-to-date checks to add some extra configuration to get those benefits.

Add a MapProperty<String, Object> templateParameters (or choose a different name) to the AbstractBndrun task. You can even prefill it with common values from the project, e.g. templateParameters.put('projectName', project.getName()).

To make the transition easier, I would also use a StrictMap that throws when an unknown key is requested. This way, users can figure out if they try to access unavailable data instead of just getting null.

Add a switch to the task, Property<Boolean> safeConfiguration which will affect the available template binding parameters.

You would probably have to either enhance BeanProperties to deal with Map fields, or just use the map values as property values, loosing the strict behavior, but reducing the complexity as well.

Now users can choose to optimize their build if they desire, or they can use the old behavior if they don't care.

bjhargrave commented 2 years ago

See https://github.com/bndtools/bnd/blob/master/gradle-plugins/README.md#gradle-configuration-cache-support for information on how to configure the Builder plugin for the Configuration Cache.

leonard84 commented 2 years ago

Thanks @bjhargrave 👍