From 526c85a64615d1d7e89a9b4d68ece17514247285 Mon Sep 17 00:00:00 2001 From: Joshua Tam <297250+joshuatam@users.noreply.github.com> Date: Fri, 6 Nov 2026 10:05:16 +0800 Subject: [PATCH] android: rebuild the drm artifacts on every package, and fail on a failed build MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The freshness guard compared mtimes: it failed if any file under an artifact's crates was newer than the artifact itself. Ages answer the wrong question, and they did so in both directions the same day. It fired on a tree that was current. A branch switch rewrote modules/gndrm-android/Cargo.toml and modules/gndrm-daemon/Cargo.toml with byte-identical content, so they were newer than libgamenative_drm.so; cargo compares content hashes, correctly reported everything fresh, and left the artifact from three days before. The guard called it stale, and rerunning the package could never clear it because nothing rewrites an artifact that cargo considers current. The other direction was worse and silent: if a builder did not rewrite an artifact — because it failed, or because it skipped one — the old artifact was newer than the sources, so the guard passed and the payload carried it. A failed `build-android.sh` was not even checked; its exit code was ignored. So the artifacts are now always built by the run that packages them. The Rust side is forced to recompile by touching the workspace's own *.rs and Cargo.toml (third-party crates stay cached, which is what holds this to ~53 s; the C shim and the C++ engine rebuild unconditionally anyway), each builder's exit code is checked, and every shipped artifact must be newer than a mark taken before the builders ran. That is the only claim the guard needs to make: the payload carries what this run built. Cost: a package run goes from ~60 s to ~111 s on a warm tree. Verified: the reported failure (identical content, newer mtime) packages and exits 0; appending a syntax error to modules/gndrm-uapi/src/lib.rs exits 1 with "error: build-android.sh failed" instead of packaging the previous artifact; restoring the file and repacking exits 0. File: android/package-gamescope.sh (the freshness guard, the builder invocations and the artifact checks; check_fresh and its per-artifact source lists are gone). --- android/package-gamescope.sh | 65 +++++++++++++++--------------------- 1 file changed, 27 insertions(+), 38 deletions(-) diff --git a/android/package-gamescope.sh b/android/package-gamescope.sh index 07a697198..32c34821a 100755 --- a/android/package-gamescope.sh +++ b/android/package-gamescope.sh @@ -88,48 +88,37 @@ LSFG_SO="$GNDRM_REL/liblsfg_engine.so" # that its source had already dropped, and nothing in the payload showed it. The # scripts are incremental, so a current tree costs a few seconds; a stale one # costs a rebuild. -bash "$GNDRM_DIR/tools/build-lsfg.sh" android -bash "$GNDRM_DIR/tools/build-shim.sh" android -bash "$GNDRM_DIR/tools/build-android.sh" +# The payload carries whatever these scripts produce, so the question the guard +# has to answer is "was this artifact written by this run?", not "is it newer +# than some file". Ages answer that wrongly in both directions: a checkout or a +# rebase rewrites bytes-identical sources and makes a current artifact look +# stale, and a build that silently did nothing leaves a stale one looking fine. +# So the Rust side is always rebuilt — touching the workspace's own sources +# keeps third-party crates cached, which is what holds this to about a minute — +# and every artifact has to come out of this run newer than the mark below. +FRESH_MARK="$(mktemp)" +# Removed once the checks have run: the staging block below replaces this +# script's EXIT trap, so a trap here would leak the file. +find "$GNDRM_DIR/modules" "$GNDRM_DIR/icd_shim" "$GNDRM_DIR/utils" \ + \( -name '*.rs' -o -name 'Cargo.toml' \) -print0 | xargs -0 touch +bash "$GNDRM_DIR/tools/build-lsfg.sh" android \ + || { echo "error: build-lsfg.sh failed" >&2; exit 1; } +bash "$GNDRM_DIR/tools/build-shim.sh" android \ + || { echo "error: build-shim.sh failed" >&2; exit 1; } +bash "$GNDRM_DIR/tools/build-android.sh" \ + || { echo "error: build-android.sh failed" >&2; exit 1; } for f in "$GAMESCOPE_BIN" "$REAPER_BIN" "$SHIM_SO" "$DRM_SO" "$ICD_SO" "$LSFG_SO" "$DMTEST_BIN" "$WSITEST_BIN" "$XTREE_BIN" "$SCALE_BIN"; do [ -e "$f" ] || { echo "error: missing $f" >&2; exit 1; } done -# An up-to-date build keeps the old file, so its age alone proves nothing. What -# must not happen is a source newer than the artifact built from it: that is the -# stale file, and the package would carry it. -# Each artifact is checked against the sources it is built from — its own crate -# plus the path dependencies it links, and the workspace manifest that sets the -# profile — not the whole tree: a crate the artifact does not link is not a -# source of staleness, and `libgamenative_drm.so` never compiles the C shim. -check_fresh() { - local f="$1"; shift - local stale - stale="$(find "$@" -type f \ - \( -name '*.rs' -o -name '*.c' -o -name '*.h' -o -name 'Cargo.toml' -o -name '*.map' \) \ - -newer "$f" -print -quit)" - [ -z "$stale" ] || { echo "error: $f is older than $stale" >&2; exit 1; } -} -check_fresh "$DRM_SO" \ - "$GNDRM_DIR/modules/gndrm-android" \ - "$GNDRM_DIR/modules/gndrm-daemon" \ - "$GNDRM_DIR/modules/gndrm-uapi" \ - "$GNDRM_DIR/modules/gndrm-obf" \ - "$GNDRM_DIR/modules/gndrm-obf-macros" \ - "$GNDRM_DIR/Cargo.toml" -check_fresh "$ICD_SO" \ - "$GNDRM_DIR/icd_shim" \ - "$GNDRM_DIR/modules/gndrm-lsfg" \ - "$GNDRM_DIR/modules/gndrm-uapi" \ - "$GNDRM_DIR/modules/gndrm-obf" \ - "$GNDRM_DIR/modules/gndrm-obf-macros" \ - "$GNDRM_DIR/Cargo.toml" -check_fresh "$SHIM_SO" "$GNDRM_DIR/modules/gndrm-shim" -check_fresh "$LSFG_SO" \ - "$GNDRM_DIR/modules/gndrm-lsfg" \ - "$GNDRM_DIR/modules/gndrm-obf" \ - "$GNDRM_DIR/modules/gndrm-obf-macros" \ - "$GNDRM_DIR/Cargo.toml" +# An artifact older than FRESH_MARK was not written by the run above: either the +# builders did nothing for it, or one of them failed on a tree whose artifact is +# still the old one. Either way the payload must not carry it. +for f in "$SHIM_SO" "$DRM_SO" "$ICD_SO" "$LSFG_SO"; do + [ "$f" -nt "$FRESH_MARK" ] \ + || { echo "error: $f was not rebuilt by this run (stale, or the build failed)" >&2; exit 1; } +done +rm -f "$FRESH_MARK" # --- staging ---------------------------------------------------------------- WORK="$(mktemp -d)"