Create ci.yml to build with GitHub Actions #1254

mickaelistria · 2023-07-28T12:30:00Z

What it does

Adds GitHub action that validates branches and PRs.
Compared to Jenkinsfile, this workflow can easily be used to enable validation in forks, just by enabling GitHub actions on the forked repository. So it makes it almost free for contributors to get some CI validation, and thus to provide higher quality PRs more easily.

Author checklist

I have thoroughly tested my changes
The change is following the coding conventions
I have signed the Eclipse Contributor Agreement (ECA)

mickaelistria · 2023-07-28T14:23:03Z

I don't think the test failure

2023-07-28T12:58:19.2029933Z org.eclipse.jdt.core.tests.model.InclusionPatternsTests.testNestedSourceFolder2 -- Time elapsed: 0.061 s <<< FAILURE!
2023-07-28T12:58:19.2030137Z junit.framework.ComparisonFailure:
2023-07-28T12:58:19.2030242Z Unexpected deltas.
2023-07-28T12:58:19.2030419Z ----------- Expected ------------
2023-07-28T12:58:19.2030573Z P[*]: {CHILDREN}\n
2023-07-28T12:58:19.2030664Z    src1/src2[*]: {CHILDREN}\n
2023-07-28T12:58:19.2030838Z            <default>[*]: {CHILDREN}\n
2023-07-28T12:58:19.2030920Z                    A.java[+]: {}
2023-07-28T12:58:19.2031083Z ------------ but was ------------
2023-07-28T12:58:19.2031164Z P[*]: {CHILDREN}\n
2023-07-28T12:58:19.2031314Z    src1[+]: {}
2023-07-28T12:58:19.2031473Z --------- Difference is ----------
2023-07-28T12:58:19.2031565Z  expected:<...: {CHILDREN}\n
2023-07-28T12:58:19.2031653Z    src1[/src2[*]: {CHILDREN}\n
2023-07-28T12:58:19.2031739Z            <default>[*]: {CHILDREN}\n
2023-07-28T12:58:19.2031984Z                    A.java][+]: {}> but was:<...: {CHILDREN}\n
2023-07-28T12:58:19.2032085Z    src1[][+]: {}>
2023-07-28T12:58:19.2032348Z    at org.eclipse.jdt.core.tests.junit.extension.TestCase.assertStringEquals(TestCase.java:266)
2023-07-28T12:58:19.2032594Z    at org.eclipse.jdt.core.tests.junit.extension.TestCase.assertEquals(TestCase.java:242)
2023-07-28T12:58:19.2032951Z    at org.eclipse.jdt.core.tests.model.AbstractJavaModelTests.assertDeltas(AbstractJavaModelTests.java:1416)
2023-07-28T12:58:19.2033318Z    at org.eclipse.jdt.core.tests.model.AbstractJavaModelTests.assertDeltas(AbstractJavaModelTests.java:1406)
2023-07-28T12:58:19.2033780Z    at org.eclipse.jdt.core.tests.model.InclusionPatternsTests.testNestedSourceFolder2(InclusionPatternsTests.java:659)

is related to this change. Is this test known as somewhat flaky?

mickaelistria · 2023-07-28T14:36:19Z

So the test indeed seems flaky, as it was already reported in #420 (comment)

mickaelistria · 2023-07-28T16:55:57Z

@rgrunber If this is merged, then I think we can close #1234 and keep only #1230 and my local build on GitHub actions should be enough to demo what works or not.

iloveeclipse · 2023-07-28T20:14:01Z

.github/workflows/ci.yml

+      uses: actions/setup-java@v3
+      with:
+        java-version: |
+          8


Does it mean, ecj will run on Java 8? That is not possible.

No, it just makes Java 8 will be available on the CI machine.

... and why do we need Java 8 on CI machine?

Tycho requires Java 8 to properly resolve Java 8 APIs. It can be turned off and the current Java runtime can be used instead (Java 17), but we've seen cases where some API disappear in newer versions and thus the code targetting Java 8 that cannot be compiled any longer by Java 17.
IIRC, Java 8 is also installed on Jenkins and configured in java toolchains.

the code targetting Java 8 that cannot be compiled any longer by Java 17.

We do not have code targeting Java 8 AFAIK, and annotations don't use any Java 8 specific API's, so why all this overhead adding something we don't need?

You need to ensure compatibility with Java 8 if you target Java 8 (in https://github.com/eclipse-jdt/eclipse.jdt.core/blob/master/org.eclipse.jdt.annotation_v1/META-INF/MANIFEST.MF#L8 ). Compatibility cannot be verified at build-time without a Java 8 installation being visible to Tycho.
The build config here is similar to Jenkins one. I wouldn't make it a requirement to chnage the build config to something too different from "main" build.
If you wish to get rid of the requirement of having Java 8 installed for build to succeed, please open another issue; it can be changed from pom.xml, but this would also affect the Jenkins build. It's another topic.

iloveeclipse · 2023-07-28T20:14:31Z

.github/workflows/ci.yml

+          17
+          20
+        mvn-toolchain-id: |
+          JavaSE-1.8


Here also Java 8?

No, this is a specific Tycho/OSGi label here: tycho wants a toolchain that has the BREE name as id to properly resolve it, and for Java 8, it's JavaSE-1.8. See eclipse-tycho/tycho#2670 .
Note that this particular workaround is necessary for the older org.eclipse.jdt.annotations bundle only.

.github/workflows/ci.yml

mickaelistria · 2023-08-25T08:50:29Z

@rgrunber Anything left that prevents from merging here?

rgrunber

If there's no other opposition, I'll merge this ~~soon~~ after the freeze ?

@mickaelistria , wait I assume branch freeze policy still applies so we're waiting until 4.30..

akurtakov · 2023-08-25T13:45:39Z

Releng improvements (like this one) are in general not under freeze together with docs/examples/tests changes.

.github/workflows/ci.yml

* --batch-mode * remove extra space * split commands as 2 lines.

mickaelistria · 2023-08-25T19:57:21Z

Thank you! That will be pretty useful for people willing to experiment with JDT to have CI enabled out-of-the-box in their fork!

stephan-herrmann · 2023-09-18T22:38:30Z

@mickaelistria Seems you're not done here, see https://www.eclipse.org/lists/jdt-dev/msg02287.html

mickaelistria · 2023-09-19T07:15:01Z

Thanks for the heads-up and sorry for this inconvenience. I hope #1389 fixes it.

iloveeclipse reviewed Jul 28, 2023

View reviewed changes

Create ci.yml to build with GitHub Actions

68f71a6

mickaelistria force-pushed the master branch from 57a5a41 to 68f71a6 Compare August 17, 2023 15:06

rgrunber reviewed Aug 21, 2023

View reviewed changes

.github/workflows/ci.yml Outdated Show resolved Hide resolved

rgrunber reviewed Aug 21, 2023

View reviewed changes

.github/workflows/ci.yml Outdated Show resolved Hide resolved

pin action SHA + don't build PRs

00b7f08

rgrunber approved these changes Aug 25, 2023

View reviewed changes

rgrunber reviewed Aug 25, 2023

View reviewed changes

.github/workflows/ci.yml Outdated Show resolved Hide resolved

Cosmetics on ci.yml

03d0f73

* --batch-mode * remove extra space * split commands as 2 lines.

rgrunber merged commit 816bc73 into eclipse-jdt:master Aug 25, 2023
6 of 7 checks passed

rgrunber added this to the 4.29 RC1 milestone Aug 25, 2023

mickaelistria mentioned this pull request Sep 19, 2023

Upload eventfile and unit test results #1389

Merged

robstryker pushed a commit to robstryker/eclipse.jdt.core that referenced this pull request Jul 18, 2024

Create ci.yml to build with GitHub Actions (eclipse-jdt#1254)

baad677

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Create ci.yml to build with GitHub Actions #1254

Create ci.yml to build with GitHub Actions #1254

mickaelistria commented Jul 28, 2023

mickaelistria commented Jul 28, 2023

mickaelistria commented Jul 28, 2023

mickaelistria commented Jul 28, 2023

iloveeclipse Jul 28, 2023

mickaelistria Aug 14, 2023

iloveeclipse Aug 14, 2023

mickaelistria Aug 14, 2023

iloveeclipse Aug 14, 2023

mickaelistria Aug 15, 2023

iloveeclipse Jul 28, 2023

mickaelistria Aug 14, 2023

mickaelistria commented Aug 25, 2023

rgrunber left a comment •

edited

Loading

akurtakov commented Aug 25, 2023

mickaelistria commented Aug 25, 2023

stephan-herrmann commented Sep 18, 2023

mickaelistria commented Sep 19, 2023

Create ci.yml to build with GitHub Actions #1254

Create ci.yml to build with GitHub Actions #1254

Conversation

mickaelistria commented Jul 28, 2023

What it does

Author checklist

mickaelistria commented Jul 28, 2023

mickaelistria commented Jul 28, 2023

mickaelistria commented Jul 28, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mickaelistria commented Aug 25, 2023

rgrunber left a comment • edited Loading

Choose a reason for hiding this comment

akurtakov commented Aug 25, 2023

mickaelistria commented Aug 25, 2023

stephan-herrmann commented Sep 18, 2023

mickaelistria commented Sep 19, 2023

rgrunber left a comment •

edited

Loading