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

Revert 1101 #1106

Merged
merged 3 commits into from Mar 13, 2024
Merged

Revert 1101 #1106

merged 3 commits into from Mar 13, 2024

Conversation

iloveeclipse
Copy link
Member

  1. Revert "Replace internal SWTEventListener directly with java.util.EventListener"

This reverts commit bd306ac.
Replacing SWTEventListener directly with java.util.EventListener breaks binary compatibility with bundles compiled against any older SWT version.

  1. Added filters for SWTEventListener used in SWT API

java.util.EventListener"

This reverts commit bd306ac.

Replacing SWTEventListener directly with java.util.EventListener breaks
binary compatibility with bundles compiled against any older SWT
version.

See discussion on
eclipse-platform#1101
Copy link
Contributor

github-actions bot commented Mar 13, 2024

Test Results

   299 files  ±0     299 suites  ±0   7m 5s ⏱️ +4s
 4 099 tests ±0   4 091 ✅ ±0   8 💤 ±0  0 ❌ ±0 
12 209 runs  ±0  12 134 ✅ ±0  75 💤 ±0  0 ❌ ±0 

Results for commit 8ef2835. ± Comparison against base commit bd306ac.

♻️ This comment has been updated with latest results.

@iloveeclipse iloveeclipse marked this pull request as ready for review March 13, 2024 09:23
@iloveeclipse
Copy link
Member Author

I do not see warnings on Linux with this patch.
On Windows I see following warnings (mostly unrelated to the problem we have discussed before, in OLE area):

Composite leaks APIs from the superclass Scrollable
Constructor for Variant with non-API parameter type IDispatch
Constructor for Variant with non-API parameter type IUnknown
GCData.ps declared as non-API type PAINTSTRUCT
OleAutomation.getTypeInfoAttributes() has non-API return type TYPEATTR
OleClientSite.appClsid declared as non-API type GUID
OleClientSite.createTempStorage() has non-API return type IStorage
OleClientSite.getClassID(String) has non-API return type GUID
OleClientSite.objDocumentView declared as non-API type IOleDocumentView
OleClientSite.objIOleCommandTarget declared as non-API type IOleCommandTarget
OleClientSite.objIOleInPlaceObject declared as non-API type IOleInPlaceObject
OleClientSite.objIOleObject declared as non-API type IOleObject
OleClientSite.objIUnknown declared as non-API type IUnknown
OleClientSite.objIViewObject2 declared as non-API type IViewObject2
OleClientSite.tempStorage declared as non-API type IStorage
OleControlSite.getLicenseInfo(GUID) has non-API parameter type GUID
OleControlSite.removeEventListener(OleAutomation, GUID, int, OleListener) has non-API parameter type GUID
Variant.getDispatch() has non-API return type IDispatch
Variant.getUnknown() has non-API return type IUnknown

I will add an additional commit on top that would mute these warnings (looking on the code they existed since very beginning), so that we would have zero API warnings on Windows too.

@iloveeclipse iloveeclipse merged commit a98c844 into eclipse-platform:master Mar 13, 2024
13 checks passed
@iloveeclipse iloveeclipse deleted the revert_1101 branch March 13, 2024 12:27
@merks
Copy link
Contributor

merks commented Mar 13, 2024

I see these errors in my workspace:

image

Is this expected?

@iloveeclipse
Copy link
Member Author

@merks :

I've tried master on Linux/Windows and see no errors.

@merks
Copy link
Contributor

merks commented Mar 13, 2024

No, restarts and cleans don't help.

@iloveeclipse
Copy link
Member Author

No, restarts and cleans don't help.

@iloveeclipse
Copy link
Member Author

Just tried on Linux with IDE on I20240313-0830 and baseline on https://download.eclipse.org/eclipse/downloads/drops4/R-4.31-202402290520/ , everything is fine.
I've already tested that before pushing with 4.31RC2 Windows build, I don't expect there is any difference.
Will double check on Windows now with 4.31 final.

@merks
Copy link
Contributor

merks commented Mar 13, 2024

I have this installed:

image

I have this API baseline:

image

The API baseline has content from this repository:

image

The problem won't go away and it seems like a real problem. Are you expecting an API filter to make it go away? Because the change it's complaining about is really a change since the last release:

image

Should the revert go farther back?

@iloveeclipse
Copy link
Member Author

Are you expecting an API filter to make it go away?

Yes, with this line:

https://github.com/eclipse-platform/eclipse.platform.swt/blob/master/binaries/org.eclipse.swt.win32.win32.x86_64/.settings/.api_filters#L578

Do you have clean workspace, may be .settings or filters deleted by occasion?

@iloveeclipse
Copy link
Member Author

So, running SDK I20240313-0830 from https://download.eclipse.org/eclipse/downloads/drops4/I20240313-0830/ with API baseline set to the extracted final 4.31 SDK zip file downloaded from https://download.eclipse.org/eclipse/downloads/drops4/R-4.31-202402290520/

Note: everything installed manually without Oomph or maven or any other tooling.

I'm looking on clean SWT git state at a98c844 and see no API errors or warnings on org.eclipse.swt.win32.win32.x86_64 project on Windows:

image

@merks
Copy link
Contributor

merks commented Mar 13, 2024

Interesting, the link is not there!

image

But the file exists in the clone:

image

I'll have to dig deeper tomorrow.

Thanks for your help so far!! 😀

@iloveeclipse
Copy link
Member Author

Just close and reopen project.

@HannesWell
Copy link
Member

Thank you Andrey, for helping in reverting the removal and adding the filters!

I will add an additional commit on top that would mute these warnings (looking on the code they existed since very beginning), so that we would have zero API warnings on Windows too.

Thanks for that as well, I as about to do that too.
In order to prevent API-leakages in the future I have created #1110.

Interesting, the link is not there!

This is a bit convoluted. Since the .settings folder from the parent-folder is linked into the fragment projects to share the settings between the fragments, I had to link the .settings/.api_filters again linking it to their real location. Otherwise the workspace didn't saw it :/.
But for me it worked as well.

@merks
Copy link
Contributor

merks commented Mar 13, 2024

Close/open didn't work either, but deleting the project from the workspace (not the repository) and running the setup tasks to import the project again worked fine. Thank you!

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

Successfully merging this pull request may close these issues.

None yet

3 participants