-
Notifications
You must be signed in to change notification settings - Fork 183
Improve support for Arrays of Strings in JNI Generator #2539
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
Conversation
...es/org.eclipse.swt.tools/JNI Generation/org/eclipse/swt/tools/internal/NativesGenerator.java
Outdated
Show resolved
Hide resolved
Test Results 88 files - 30 88 suites - 30 6m 25s ⏱️ - 4m 40s For more details on these failures, see this check. Results for commit 45f54ec. ± Comparison against base commit f38e115. This pull request removes 37 tests.
♻️ This comment has been updated with latest results. |
Why are .so files changed? Shouldn't the binaries be rebuilt by the SWT build job? |
The 5 test failures here all seem to be what is being investigated in #2532 and not caused by this PR. |
I don't know - they are changed in my workspace so I included them in the commit. I can exclude these change from this (and similar PRs) but how do tests work? Does the PR do the SWT build job? Actually I don't know what the SWT build job is, but I assumed it was something on Jenkins that runs on main branch? |
@HannesWell could explain it better, I believe you don't need to checkin any binaries in a PR, if native code is changed, Jenkins job will build everything needed and test with the updated binaries, and if merged, a job will be scheduled to push updated libraries to. I believe you should be able to see this in the git history. |
OK - I will look around and update this soon. |
71b077d
to
0075112
Compare
Nope, cannot explain it better, that's exactly how it is. :) |
Thanks @HannesWell - I have moved follow up questions on this to #2544 |
0075112
to
ed0b218
Compare
This pull request changes some projects for the first time in this development cycle.
An additional commit containing all the necessary changes was pushed to the top of this PR's branch. To obtain these changes (for example if you want to push more changes) either fetch from your fork or apply the git patch. Git patch
Further information are available in Common Build Issues - Missing version increments. |
ed0b218
to
0ea736d
Compare
A number of places in the GTK4 API there is the possibility of passing in arrays of Strings, i.e. Java String[] -> char ** as input argument to C side. This change adds better support to the JNI generator for this use case by converting the jobjectArray (String[]) to char ** on the C side. Part of #2126 Split out of #2538
0ea736d
to
45f54ec
Compare
The only failing tests are the Windows KeyEvent ones tracked in #2156. This is ready to merge. As a few other people have had a look in here I will wait a bit for last minute comments before merging myself |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.
Test failures are keyevent on windows and generally crashing tests on gtk4. |
OK - I didn't realize that gtk4 label changed the tests. I see it now in
|
A number of places in the GTK4 API there is the possibility of passing in arrays of Strings, i.e. Java String[] -> char ** as input argument to C side.
This change adds better support to the JNI generator for this use case by converting the jobjectArray (String[]) to char ** on the C side.
Part of #2126
Split out of #2538