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

Fix potential github action smells #2684

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Fix potential github action smells #2684

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f136145
Fix gha smells:
ceddy4395 May 24, 2024
1bde077
Include suggestions from PR:
ceddy4395 May 27, 2024
2b20b0b
Fix gha smells in other workflows as well:
ceddy4395 May 27, 2024
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
27 changes: 14 additions & 13 deletions .github/workflows/build.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,8 +16,8 @@ jobs:
- java: 21
# Disable Enforcer check which (intentionally) prevents using JDK 21 for building
extra-mvn-args: -Denforcer.fail=false
runs-on: ubuntu-latest

runs-on: ubuntu-22.04
timeout-minutes: 5
steps:
- uses: actions/checkout@0ad4b8fadaa221de15dcec353f45205ec38ea70b # v4.1.4
- name: "Set up JDK ${{ matrix.java }}"
Expand All @@ -32,8 +32,8 @@ jobs:

native-image-test:
name: "GraalVM Native Image test"
runs-on: ubuntu-latest

runs-on: ubuntu-22.04
timeout-minutes: 10
steps:
- uses: actions/checkout@0ad4b8fadaa221de15dcec353f45205ec38ea70b # v4.1.4
- name: "Set up GraalVM"
Expand All @@ -51,8 +51,8 @@ jobs:

verify-reproducible-build:
name: "Verify reproducible build"
runs-on: ubuntu-latest

runs-on: ubuntu-22.04
timeout-minutes: 10
steps:
- uses: actions/checkout@0ad4b8fadaa221de15dcec353f45205ec38ea70b # v4.1.4
- name: "Set up JDK 17"
Expand All @@ -65,11 +65,12 @@ jobs:
- name: "Verify no plugin issues"
run: mvn artifact:check-buildplan --batch-mode --no-transfer-progress

- name: "Verify reproducible build"
# See https://maven.apache.org/guides/mini/guide-reproducible-builds.html#how-to-test-my-maven-build-reproducibility
Copy link
Collaborator

Choose a reason for hiding this comment

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

Might be good to move this # See ... comment above the name: "Do clean ... since it applies to both steps now and not just the mvn clean install.

Copy link
Author

Choose a reason for hiding this comment

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

Ah oke yes makes sense. I've updated this.

run: |
mvn clean install --batch-mode --no-transfer-progress -Dproguard.skip -DskipTests
# Run with `-Dbuildinfo.attach=false`; otherwise `artifact:compare` fails because it creates a `.buildinfo` file which
# erroneously references the existing `.buildinfo` file (respectively because it is overwriting it, a file with size 0)
# See https://issues.apache.org/jira/browse/MARTIFACT-57
mvn clean verify artifact:compare --batch-mode --no-transfer-progress -Dproguard.skip -DskipTests -Dbuildinfo.attach=false
- name: "Build project"
run: mvn clean install --batch-mode --no-transfer-progress -Dproguard.skip -DskipTests

- name: "Verify reproducible build"
# Run with `-Dbuildinfo.attach=false`; otherwise `artifact:compare` fails because it creates a `.buildinfo` file which
# erroneously references the existing `.buildinfo` file (respectively because it is overwriting it, a file with size 0)
# See https://issues.apache.org/jira/browse/MARTIFACT-57
run: mvn clean verify artifact:compare --batch-mode --no-transfer-progress -Dproguard.skip -DskipTests -Dbuildinfo.attach=false
9 changes: 4 additions & 5 deletions .github/workflows/check-android-compatibility.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ permissions:

jobs:
check-android-compatibility:
runs-on: ubuntu-latest

runs-on: ubuntu-22.04
timeout-minutes: 5
steps:
- uses: actions/checkout@0ad4b8fadaa221de15dcec353f45205ec38ea70b # v4.1.4

Expand All @@ -24,6 +24,5 @@ jobs:
cache: 'maven'

- name: Check Android compatibility
run: |
# Run 'test' phase because plugin normally expects to be executed after tests have been compiled
mvn --batch-mode --no-transfer-progress test animal-sniffer:check@check-android-compatibility -DskipTests
# Run 'test' phase because plugin normally expects to be executed after tests have been compiled
run: mvn --batch-mode --no-transfer-progress test animal-sniffer:check@check-android-compatibility -DskipTests
20 changes: 9 additions & 11 deletions .github/workflows/check-api-compatibility.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ on: pull_request

jobs:
check-api-compatibility:
runs-on: ubuntu-latest

runs-on: ubuntu-22.04
timeout-minutes: 5
steps:
- name: Checkout old version
uses: actions/checkout@0ad4b8fadaa221de15dcec353f45205ec38ea70b # v4.1.4
Expand All @@ -22,21 +22,19 @@ jobs:
java-version: '11'
cache: 'maven'

- name: Build old version
run: |
cd gson-old-japicmp
# Set dummy version
mvn --batch-mode --no-transfer-progress org.codehaus.mojo:versions-maven-plugin:2.11.0:set -DnewVersion=JAPICMP-OLD
# Install artifacts with dummy version in local repository; used later by Maven plugin for comparison
mvn --batch-mode --no-transfer-progress install -DskipTests
- name: Set dummy version
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe the "used later by Maven plugin for comparison" should be preserved in some way (e.g. as comment) to make it clearer why this is done.

What about this?

Suggested change
- name: Set dummy version
# First sets a dummy project version, then installs the artifacts with that version in the local
# Maven repository, and in a later step uses it as 'old version' for the API compatibility check.
- name: Set dummy version

working-directory: gson-old-japicmp
run: mvn --batch-mode --no-transfer-progress org.codehaus.mojo:versions-maven-plugin:2.11.0:set -DnewVersion=JAPICMP-OLD
- name: Install artifacts with dummy version
working-directory: gson-old-japicmp
run: mvn --batch-mode --no-transfer-progress install -DskipTests

- name: Checkout new version
uses: actions/checkout@0ad4b8fadaa221de15dcec353f45205ec38ea70b # v4.1.4

- name: Check API compatibility
id: check-compatibility
run: |
mvn --batch-mode --fail-at-end --no-transfer-progress package japicmp:cmp -DskipTests
run: mvn --batch-mode --fail-at-end --no-transfer-progress package japicmp:cmp -DskipTests

- name: Upload API differences artifacts
uses: actions/upload-artifact@65462800fd760344b1a7b4382951275a0abb4808 # v4.3.3
Expand Down
3 changes: 2 additions & 1 deletion .github/workflows/cifuzz.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,8 @@ name: CIFuzz
on: [pull_request]
jobs:
Fuzzing:
runs-on: ubuntu-latest
runs-on: ubuntu-22.04
timeout-minutes: 20
steps:
- name: Build Fuzzers
id: build
Expand Down
6 changes: 3 additions & 3 deletions .github/workflows/codeql-analysis.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@ on:
jobs:
analyze:
name: Analyze
runs-on: ubuntu-latest
runs-on: ubuntu-22.04
timeout-minutes: 5
permissions:
security-events: write

Expand Down Expand Up @@ -46,8 +47,7 @@ jobs:
# be that relevant (though GitHub security view also allows filtering by source type)
# Can replace this with github/codeql-action/autobuild action to run complete build
- name: Compile sources
run: |
mvn compile --batch-mode --no-transfer-progress
run: mvn compile --batch-mode --no-transfer-progress

- name: Perform CodeQL Analysis
uses: github/codeql-action/analyze@d39d31e687223d841ef683f52467bd88e9b21c14 # v3.25.3