Plugins should not access a task's project at execution time

[abstratt]

Plugins should not access a task's project at execution time. This will be deprecated in 8.12, and will be forbidden in a future release. One case:

https://github.com/bndtools/bnd/blob/4550adf924c46d7c5860bbedadc22ea8f946dc11/gradle-plugins/biz.aQute.bnd.gradle/src/main/java/aQute/bnd/gradle/AbstractBndrun.java#L345

Invocation of Task.project at execution time has been deprecated. This will fail with an error in Gradle 9.0. Consult the upgrading guide for further information: https://docs.gradle.org/8.12-20241030030000+0000/userguide/upgrading_version_7.html#task_project
        ...
	at org.gradle.api.internal.AbstractTask.getProject(AbstractTask.java:238)
	at org.gradle.api.DefaultTask.getProject(DefaultTask.java:59)
	at aQute.bnd.gradle.AbstractBndrun.lambda$bndrunAction$3(AbstractBndrun.java:345)
	at java.base/java.util.concurrent.ConcurrentHashMap.computeIfPresent(ConcurrentHashMap.java:1828)
	at java.base/java.util.Properties.computeIfPresent(Properties.java:1459)
	at aQute.bnd.gradle.AbstractBndrun.bndrunAction(AbstractBndrun.java:345)
        ...

Context: https://github.com/gradle/gradle/issues/30860

[pkriens]

@bjhargrave any idea how much work this will be?

[bjhargrave]

This has long been a known concern and there is a workaround. See https://github.com/bndtools/bnd/tree/master/gradle-plugins#gradle-configuration-cache-support

Certain users of Bnd need access to the open-ended properties in the Gradle Project object.

One can do:

tasks.named("jar") {
  bundle {
    properties.put("project.group", provider({project.group}))
  }
}

for each Gradle Project property they need to use. But sometimes the list of needed properties is not obvious due to Bnd's support for including other bnd files and macros, etc. So this could be a trail-and-error process which can break when a user modifies a bnd file and not the gradle build file.

A possible solution could be to change:

https://github.com/bndtools/bnd/blob/3f2c1d34ddb2e2b1dc778f55f4abc7fade812daa/gradle-plugins/biz.aQute.bnd.gradle/src/main/java/aQute/bnd/gradle/AbstractBndrun.java#L292

to something like:

MapProperty<String, Object> projectProperties = objects.mapProperty(String.class, Object.class)
   .convention(project.provider(() -> project.getProperties()));
properties.convention(Maps.of("project", projectProperties));

This would require the projectProperties MapProperty to fully collect all the Gradle Project properties when the provider is called. I don't know if it does an exhaustive collection or only collects the keys used during configuration (which are zero in this case since the MapProperty is not called until execution time). Since Gradle Projects have dynamic properties, I am not confident this will happen. But perhaps you can shed some light on this and suggest how to collect all Gradle Project properties at (the end of) configuration time so their values could be used at execution time.

[pkriens]

@abstratt did you notice the question from @bjhargrave ?

But perhaps you can shed some light on this and suggest how to collect all Gradle Project properties at (the end of) configuration time so their values could be used at execution time.

[abstratt]

Actually, such provider would be evaluated at configuration (store) time. However, the value it returns must contain only objects supported by the configuration cache. Project.properties includes many objects that are not supported by the configuration cache, so you would need to somehow extract only values that are supported.

[abstratt]

At least you have a workaround. But please beware that (unless you change your implementation's default behavior) builds will soon start producing deprecation warnings if your users do not set the task's own properties property to some value. Maybe falling back to project properties should be an opt-in, not the default behavior?

[pkriens]

I am not sure if I am getting this correctly

• (Some) bnd files refer to Gradle project information,
• To support this, we have code that will soon create a warning,
• This is (should) rarely be used.

Although I can see the utility, the fact that you make your build depend on Gradle seems off. These features won't work in Eclipse, IDEA, or the bnd command line.

I think I agree that we should remove the default code It seems to make sense to me that a project that uses this feature will somehow transfer these properties from the Gradle project to the bnd project?

I am a nitwit in gradle but could we do this in a build.gradle file in the project that needs this information? Or does this need to be done in the root build.gradle?

We're planning to start releasing 7.1 so it would be appreciated if we could expedite this. It would be awkward if all the builds started to have warnings.

[bjhargrave]

@pkriens This issue is not for Bnd Workspace builds. It is for standard gradle builds where the gradle project uses the biz.aQute.bnd.builder gradle plugin. This type of use can be with a bnd file or with bnd configuration in the project's build.gradle file. Such bnd configuration is used to build a bundle in the gradle project and the bundle may want to reference standard gradle project properties like project.name, project.version, project.description. This is why the default behavior was to use the real gradle project object (DRY) instead of requiring the developer to duplicate this information in the bnd configuration.

See some examples:

https://github.com/junit-team/junit5/blob/2be6e9e396b7b1e7be6a3b5933c62e0b8599b3f2/gradle/plugins/common/src/main/kotlin/junitbuild.osgi-conventions.gradle.kts#L35

https://github.com/bndtools/bnd/blob/7d340b35175e26112ce35b1e76d365d9c186b677/gradle-plugins/biz.aQute.bnd.gradle/testresources/builderplugin2/build.gradle#L50-L52

[bjhargrave]

I will note that JUnit5 example already uses the workaround to avoid using the gradle project object at execution time.

https://github.com/junit-team/junit5/blob/2be6e9e396b7b1e7be6a3b5933c62e0b8599b3f2/gradle/plugins/common/src/main/kotlin/junitbuild.osgi-conventions.gradle.kts#L21-L23

[pkriens]

ah, makes sense.

Might be stupid but can't the macro expansion happen at configuration time?

[bjhargrave]

Might be stupid but can't the macro expansion happen at configuration time?

Some could happen but it is difficult to know which bnd macros can be resolved early during gradle configuration time and which bnd macros depend upon information calculated by bnd during gradle execution time when bnd build is done. The bnd gradle plugin would need a feature in bndlib which determines all macros whose value is not affected by build time calculations. Then the bnd gradle plugin could resolve the values for those macros and insert them into the properties map at the end of gradle configuration.

The current workaround asks the user to make this determination and load the values into the properties map.

[bjhargrave]

Also note that the same issue exists for bndruns (run, test, resolve, export) as well as building. The original comment referenced bndrun but I was mostly thinking about building. So I just wanted to highlight that in both cases this issue is present.

[pkriens]

still not completely getting it .. sorry. @abstratt warns us that we will start creating deprecation warnings and finally it won't work anymore. To be sure, if they get this warning they can prevent the warning with the workaround?

So we have nothing to do?

[pkriens]

Ok, the plan is:

1. I will add a feature to the Processor to get all referenced properties
2. In the coming months @bjhargrave will update the gradle plugin code
3. I will add some text in the description of the workaround

So it will not be in 7.1

[chrisrueger]

Note to remember: In PR https://github.com/bndtools/bnd/pull/6558/files#diff-22fee05083d1c220d596231f9101353197f9a5ec1b738d8d562af46a27fdcc32 I changed REBUILD from --warning-mode=fail to --warning-mode=all which does not fail the build, because of a "Deprecated gradle feature used" warning.

In case PR #6558 is merged before this PR, after this PR here has been fixed we should revert it back to --warning-mode=fail

[chrisrueger]

@bjhargrave Do you plan to work on this? Or should we close?

[bjhargrave]

I would like to work on it. I just keep being otherwise busy... :-/

We need to do this. So leave it open.

[chrisrueger]

Sure, we went over all issues in the bnd call. thanks for getting back @bjhargrave

[abstratt]

I am not sure yet whether the scope would be a good fit for our Google Summer of Code contributor (need to review the scope of changes required), but we are looking into contributing CC fixes for community plugins. @bjhargrave Would you be interested in an external contribution for this?

[bjhargrave]

I think this is non-trivial but go ahead and take a go at it. I can review a PR.

[abstratt]

Thanks, BJ. We decided to focus on more trivial plugins in the context of the Summer of Code project. Sorry for the distraction.

On the brioght side of things, the Gradle Build Tool team is looking into making it easier for plugin authors to make their plugins CC-compatible (could be just documentation, possibly more, such as new APi).

[ejjcase]

@chrisrueger asked me to have a look at this.

It looks like this is what we have to do in the Gradle tasks:

• At configuration time, load the bnd/bndrun file together with its include chain. I can see this happening in AbstractBndrun with biz.aQute.resolve.Bndrun.createBndrun(Workspace, File), or maybe we just create a Processor. Handle errors appropriately, e.g. syntax errors, missing included files, cycles in the chain.
• Call Processor.getMacroReferences(…) to get a set of property names. I guess we're looking for the UNKNOWN ones because if it's already known to Bnd then Bnd doesn't want to get it from Gradle, or is Gradle allowed to override Bnd properties? Or do we only care about the ones beginning with project.?
• For each property name, find its Gradle property value. Discard the values whose types are incompatible with the configuration cache, and any properties that are unset.
• Use these properties and values at runtime instead of going to the project. Also take whatever steps to get them into the configuration cache.

Does that sound right?

If we're going to load the include chain at configuration time then there's something else I would like to fix: all the files in the chain should be Gradle task inputs.

[ejjcase]

I'm working on this today. Observing how it currently works:

• Property values that are not strings can go in bundle.properties in the Gradle config. The configuration cache can still work as long as they are Serializable.
• They can be quoted in macro references, but not in deep macro references. For example, if I add a java.net.URI as the value of website.uri then I can quote ${website.uri} but not ${website.uri.path}.
• Macro references in properties that are set in the Gradle config are honoured just like ones in bnd.bnd.

So I want to preserve the handling of macro references in properties in BundleTaskExtension.getProperties(), but I guess I don't need to worry about letting a BeanProperties see inside complex objects in BundleTaskExtension.getProperties() the way it can see inside the Project and Task.

Does that sound right?

[chrisrueger]

Thanks @ejjcase . @bjhargrave can you help here?

[laeubi]

@ejjcase as everyone seems quite busy I now took the opportunity to spin-up a task for the AI and hope it will reveal some helpful insights for you to work on the topic (it will take some time to complete):

• https://github.com/laeubi/bnd/pull/6

in general I think, give it a try, change some code, push the changes and we will see if anything breaks... that probably the best way forward!

[bjhargrave]

I am wrapping up a fix for this. Will make a PR shortly.

[ejjcase]

I am wrapping up a fix for this. Will make a PR shortly.

I see I should have been more communicative about what I was doing yesterday. Didn't want to push it before it was ready but here's what I have so far.

https://github.com/bndtools/bnd/pull/6914