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] Extend GH workflow matrix for jdkVersion and targetPlatform #2618

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

HannesWell
Copy link
Contributor

This PR extends the GH workflow matrix for Xtext with an axis for the jdkVersion and targetPlatform version. So that all selected combinations are always build.
In order to limit the number of combinations only the oldest and latest supported Eclipse TP are build.

The incompatible combination of Java-11 and Eclipse 2023-06 (aka latest), that matrix cell is excluded.

Because the coactions/setup-xvfb can only execute a single command I had to adjust the strict-release-jdk a bit so that the the jdk can be configured via an extra property. I didn't found a solution to use the same approach as in the Jenkins file.

I tried

      - name: Build and test
        uses: coactions/setup-xvfb@v1
        with: 
          run: "(if [[ ${{ matrix.jdkVersion }} == 11 ]]; then toolchainArgs='--toolchains ../releng/toolchains.xml -Pstrict-release-jdk';fi && \
            mvn clean verify -PuseJenkinsSnapshots $toolchainArgs -Dtarget-platform-classifier=xtext-${{ matrix.targetPlatform }})"

But then setup-xvfb only tried to run the command if and failed.
Personally I also find the approach used now simpler to read.

--toolchains releng/toolchains.xml -Pstrict-release-jdk -Dtoolchain.jdk=17 is actually a NoOp and not specifying has the same result since JAVA_HOME points to the same JDK as jdk-17 from the toolchain.
Since I found it simpler to read I applied the same approach in the Jenkins workflow, but if you prefer the previous way I'll revert that part.

In order to make the job labels not too long I also adjusted the os axis values.

You can see the results in my fork at:
https://github.com/HannesWell/eclipse.xtext/actions/runs/4757063052

From #2223 (comment)

It would be nice to have such a matrix in this repository, but due to the limitations of the usage of GitHub Actions shared by all Eclipse repositories, it wouldn't be feasible. Maybe just for Ubuntu and not macOS... maybe just nightly. We can investigate that in the future.

AFAIK the workflow resource quota is per organization, so if Xtext would have its own GH-organisation more resources would probably be available.

@LorenzoBettini
Copy link
Contributor

@HannesWell , as I suggested in the other PR, I don't think we want to change the maven.yml; what might help would be another workflow that only runs periodically (even nightly might be too much, maybe weekly and/or on demand, AKA workflow dispatch, or both).

The combinations currently in the matrix of this PR maybe are not that important because we already have those configurations in Jenkins, so I wouldn't repeat the effort. Something more along the lines of the experiments you made for the other PR (https://github.com/HannesWell/eclipse.xtext/actions/runs/4746875852), where you were testing also with the other target platforms, that, IIRC, are never tested automatically in Jenkins. I would stick with Linux only, which is way faster. I don't think it's worthwhile to test with macOS with so many configurations. (this would also simplify the maven command since it could be simply be always prefixed with xvfb-run)

The Jenkinsfile should not be changed either: really, I wouldn't want to always run with toolchains, even only because locally I wouldn't want to run with a toolchain unless strictly required. So, I prefer to have a run with no toolchain at all.

I also guess it was decided not to switch to another organization when we switched to the monorepo. In that respect, if I need to do GitHub Actions CI heavily, I do it on my fork (this was also suggested on the cross dev mailing list).

However, this is just my opinion, we should hear from @cdietrich and @szarnekow

@cdietrich cdietrich modified the milestones: Release_2.31, Release_2.32 May 29, 2023
@cdietrich cdietrich modified the milestones: Release_2.32, Release_2.33 Aug 27, 2023
@cdietrich cdietrich modified the milestones: Release_2.33, Release_2.34 Nov 17, 2023
@cdietrich cdietrich modified the milestones: Release_2.34, Release_2.35 Feb 27, 2024
@cdietrich cdietrich modified the milestones: Release_2.35, Release_2.36 May 23, 2024
@cdietrich cdietrich modified the milestones: Release_2.36, Release_2.37 Aug 25, 2024
@cdietrich cdietrich modified the milestones: Release_2.37, Release_2.38 Nov 16, 2024
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.

3 participants