Composite actions and a new native matrix build#700
Conversation
There was a problem hiding this comment.
Code Review
This pull request modularizes the build and deployment workflow by introducing several GitHub composite actions and refactoring the installer scripts to support parameterized, matrix-based builds. Key feedback includes addressing a missing variable reference and commented-out build commands in build-installers.sh, setting the correct working directory for installer actions, avoiding bracket expressions in GitHub Actions if conditions, and correcting copy-pasted metadata in the deployment action.
I am having trouble creating individual review comments. Click here to see my feedback.
installers/build-installers.sh (14-15)
The variable nbpackage_version was completely removed from this script, but it is still referenced on line 17 (./nbpackage/nbpackage-$nbpackage_version/bin/nbpackage). This will cause the script to fail with a "No such file or directory" error because the path will resolve incorrectly. Please retain the nbpackage_version definition.
nbpackage_version="1.0"
.github/actions/build-installers/action.yml (18-24)
The scripts download-jdks.sh and build-installers.sh are located in the installers/ directory. Running them from the root directory without specifying the working directory will result in a "No such file or directory" error. Please set working-directory: installers for these steps.
- name: Download JDKs
run: bash download-jdks.sh ${{ inputs.os }} ${{ inputs.arch }}
working-directory: installers
shell: bash
- name: Build installers
run: bash build-installers.sh ${{ inputs.tag }} ${{ inputs.os }} ${{ inputs.arch }}
working-directory: installers
shell: bashinstallers/build-installers.sh (92-93)
The installer build commands are currently commented out, meaning the script will not build any installers. Additionally, with the new matrix build, the script should be updated to only build the specific OS and architecture passed as arguments, rather than building all architectures or using outdated arguments.
if [ "$2" == "linux" ]; then
build_nbpackage linux "$3" "jmonkeyengine-$3-deb.properties" "$versionString"
elif [ "$2" == "windows" ]; then
setup_inno_setup "headless"
build_nbpackage windows "$3" "jmonkeyengine-windows-$3.properties" "$versionString"
fi
.github/actions/build-sdk/action.yml (31-39)
Using ${{ }} inside an if condition is not recommended in GitHub Actions and can lead to expression parsing errors. It is cleaner and safer to reference the inputs directly.
- name: Build the SDK (with tag)
if: inputs.tag != ''
run: ./gradlew buildSdk -Ptag_name=${{ inputs.tag }}
shell: bash
- name: Build the SDK
if: inputs.tag == ''
run: ./gradlew buildSdk
shell: bash.github/actions/nbm-deployment/action.yml (1-2)
The name and description of this action appear to be copy-pasted from the build-sdk action. Please update them to accurately describe the NBM deployment / nightly trigger action.
name: "NBM Deployment"
description: "Deploys NBMs to the SDK Update Center"There was a problem hiding this comment.
Pull request overview
This PR refactors the CI/release pipeline toward reusable composite actions and introduces a new “native” matrix packaging stage intended to build installers on their target OS/arch (per #699).
Changes:
- Added composite actions for SDK build, installer preparation, installer build, and nightly NBM deployment.
- Refactored installer preparation into a dedicated script and adjusted JDK download/build scripts to accept OS/arch parameters.
- Reworked the release workflow into a 3-stage pipeline: build+bundle (Linux) → matrix package → publish release.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
installers/prepare-installers.sh |
New script to download and extract NBPackage for installer builds. |
installers/download-jdks.sh |
Changed to build/download a single JDK based on passed OS/arch args. |
installers/build-installers.sh |
Removed inline NBPackage setup and adjusted parameters/logging; installer invocation currently disabled. |
.github/workflows/release.yml |
Replaced inline steps with composite actions and added a native packaging matrix + release stage. |
.github/workflows/gradle.yml |
Refactored nightly build/deploy to composite actions. |
.github/actions/prepare-installers/action.yml |
New composite action to run NBPackage preparation. |
.github/actions/nbm-deployment/action.yml |
New composite action to trigger sdk-update-center nightly builds. |
.github/actions/build-sdk/action.yml |
New composite action to checkout, setup Java, and build SDK (+ bundle for releases). |
.github/actions/build-installers/action.yml |
New composite action to download JDKs and run installer build script. |
Comments suppressed due to low confidence (1)
installers/build-installers.sh:18
nbpackage_versionis no longer defined in this script, but it’s still used to build the nbpackage path (./nbpackage/nbpackage-$nbpackage_version/...). This will resolve to./nbpackage/nbpackage-/...and fail.
set -e # Quit on Error
inno_setup_url="https://files.jrsoftware.org/is/6/innosetup-6.5.1.exe"
function build_nbpackage {
echo ">> Building the nbpackage installer for $1-$2"
./nbpackage/nbpackage-$nbpackage_version/bin/nbpackage --input ../dist/jmonkeyplatform.zip --config "$1-$2/$3" --output ../dist/ -v -Ppackage.version="$4"
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - name: Trigger nightly builds | ||
| working-directory: nightly | ||
| run: git add target && git commit -m "Trigger a fresh nightly build for https://github.com/jMonkeyEngine/sdk/commit/${{ github.sha }}" && git push | ||
| token: ${{ secrets. UPDATE_CENTER_PAT }} No newline at end of file |
| - name: Build the SDK (with tag) | ||
| if: ${{ inputs.tag }} != null | ||
| run: ./gradlew buildSdk -Ptag_name=${{ inputs.tag }} | ||
| shell: bash | ||
|
|
||
| - name: Build the SDK | ||
| if: ${{ inputs.tag }} == null | ||
| run: ./gradlew buildSdk | ||
| shell: bash |
| - name: Override Harness (custom icon) | ||
| if: ${{ inputs.createBundle }} | ||
| run: ./gradlew overrideHarness -Ptag_name=${{ github.ref_name }} | ||
| shell: bash | ||
|
|
||
| - name: Build distributable ZIP | ||
| if: ${{ inputs.createBundle }} | ||
| run: ant -Dstorepass="$NBM_SIGN_PASS" -Dpack200.enabled=false set-spec-version build-zip unset-spec-version |
| required: false | ||
| default: false | ||
| type: boolean | ||
| description: "Tag should be used for releases" |
| - name: Download JDKs | ||
| run: bash download-jdks.sh ${{ inputs.os }} ${{ inputs.arch }} | ||
| shell: bash | ||
|
|
||
| - name: Build installers | ||
| run: bash build-installers.sh ${{ inputs.tag }} ${{ inputs.os }} ${{ inputs.arch }} | ||
| shell: bash No newline at end of file |
| echo "Building JDK with on $1 arch $2" | ||
| get_jdk $1 $2 |
| #build_linux_deb "$versionString" | ||
| #build_windows_installer "$versionString" "$2" |
| name: "Build SDK" | ||
| description: "Setups and builds the SDK (Linux)" | ||
|
|
| - name: Publish release | ||
| uses: ./.github/actions/publish-release | ||
| with: | ||
| tag: ${{ github.ref }} |
This reverts commit 957e25b.
Resolves #699, resolves #697