-
Notifications
You must be signed in to change notification settings - Fork 149
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
Build GTK3 support for linux x86-64 to maximize compatibility #1795
base: master
Are you sure you want to change the base?
Conversation
@laeubi and/or @HannesWell How to test this? Can I push to a branch of SWT so I can run builds on https://ci.eclipse.org/releng/job/eclipse.platform.swt/? If so, any objections to me merging eclipse-platform/eclipse.platform.releng.aggregator#2802 now so that I can start to test this? Also, assuming that is the way to test, can you check out if always push new natives is ok? |
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.
Also, assuming that is the way to test, can you check out if always push new natives is ok?
In general the policy is that all SWT native binaries have to be build on project infrastructure and to ensure this we usually don't accept changes in the binaries directly from PRs but just receive the code changes and only rebuild on master what is eventually added to the git/p2-repo.
In order to test the newly built binaries you can just download the built artifacts from Jenkins. E.g. for the second build:
https://ci.eclipse.org/releng/job/eclipse.platform.swt/job/PR-1795/2/artifact/eclipse.platform.swt/binaries/org.eclipse.swt.gtk.linux.x86_64/target/
I've just created a PR to clean up the archived artifacts a bit: #1799
In the past there have been discussions about how to stream-line this process, especially as the required build-resources are currently rare (I hope this changes in the future: https://gitlab.eclipse.org/eclipsefdn/helpdesk/-/issues/4231).
But I think if this is changed, it should be a separate, dedicated PR.
For all the other changes I need a bit more time to review. I intend to do it tomorrow.
I will remove that commit then and apply your ternary change. |
@jonahgraham If you change any of the C files or build scripts a little bit (e.g. add a temporary comment) then the "build natives" workflow should kick in and build things with your changed containers. What I have done for GTK4 back then was that I first checked the build is running, and then look at the build output (e.g. did the GTK4 binaries produced) and finally that all the so files are attached as desired. If that works (and the tests not go completely wild) there is a good chance that the change is good to be submited and a committer can then run a build with forced binary generation. |
Alternatively you can just start a build via With #1799 being submitted the build native fragments are now prominently displayed as the only Build artifacts in the corresponding Jenkins job and can be downloaded and further inspected and tests. The current build is waiting for the Y-build tests to complete and will therefore need some more time, but the previous build failed: For example for the master you can see it already: I have to leave now for a few hours but, when back I'll review this in detail and make a few more suggestions for enhancements. |
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.
Below I have made a few suggestions for making this change a bit more compact and one required change to fix the pipeline failure.
With that applied the build succeeds:
https://ci.eclipse.org/releng/job/eclipse.platform.swt/job/PR-1795/7/
You can obtain the newly built native jars from that runs Build-artifacts
section on the overview page or just visit it directly at:
https://ci.eclipse.org/releng/job/eclipse.platform.swt/job/PR-1795/7/artifact/eclipse.platform.swt/binaries/
Is the "that" in this comment that you changed the Jenkinsfile via "Replay" feauture? Because AFAICT run 6 failed and run 7 succeeded, but they were both built from same SHA1 but there is a diff in the replay https://ci.eclipse.org/releng/job/eclipse.platform.swt/job/PR-1795/7/replay/diff Not a feature I had been familiar with until now. Thanks for the demo. |
Yes but this only works for committers and especially when testing different approaches it is a bit inconvenient, but of course many things work (including replay). |
Is this still in the draft state? Would be great to have it before M3. |
Recently all SWT builds for Linux x86-64 have been done on a new Debian image. This leads to requiring such a new distro to be able to run SWT successfully. This change builds GTK3 and common code on an older debian container to increase the range of versions we support. Only GTK4's key library, libswt-pi4-gtk*.so is shipped from a build on the new debaian container. We continue to build all of the natives on latest debian so we benefit from compiler warnings/errors and other such tooling. Prerequisite eclipse-platform/eclipse.platform.releng.aggregator#2802 Fixes eclipse-platform#1791
This reverts commit 4109c4a.
Yes "that" was meant to be the changes suggested/described before. Should have been more clear.
Yes that's right. I just wanted to show an alternative that IMO suitable if one just wants try a change only ones or twice.
@jonahgraham said in #1795 (comment) that he want's to enhance this a bit further but in general this looks good to me and I assume we can land this very soon (after the commits have been squashed). One can already test the built native fragments from: |
Recently all SWT builds for Linux x86-64 have been done on a new Debian image. This leads to requiring such a new distro to be able to run SWT successfully.
This change builds GTK3 and common code on an older debian container to increase the range of versions we support. Only GTK4's key library, libswt-pi4-gtk*.so is shipped from a build on the new debaian container.
We continue to build all of the natives on latest debian so we benefit from compiler warnings/errors and other such tooling.
Prerequisite eclipse-platform/eclipse.platform.releng.aggregator#2802
Fixes #1791
The way I have tested this concept locally is to run the build.sh twice on my local machine using the two docker containers mentioned in runOnNativeBuildAgent of the Jenkinsfile*:
cd /scratch/eclipse/oomph/swt-master/git/eclipse.platform.swt/
java -Dws=gtk build-scripts/CollectSources.java -nativeSources 'target/natives-build-temp/gtk'
cd 'target/natives-build-temp/gtk'
docker run --rm -it -v/scratch:/scratch -w$PWD -e SWT_JAVA_HOME=/scratch/java/jdk-17.0.13+11 -e OUTPUT_DIR=/scratch/eclipse/oomph/swt-master/git/eclipse.platform.swt/binaries/org.eclipse.swt.gtk.linux.x86_64 swtnativebuild ./build.sh -gtk4 install
docker run --rm -it -v/scratch:/scratch -w$PWD -e SWT_JAVA_HOME=/scratch/java/jdk-17.0.13+11 -e OUTPUT_DIR=/scratch/eclipse/oomph/swt-master/git/eclipse.platform.swt/binaries/org.eclipse.swt.gtk.linux.x86_64 swtgtk3nativebuild ./build.sh -gtk3 install
* Note that I actually built them manually with USER 1000 at the end of the file so I can share directories with the container. There are more details in the step-by-step in eclipse-platform/eclipse.platform.releng.aggregator#2802 (comment)