From 49d179738bd6f53f3bda8139241dca16debb5fea Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Fri, 23 Oct 2026 11:17:22 -0400 Subject: [PATCH] packaging: use [[ tests, named locals and explicit returns (SonarCloud S7688/S7679/S7682) (#2187) Switch the bash packaging scripts from [ to [[ for conditional tests, give positional parameters descriptive local names, and end the shared env.sh helpers with an explicit return. No behaviour change: every comparison is against a literal, and a bare return keeps the last command's status. --- packaging/env.sh | 15 ++++++++++----- packaging/freebsd/package.sh | 16 ++++++++-------- packaging/icons.sh | 5 ++++- packaging/linux/flatpak-bundle.sh | 8 ++++---- packaging/linux/package.sh | 21 ++++++++++++--------- packaging/macos/import-cert.sh | 6 +++--- packaging/macos/package.sh | 26 +++++++++++++------------- packaging/web/package.sh | 6 +++--- 8 files changed, 57 insertions(+), 46 deletions(-) diff --git a/packaging/env.sh b/packaging/env.sh index 6a1bfa28..68b57026 100644 --- a/packaging/env.sh +++ b/packaging/env.sh @@ -19,10 +19,11 @@ workspace_version() { /^\[/ { in_pkg = ($0 == "[workspace.package]") ; next } in_pkg && $1 == "version" { gsub(/[" ]/, "", $3); print $3; exit } ' "$ROOT/Cargo.toml" + return } VERSION="${EFFECTCRAFT_VERSION:-$(workspace_version)}" -if [ -z "$VERSION" ]; then +if [[ -z "$VERSION" ]]; then echo "error: could not read [workspace.package] version from $ROOT/Cargo.toml" >&2 exit 1 fi @@ -32,7 +33,7 @@ DIST="${DIST:-$ROOT/dist/release}" mkdir -p "$DIST" export DIST -if [ -z "${EFFECTCRAFT_BUILD_SHA:-}" ]; then +if [[ -z "${EFFECTCRAFT_BUILD_SHA:-}" ]]; then EFFECTCRAFT_BUILD_SHA="$(git -C "$ROOT" rev-parse HEAD 2>/dev/null || true)" fi export EFFECTCRAFT_BUILD_SHA @@ -41,18 +42,22 @@ export CARGO_TARGET_DIR="${CARGO_TARGET_DIR:-$ROOT/target}" # Emit a GitHub Actions warning (plain stderr outside Actions). warn() { - if [ -n "${GITHUB_ACTIONS:-}" ]; then echo "::warning::$*"; else echo "warning: $*" >&2; fi + if [[ -n "${GITHUB_ACTIONS:-}" ]]; then echo "::warning::$*"; else echo "warning: $*" >&2; fi + return } # Copy licence and readme files that exist into a package directory. copy_docs() { local dest="$1" f for f in README.md LICENSE LICENSE-MIT LICENSE-APACHE COPYRIGHT; do - if [ -f "$ROOT/$f" ]; then cp "$ROOT/$f" "$dest/"; fi + if [[ -f "$ROOT/$f" ]]; then cp "$ROOT/$f" "$dest/"; fi done + return } # Portable SHA-256 of a file (prints just the hash). sha256() { - if command -v sha256sum >/dev/null; then sha256sum "$1" | cut -d' ' -f1; else shasum -a 256 "$1" | cut -d' ' -f1; fi + local file="$1" + if command -v sha256sum >/dev/null; then sha256sum "$file" | cut -d' ' -f1; else shasum -a 256 "$file" | cut -d' ' -f1; fi + return } diff --git a/packaging/freebsd/package.sh b/packaging/freebsd/package.sh index 7418dd85..b5d1dc44 100755 --- a/packaging/freebsd/package.sh +++ b/packaging/freebsd/package.sh @@ -20,7 +20,7 @@ APP_ID=ai.storyteller.effectcraft SKIP_BUILD=0 DRY_RUN=0 -while [ $# -gt 0 ]; do +while [[ $# -gt 0 ]]; do case "$1" in --skip-build) SKIP_BUILD=1; shift ;; --dry-run) DRY_RUN=1; SKIP_BUILD=1; shift ;; @@ -36,7 +36,7 @@ case "$MACHINE" in arm64 | aarch64) ARCH=aarch64 ;; *) echo "unsupported architecture $MACHINE" >&2; exit 2 ;; esac -if [ "$DRY_RUN" = 0 ] && [ "$(uname -s)" != FreeBSD ]; then +if [[ "$DRY_RUN" = 0 ]] && [[ "$(uname -s)" != FreeBSD ]]; then echo "error: build on FreeBSD (or pass --dry-run to check the tree layout)" >&2 exit 2 fi @@ -50,11 +50,11 @@ export CARGO_BUILD_JOBS="${CARGO_BUILD_JOBS:-4}" # natively under QEMU). With one codegen unit and thin LTO the largest crates each optimize on a # single emulated core and the job runs past its 5-hour limit, so trade a little runtime speed for # parallel codegen there. x86_64 keeps the workspace's release profile. -if [ "$ARCH" = aarch64 ]; then +if [[ "$ARCH" = aarch64 ]]; then export CARGO_PROFILE_RELEASE_CODEGEN_UNITS="${CARGO_PROFILE_RELEASE_CODEGEN_UNITS:-16}" export CARGO_PROFILE_RELEASE_LTO="${CARGO_PROFILE_RELEASE_LTO:-off}" fi -if [ "$SKIP_BUILD" = 0 ]; then +if [[ "$SKIP_BUILD" = 0 ]]; then (cd "$ROOT" && cargo build --release --locked -p effectcraft -p effectcraft-cli) fi WORK="$CARGO_TARGET_DIR/freebsd-package" @@ -64,7 +64,7 @@ mkdir -p "$WORK" BIN="$CARGO_TARGET_DIR/release" OUT_DIR="$DIST" -if [ "$DRY_RUN" = 1 ]; then +if [[ "$DRY_RUN" = 1 ]]; then # Stub binaries, and the tarball stays in the work dir so dist/ only ever holds real builds. BIN="$WORK/stub-bin" OUT_DIR="$WORK" @@ -89,13 +89,13 @@ sed -e "s/@VERSION@/$VERSION/g" -e "s/@DATE@/$EFFECTCRAFT_BUILD_DATE/g" \ cp -R "$ROOT/assets/app-icon/hicolor" "$STAGE/share/icons/" copy_docs "$STAGE/share/doc/effectcraft" for f in NOTICE ATTRIBUTION.md; do - if [ -f "$ROOT/$f" ]; then install -m 644 "$ROOT/$f" "$STAGE/share/doc/effectcraft/"; fi + if [[ -f "$ROOT/$f" ]]; then install -m 644 "$ROOT/$f" "$STAGE/share/doc/effectcraft/"; fi done for f in bin/effectcraft bin/effectcraft-cli "share/applications/$APP_ID.desktop" \ "share/icons/hicolor/256x256/apps/$APP_ID.png" "share/icons/hicolor/scalable/apps/$APP_ID.svg" \ share/doc/effectcraft/LICENSE-MIT share/doc/effectcraft/LICENSE-APACHE; do - if [ ! -e "$STAGE/$f" ]; then echo "error: $f is missing from the package" >&2; exit 1; fi + if [[ ! -e "$STAGE/$f" ]]; then echo "error: $f is missing from the package" >&2; exit 1; fi done # ---- .tar.gz ------------------------------------------------------------------------------------ @@ -104,7 +104,7 @@ tar -C "$WORK" -czf "$OUT_DIR/$BASENAME.tar.gz" "$BASENAME" echo "wrote $OUT_DIR/$BASENAME.tar.gz" "$STAGE/bin/effectcraft-cli" --version -if [ "$DRY_RUN" = 1 ]; then +if [[ "$DRY_RUN" = 1 ]]; then echo "==> dry run: tarball contents" tar -tzvf "$OUT_DIR/$BASENAME.tar.gz" fi diff --git a/packaging/icons.sh b/packaging/icons.sh index afeb13f8..770cda91 100755 --- a/packaging/icons.sh +++ b/packaging/icons.sh @@ -18,7 +18,10 @@ trap 'rm -rf "$TMP"' EXIT command -v resvg >/dev/null || { echo "error: resvg not found (brew install resvg)" >&2; exit 1; } -render() { resvg -w "$2" -h "$2" "$1" "$3" &2; exit 1; } +[[ -f "$TARBALL" ]] || { echo "error: $TARBALL not found (run packaging/linux/package.sh --formats tar first)" >&2; exit 1; } for tool in flatpak flatpak-builder; do command -v "$tool" >/dev/null || { echo "error: $tool not found (apt install flatpak flatpak-builder librsvg2-common)" >&2; exit 1; } done @@ -49,7 +49,7 @@ WORK="$CARGO_TARGET_DIR/flatpak-bundle" rm -rf "$WORK" mkdir -p "$WORK/stage" tar -xzf "$TARBALL" -C "$WORK/stage" --strip-components=1 -[ -x "$WORK/stage/bin/effectcraft" ] || { echo "error: $TARBALL has no bin/effectcraft" >&2; exit 1; } +[[ -x "$WORK/stage/bin/effectcraft" ]] || { echo "error: $TARBALL has no bin/effectcraft" >&2; exit 1; } cp "$HERE/flatpak/$APP_ID.bundle.yml" "$WORK/$APP_ID.yml" flatpak remote-add --user --if-not-exists flathub "$FLATHUB" @@ -62,7 +62,7 @@ OUT="$DIST/$BASENAME.flatpak" flatpak build-bundle --runtime-repo="$FLATHUB" "$WORK/repo" "$OUT" "$APP_ID" stable echo "wrote $OUT" -if [ "$TEST" = 1 ]; then +if [[ "$TEST" = 1 ]]; then flatpak install --user -y --noninteractive --reinstall "$OUT" flatpak info --user "$APP_ID" flatpak run --command=effectcraft-cli "$APP_ID" --version diff --git a/packaging/linux/package.sh b/packaging/linux/package.sh index e32d66ac..9a9693aa 100755 --- a/packaging/linux/package.sh +++ b/packaging/linux/package.sh @@ -25,7 +25,7 @@ APP_ID=ai.storyteller.effectcraft SKIP_BUILD=0 FORMATS="appimage deb rpm tar" -while [ $# -gt 0 ]; do +while [[ $# -gt 0 ]]; do case "$1" in --skip-build) SKIP_BUILD=1; shift ;; --formats) FORMATS="$2"; shift 2 ;; @@ -46,8 +46,8 @@ BASENAME="effectcraft-$VERSION-linux-$ARCH" echo "==> EffectCraft $VERSION for Linux $ARCH ($FORMATS)" -if [ "$SKIP_BUILD" = 0 ]; then - if [ -n "${CROSS_TARGET:-}" ]; then +if [[ "$SKIP_BUILD" = 0 ]]; then + if [[ -n "${CROSS_TARGET:-}" ]]; then (cd "$ROOT" && cargo build --release --locked -p effectcraft -p effectcraft-cli --target "$CROSS_TARGET") else (cd "$ROOT" && cargo build --release --locked -p effectcraft -p effectcraft-cli) @@ -79,7 +79,10 @@ if command -v appstreamcli >/dev/null; then appstreamcli validate --no-net --explain "$STAGE/usr/share/metainfo/$APP_ID.metainfo.xml" fi -has() { case " $FORMATS " in *" $1 "*) return 0 ;; *) return 1 ;; esac; } +has() { + local format="$1" + case " $FORMATS " in *" $format "*) return 0 ;; *) return 1 ;; esac +} # ---- .tar.gz ------------------------------------------------------------------------------------ if has tar; then @@ -112,9 +115,9 @@ if has appimage; then ln -s "$APP_ID.png" "$APPDIR/.DirIcon" TOOL="${APPIMAGETOOL:-$(command -v appimagetool || true)}" - if [ -z "$TOOL" ]; then + if [[ -z "$TOOL" ]]; then TOOL="$CARGO_TARGET_DIR/appimagetool-$ARCH.AppImage" - if [ ! -x "$TOOL" ]; then + if [[ ! -x "$TOOL" ]]; then curl --proto '=https' -fsSL -o "$TOOL" "https://github.com/AppImage/appimagetool/releases/download/continuous/appimagetool-$ARCH.AppImage" chmod +x "$TOOL" fi @@ -137,16 +140,16 @@ if has appimage; then # package) appimagetool also writes the .zsync, into its working directory, hence the cd. (cd "$DIST" && ARCH="$ARCH" APPIMAGE_EXTRACT_AND_RUN=1 "$TOOL" --no-appstream -u "$UPDATE_INFO" "$APPDIR" "$OUT") echo "wrote $OUT" - if [ -s "$OUT.zsync" ]; then + if [[ -s "$OUT.zsync" ]]; then echo "wrote $OUT.zsync" else warn "zsyncmake not found, so $OUT.zsync was not written; AppImage delta updates need it" fi fi -if [ -z "${CROSS_TARGET:-}" ]; then +if [[ -z "${CROSS_TARGET:-}" ]]; then "$STAGE/usr/bin/effectcraft-cli" --version -elif [ -n "${EMULATOR:-}" ]; then +elif [[ -n "${EMULATOR:-}" ]]; then "$EMULATOR" "$STAGE/usr/bin/effectcraft-cli" --version fi echo "==> done" diff --git a/packaging/macos/import-cert.sh b/packaging/macos/import-cert.sh index e0178b6a..8e3462cd 100755 --- a/packaging/macos/import-cert.sh +++ b/packaging/macos/import-cert.sh @@ -12,7 +12,7 @@ umask 077 # shellcheck source=../env.sh . "$(dirname "${BASH_SOURCE[0]}")/../env.sh" -if [ -z "${APPLE_CERTIFICATE:-}" ]; then +if [[ -z "${APPLE_CERTIFICATE:-}" ]]; then warn "APPLE_CERTIFICATE is not set: macOS builds will be ad-hoc signed and not notarized" exit 0 fi @@ -36,13 +36,13 @@ security set-key-partition-list -S apple-tool:,apple:,codesign: -s -k "$KC_PASS" security list-keychains -d user -s "$KEYCHAIN" $(security list-keychains -d user | tr -d '"') IDENTITY="$(security find-identity -v -p codesigning "$KEYCHAIN" | awk '/Developer ID Application/ { print $2; exit }')" -if [ -z "$IDENTITY" ]; then +if [[ -z "$IDENTITY" ]]; then security find-identity -v -p codesigning "$KEYCHAIN" >&2 echo "error: no 'Developer ID Application' identity in APPLE_CERTIFICATE" >&2 exit 1 fi echo "Imported signing identity $IDENTITY" -if [ -n "${GITHUB_ENV:-}" ]; then +if [[ -n "${GITHUB_ENV:-}" ]]; then { echo "MACOS_SIGN_IDENTITY=$IDENTITY"; echo "MACOS_KEYCHAIN=$KEYCHAIN"; } >>"$GITHUB_ENV" else echo "MACOS_SIGN_IDENTITY=$IDENTITY" diff --git a/packaging/macos/package.sh b/packaging/macos/package.sh index 37f81f48..95827b22 100755 --- a/packaging/macos/package.sh +++ b/packaging/macos/package.sh @@ -19,7 +19,7 @@ HERE="$ROOT/packaging/macos" ARCH=universal SKIP_BUILD=0 -while [ $# -gt 0 ]; do +while [[ $# -gt 0 ]]; do case "$1" in --arch) ARCH="$2"; shift 2 ;; --skip-build) SKIP_BUILD=1; shift ;; @@ -44,9 +44,9 @@ DMG="$DIST/effectcraft-$VERSION-macos-$ARCH.dmg" CLI_ZIP="$DIST/effectcraft-cli-$VERSION-macos-$ARCH.zip" NOTARIZE=0 -if [ "$IDENTITY" = "-" ]; then +if [[ "$IDENTITY" = "-" ]]; then warn "macOS: ad-hoc signing (no MACOS_SIGN_IDENTITY); artifacts are not notarized" -elif [ -n "${APPLE_ID:-}" ] && [ -n "${APPLE_PASSWORD:-}" ] && [ -n "${APPLE_TEAM_ID:-}" ]; then +elif [[ -n "${APPLE_ID:-}" ]] && [[ -n "${APPLE_PASSWORD:-}" ]] && [[ -n "${APPLE_TEAM_ID:-}" ]]; then NOTARIZE=1 else warn "macOS: APPLE_ID / APPLE_PASSWORD / APPLE_TEAM_ID incomplete; signed but not notarized" @@ -55,7 +55,7 @@ fi echo "==> EffectCraft $VERSION for macOS ($ARCH), identity: $IDENTITY, notarize: $NOTARIZE" # ---- build ------------------------------------------------------------------------------------- -if [ "$SKIP_BUILD" = 0 ]; then +if [[ "$SKIP_BUILD" = 0 ]]; then args=() for t in "${TARGETS[@]}"; do args+=(--target "$t"); done (cd "$ROOT" && cargo build --release --locked -p effectcraft -p effectcraft-cli "${args[@]}") @@ -74,9 +74,9 @@ done sign() { # Hardened runtime + secure timestamp for a real identity; ad-hoc can't be timestamped. local ts=(--timestamp) - [ "$IDENTITY" = "-" ] && ts=(--timestamp=none) + [[ "$IDENTITY" = "-" ]] && ts=(--timestamp=none) local kc=() - [ -n "${MACOS_KEYCHAIN:-}" ] && kc=(--keychain "$MACOS_KEYCHAIN") + [[ -n "${MACOS_KEYCHAIN:-}" ]] && kc=(--keychain "$MACOS_KEYCHAIN") codesign --force --sign "$IDENTITY" ${kc[@]+"${kc[@]}"} "${ts[@]}" "$@" } @@ -88,8 +88,8 @@ notarize() { echo "$out" id="$(printf '%s' "$out" | plutil -extract id raw -o - - 2>/dev/null || true)" status="$(printf '%s' "$out" | plutil -extract status raw -o - - 2>/dev/null || true)" - if [ "$status" != "Accepted" ]; then - if [ -n "$id" ]; then + if [[ "$status" != "Accepted" ]]; then + if [[ -n "$id" ]]; then xcrun notarytool log "$id" --apple-id "$APPLE_ID" --password "$APPLE_PASSWORD" --team-id "$APPLE_TEAM_ID" || true fi echo "error: notarization of $(basename "$file") failed (status: ${status:-unknown})" >&2 @@ -115,7 +115,7 @@ sign --options runtime --entitlements "$HERE/entitlements.plist" "$APP/Contents/ sign --options runtime --entitlements "$HERE/entitlements.plist" "$APP" codesign --verify --strict --deep --verbose=2 "$APP" -if [ "$NOTARIZE" = 1 ]; then +if [[ "$NOTARIZE" = 1 ]]; then ditto -c -k --keepParent "$APP" "$WORK/EffectCraft-notarize.zip" notarize "$WORK/EffectCraft-notarize.zip" xcrun stapler staple "$APP" @@ -134,7 +134,7 @@ ln -s /Applications "$STAGE/Applications" # CI (the shared release workflow doesn't install resvg; a warm cargo cache may hold it): install # the pinned version. Locally, say which version to install. RESVG_VERSION=0.48.1 -if [ -n "${CI:-}" ] && ! command -v resvg >/dev/null; then +if [[ -n "${CI:-}" ]] && ! command -v resvg >/dev/null; then cargo install resvg --locked --version "$RESVG_VERSION" fi command -v resvg >/dev/null || { echo "resvg not found (cargo install resvg --locked --version $RESVG_VERSION)" >&2; exit 1; } @@ -160,7 +160,7 @@ hdiutil_retry() { if hdiutil "$@"; then return 0 fi - [ "$attempt" = 5 ] && { echo "hdiutil $what failed 5 times" >&2; return 1; } + [[ "$attempt" = 5 ]] && { echo "hdiutil $what failed 5 times" >&2; return 1; } echo "hdiutil $what failed (attempt $attempt of 5); retrying in $((attempt * 10))s" >&2 sleep $((attempt * 10)) done @@ -178,7 +178,7 @@ if ! codesign --verify --strict --deep --verbose=2 "$MOUNT/EffectCraft.app"; the exit 1 fi hdiutil_retry detach detach "$MOUNT" >/dev/null -if [ "$NOTARIZE" = 1 ]; then +if [[ "$NOTARIZE" = 1 ]]; then notarize "$DMG" xcrun stapler staple "$DMG" xcrun stapler validate "$DMG" @@ -196,7 +196,7 @@ codesign --verify --strict --verbose=2 "$CLI_DIR/effectcraft-cli" rm -f "$CLI_ZIP" ditto -c -k --keepParent "$CLI_DIR" "$CLI_ZIP" # A bare Mach-O can't carry a stapled ticket; Gatekeeper looks the notarization up online. -if [ "$NOTARIZE" = 1 ]; then notarize "$CLI_ZIP"; fi +if [[ "$NOTARIZE" = 1 ]]; then notarize "$CLI_ZIP"; fi "$WORK/bin/effectcraft-cli" --version echo "==> done" diff --git a/packaging/web/package.sh b/packaging/web/package.sh index 89d767ea..dc3aaa90 100755 --- a/packaging/web/package.sh +++ b/packaging/web/package.sh @@ -11,10 +11,10 @@ set -euo pipefail . "$(dirname "${BASH_SOURCE[0]}")/../env.sh" HERE="$ROOT/packaging/web" -if [ "${1:-}" != "--skip-build" ]; then +if [[ "${1:-}" != "--skip-build" ]]; then # CI (the shared release workflow installs only trunk): install the wasm-bindgen CLI at the # crate's exact version. Locally, `cargo xtask web` says which version to install. - if [ -n "${CI:-}" ]; then + if [[ -n "${CI:-}" ]]; then WASM_BINDGEN_VERSION="$(sed -n 's/^const WASM_BINDGEN: &str = "\(.*\)";$/\1/p' "$ROOT/xtask/src/main.rs")" if ! { command -v wasm-bindgen >/dev/null && wasm-bindgen --version | grep -qF "$WASM_BINDGEN_VERSION"; }; then cargo install wasm-bindgen-cli --version "$WASM_BINDGEN_VERSION" --locked @@ -24,7 +24,7 @@ if [ "${1:-}" != "--skip-build" ]; then fi SITE="$CARGO_TARGET_DIR/web/dist" -[ -f "$SITE/index.html" ] || { echo "error: $SITE/index.html missing; run without --skip-build" >&2; exit 1; } +[[ -f "$SITE/index.html" ]] || { echo "error: $SITE/index.html missing; run without --skip-build" >&2; exit 1; } # Paths must be relative so the site works under any prefix. if grep -Eq '(src|href)="/[^/]' "$SITE/index.html"; then echo "error: $SITE/index.html has root-absolute URLs; it would break when served from a sub-path" >&2