Fix the review findings: never destroy a working anchor
Secret Scan / scan (push) Successful in 4s
Build App (Preview) / compute-version (pull_request) Successful in 3s
Secret Scan / scan (pull_request) Successful in 4s
Build App (Preview) / create-release (pull_request) Successful in 1s
Build App (Preview) / build-linux (pull_request) Failing after 1m49s
Build App (Preview) / build-macos (pull_request) Successful in 2m41s
Build App (Preview) / build-windows (pull_request) Successful in 4m55s
Build App (Preview) / prune-previews (pull_request) Skipped
Secret Scan / scan (push) Successful in 4s
Build App (Preview) / compute-version (pull_request) Successful in 3s
Secret Scan / scan (pull_request) Successful in 4s
Build App (Preview) / create-release (pull_request) Successful in 1s
Build App (Preview) / build-linux (pull_request) Failing after 1m49s
Build App (Preview) / build-macos (pull_request) Successful in 2m41s
Build App (Preview) / build-windows (pull_request) Successful in 4m55s
Build App (Preview) / prune-previews (pull_request) Skipped
An adversarial review of the previous commit found six real problems and corrected one of my claims. Taking all of it. **The anchoring could kill the channel it exists to protect.** It did DELETE-then-POST so the tag would name the current build. If the POST failed for any transient reason the script aborted having already deleted the anchor a previous run put there, and the next mirror run pruned GitHub's copy — a transient Gitea error converting a healthy channel into a dead one, which is strictly worse than the step not existing. There was also a real window between the two calls with no tag at all. The DELETE bought nothing. The update string resolves the tag by *name* and the assets hang off the release object, so nothing about the channel depends on which commit the tag points at; moving it changes only the source-zip link. It existed solely to get past a 409, since Gitea's POST /tags has no force semantics. Now the tag is created if absent and otherwise left alone, which removes the window too. **My "no window where the two disagree" claim was wrong, and it is the third time in this area I have asserted something I had not established.** The release POST sets no `target_commitish`, so GitHub creates its tag at its own default-branch HEAD, not at `GITEA_SHA`; the two agree only because `sync_on_commit` pushes main minutes earlier. And the DELETE actively created the window. What the ordering genuinely buys is narrower: if anchoring fails, the script aborts before creating a GitHub release that would be orphaned. **Orphaned drafts were invisible to the release lookup.** GitHub demotes a release to a draft when its tag is deleted, and `/releases/tags/` never returns drafts — precisely the state every mirror run left behind. The by-tag lookup reported "absent" while 86 MB drafts accumulated, one per release. The lookup now reads the authenticated list, republishes the newest, and deletes the rest. **A guard that could not catch what it named.** The update-info assertion was a substring match on the tag, so it passed for a wrong host, path, filename or transport — verified: an `evil.example.com/.../linux-latest/...` string passes the old check and fails the new one. Now a fixed full-string match. Also from the review: an absent bundled library no longer exits early, because that skipped the metadata *and* left `update-channel/` uncreated, killing the publish step on a missing directory and taking the tag and mirror jobs with it; the Categories guard asserts the absence of an empty value rather than the presence of any filled one; the channel directory is cleared before use so a stale zsync cannot satisfy an existence check while describing the previous build; the AppImage count uses a glob array, since `ls | wc -l` aborted under pipefail before the message it promised could print; uploads carry the retry/http1.1 hardening this repo's other upload steps already learned to need; verification compares served size against built size, because a status code only proves something is served; and the release workflow now fails on empty artifacts instead of publishing a release with no AppImage. The metainfo file is installed as `Triple-C.appdata.xml`. appimagetool derives the name it looks for from the .desktop basename, so under the id-based name it warned the metadata was missing on every build while this script reported it present. Now it prints "AppStream upstream metadata found in usr/share/metainfo/Triple-C.appdata.xml" — the AppStream id inside the file is unchanged and is what identifies the component. Two review hypotheses did not hold and nothing was changed for them: `set -e` does not abort on a failing `&&` list mid-script, and my claim of a `trap` reassignment was wrong — there is one trap, installed once. Verified against the real 0.4.19 artifact: exit 0, one AppImage beside the release, channel pair in its own directory, appimagetool reporting the metadata found, and the wayland fallback intact. Guards exercised individually — the duplicate one bites, the exact-match one rejects an impostor carrying the tag, the empty directory reports cleanly, and all four publisher preconditions refuse rather than half-publishing. Header parsing for the size check was tested against a real redirecting GitHub asset URL. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011YPqHpjV4EL6RNEwrRKqQm
This commit is contained in:
@@ -103,8 +103,13 @@ CATEGORIES="Development;Utility;"
|
||||
|
||||
repo_root="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)"
|
||||
appdata_src="$repo_root/packaging/appimage/$APP_ID.appdata.xml"
|
||||
# appimagetool looks for `<desktop basename>.appdata.xml` and warns the
|
||||
# metadata is missing under any other name — while the script cheerfully
|
||||
# reported it present. The AppStream id inside the file is unchanged and is
|
||||
# what actually identifies the component; only the filename follows the tool.
|
||||
appdata_installed_as="Triple-C.appdata.xml"
|
||||
|
||||
dir="${1:?usage: unbundle-wayland-client.sh <bundle/appimage directory>}"
|
||||
dir="${1:?usage: finalize-appimage.sh <bundle/appimage directory>}"
|
||||
cd "$dir"
|
||||
|
||||
shopt -s nullglob
|
||||
@@ -125,15 +130,15 @@ echo "Inspecting $appimage"
|
||||
( cd "$work" && "$here/$appimage" --appimage-extract >/dev/null )
|
||||
root="$work/squashfs-root"
|
||||
|
||||
if [ ! -e "$root/usr/lib/$LIB" ]; then
|
||||
# Not a failure: linuxdeploy may have stopped bundling it, which is the
|
||||
# outcome this script exists to produce.
|
||||
echo "$LIB is not bundled — leaving $appimage alone."
|
||||
exit 0
|
||||
fi
|
||||
# The demotion and the metadata are independent jobs, and an absent library
|
||||
# must not skip the second. An early exit here also left `update-channel/`
|
||||
# uncreated, which killed the publish step on a missing directory and took the
|
||||
# tag and mirror jobs down with it — a half-published release.
|
||||
demoted=false
|
||||
if [ -e "$root/usr/lib/$LIB" ]; then
|
||||
|
||||
mkdir -p "$root/$FALLBACK_DIR"
|
||||
mv "$root/usr/lib/$LIB" "$root/$FALLBACK_DIR/$LIB"
|
||||
mkdir -p "$root/$FALLBACK_DIR"
|
||||
mv "$root/usr/lib/$LIB" "$root/$FALLBACK_DIR/$LIB"
|
||||
|
||||
cat > "$root/$HOOK" <<'HOOK_EOF'
|
||||
#! /usr/bin/env bash
|
||||
@@ -182,6 +187,11 @@ src = src.replace(
|
||||
)
|
||||
open(path, "w").write(src)
|
||||
PATCH_EOF
|
||||
fi
|
||||
demoted=true
|
||||
echo "Demoted $LIB to $FALLBACK_DIR."
|
||||
else
|
||||
echo "$LIB is not bundled — nothing to demote."
|
||||
fi
|
||||
|
||||
# --- metadata -------------------------------------------------------------
|
||||
@@ -193,7 +203,7 @@ version="$(printf '%s' "$appimage" | sed -n 's/.*_\([0-9][0-9.]*\)_.*/\1/p')"
|
||||
if [ -f "$appdata_src" ]; then
|
||||
mkdir -p "$root/usr/share/metainfo"
|
||||
sed -e "s/@VERSION@/$version/" -e "s/@DATE@/$(date -u +%Y-%m-%d)/" \
|
||||
"$appdata_src" > "$root/usr/share/metainfo/$APP_ID.appdata.xml"
|
||||
"$appdata_src" > "$root/usr/share/metainfo/$appdata_installed_as"
|
||||
echo "Added AppStream metadata for $version."
|
||||
else
|
||||
echo "No AppStream source at $appdata_src — skipping." >&2
|
||||
@@ -208,7 +218,7 @@ for desktop in "$root"/*.desktop; do
|
||||
fi
|
||||
done
|
||||
|
||||
echo "Demoted $LIB to $FALLBACK_DIR; repacking."
|
||||
echo "Repacking."
|
||||
|
||||
tool="$work/appimagetool"
|
||||
curl -fsSL -o "$tool" "$APPIMAGE_TOOL_URL"
|
||||
@@ -216,6 +226,7 @@ chmod +x "$tool"
|
||||
|
||||
# --appimage-extract-and-run: CI runners generally have no FUSE.
|
||||
# -u embeds the update string and writes "$STABLE_NAME.zsync" beside the image.
|
||||
rm -rf "$CHANNEL_DIR"
|
||||
mkdir -p "$CHANNEL_DIR"
|
||||
ARCH=x86_64 "$tool" --appimage-extract-and-run \
|
||||
-u "$UPDATE_INFO" "$root" "$CHANNEL_DIR/$STABLE_NAME" >/dev/null
|
||||
@@ -237,16 +248,18 @@ out="$check/squashfs-root"
|
||||
|
||||
fail() { echo "FAILED: $1" >&2; exit 1; }
|
||||
|
||||
[ -e "$out/usr/lib/$LIB" ] && fail "$LIB is still on the loader path."
|
||||
[ -e "$out/$FALLBACK_DIR/$LIB" ] || fail "the fallback copy of $LIB is missing."
|
||||
[ -e "$out/$HOOK" ] || fail "the fallback hook is missing."
|
||||
grep -q "triple-c-wayland-fallback" "$out/AppRun" || fail "AppRun does not source the hook."
|
||||
if [ "$demoted" = true ]; then
|
||||
[ -e "$out/usr/lib/$LIB" ] && fail "$LIB is still on the loader path."
|
||||
[ -e "$out/$FALLBACK_DIR/$LIB" ] || fail "the fallback copy of $LIB is missing."
|
||||
[ -e "$out/$HOOK" ] || fail "the fallback hook is missing."
|
||||
grep -q "triple-c-wayland-fallback" "$out/AppRun" || fail "AppRun does not source the hook."
|
||||
fi
|
||||
[ -x "$out/usr/bin/triple-c" ] || fail "no executable usr/bin/triple-c."
|
||||
|
||||
# An empty Categories or missing metadata ships an image a manager cannot file
|
||||
# or describe, and both fail silently at runtime rather than at build time.
|
||||
grep -q "^Categories=.\+" "$out"/*.desktop || fail "Categories is still empty."
|
||||
[ -f "$appdata_src" ] && { [ -e "$out/usr/share/metainfo/$APP_ID.appdata.xml" ] \
|
||||
! grep -q "^Categories=$" "$out"/*.desktop || fail "a desktop file still has an empty Categories."
|
||||
[ -f "$appdata_src" ] && { [ -e "$out/usr/share/metainfo/$appdata_installed_as" ] \
|
||||
|| fail "AppStream metadata did not make it into the image."; }
|
||||
|
||||
# The update string is the difference between adoptable and updatable. It
|
||||
@@ -258,15 +271,18 @@ grep -q "^Categories=.\+" "$out"/*.desktop || fail "Categories is still empty."
|
||||
[ -e "$CHANNEL_DIR/$STABLE_NAME" ] || fail "the stable-named image is missing."
|
||||
[ -e "$CHANNEL_DIR/$STABLE_NAME.zsync" ] || fail "appimagetool wrote no .zsync."
|
||||
|
||||
readelf -p .upd_info "$CHANNEL_DIR/$STABLE_NAME" 2>/dev/null | grep -q "$UPDATE_TAG" \
|
||||
|| fail "the image carries no update information for the $UPDATE_TAG tag."
|
||||
readelf -p .upd_info "$CHANNEL_DIR/$STABLE_NAME" 2>/dev/null | grep -qF "$UPDATE_INFO" \
|
||||
|| fail "the image does not carry exactly the expected update information."
|
||||
grep -aq "^Filename: $STABLE_NAME$" "$CHANNEL_DIR/$STABLE_NAME.zsync" \
|
||||
|| fail "the .zsync names something other than $STABLE_NAME."
|
||||
|
||||
# The versioned release must carry one AppImage, not two. This is the guard
|
||||
# for the duplicate that shipped in 0.4.20 and 0.4.21.
|
||||
count="$(ls -1 *.AppImage 2>/dev/null | wc -l)"
|
||||
[ "$count" = "1" ] || fail "expected 1 AppImage beside the release, found $count."
|
||||
shopt -s nullglob
|
||||
beside=(*.AppImage)
|
||||
shopt -u nullglob
|
||||
[ "${#beside[@]}" -eq 1 ] \
|
||||
|| fail "expected 1 AppImage beside the release, found ${#beside[@]}."
|
||||
|
||||
echo "OK: $appimage prefers the host $LIB (fallback kept) and carries AppStream"
|
||||
echo " metadata. Channel pair in $CHANNEL_DIR/, updating from the $UPDATE_TAG tag."
|
||||
|
||||
Reference in New Issue
Block a user