Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
abstratt opened this issue Oct 31, 2024 · 13 comments
Open

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

abstratt opened this issue Oct 31, 2024 · 13 comments
Assignees

Comments

@abstratt
Copy link

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:

gradleProperties.computeIfPresent("project", (k, v) -> "__convention__".equals(v) ? getProject() : v);

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: gradle/gradle#30860

@pkriens
Copy link
Member

pkriens commented Oct 31, 2024

@bjhargrave any idea how much work this will be?

@bjhargrave
Copy link
Member

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:

properties.convention(Maps.of("project", "__convention__"));

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
Copy link
Member

pkriens commented Nov 4, 2024

@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
Copy link
Author

abstratt commented Nov 5, 2024

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
Copy link
Author

abstratt commented Nov 5, 2024

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
Copy link
Member

pkriens commented Nov 5, 2024

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
Copy link
Member

@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

bnd '''
Bundle-Name: ${project.group}:${task.archiveBaseName}-${task.archiveClassifier}
'''

@bjhargrave
Copy link
Member

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
Copy link
Member

pkriens commented Nov 6, 2024

ah, makes sense.

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

@bjhargrave
Copy link
Member

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
Copy link
Member

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
Copy link
Member

pkriens commented Nov 6, 2024

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
Copy link
Member

pkriens commented Nov 6, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants