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

Minor simplifications and clean-ups of build pipeline/workflows #2214

Merged
merged 1 commit into from
Apr 14, 2023
Merged

Minor simplifications and clean-ups of build pipeline/workflows #2214

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 6 additions & 14 deletions .github/workflows/external-pr.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,17 +40,9 @@ jobs:
restore-keys: ${{ runner.os }}-m2
if: ${{ env.BASE_REPO != env.PR_HEAD_REPO }}

- name: Show Maven version
run: |
which mvn
mvn -version

- name: Build and test (headless with xvfb in Linux)
run: xvfb-run mvn clean verify -PuseJenkinsSnapshots -Dsurefire.rerunFailingTestsCount=3
if: ${{ env.BASE_REPO != env.PR_HEAD_REPO && matrix.os == 'ubuntu-latest' }}
working-directory: org.eclipse.xtext.full.releng

- name: Build and test (in other OSes)
run: mvn clean verify -PuseJenkinsSnapshots -Dsurefire.rerunFailingTestsCount=3
if: ${{ env.BASE_REPO != env.PR_HEAD_REPO && matrix.os != 'ubuntu-latest' }}
working-directory: org.eclipse.xtext.full.releng
- name: Build and test
uses: coactions/setup-xvfb@v1
with:
run: mvn clean verify --show-version -PuseJenkinsSnapshots -Dsurefire.rerunFailingTestsCount=3
working-directory: org.eclipse.xtext.full.releng
if: ${{ env.BASE_REPO != env.PR_HEAD_REPO }}
26 changes: 6 additions & 20 deletions .github/workflows/maven.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,20 +30,11 @@ jobs:
key: ${{ runner.os }}-m2-${{ hashFiles('**/pom.xml', '**/*.target') }}
restore-keys: ${{ runner.os }}-m2

- name: Show Maven version
run: |
which mvn
mvn -version

- name: Build and test (headless with xvfb in Linux)
run: xvfb-run mvn clean verify -PuseJenkinsSnapshots -Dsurefire.rerunFailingTestsCount=3
if: matrix.os == 'ubuntu-latest'
working-directory: org.eclipse.xtext.full.releng

- name: Build and test (in other OSes)
run: mvn clean verify -PuseJenkinsSnapshots -Dsurefire.rerunFailingTestsCount=3
if: matrix.os != 'ubuntu-latest'
working-directory: org.eclipse.xtext.full.releng
- name: Build and test
Copy link
Contributor

Choose a reason for hiding this comment

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

This actions seems to trigger the installation of additional packages in GitHub Actions:

Run coactions/setup-xvfb@v1
  with:
    run: mvn clean verify --show-version -PuseJenkinsSnapshots -Dsurefire.rerunFailingTestsCount=3
    working-directory: org.eclipse.xtext.full.releng
  env:
    JAVA_HOME: /opt/hostedtoolcache/Java_Temurin-Hotspot_jdk/17.0.6-10/x64
    JAVA_HOME_17_X64: /opt/hostedtoolcache/Java_Temurin-Hotspot_jdk/17.0.6-10/x64
/usr/bin/sudo apt-get install -y xvfb x11-xserver-utils
Reading package lists...
Building dependency tree...
Reading state information...
xvfb is already the newest version (2:21.1.3-2ubuntu2.9).
Suggested packages:
  nickle cairo-5c xorg-docs-core
The following NEW packages will be installed:
  x11-xserver-utils
0 upgraded, 1 newly installed, 0 to remove and 9 not upgraded.

while the "if" solution did not require any additional package installation.

Besides not having the "if" and 2 steps, does the use of this action provide additional benefits?
Have you used this action in other projects?

Copy link
Member

Choose a reason for hiding this comment

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

question would be: why is xvfb-run insufficient.
we used that cause the original GabrielBB/xvfb-action
was no longer maintained
coactions/setup-xvfb
seems to be the new fork.
but i am not sure if needed cause xvfb-run seem to work.
should this be reported to coactions/setup-xvfb

Copy link
Member Author

Choose a reason for hiding this comment

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

Besides not having the "if" and 2 steps, does the use of this action provide additional benefits?

No more benefits I'm aware of.

Have you used this action in other projects?

Yes, for example we used the predecessor (as Christian mentioned) in M2E for a while and I just changed to the new one recently: eclipse-m2e/m2e-core#1344
And SWT uses it as well (I should probably update it there too):
https://github.com/eclipse-platform/eclipse.platform.swt/blob/6912a16abe78c905180908ea4d59218f6519c272/.github/workflows/maven.yml#L60

question would be: why is xvfb-run insufficient.

Probably, yes. I have not digged into the details, but as far as I understand this is mainly for conveniences and avoids the need for the if.
From https://github.com/coactions/setup-xvfb

This action installs [XVFB](http://elementalselenium.com/tips/38-headless) and runs your headless tests with it. It cleans up the xvfb process after your tests are done. If it detects you're not using linux then your tests still run, but without xvfb, which is very practical for multi-platform workflows.

So that's probably it.

Copy link
Member

Choose a reason for hiding this comment

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

we were using the garielbb for rerference project integ tests nor about 5 years
but we stopped using once it was no longer maintained
and github was issuing warnings about the deprecation feature use of that
this is why we then switchted to the alternative approach
xtext/xtext-reference-projects@bc0256f

Copy link
Member Author

Choose a reason for hiding this comment

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

this is why we then switchted to the alternative approach

Understand, but since the action has been revived I think things changed a bit.
Any way, just let me know which way you want and I can act accordingly.

Copy link
Member

Choose a reason for hiding this comment

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

using the new action is ok. the question is the warning already reported upstream or do we do something wrong

Copy link
Member Author

Choose a reason for hiding this comment

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

OK. 👍🏽
Probably not reported, at least I cannot see any issue on that topic.
This extra package installation was just recently added in coactions/setup-xvfb#5. Unfortunately there is not much context.
Maybe this can be made smarter by just installing the recommended packages with it, but I'm not an apt expert?

Copy link
Member

Choose a reason for hiding this comment

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

hmmm ok

Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see the warning in GitHub Actions.
If the action has been revived, I'm OK with that.
The apt installation doesn't seem to take a lot of time.
If something goes wrong we can revert.
@cdietrich I'd approve the PR, what about you?

uses: coactions/setup-xvfb@v1
with:
run: mvn clean verify --show-version -PuseJenkinsSnapshots -Dsurefire.rerunFailingTestsCount=3
working-directory: org.eclipse.xtext.full.releng

build-maven-artifacts:
runs-on: ubuntu-latest
Expand All @@ -65,11 +56,6 @@ jobs:
key: ${{ runner.os }}-maven-${{ hashFiles('**/pom.xml', '**/*.target') }}
restore-keys: ${{ runner.os }}-maven

- name: Show Maven version
run: |
which mvn
mvn -version

- name: Build Maven artifacts
run: mvn clean verify -PuseJenkinsSnapshots
run: mvn clean verify --show-version -PuseJenkinsSnapshots
working-directory: org.eclipse.xtext.maven.releng
20 changes: 10 additions & 10 deletions Jenkinsfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,16 +35,16 @@ pipeline {
stage('Initialize') {
steps {
checkout scm

script {
currentBuild.displayName = String.format("#%s(JDK%s,Eclipse%s)", BUILD_NUMBER, javaVersion(), eclipseVersion())
}

sh '''
if [ -f "/sys/fs/cgroup/memory/memory.limit_in_bytes" ]; then
echo "Available memory: $(cat /sys/fs/cgroup/memory/memory.limit_in_bytes | numfmt --to iec --format '%f')"
fi

sed_inplace() {
if [[ "$OSTYPE" == "darwin"* ]]; then
sed -i '' "$@"
Expand All @@ -57,12 +57,12 @@ pipeline {
}

stage('Maven/Tycho Build & Test') {
environment {
MAVEN_OPTS = "-Xmx1500m"
}
steps {
wrap([$class: 'Xvnc', takeScreenshot: false, useXauthority: true]) {
sh """
export MAVEN_OPTS=-Xmx1500m
./full-build.sh --tp=${selectedTargetPlatform()}
"""
xvnc(useXauthority: true) {
sh "./full-build.sh --tp=${selectedTargetPlatform()}"
}
}// END steps
} // END stage
Expand Down Expand Up @@ -138,7 +138,7 @@ def eclipseVersion() {
def targetDate = java.time.LocalDate.parse(targetPlatform.substring(1)+"01", df)
long monthsBetween = java.time.temporal.ChronoUnit.MONTHS.between(baseDate, targetDate);
return "4."+ (8+(monthsBetween/3))
}
}
}

/**
Expand All @@ -150,7 +150,7 @@ def selectedTargetPlatform() {
def tp = params.TARGET_PLATFORM
def isUpstream = isTriggeredByUpstream()
def javaVersion = javaVersion()

if (isTriggeredByUpstream() && javaVersion>=17) {
println("Choosing 'latest' target since this build was triggered by upstream with Java ${javaVersion}")
return 'latest'
Expand Down
2 changes: 1 addition & 1 deletion jenkins/nightly-deploy/Jenkinsfile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ pipeline {
steps {
withCredentials([file(credentialsId: 'secret-subkeys.asc', variable: 'KEYRING')]) {
sh 'gpg --batch --import "${KEYRING}"'
sh 'for fpr in $(gpg --list-keys --with-colons | awk -F: \'/fpr:/ {print $10}\' | sort -u); do echo -e "5\ny\n" | gpg --batch --command-fd 0 --expert --edit-key ${fpr} trust; done'
sh 'for fpr in $(gpg --list-keys --with-colons | awk -F: \'/fpr:/ {print $10}\' | sort -u); do echo -e "5\ny\n" | gpg --batch --command-fd 0 --expert --edit-key ${fpr} trust; done'
}
sshagent(['projects-storage.eclipse.org-bot-ssh']) {
sh './full-deploy.sh -Peclipse-sign,sonatype-oss-release,release-snapshot'
Expand Down