Recommend OSGi EventHandler for APP_STARTUP_COMPLETE over IStartup

[HannesWell]

And migrate MonitoringStartup accordingly.

This also uses the org.osgi.service.event.propertytypes.EventTopics ComponentPropertyType for which PDE has very recently got support for via eclipse-pde/eclipse.pde#923.

Instead of just recommending the OSGi EventHandler over IStartup, we could also deprecate IStartup and the associcate extension-point in favor of this approach? I can then migrate the only remaining usage in the SDK in P2.

[github-actions]

Test Results

     900 files  +       1       900 suites  +1   1h 4m 17s ⏱️ + 28m 22s
  7 468 tests ±       0    7 315 ✔️ +       1  153 💤 ±    0  0 ❌  - 1 
23 553 runs  +1 576  23 044 ✔️ +1 457  509 💤 +120  0 ❌  - 1 

Results for commit e9b5ee4. ± Comparison against base commit 7d88c88.

♻️ This comment has been updated with latest results.

[laeubi]

You can even define a new composite annotation like this

@Component(service = EventHandler.class)
@EventTopics(value = UIEvents.UILifeCycle.APP_STARTUP_COMPLETE)
public @interface StartupComponent {

}

what the can be used like this

Suggested change

	public class MonitoringStartup implements EventHandler {
	@StartupComponent
	public class MonitoringStartup implements EventHandler {

[HannesWell]

If I try to add for example to the UILifeCycle class:

@Component(service = EventHandler.class)
@EventTopics(value = APP_STARTUP_COMPLETE)
public static @interface StartupCompleted {
}

I get the error Invalid component implementation class 'StartupCompleted': annotation type.

[laeubi]

Maybe @Component can't be placed on meta annotation, maybe its just a bug in PDE-DS ...

[sratz]

An IStartups execution can be prevented via Window -> Preferences -> General -> Startup and Shutdown -> Plug-ins activated on startup ('activated' is a bit misleading here, but the list contains all plug-ins contributing an IStartup extension and controls whether those are actually started or not).

This gives users a choice to enable or disable certain startups.

I suppose the OSGI mechanism does not respect the preference page?

In that case I don't think EventHandler can be considered a drop-in replacement over IStartup and IStartup does have its right to exist. The Javadoc should probably be updated to make this distinction of whether an end-user may disable it or not.

[vogella]

I'm in favor of deprecating IStartup and marking it for deletion. The user option to disable IStartup was always very dangerous in my opinion as this would allow the user to disable potential critical infrastructure.

[sratz]

I'm in favor of deprecating IStartup and marking it for deletion. The user option to disable IStartup was always very dangerous in my opinion as this would allow the user to disable potential critical infrastructure.

I'm not sure, I consider the preference a feature, e.g. we have a 'Release notes' feature for our product that is triggered via an IStartup, which users can disable there if they don't like it.

With the two alternatives we now have to chance to separate mandatory/critical startups from optional ones and convert those to OSGI event handlers.

My vote would be to state the fact in the Javadoc of IStartup and give providers the choice.

[laeubi]

I'm not sure, I consider the preference a feature, e.g. we have a 'Release notes' feature for our product that is triggered via an IStartup, which users can disable there if they don't like it.

I think that would be much more suitable to have a custom preference (page) for that that having to go into some generic dialogs.

[vogella]

I'm not sure, I consider the preference a feature, e.g. we have a 'Release notes' feature for our product that is triggered via an IStartup, which users can disable there if they don't like it.

I think that would be much more suitable to have a custom preference (page) for that that having to go into some generic dialogs.

+1

[BeckerWdf]

I'm not sure, I consider the preference a feature, e.g. we have a 'Release notes' feature for our product that is triggered via an IStartup, which users can disable there if they don't like it.

I think that would be much more suitable to have a custom preference (page) for that that having to go into some generic dialogs.

But that's we is out there today. I assume other RCP products may also use this feature. If platform now changes how this behaves: this is incompatible and will force RCP implementers to adapt their code. And what's the value of it?

Besides that: I as a user find it quite nice to have one central place where I can turn off / on such start ups.

[vogella]

FWIW - I have never seen (nor recommended) using IStartup in RCP applications. The ability to disable them, make it impossible to rely on the functionality for the regular use case that "something should start early".

[vogella]

@HannesWell can we apply this? The recommendation is fine for now IMHO. Once we have removed the usage of IStartup from p2 we can discuss to deprecate it, even though maybe not mark it for deletion yet due to the concerns from @BeckerWdf and @sratz. But it might be good to guide new users to use the new approach).

Please also add to N&N so that people can find it.

[HannesWell]

An IStartups execution can be prevented via Window -> Preferences -> General -> Startup and Shutdown -> Plug-ins activated on startup ('activated' is a bit misleading here, but the list contains all plug-ins contributing an IStartup extension and controls whether those are actually started or not).

This gives users a choice to enable or disable certain startups.

Thank you @sratz for mentioning this, I didn't knew it. And yes, at least to my knowledge, one cannot disable the execution of OSGi event handlers.

And like Lars I would consider this, at least in general, a dangerous feature if it is used for critical tasks. And since it was not mention in the doc, I'm not sure if everyone that uses it is aware of it (I wasn't).
Like Christoph suggested I think it would be more suitable to have dedicated and explicit preferences for tasks that are optional and where the user should be able to disable it.

Besides that: I as a user find it quite nice to have one central place where I can turn off / on such start ups.

To some extend I understand that demand. But not everything that runs at start-up currently implements IStartup. Many things happens in Activators or code triggered by activators. So you will only see a sub-set of everything any ways.

@HannesWell can we apply this? The recommendation is fine for now IMHO. Once we have removed the usage of IStartup from p2 we can discuss to deprecate it, even though maybe not mark it for deletion yet due to the concerns from @BeckerWdf and @sratz. But it might be good to guide new users to use the new approach).

Applying this to the Freeze-Monitor currently requires the latest Features in the PDE DS support which was just implemented by @laeubi in eclipse-pde/eclipse.pde#923. It is currently only available only available in I-builds.
I don't know if it is an acceptable requirement for other platform contributors?
Furthermore with the objections raised we have to think again if the Freeze-Monitor should possible be disabled by the user?

[HannesWell]

I also changed the language in the doc to be less strict.

Furthermore with the objections raised we have to think again if the Freeze-Monitor should possible be disabled by the user?

In fact there is already a dedicated preference:

So it would just be the question if it is OK to require the latest PDE DS version for developing the platfrom, but if one only uses the latest milestone in most cases it is probably also ok if the freeze-monitoring does not work temporarily.

[vogella]

Lets reduce our usage of IStartup, the deprecation may come later, we may want to "only" deprecate for now and not mark for deletion so that new usages of this "feature" are avoided.