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.

Sign up for GitHub

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

Jump to bottom

Switch SWT to Java 17 runtime #627

Merged
merged 1 commit into from
Apr 13, 2023
Merged

Switch SWT to Java 17 runtime #627

merged 1 commit into from
Apr 13, 2023

Conversation

iloveeclipse
Copy link
Member

Note: there are still build files requiring Java 11 for execution (Jenkinsfile and buildSWT.xml).

See #626

Fixes #625

@github-actions
Copy link
Contributor

github-actions bot commented Apr 12, 2023 •
edited
Loading

Test Results

     299 files  ±0       299 suites  ±0   5m 42s ⏱️ -15s
  4 049 tests ±0    4 038 ✔️ ±0    8 💤 ±0  3 ❌ ±0 
12 065 runs  ±0  11 992 ✔️ ±0  70 💤 ±0  3 ❌ ±0 

For more details on these failures, see this check.

Results for commit 717cc0d. ± Comparison against base commit 04678a2.

♻️ This comment has been updated with latest results.

@iloveeclipse
Copy link
Member Author

@HannesWell : I assume something Jenkins related doesn't work as expected.
Maven build seem to be successful: see https://ci.eclipse.org/releng/job/eclipse.platform.swt/job/PR-627/1/console

16:08:25  [INFO] Reactor Summary:
16:08:25  [INFO] 
16:08:25  [INFO] eclipse.platform.swt 4.28.0-SNAPSHOT ............... SUCCESS [  0.340 s]
16:08:25  [INFO] org.eclipse.swt 3.124.0-SNAPSHOT ................... SUCCESS [ 33.649 s]
16:08:25  [INFO] org.eclipse.swt.fragments.localbuild 3.105.0-SNAPSHOT SUCCESS [  0.567 s]
16:08:25  [INFO] org.eclipse.swt.tools.base 3.107.400-SNAPSHOT ...... SUCCESS [  4.967 s]
16:08:25  [INFO] org.eclipse.swt.tools.spies 3.109.0-SNAPSHOT ....... SUCCESS [  2.437 s]
16:08:25  [INFO] org.eclipse.swt.tools 3.110.0-SNAPSHOT ............. SUCCESS [  3.465 s]
16:08:25  [INFO] org.eclipse.swt.examples 3.108.0-SNAPSHOT .......... SUCCESS [  2.467 s]
16:08:25  [INFO] org.eclipse.swt.examples.browser.demos 3.108.0-SNAPSHOT SUCCESS [  1.059 s]
16:08:25  [INFO] org.eclipse.swt.examples.launcher 3.108.0-SNAPSHOT . SUCCESS [  0.797 s]
16:08:25  [INFO] org.eclipse.swt.examples.ole.win32 3.108.0-SNAPSHOT  SUCCESS [  8.188 s]
16:08:25  [INFO] org.eclipse.swt.examples.views 3.108.0-SNAPSHOT .... SUCCESS [  0.765 s]
16:08:25  [INFO] org.eclipse.swt.tests 3.107.0-SNAPSHOT ............. SUCCESS [03:18 min]
16:08:25  [INFO] org.eclipse.swt.tools.feature 3.109.0-SNAPSHOT ..... SUCCESS [  0.655 s]
16:08:25  [INFO] org.eclipse.swt.tests.gtk 3.109.0-SNAPSHOT ......... SUCCESS [  2.114 s]
16:08:25  [INFO] ------------------------------------------------------------------------
16:08:25  [INFO] BUILD SUCCESS

but later on, it looks like something related to eclipse.platform.swt.binaries started that probably causes the PR result to fail:

groovy.lang.MissingPropertyException: No such property: skipCommit for class: groovy.lang.Binding
	at groovy.lang.Binding.getVariable(Binding.java:63)
	at org.jenkinsci.plugins.scriptsecurity.sandbox.groovy.SandboxInterceptor.onGetProperty(SandboxInterceptor.java:285)
	at org.kohsuke.groovy.sandbox.impl.Checker$7.call(Checker.java:375)
	at org.kohsuke.groovy.sandbox.impl.Checker.checkedGetProperty(Checker.java:379)
	at org.kohsuke.groovy.sandbox.impl.Checker.checkedGetProperty(Checker.java:355)
	at org.kohsuke.groovy.sandbox.impl.Checker.checkedGetProperty(Checker.java:355)
	at org.kohsuke.groovy.sandbox.impl.Checker.checkedGetProperty(Checker.java:355)
	at org.kohsuke.groovy.sandbox.impl.Checker.checkedGetProperty(Checker.java:355)
	at com.cloudbees.groovy.cps.sandbox.SandboxInvoker.getProperty(SandboxInvoker.java:29)
	at com.cloudbees.groovy.cps.impl.PropertyAccessBlock.rawGet(PropertyAccessBlock.java:20)
	at WorkflowScript.run(WorkflowScript:328)
	at ___cps.transform___(Native Method)
	at com.cloudbees.groovy.cps.impl.PropertyishBlock$ContinuationImpl.get(PropertyishBlock.java:73)
	at com.cloudbees.groovy.cps.LValueBlock$GetAdapter.receive(LValueBlock.java:30)
	at com.cloudbees.groovy.cps.impl.PropertyishBlock$ContinuationImpl.fixName(PropertyishBlock.java:65)
	at jdk.internal.reflect.GeneratedMethodAccessor177.invoke(Unknown Source)
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:566)
	at com.cloudbees.groovy.cps.impl.ContinuationPtr$ContinuationImpl.receive(ContinuationPtr.java:72)
	at com.cloudbees.groovy.cps.impl.ConstantBlock.eval(ConstantBlock.java:21)
	at com.cloudbees.groovy.cps.Next.step(Next.java:83)
	at com.cloudbees.groovy.cps.Continuable$1.call(Continuable.java:152)
	at com.cloudbees.groovy.cps.Continuable$1.call(Continuable.java:146)
	at org.codehaus.groovy.runtime.GroovyCategorySupport$ThreadCategoryInfo.use(GroovyCategorySupport.java:136)
	at org.codehaus.groovy.runtime.GroovyCategorySupport.use(GroovyCategorySupport.java:275)
	at com.cloudbees.groovy.cps.Continuable.run0(Continuable.java:146)
	at org.jenkinsci.plugins.workflow.cps.SandboxContinuable.access$001(SandboxContinuable.java:18)
	at org.jenkinsci.plugins.workflow.cps.SandboxContinuable.run0(SandboxContinuable.java:51)
	at org.jenkinsci.plugins.workflow.cps.CpsThread.runNextChunk(CpsThread.java:187)
	at org.jenkinsci.plugins.workflow.cps.CpsThreadGroup.run(CpsThreadGroup.java:420)
	at org.jenkinsci.plugins.workflow.cps.CpsThreadGroup$2.call(CpsThreadGroup.java:330)
	at org.jenkinsci.plugins.workflow.cps.CpsThreadGroup$2.call(CpsThreadGroup.java:294)
	at org.jenkinsci.plugins.workflow.cps.CpsVmExecutorService$2.call(CpsVmExecutorService.java:67)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at hudson.remoting.SingleLaneExecutorService$1.run(SingleLaneExecutorService.java:139)
	at jenkins.util.ContextResettingExecutorService$1.run(ContextResettingExecutorService.java:30)
	at jenkins.security.ImpersonatingExecutorService$1.run(ImpersonatingExecutorService.java:70)
	at java.base/java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515)
	at java.base/java.util.concurrent.FutureTask.run(FutureTask.java:264)
	at java.base/java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128)
	at java.base/java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628)
	at java.base/java.lang.Thread.run(Thread.java:839)
[GitHub Checks] GitHub check (name: Jenkins, status: completed) has been published.

Is that a known Jenkins build issue in SWT? I don't see any relationship to Java 17 changes.

@iloveeclipse iloveeclipse mentioned this pull request Apr 12, 2023
Closed
@HannesWell
Copy link
Member

but later on, it looks like something related to eclipse.platform.swt.binaries started that probably causes the PR result to fail:

That's indeed very likely not related to this change. I assume the parameter in the (now) GString block has to be accessed through the params object, but that somehow did not show up in the verification of #628 and the subsequent master branch builds. IIRC those params are also injected as environment variables, but maybe not on the first run of a PR?
Nevertheless I created #613 to add it.

@HannesWell
Copy link
Member

As expected a second build succeeded, even without #613. It looks like the build parameters are only injected as environment variables on the second run. From my previous work on the SWT pipeline I believe to remember that the value of the parameter value was only available in a subsequent run as env. For whatever reason. Using the params-object seems to make it available immediately.

Since you changed the way how the natives are build, without changing a file in a natives source directory (and therefore did not implicitly trigger a build of the natives), the natives build has to be enforced, by re-triggering the job in Jenkins via Build with parameters and the forceNativeBuilds parameter set to true. I just did that, so lets see if the builds still passes.

@iloveeclipse
Copy link
Member Author

Since you changed the way how the natives are build

What do you mean by that? Where did I changed native build?

@HannesWell
Copy link
Member

Since you changed the way how the natives are build

What do you mean by that? Where did I changed native build?

You changed the JDKs that provide the header files for JNI and if there is a no JDK at the new path the SWT natives build for that platform likely fails because of the missing header, unless there is some auto-search like in the Windows build.bat.

@iloveeclipse
Copy link
Member Author

So should I revert changes on Jenkinsfile?

@HannesWell
Copy link
Member

So should I revert changes on Jenkinsfile?

At least for the platforms gtk.linux.aarch64 and gtk.linux.ppc64le the new paths are invalid and no JDK is present.
The other linux and mac cases seem to be fine.
For windows that's the result for the auto-search.

'SWT_JAVA_HOME' was not provided, auto-searching for JDK...
-- JDK 'C:\Program Files\AdoptOpenJDK\jdk-11.0.11.9-hotspot' looks good, selecting it
SWT_JAVA_HOME x64: C:\Program Files\AdoptOpenJDK\jdk-11.0.11.9-hotspot

So unless you are fine with a mixture of Java-17 and Java-11 headers you should revert all, yes.
Then we should ask the infra team for new JDKs for linux aarch64 and ppc and win32 and adjust the auto-search for Windows accordingly.

@iloveeclipse
Switch SWT to Java 17 runtime
717cc0d 
Note: there are still build files requiring Java 11 for execution
(Jenkinsfile and buildSWT.xml).

See eclipse-platform#626

Fixes eclipse-platform#625
@iloveeclipse iloveeclipse mentioned this pull request Apr 13, 2023
Closed
@iloveeclipse
Copy link
Member Author

So unless you are fine with a mixture of Java-17 and Java-11 headers you should revert all, yes.

So I did now.

Then we should ask the infra team for new JDKs for linux aarch64 and ppc and win32 and adjust the auto-search for Windows accordingly.

I've updated #626

@iloveeclipse
Copy link
Member Author

Then we should ask the infra team for new JDKs for linux aarch64 and ppc and win32 and adjust the auto-search for Windows accordingly.

@HannesWell : in case you asked already what to install and where, can you please ask infra team to install Java 17 variants in same workstations / hosts / whatever and link to #626?

@iloveeclipse iloveeclipse merged commit 1935816 into eclipse-platform:master Apr 13, 2023
@iloveeclipse iloveeclipse deleted the issue_625 branch April 13, 2023 06:40
@HannesWell
Copy link
Member

Then we should ask the infra team for new JDKs for linux aarch64 and ppc and win32 and adjust the auto-search for Windows accordingly.

@HannesWell : in case you asked already what to install and where, can you please ask infra team to install Java 17 variants in same workstations / hosts / whatever and link to #626?

Yes I can do that this evening.

After thinking about the problem again, what maybe could also work is to collect/stash the JNI headers from the JVM at the 'main' Jenkins executor before the matrix is forked (this will likely be a linux VM), using the headers at the location returned by tool(type:'jdk', name:'temurin-jdk17-latest'), then unstash it in each executor of a platform specific SWT natives build and set up the environment variables accordingly. This way the special executors configured to build the SWT natives don't need to provide any JVM.
But of course this requires the JNI headers to be the same or at least equivalent for all platforms. I don't know that and first have to check if that is the case.

@iloveeclipse
Copy link
Member Author

Yes I can do that this evening.

Thanks.

I don't know that and first have to check if that is the case.

I have no opinion as I have no real experience with "right" Jenkins builds configuration way.
Let us continue discussion on #626

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

SWT should require Java 17 runtime in 4.28
2 participants
@iloveeclipse @HannesWell