Stop using SWTEventListener internally and mark it as deprecated forRemoval

[akurtakov]

As it:

• is internal
• makes api tools fail thanks to it
• serves no purpose since SWT no longer runs on Java ME

Internally SWT is moved to use java.util.EventListener and as it's the
superclass of SWTEventListener makes all the APIs that taken
SWTEventListener to keep working.

[laeubi]

There already was an attempt here:

• Replace internal SWTEventListener directly with java.util.EventListener #1101
• I20240312-1800 failed many junit related to "width" #1105
• Revert 1101 #1106

but there was some concerns raised.

[github-actions]

Test Results

   483 files  ±0     483 suites  ±0   7m 21s ⏱️ -55s
 4 095 tests ±0   4 085 ✅ ±0   7 💤 ±0  3 ❌ ±0 
16 173 runs  ±0  16 080 ✅ ±0  90 💤 ±0  3 ❌ ±0 

For more details on these failures, see this check.

Results for commit d80bcdf. ± Comparison against base commit cfa74b4.

♻️ This comment has been updated with latest results.

[akurtakov]

Thanks for the pointers @laeubi . The new versions keeps SWTEventListener (marked as deprecated for removal) and should be compatible for all the cases except:

• SomeSWTListener instanceof SWTEventListener
• TypedListener.getEventListener returning EventListener and not SWTEventListener

@Phillipus and @iloveeclipse can you check whether this PR works for you?

[Phillipus]

I checked out the PR and get some errors in the Widget class:

protected void removeListener (int eventType, SWTEventListener listener) {
	removeTypedListener(eventType, listener);
}

[akurtakov]

I checked out the PR and get some errors in the Widget class:

protected void removeListener (int eventType, SWTEventListener listener) {
	removeTypedListener(eventType, listener);
}

I did the change for Gtk only, new version should have the changes for mac/win too.

[Phillipus]

I'm using Nebula Grid and Gallery Widgets and get:

java.lang.NoClassDefFoundError: org/eclipse/swt/internal/SWTEventListener
	at org.eclipse.nebula.jface.gridviewer.GridTableViewer.<init>(GridTableViewer.java:138)

Does Nebula need a change? I'm using 3.1.1

[akurtakov]

I'm using Nebula Grid and Gallery Widgets and get:

java.lang.NoClassDefFoundError: org/eclipse/swt/internal/SWTEventListener
	at org.eclipse.nebula.jface.gridviewer.GridTableViewer.<init>(GridTableViewer.java:138)

Does Nebula need a change? I'm using 3.1.1

The goal is to not need a change (for majority of cases). Please try with latest version.

[Phillipus]

The goal is to not need a change (for majority of cases). Please try with latest version.

Now I get:

java.lang.NoSuchMethodError: 'void org.eclipse.swt.widgets.TypedListener.<init>(org.eclipse.swt.internal.SWTEventListener)'
	at org.eclipse.nebula.widgets.grid.Grid.addTreeListener(Grid.java:995)

This is:

addListener(SWT.Expand, new TypedListener(listener));

Also in Gallery Nebula Widget:

java.lang.NoSuchMethodError: 'void org.eclipse.swt.widgets.TypedListener.<init>(org.eclipse.swt.internal.SWTEventListener)'
	at org.eclipse.nebula.widgets.gallery.Gallery.addSelectionListener(Gallery.java:316)

Seems that Nebula is expecting the old version of TypedListener:

public TypedListener (SWTEventListener listener) {
	eventListener = listener;
}

[HannesWell]

As Christoph mentioned I tried the same in #1101 and failed.
While I'm note happy about the current situation and would like to get rid of SWTEventListener too I think we have to be much more careful here because although it should not have been used, SWTEventListener is probably used in more places than we would like it happen. It seems it has become a de-factor API and we should treat it like this to not potentially break everything.

• SomeSWTListener instanceof SWTEventListener

Furthermore assigning SWT-events of a specific type to a variable of type SWTEventListener does not work anymore too.
But in that case either a specific event type or the more general EventListener can be used immediately as an replacement.
And the same solution can be used for existing instanceof checks.
But of course there are no runtime errors like in the first case but the case will just always be false at runtime.

• TypedListener.getEventListener returning EventListener and not SWTEventListener

That could be a bigger problem, because although it again should not have been used, it was necessary to use it because a lack of API added only two/three releases ago and this change is binary incompatible.

I have to check the details again.

Does Nebula need a change? I'm using 3.1.1

I did a lot of migration in the past in the context of #1101 (comment) to use new APIs:
https://github.com/eclipse/nebula/pulls?q=is%3Apr+author%3AHannesWell+is%3Aclosed

But IIRC since the PRs were open for some time new code using the old APIs have been added, so another pass might be necessary. I'll check that.

[HannesWell]

The past effort to really remove/hide SWT internals did not only affect SWTEventListener but also TypedListener and I think we should also deprecate the latter for removal, to make it really internal. But this can probably be a separate change.

[HannesWell]

The goal is to not need a change (for majority of cases). Please try with latest version.

Now I get:

java.lang.NoSuchMethodError: 'void org.eclipse.swt.widgets.TypedListener.<init>(org.eclipse.swt.internal.SWTEventListener)'
	at org.eclipse.nebula.widgets.grid.Grid.addTreeListener(Grid.java:995)

You are not using the latest version of Nebula are you?
Because this was changed a few months ago in

• https://github.com/eclipse/nebula/pull/583/files#diff-c3b4ffebdc875a02b8d0c4103d43376486a3dc50672e536164834cc6873ed2bd

But I don't know how often Nebula releases new versions.

Btw. with EclipseNebula/nebula#613, all references to SWT's TypedListener and SWTEventListener should be removed from Nebula. Certain use cases of TypedListener also required SWTEventListener as your error shows.
There is only one utility method left, which is deprecated and not necessary and used (in Nebula itself) anymore.

[Phillipus]

You are not using the latest version of Nebula are you?

If "latest" means nightly build then no, because:

The goal is to not need a change (for majority of cases).

It's OK with the nightly build of Nebula.

[HannesWell]

But I don't know how often Nebula releases new versions.

Looking at https://github.com/eclipse/nebula/releases, V3.1.1 seems to be the latest release and it's from February and therefore does not include all the mentioned changes.
But https://projects.eclipse.org/projects/technology.nebula says there is a 3.2.0 release done after the mentioned changes were done.
But at https://download.eclipse.org/nebula/updates/ I can also only see 3.1.1.

@Phillipus can you try the latest master?

[HannesWell]

It's OK with the nightly build of Nebula.

Thanks for already trying. Then we should ask for a new Nebula release after EclipseNebula/nebula#613 has landed.

[akurtakov]

I've added back the TypedListener constructor using SWTEventListener but marked it as deprecated forRemoval. That should fix this issue too.
@Phillipus Huge thanks for baring with us to help getting some sense back into SWT . If we can't manage to get our tools (apitools) work with it we can just give up on getting new contributors right now.

[HannesWell]

@akurtakov have you seen my comments in

• Stop using SWTEventListener internally and mark it as deprecated forRemoval #1615 (comment)
• Stop using SWTEventListener internally and mark it as deprecated forRemoval #1615 (comment)

[akurtakov]

@Phillipus if we manage to get to a state where using your product as an experiment we can land this PR without any breakage in it that would be huge step.

[Phillipus]

Latest push to PR is working with existing Nebula Grid and Gallery widgets (3.1.1)

[akurtakov]

I plan to merge this one in once new stream opens if there are no identified issues.

[Phillipus]

I plan to merge this one in once new stream opens if there are no identified issues.

I'll test again when that happens. But I only use two of the Nebula widgets, Grid and Gallery, Wouldn't it make more sense for the Nebula committers to test all their widgets against these changes in SWT using automated tests?

[akurtakov]

Ideally every downstream project (not only Nebula) should test with latest I-builds constantly so issues are reported and identified ASAP. I am very grateful to everyone that tests and reports timely and of course this works in the other direction too - early testers don't face (or at least less often) face regressions.

[merks]

FYI, it looks like datechooser has not gotten any attention:

Builds are definitely not being built against the latest, though making it configurable for that wouldn't be so much work...

[akurtakov]

FYI, it looks like datechooser has not gotten any attention:

Builds are definitely not being built against the latest, though making it configurable for that wouldn't be so much work...

I can not reproduce locally. I have this project (+deps ) open in my swt workspace and they compile just fine. What am I missing?

[merks]

Probably/maybe related to the rwt thing:

I don't have time to investigate further now...

[merks]

@HannesWell

I've never seen this bundle-version thing before. It looks like this would maybe make the plugin not work with the latest rwt-providing bundle:

I think RWT has not been adapted, so the code doesn't work there, and that there is confusion here when RWT is in the target platform...

[akurtakov]

So what I understand is that Nebula fails when RWT (RAP?) is used instead of SWT aka not not related to SWT itself.

[merks]

Yes, I think that's a reasonable theory at this point. (I'm not sure if resolving to both rwt and swt is new PDE behavior, but it's a complete confusing mess in the IDE.)

[HannesWell]

I've never seen this bundle-version thing before. It looks like this would maybe make the plugin not work with the latest rwt-providing bundle:

It's indeed not used often but according to the spec it legal:
https://docs.osgi.org/specification/osgi.core/8.0.0/framework.module.html#framework.module.importpackage

bundle-version - A version-range to select the bundle version of the exporting bundle.
The default value is [0.0.0, ∞). See Semantic Versioning.
In the case of a fragment bundle, the version is from the host bundle.

I think RWT has not been adapted, so the code doesn't work there, and that there is confusion here when RWT is in the target platform...

Looks like they have copied the class...
https://github.com/eclipse-rap/org.eclipse.rap/blob/924a3e0bf8f0669308eba601b9ed08cf95b53b60/bundles/org.eclipse.rap.rwt/src/org/eclipse/swt/widgets/TypedListener.java#L66

Don't know why they do it but it's for sure their responsibility to update it.

[wimjongman]

But I don't know how often Nebula releases new versions.

I'm prepping a new release. Let me know if you need something else.

[akurtakov]

But I don't know how often Nebula releases new versions.

I'm prepping a new release. Let me know if you need something else.

Would you please run your full test suite against SWT patched with this PR and report back ?

[merks]

@akurtakov

Does the build produce an update site?

[akurtakov]

@akurtakov

Does the build produce an update site?

Nope. In order for people to try it they have to build it themself .

At some point I hope that every git repo will provide update site for usage but I start from the leaves and this one is kind of 'root'. It feels right to ask you about "update sites" - eclipse-pde/eclipse.pde#1136 - would you please look into this one?

If someone (I definitely can not now) can spend the time to make such update sites being generated and usable they might be able to team up at #1621

[HannesWell]

Does the build produce an update site?

If the build would create a P2 repo just for SWT the result could be archived by the Jenkins build. The URL to the corresponding build artifact can then be used just like any other URL for a p2-repo, e.g. in a TP or to install new software.
But it would increase the disk-storage demand on Jenkins and I don't know if the rare occasions where this is needed are worth it.

[Phillipus]

I plan to merge this one in once new stream opens if there are no identified issues.

I'll test again when that happens.

This merge is working with the Nebula Grid and Gallery widgets (3.1.1). Thank-you!

[akurtakov]

I plan to merge this one in once new stream opens if there are no identified issues.

I'll test again when that happens.

This merge is working with the Nebula Grid and Gallery widgets (3.1.1). Thank-you!

@Phillipus It wouldn't have been the case without you!