avm/hassio-addons
1
0
Fork 0
mirror of https://github.com/alexbelgium/hassio-addons.git synced 2026-09-10 19:59:09 +02:00
Code Issues Packages Projects Releases Wiki Activity

birdnet-go-dev: add merge-prs.sh --check to catch build-breaking PR conflicts (#3053)

Browse Source 
* birdnet-go-dev: add merge-prs.sh --check to catch build-breaking PR conflicts

The add-on build merges every open non-draft fork PR onto an upstream-synced
main. GitHub's `mergeable` field answers a different question: it compares a PR
against its *own* base ref, which for the stacked dashboard PRs is another
feature branch - sometimes one belonging to an already-closed PR. So a PR can
read MERGEABLE/CLEAN and still fail the build.

That is how run 34101694542 broke: PR #57 is MERGEABLE against the frozen branch
of closed PR #56, but conflicts with main on DetectionCard.svelte.

--check replays the exact same merge sequence, skips past conflicts instead of
stopping at the first one, and reports every offender as

    !!! CONFLICT pr=#N conflicts-with=<main|accumulated> files=... title=...

conflicts-with is probed in a throwaway worktree against the pristine synced
main, so it separates a PR that is merely stale (fixable in its own branch) from
one that only clashes with another open PR (needs a cross-PR decision).

Build behaviour is unchanged; --check is a no-op unless asked for, so no version
bump.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

* birdnet-go-dev: fix --check misclassification, arg handling and temp leak

Review findings on #3053, all reproduced against git fixtures before fixing.

1. The probe did not apply the build's own merge policy (Codex).
   The real merge treats a *sole* frontend/package-lock.json conflict as
   non-fatal, but merges_onto_main() did a raw merge. A PR whose only clash
   with main is the generated lockfile was therefore reported as
   conflicts-with=main, which prints "the PR is stale against main; merge main
   into its branch" when in truth it merges onto main fine and only clashes
   with another open PR — the exact opposite remediation, from the feature
   whose entire job is to say which one it is.

   Extracted the policy into resolve_sole_lockfile() and routed both the real
   merge and the probe through it, so the two cannot drift apart again.

2. A second positional operand silently won (CodeRabbit).
   `merge-prs.sh a b` ran against b, where the pre-flag script used "${1}".
   A stray argument would have cloned into the wrong directory. Now exits 64.

3. mktemp parent directory leaked (CodeRabbit, Copilot).
   probe="$(mktemp -d)/probe" and only the child was removed, leaking one
   empty dir per checked conflict. Measured 3 leaked dirs over 3 calls; now 0.

Verified with throwaway repos: a lockfile-only clash now classifies as
"accumulated" (was "main"), a real source conflict with main still classifies
as "main", a clean PR still classifies as "accumulated", 0 leaked temp dirs,
0 stray worktrees, and the argument matrix behaves. shellcheck clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: BirdNET-Go Addon Builder <addon-builder@users.noreply.github.com>
This commit is contained in:
Alexandre
2026-09-07 14:05:42 +02:00
committed by GitHub
parent bcde474c15
commit abf3d76873
1 changed files with 103 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files

115
birdnet-go-dev/merge-prs.sh
View File

@@ -11,6 +11,15 @@
# stamping) is written to the directory given as $1 so the Docker build can
# compile it.
#
# With --check the script does not build anything: it performs the exact same
# merge sequence, skips (instead of failing on) every conflicting PR, and prints
# one "!!! CONFLICT pr=#N conflicts-with=... files=... " line per offender before
# exiting 2. Use it to find conflicts *before* a build burns on them. This is a
# different question from GitHub's `mergeable` field, which compares a PR against
# its own base ref - for a stacked PR that base is another feature branch (often
# stale, sometimes belonging to a closed PR), so GitHub can report CLEAN for a PR
# that does not merge onto main at all.
#
# Environment:
# BIRDNET_FORK owner/repo of the fork (default alexbelgium/birdnet-go)
# BIRDNET_UPSTREAM owner/repo of the upstream (default tphakala/birdnet-go)
@@ -19,7 +28,26 @@
#
set -euo pipefail
TARGET_DIR="${1:?usage: merge-prs.sh <target-dir>}"
CHECK_ONLY="${MERGE_PRS_CHECK:-0}"
TARGET_DIR=""
while [ "$#" -gt 0 ]; do
case "$1" in
--check) CHECK_ONLY=1 ;;
-*) echo "unknown option: $1" >&2; exit 64 ;;
*)
# Last-one-wins would silently clone into the wrong directory if a caller ever
# appended an argument; the pre-flag script used "${1}", so refuse rather than
# quietly change which operand counts.
if [ -n "${TARGET_DIR}" ]; then
echo "usage: merge-prs.sh [--check] <target-dir>" >&2
exit 64
fi
TARGET_DIR="$1"
;;
esac
shift
done
: "${TARGET_DIR:?usage: merge-prs.sh [--check] <target-dir>}"
FORK="${BIRDNET_FORK:-alexbelgium/birdnet-go}"
UPSTREAM="${BIRDNET_UPSTREAM:-tphakala/birdnet-go}"
@@ -33,6 +61,52 @@ GH_TOKEN="${GH_TOKEN:-${GITHUB_TOKEN:-}}"
log() { echo ">>> $*"; }
# Conflicting PRs collected in --check mode: "number|scope|files|title".
conflicting=()
LOCKFILE="frontend/package-lock.json"
# The single place that decides whether a conflicted merge is still acceptable.
# package-lock.json is generated content and stacked PRs can carry an older copy even when
# their source changes merge cleanly, so keep the lockfile already assembled on the base side
# — but only when it is the sole conflict. Any source conflict stays fatal.
# Returns 0 when it resolved and committed such a merge, 1 when the conflict is real.
# BOTH the real merge and the --check probe must go through here: when only the real merge
# applied the policy, the probe called a PR "conflicts-with=main" that the build would have
# merged fine, and printed the opposite remediation to the true one.
resolve_sole_lockfile() {
local dir="$1"
local -a conflicted
mapfile -t conflicted < <(git -C "${dir}" diff --name-only --diff-filter=U)
if [ "${#conflicted[@]}" -ne 1 ] || [ "${conflicted[0]}" != "${LOCKFILE}" ]; then
return 1
fi
log "Resolving generated ${LOCKFILE} conflict using the base tree"
git -C "${dir}" checkout --ours -- "${LOCKFILE}" || return 1
git -C "${dir}" add "${LOCKFILE}" || return 1
git -C "${dir}" commit --no-edit > /dev/null || return 1
}
# Does ${1} merge cleanly onto the pristine upstream-synced main? Probed in a
# throwaway worktree so the accumulated tree is left untouched. Tells apart a PR
# that is simply stale against main (fixable inside that PR's own branch) from
# one that only clashes with another open PR (needs a cross-PR decision).
merges_onto_main() {
local sha="$1" tmpdir probe rc=0
tmpdir="$(mktemp -d)"
probe="${tmpdir}/probe"
git worktree add --quiet --detach "${probe}" "${MAIN_SYNCED}"
if ! git -C "${probe}" merge --no-edit --no-ff -m probe "${sha}" > /dev/null 2>&1; then
# Same policy as the real merge, or this misclassifies a lockfile-only clash.
resolve_sole_lockfile "${probe}" > /dev/null 2>&1 || rc=1
fi
git worktree remove --force "${probe}" > /dev/null 2>&1 || true
# worktree remove only takes the child back; without this the mktemp parent is left behind
# on every checked conflict.
rmdir "${tmpdir}" > /dev/null 2>&1 || true
return "${rc}"
}
git config --global user.email "addon-builder@users.noreply.github.com"
git config --global user.name "BirdNET-Go Addon Builder"
git config --global advice.detachedHead false
@@ -47,6 +121,7 @@ git remote add upstream "${UPSTREAM_URL}"
git fetch --no-tags upstream main
# --no-ff keeps an explicit sync commit; a no-op when main is already current.
git merge --no-edit --no-ff upstream/main
MAIN_SYNCED="$(git rev-parse HEAD)"
log "Querying open non-draft PRs from ${FORK}"
auth_header=()
@@ -79,27 +154,43 @@ for entry in "${prs[@]}"; do
if ! git merge --no-edit --no-ff -m "Merge PR #${number}: ${title}" "${sha}"; then
mapfile -t conflicted_files < <(git diff --name-only --diff-filter=U)
# package-lock.json is generated content and stacked PRs can carry an
# older copy even when their source changes merge cleanly. Keep the
# lockfile already assembled from upstream and earlier PRs, but only
# when it is the sole conflict. Any source conflict remains fatal.
if [ "${#conflicted_files[@]}" -eq 1 ] \
&& [ "${conflicted_files[0]}" = "frontend/package-lock.json" ]; then
log "Resolving generated frontend/package-lock.json conflict using the accumulated tree"
git checkout --ours -- frontend/package-lock.json
git add frontend/package-lock.json
git commit --no-edit
if resolve_sole_lockfile .; then
: # generated lockfile only - the merge is committed and the build continues
else
echo "!!! Merge conflict while merging PR #${number} (${title})." >&2
if [ "${#conflicted_files[@]}" -gt 0 ]; then
printf '!!! Conflicting file: %s\n' "${conflicted_files[@]}" >&2
fi
echo "!!! Resolve the conflict in the fork or pause this PR, then rebuild." >&2
git merge --abort || true
if [ "${CHECK_ONLY}" = "1" ]; then
scope="accumulated"
merges_onto_main "${sha}" || scope="main"
conflicting+=("${number}|${scope}|${conflicted_files[*]:-}|${title}")
log "check mode: skipping PR #${number}, continuing with the rest"
continue
fi
echo "!!! Resolve the conflict in the fork or pause this PR, then rebuild." >&2
exit 1
fi
fi
done
if [ "${CHECK_ONLY}" = "1" ]; then
if [ "${#conflicting[@]}" -eq 0 ]; then
log "CHECK OK: every open non-draft PR merges into the combined build tree"
exit 0
fi
echo "!!! CHECK FAILED: ${#conflicting[@]} PR(s) would break the add-on build" >&2
for entry in "${conflicting[@]}"; do
IFS='|' read -r number scope files title <<<"${entry}"
echo "!!! CONFLICT pr=#${number} conflicts-with=${scope} files=${files} title=${title}" >&2
done
echo "!!! conflicts-with=main -> the PR is stale against main; merge main into its branch and resolve there." >&2
echo "!!! conflicts-with=accumulated -> the PR only clashes with another open PR; decide which one owns the hunk." >&2
exit 2
fi
log "Merged HEAD: $(git rev-parse --short HEAD)"
log "Source tree ready at ${TARGET_DIR}"