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

Build SWT-natives against Java-17 JDK header files #633

Merged
merged 2 commits into from May 2, 2023

Conversation

HannesWell
Copy link
Member

@HannesWell HannesWell commented Apr 13, 2023

As described in #626 (comment) the SWT-natives are currently build against the JDK header files for different Java versions raging from 11 to 19. Because SWT now requires Java-17 (#625), they all should be build against Java-17 headers.

This PR aims to achieve that and at the same time fixes the inconsistencies regarding the headers/libs in the natives build described in #626 (comment).

  • Use the C header files and shared native libraries from minimal JustJ JDKs when building the SWT native binaries on specialized build machines.
  • Always specify the location of the JDK providing the headers/libs using the 'SWT_JAVA_HOME' environment variable. This allows to use another JDK as the one specified in the global JAVA_HOME variable.
  • When building the natives for individual platforms via Maven (with the build-individual-platform profile active and -Dnative=<platform> specified) set the value of SWT_JAVA_HOME to the location of the running JDK (which must of course be present). If one wants to use a custom JDK to build the SWT natives against, one can set set for example the Maven CLI argument
    -DSWT_JAVA_HOME=<path-to-desired-JDK>

Because in the CI and for Maven builds that build natives now always have a JDK specified the auto-search algorithms to find one is now obsolete and removed from the scripts.

Fixes #626 (at least the part regarding the Java-11 headers).

@github-actions
Copy link
Contributor

github-actions bot commented Apr 13, 2023

Test Results

     299 files  ±0       299 suites  ±0   6m 0s ⏱️ -20s
  4 085 tests ±0    4 074 ✔️ +23    8 💤 ±0  3  - 23 
12 173 runs  ±0  12 100 ✔️ +23  70 💤 ±0  3  - 23 

For more details on these failures, see this check.

Results for commit 122911d. ± Comparison against base commit 710fcc5.

♻️ This comment has been updated with latest results.

@HannesWell
Copy link
Member Author

Created https://gitlab.eclipse.org/eclipsefdn/helpdesk/-/issues/2957 to ask the Infra-team to add corresponding JDKs to the SWT-natives build machines.
Maybe we are lucky and they are already there. As far as I know SWT is not the only user of those machines, so I believe the chances are not too low, but I don't know the list of available JDKs in these specific agents.

@HannesWell HannesWell force-pushed the java17_headers branch 7 times, most recently from f781f88 to eb0fc42 Compare April 22, 2023 12:50
Jenkinsfile Outdated
for(os in [ 'macosx', 'linux', 'win32' ]) {
sh """
curl ${getJDKHeaderProviderURL(os)} -o jdk.tar.gz
tar -xzf jdk.tar.gz ${ os == 'win32' ? './include/': 'include/'}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@merks can you tell why in the Windows JDKs available at https://download.eclipse.org/justj/jres/17/downloads/20230204_0657/ there is always an extra folder named . in the tar.gz root which contains the jdk content?
For Linux and Macos the jdk is always contained directly in the zip's root.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's produced the same way on all platforms by this:

https://github.com/eclipse-justj/justj/blob/400371be17b1e5b4819ef1d977cb4133d7bbf277/releng/org.eclipse.justj.releng/build-jre.sh#L373-L376

It should be logged here:

https://ci.eclipse.org/justj/job/build-jres/248/

But there I see the following as if the * is expanded to "." which seems rather strange behavior.

+ tar -czf ../org.eclipse.justj.openjdk.hotspot.jre.full-17.0.6-win32-x86_64.tar.gz .

It should be possible to run this script locally and reproduce such a problem. But I won't have time today or tomorrow to investigate that.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Found time for a quick look...

It's this that is repackaging after signing all the executables and dlls:

https://github.com/eclipse-justj/justj/blob/400371be17b1e5b4819ef1d977cb4133d7bbf277/releng/org.eclipse.justj.releng/Jenkinsfile#L513

I thing that should be '*' and not '.' to avoid this problem.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've kicked off a build to build new JREs that should fix this problem...

BTW, this file contains relative URIs of all the latest Java 17 JREs:

https://download.eclipse.org/justj/jres/17/downloads/latest/justj.manifest

So in principle one could use that to avoid hard coding the specific URL of a current latest...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've kicked off a build to build new JREs that should fix this problem...

Thanks a lot! The windows tar.gz at https://download.eclipse.org/justj/www/download.eclipse.org.php?file=jres/17/downloads/20230423_0633 look perfectly fine now!

I've kicked off a build to build new JREs that should fix this problem...

BTW, this file contains relative URIs of all the latest Java 17 JREs:

https://download.eclipse.org/justj/jres/17/downloads/latest/justj.manifest

So in principle one could use that to avoid hard coding the specific URL of a current latest...

Thanks for that hint. I was already wondering if there is a latest URL from which one can just download them directly.
But since actually only the C header files provided by a JDK (in the include) are of interest in this case and I believe that they don't change often I was fine with how it is.
But I'll have a look so that always the latest tar.gz are used.

@HannesWell HannesWell force-pushed the java17_headers branch 5 times, most recently from 5786a0f to 4f9e7d5 Compare April 28, 2023 22:12
@HannesWell HannesWell marked this pull request as ready for review April 28, 2023 22:21
@HannesWell HannesWell force-pushed the java17_headers branch 2 times, most recently from e46a3f6 to 5b39bce Compare April 28, 2023 22:33
@HannesWell
Copy link
Member Author

For me this PR is ready.
I just updated the PR description to reflect the final state.
@iloveeclipse or someone else, do you want to review this?

Use the C header files and shared native libraries from minimal JustJ
JDKs when building the SWT native binaries on specialized build
machines.

Always specify the location of the JDK providing the headers/libs using
the 'SWT_JAVA_HOME' environment variable. This allows to use another JDK
as the one specified in the global JAVA_HOME variable.

When building the natives for individual platforms via Maven (with the
'build-individual-platform' profile active and -Dnative=<platform>
specified) set the value of 'SWT_JAVA_HOME' to the location of the
running JDK (which must of course be present). If one wants to use a
custom JDK to build the SWT natives against, one can set set for example
the Maven CLI argument
-DSWT_JAVA_HOME=<path-to-desired-JDK>

Because in the CI and for Maven builds that build natives now always
have a JDK specified the auto-search algorithms to find one can be
removed from the scripts.

Fixes eclipse-platform#626
@akurtakov
Copy link
Member

Even if this one causes some issues it's a move in the right direction so we would better merge it now and deal with possible consequences.

@HannesWell
Copy link
Member Author

Even if this one causes some issues it's a move in the right direction so we would better merge it now and deal with possible consequences.

As far as I can tell and as far as I have tested it, everything should be fine. But eventually we will only know for sure if we try it, therefore I'm merging this one.
Thanks Aleks for the review.

@HannesWell HannesWell merged commit 24f5e7c into eclipse-platform:master May 2, 2023
10 of 13 checks passed
@HannesWell HannesWell deleted the java17_headers branch May 2, 2023 10:23
HannesWell added a commit to HannesWell/eclipse.platform.swt that referenced this pull request Jan 13, 2024
to make it usable in all stages.
Since eclipse-platform#633
the JDK on the native-build-agent's PATH is not used anymore to build
the natives and therefore it is not required anymore to keep the PATH
untouched.
HannesWell added a commit to HannesWell/eclipse.platform.swt that referenced this pull request Jan 14, 2024
in order to replace ANT tasks with JavaScript (which require Java-11) in
Jenkins pipeline.
Stash native sources not-zipped to save the zip and unzip steps.

Additionally move all remaining ANT tasks to run a native build in a
local Maven for the running platform completely to the
maven-antrun-plugin configuration.

Furthermore move declaration of tools to the common Jenkins pipeline
section to make them usable in all stages.
Since eclipse-platform#633
the JDK on the native-build-agent's PATH is not used anymore when
building the native binaries and thus it is not required anymore to keep
the PATH untouched.

Part of
- eclipse-platform#513
- eclipse-platform#626
HannesWell added a commit to HannesWell/eclipse.platform.swt that referenced this pull request Jan 14, 2024
in order to replace ANT tasks with JavaScript (which require Java-11) in
Jenkins pipeline.
Stash native sources not-zipped to save the zip and unzip steps.

Additionally move all remaining ANT tasks to run a native build in a
local Maven for the running platform completely to the
maven-antrun-plugin configuration.

Furthermore move declaration of tools to the common Jenkins pipeline
section to make them usable in all stages.
Since eclipse-platform#633
the JDK on the native-build-agent's PATH is not used anymore when
building the native binaries and thus it is not required anymore to keep
the PATH untouched.

Part of
- eclipse-platform#513
- eclipse-platform#626
HannesWell added a commit to HannesWell/eclipse.platform.swt that referenced this pull request Jan 14, 2024
in order to replace ANT tasks with JavaScript (which require Java-11) in
Jenkins pipeline.
Stash native sources not-zipped to save the zip and unzip steps.

Additionally move all remaining ANT tasks to run a native build in a
local Maven for the running platform completely to the
maven-antrun-plugin configuration.

Furthermore move declaration of tools to the common Jenkins pipeline
section to make them usable in all stages.
Since eclipse-platform#633
the JDK on the native-build-agent's PATH is not used anymore when
building the native binaries and thus it is not required anymore to keep
the PATH untouched.

Part of
- eclipse-platform#513
- eclipse-platform#626
HannesWell added a commit that referenced this pull request Jan 14, 2024
in order to replace ANT tasks with JavaScript (which require Java-11) in
Jenkins pipeline.
Stash native sources not-zipped to save the zip and unzip steps.

Additionally move all remaining ANT tasks to run a native build in a
local Maven for the running platform completely to the
maven-antrun-plugin configuration.

Furthermore move declaration of tools to the common Jenkins pipeline
section to make them usable in all stages.
Since #633
the JDK on the native-build-agent's PATH is not used anymore when
building the native binaries and thus it is not required anymore to keep
the PATH untouched.

Part of
- #513
- #626
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.

SWT build still requires Java 11
3 participants