eclipse.platform icon indicating copy to clipboard operation
eclipse.platform copied to clipboard

Add IContextFunction.ServiceContextKey OSGi component property type

Open HannesWell opened this issue 1 year ago • 1 comments

This annotation simplifies the specification of the 'service.context.key' service property for IContextFunction implementations and makes it type-safe and more robust:

@Component(service = IContextFunction.class)
@IContextFunction.ServiceContextKey(IProgressService.class)
public class ProgressServiceCreationFunction extends ContextFunction {
...

HannesWell avatar Oct 01 '24 21:10 HannesWell

Test Results

 1 758 files  ±0   1 758 suites  ±0   1h 29m 23s :stopwatch: - 3m 54s  4 170 tests ±0   4 148 :white_check_mark: ±0   22 :zzz: ±0  0 :x: ±0  13 107 runs  ±0  12 943 :white_check_mark: ±0  164 :zzz: ±0  0 :x: ±0 

Results for commit 12af7b71. ± Comparison against base commit 7fa51b7a.

:recycle: This comment has been updated with latest results.

github-actions[bot] avatar Oct 01 '24 21:10 github-actions[bot]

@laeubi can you tell why this build has compilation errors and how to fix them?

The existing AdapterTypes, that is the other ComponentPropertyType in the SDK, also seems to compile without any special setup: https://github.com/eclipse-equinox/equinox/blob/19b406bea5b340cee4009cd9f2c358f23b3840da/bundles/org.eclipse.equinox.common/src/org/eclipse/core/runtime/AdapterTypes.java#L44

HannesWell avatar Oct 12 '24 08:10 HannesWell

The existing AdapterTypes, that is the other ComponentPropertyType in the SDK, also seems to compile without any special setup:

Maybe because it is a top-level class 😛

Beside from that I would assume that enable DS Annotations for the project could solve the issue.

laeubi avatar Oct 12 '24 09:10 laeubi

The existing AdapterTypes, that is the other ComponentPropertyType in the SDK, also seems to compile without any special setup:

Maybe because it is a top-level class 😛

I expected that answer :P And therefore checked Tycho's DeclarativeServicesClasspathContributor, where I didn't see any sign that the class is scanned at all. Furthermore I assumed that one first has to compile the class to be able to scan it for annotations, unless one operates only on the AST.

Beside from that I would assume that enable DS Annotations for the project could solve the issue.

I thought about this as well, but at least in org.eclipse.equinox.common I didn't find any sign of such configuration, neither in the project files nor in a parent pom. And actually the project does not contain an DS components, it just needs the classpath contribution.

HannesWell avatar Oct 12 '24 10:10 HannesWell

I thought about this as well, but at least in org.eclipse.equinox.common I didn't find any sign of such configuration, neither in the project files nor in a parent pom. And actually the project does not contain an DS components, it just needs the classpath contribution.

It might pull it in by a transitive dependency (that uses DS annotations), and because DeclarativeServicesClasspathContributor actually looks for such configuration at the moment.

laeubi avatar Oct 12 '24 10:10 laeubi

I thought about this as well, but at least in org.eclipse.equinox.common I didn't find any sign of such configuration, neither in the project files nor in a parent pom. And actually the project does not contain an DS components, it just needs the classpath contribution.

It might pull it in by a transitive dependency (that uses DS annotations), and because DeclarativeServicesClasspathContributor actually looks for such configuration at the moment.

I still have not found out why it works in o.e.equinox.common, but adding the pde.ds preference file indeed fixes the build failure. 🎉 Thanks for the hints.

Therefore this is now ready for submission. Since we have a 2:1 result pro inner-class in above's discussion I plan to submit this as it is this evening unless somebody has strong new arguments. We can the finally complete https://github.com/eclipse-platform/eclipse.platform.ui/pull/2344.

HannesWell avatar Oct 12 '24 13:10 HannesWell