From 84c4b461a9373d9ada9e953cd8b2d1feba8402ef Mon Sep 17 00:00:00 2001 From: nrobi144 Date: Fri, 17 Apr 2026 11:03:01 +0300 Subject: [PATCH] fix(release): address code review findings (P1-P3) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit P1 fixes (must-fix): - AppRun: VLC path corrected to usr/lib/app/linux/vlc (jpackage actual path), add VLC_PLUGIN_PATH env var for codec discovery - linuxdeploy: use APPIMAGE_EXTRACT_AND_RUN=1 env var to bypass FUSE on CI runners instead of fragile pre-extract + symlink approach - Prerelease classifier: inverted to positive-match stable format (^v[0-9]+\.[0-9]+\.[0-9]+$) across desktop, Android, and composite action — eliminates regex drift between allowlists - assert-stable-release: now accepts inputs (tag, is_prerelease, is_draft) so workflow_dispatch on bump workflows also validates P2 fixes (should-fix): - .gitignore: add linuxdeploy-*.AppImage and extracted dirs - RPM version: use replace("-", "~") instead of substringBefore("-") for correct RPM prerelease ordering (1.08.0~rc1 < 1.08.0) - linux-portable → linux alias moved into collect_assets() in scripts/asset-name.sh (single source of truth) - Android cache key: add gradle/libs.versions.toml to hashFiles - r0adkll/sign-android-release: SHA-pinned to 349ebdef (v1) P3 fixes (nice-to-have): - LINUXDEPLOY_OUTPUT_VERSION env var replaced with APPIMAGE_EXTRACT_AND_RUN (misleading comment + redundant var) - nick-fields/retry timeout: 40m → 15m (surface real hangs) - generate_release_notes: true only on Android job (avoid last-writer-wins race across 6 concurrent uploaders) - apt-get install rpm: tightened guard to matrix.family == 'linux' (linux-portable leg doesn't need rpm tooling) --- .../actions/assert-stable-release/action.yml | 33 ++++++++------ .github/workflows/bump-homebrew.yml | 5 ++- .github/workflows/bump-winget.yml | 5 ++- .github/workflows/create-release.yml | 45 ++++++++----------- .gitignore | 5 +++ desktopApp/build.gradle.kts | 14 +++--- packaging/appimage/AppRun | 8 ++-- scripts/asset-name.sh | 2 + 8 files changed, 65 insertions(+), 52 deletions(-) diff --git a/.github/actions/assert-stable-release/action.yml b/.github/actions/assert-stable-release/action.yml index 85358cb06..9d754a447 100644 --- a/.github/actions/assert-stable-release/action.yml +++ b/.github/actions/assert-stable-release/action.yml @@ -2,9 +2,20 @@ name: Assert Stable Release description: >- Defense-in-depth guard for package-manager bump workflows. Re-validates tag format, prerelease flag, and draft status before invoking third-party - actions that hold write credentials to external package manager repos - (Homebrew, Winget, AUR, Scoop). Prevents RC builds from reaching stable - channels even if the `release.released` event gating is bypassed. + actions that hold write credentials to external package manager repos. + +inputs: + tag: + description: "Tag to validate (e.g. v1.08.0)" + required: true + is_prerelease: + description: "Whether the release is marked as prerelease (empty string treated as false)" + required: false + default: "false" + is_draft: + description: "Whether the release is a draft (empty string treated as false)" + required: false + default: "false" runs: using: composite @@ -12,22 +23,16 @@ runs: - name: Assert release is stable shell: bash env: - TAG: ${{ github.event.release.tag_name }} - IS_PRERELEASE: ${{ github.event.release.prerelease }} - IS_DRAFT: ${{ github.event.release.draft }} + TAG: ${{ inputs.tag }} + IS_PRERELEASE: ${{ inputs.is_prerelease }} + IS_DRAFT: ${{ inputs.is_draft }} run: | set -euo pipefail echo "tag=$TAG prerelease=$IS_PRERELEASE draft=$IS_DRAFT" - # Reject prerelease suffix even if GitHub's flag says false. - if [[ "$TAG" =~ -(rc|beta|alpha|dev|snapshot) ]]; then - echo "::error::Tag $TAG contains prerelease suffix; refusing bump" - exit 1 - fi - - # Enforce strict vMAJOR.MINOR.PATCH format. + # Stable = exactly vMAJOR.MINOR.PATCH; reject everything else. if ! [[ "$TAG" =~ ^v[0-9]+\.[0-9]+\.[0-9]+$ ]]; then - echo "::error::Tag $TAG does not match vMAJOR.MINOR.PATCH" + echo "::error::Tag $TAG does not match stable vMAJOR.MINOR.PATCH format; refusing bump" exit 1 fi diff --git a/.github/workflows/bump-homebrew.yml b/.github/workflows/bump-homebrew.yml index 993ffba5d..478409386 100644 --- a/.github/workflows/bump-homebrew.yml +++ b/.github/workflows/bump-homebrew.yml @@ -31,8 +31,11 @@ jobs: uses: actions/checkout@v6 - name: Re-assert stable release - if: github.event_name == 'release' uses: ./.github/actions/assert-stable-release + with: + tag: ${{ github.event.release.tag_name || inputs.tag }} + is_prerelease: ${{ github.event.release.prerelease || 'false' }} + is_draft: ${{ github.event.release.draft || 'false' }} - name: Bump cask (push-or-update PR) uses: macauley/action-homebrew-bump-cask@ad984534de44a9489a53aefd81eb77f87c70dc60 # v4.0.0 diff --git a/.github/workflows/bump-winget.yml b/.github/workflows/bump-winget.yml index 06a806a2f..be3cc9115 100644 --- a/.github/workflows/bump-winget.yml +++ b/.github/workflows/bump-winget.yml @@ -27,8 +27,11 @@ jobs: uses: actions/checkout@v6 - name: Re-assert stable release - if: github.event_name == 'release' uses: ./.github/actions/assert-stable-release + with: + tag: ${{ github.event.release.tag_name || inputs.tag }} + is_prerelease: ${{ github.event.release.prerelease || 'false' }} + is_draft: ${{ github.event.release.draft || 'false' }} - name: Submit manifest to winget-pkgs uses: vedantmgoyal9/winget-releaser@4ffc7888bffd451b357355dc214d43bb9f23917e # v2 diff --git a/.github/workflows/create-release.yml b/.github/workflows/create-release.yml index 9a334fc8f..8b81801f2 100644 --- a/.github/workflows/create-release.yml +++ b/.github/workflows/create-release.yml @@ -80,8 +80,8 @@ jobs: echo "version=$VER" >> "$GITHUB_OUTPUT" echo "toml=$TOML_VER" >> "$GITHUB_OUTPUT" - - name: Install RPM tooling (linux families) - if: startsWith(matrix.family, 'linux') + - name: Install RPM tooling (deb+rpm leg only) + if: matrix.family == 'linux' run: sudo apt-get update && sudo apt-get install -y rpm fakeroot - name: Fetch linuxdeploy (linux-portable only, SHA-verified) @@ -95,18 +95,12 @@ jobs: exit 1 fi chmod +x packaging/appimage/linuxdeploy-x86_64.AppImage - # linuxdeploy needs FUSE; on newer runners, --appimage-extract-and-run is required. - # Bypass FUSE requirement by pre-extracting the AppImage. - (cd packaging/appimage && ./linuxdeploy-x86_64.AppImage --appimage-extract >/dev/null && \ - mv squashfs-root linuxdeploy-extracted && \ - ln -sf linuxdeploy-extracted/AppRun linuxdeploy-x86_64.bin && \ - chmod +x linuxdeploy-x86_64.bin) - name: Build desktop artifacts uses: nick-fields/retry@ce71cc2ab81d554ebbe88c79ab5975992d79ba08 # v3.0.2 with: max_attempts: 2 - timeout_minutes: 40 + timeout_minutes: 15 command: ./gradlew --no-daemon :desktopApp:${{ matrix.tasks }} - name: Build portable archives (windows + linux-portable) @@ -127,11 +121,8 @@ jobs: set -euo pipefail # shellcheck source=scripts/asset-name.sh source scripts/asset-name.sh - # linux-portable family holds AppImage + tar.gz, but we re-tag with plain `linux` for the asset name. - FAMILY="${{ matrix.family }}" - [[ "$FAMILY" == "linux-portable" ]] && FAMILY="linux" - mkdir -p dist - collect_assets "$FAMILY" "${{ matrix.arch }}" "${{ steps.ver.outputs.version }}" dist + # collect_assets normalizes linux-portable → linux internally. + collect_assets "${{ matrix.family }}" "${{ matrix.arch }}" "${{ steps.ver.outputs.version }}" dist - name: Enforce asset size budget (1 GB per asset) run: | @@ -155,10 +146,11 @@ jobs: id: classify run: | TAG="${{ steps.ver.outputs.tag }}" - if [[ "$TAG" =~ -(rc|beta|alpha|dev|dryrun|snapshot) ]]; then - echo "prerelease=true" >> "$GITHUB_OUTPUT" - else + # Stable = exactly vMAJOR.MINOR.PATCH; everything else is prerelease. + if [[ "$TAG" =~ ^v[0-9]+\.[0-9]+\.[0-9]+$ ]]; then echo "prerelease=false" >> "$GITHUB_OUTPUT" + else + echo "prerelease=true" >> "$GITHUB_OUTPUT" fi - name: Upload to GH Release (skip on dry-run) @@ -170,7 +162,7 @@ jobs: prerelease: ${{ steps.classify.outputs.prerelease }} draft: false fail_on_unmatched_files: true - generate_release_notes: true + generate_release_notes: false # Android job writes release notes (last-writer-wins race) - name: Dry-run summary if: github.event_name == 'workflow_dispatch' && github.event.inputs.dry_run == 'true' @@ -205,7 +197,7 @@ jobs: path: | ~/.gradle/caches ~/.gradle/wrapper - key: ${{ runner.os }}-android-gradle-${{ hashFiles('**/*.gradle*', '**/gradle-wrapper.properties') }} + key: ${{ runner.os }}-android-gradle-${{ hashFiles('**/*.gradle*', '**/gradle-wrapper.properties', 'gradle/libs.versions.toml') }} restore-keys: | ${{ runner.os }}-android-gradle- @@ -213,7 +205,7 @@ jobs: run: ./gradlew clean bundleRelease --stacktrace - name: Sign AAB (Google Play) - uses: r0adkll/sign-android-release@v1 + uses: r0adkll/sign-android-release@349ebdef58775b1e0d8099458af0816dc79b6407 # v1 with: releaseDirectory: amethyst/build/outputs/bundle/playRelease signingKeyBase64: ${{ secrets.SIGNING_KEY }} @@ -224,7 +216,7 @@ jobs: BUILD_TOOLS_VERSION: "36.0.0" - name: Sign AAB (F-Droid) - uses: r0adkll/sign-android-release@v1 + uses: r0adkll/sign-android-release@349ebdef58775b1e0d8099458af0816dc79b6407 # v1 with: releaseDirectory: amethyst/build/outputs/bundle/fdroidRelease signingKeyBase64: ${{ secrets.SIGNING_KEY }} @@ -238,7 +230,7 @@ jobs: run: ./gradlew assembleRelease --stacktrace - name: Sign APK (Google Play) - uses: r0adkll/sign-android-release@v1 + uses: r0adkll/sign-android-release@349ebdef58775b1e0d8099458af0816dc79b6407 # v1 with: releaseDirectory: amethyst/build/outputs/apk/play/release signingKeyBase64: ${{ secrets.SIGNING_KEY }} @@ -249,7 +241,7 @@ jobs: BUILD_TOOLS_VERSION: "36.0.0" - name: Sign APK (F-Droid) - uses: r0adkll/sign-android-release@v1 + uses: r0adkll/sign-android-release@349ebdef58775b1e0d8099458af0816dc79b6407 # v1 with: releaseDirectory: amethyst/build/outputs/apk/fdroid/release signingKeyBase64: ${{ secrets.SIGNING_KEY }} @@ -288,10 +280,11 @@ jobs: id: classify run: | TAG="${GITHUB_REF_NAME}" - if [[ "$TAG" =~ -(rc|beta|alpha|dev|snapshot) ]]; then - echo "prerelease=true" >> "$GITHUB_OUTPUT" - else + # Stable = exactly vMAJOR.MINOR.PATCH; everything else is prerelease. + if [[ "$TAG" =~ ^v[0-9]+\.[0-9]+\.[0-9]+$ ]]; then echo "prerelease=false" >> "$GITHUB_OUTPUT" + else + echo "prerelease=true" >> "$GITHUB_OUTPUT" fi - name: Upload Android assets to GH Release diff --git a/.gitignore b/.gitignore index aa8cf0a66..387265dae 100644 --- a/.gitignore +++ b/.gitignore @@ -164,6 +164,11 @@ desktopApp/src/jvmMain/appResources/linux/ desktopApp/src/jvmMain/appResources/macos/ desktopApp/src/jvmMain/appResources/windows/ +# CI-fetched AppImage tooling (downloaded by create-release workflow; not committed) +packaging/appimage/linuxdeploy-x86_64.AppImage +packaging/appimage/linuxdeploy-extracted/ +packaging/appimage/squashfs-root/ + # Git worktrees .worktrees/ .claude/worktrees/ diff --git a/desktopApp/build.gradle.kts b/desktopApp/build.gradle.kts index 742051e2e..bd014b97b 100644 --- a/desktopApp/build.gradle.kts +++ b/desktopApp/build.gradle.kts @@ -9,10 +9,9 @@ plugins { id("ir.mahozad.vlc-setup") version "0.1.0" } -// RPM rejects dashes in version strings — strip prerelease suffix for Linux RPM only. -// Other formats accept full semver (DEB uses ~rc1, DMG/MSI accept bare versions). +// RPM rejects dashes in version strings — replace with tilde (~) which RPM uses +// for prerelease ordering: 1.08.0~rc1 < 1.08.0 per RPM version comparison rules. val appVersion: String = project.version.toString() -val appVersionRpm: String = appVersion.substringBefore("-") sourceSets { main { @@ -120,8 +119,8 @@ compose.desktop { appCategory = "Network" debMaintainer = "vitor@vitorpamplona.com" rpmLicenseType = "MIT" - // RPM version field rejects dashes; strip prerelease suffix for RPM builds. - rpmPackageVersion = appVersionRpm + // RPM version: replace dashes with tilde (1.08.0~rc1 < 1.08.0 per RPM ordering). + rpmPackageVersion = appVersion.replace("-", "~") } } } @@ -201,6 +200,7 @@ val createReleaseAppImage by tasks.registering(Exec::class) { ) environment("OUTPUT", outFile.get().asFile.absolutePath) environment("ARCH", "x86_64") - // Suppress linuxdeploy's verbose library-scanner output; keep errors. - environment("LINUXDEPLOY_OUTPUT_VERSION", appVersion) + // Bypass FUSE requirement on CI runners (ubuntu-latest lacks libfuse.so.2). + // AppImage standard env var: extracts + runs without mounting. + environment("APPIMAGE_EXTRACT_AND_RUN", "1") } diff --git a/packaging/appimage/AppRun b/packaging/appimage/AppRun index 9b2fb923d..d4065af01 100755 --- a/packaging/appimage/AppRun +++ b/packaging/appimage/AppRun @@ -1,9 +1,11 @@ #!/bin/bash # AppImage launcher for Amethyst Desktop. -# Sets LD_LIBRARY_PATH to find bundled VLC natives (vlcj dlopens libvlc.so at runtime). -set -e +# Sets LD_LIBRARY_PATH so vlcj finds bundled libvlc.so at runtime. +# jpackage puts app resources at usr/lib/app//vlc/ inside the AppDir. +set -eu HERE="$(dirname "$(readlink -f "${0}")")" -export LD_LIBRARY_PATH="${HERE}/usr/lib:${HERE}/usr/lib/vlc:${HERE}/usr/lib/x86_64-linux-gnu:${LD_LIBRARY_PATH:-}" +export LD_LIBRARY_PATH="${HERE}/usr/lib/app/linux/vlc:${HERE}/usr/lib:${HERE}/usr/lib/x86_64-linux-gnu:${LD_LIBRARY_PATH:-}" +export VLC_PLUGIN_PATH="${HERE}/usr/lib/app/linux/vlc/plugins" export PATH="${HERE}/usr/bin:${PATH}" export APPDIR="${HERE}" exec "${HERE}/usr/bin/Amethyst" "$@" diff --git a/scripts/asset-name.sh b/scripts/asset-name.sh index 9010e1d02..eab8f6bed 100755 --- a/scripts/asset-name.sh +++ b/scripts/asset-name.sh @@ -38,6 +38,8 @@ asset_name() { # Expects build outputs under desktopApp/build/... (Compose binaries + custom tasks + portable archives). collect_assets() { local family="$1" arch="$2" version="$3" dest="$4" + # Normalize internal matrix family aliases to canonical asset-name families. + [[ "$family" == "linux-portable" ]] && family="linux" mkdir -p "$dest" shopt -s nullglob