Skip to content

fix(python-multiversion-uv): derive broker arch from AVOCADO_SDK_TARGET #22

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
bbrock25 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(python-multiversion-uv): derive broker arch from AVOCADO_SDK_TARGET #22

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 9 additions & 10 deletions python-multiversion-uv/broker-compile.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,19 @@

set -e

# Detect the TARGET arch, not the SDK shell arch. The SDK container runs on the
# x86_64 host even for aarch64 targets (only the target python is emulated via
# binfmt), so `uname -m` would lie. The target python3 reports the real target
# arch: x86_64 for qemux86-64, aarch64 for the Pi.
TARGET_ARCH="$(python3 -c 'import platform; print(platform.machine())')"
case "$TARGET_ARCH" in
x86_64) NARCH=amd64 ;;
aarch64 | arm64) NARCH=arm64 ;;
*) echo "[broker] unsupported target arch: $TARGET_ARCH" >&2; exit 1 ;;
# Resolve the TARGET arch from AVOCADO_SDK_TARGET, the same way the app

@jetm jetm Jul 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment says this derives arch "the same way the app compile scripts do," but none of them do - app311/app314 rely on uv autodetecting the arch inside the SDK container and app312 uses the system python3. The broker genuinely differs (it downloads a prebuilt GitHub binary, so it can't lean on uv autodetection), so the approach is fine - but the cited precedent doesn't exist, and the new comment ("runtime probes report the HOST arch") contradicts the old one it replaced ("the target python3 reports the real target arch"). Worth reconciling the rationale.

# compile scripts do. Do NOT probe python3/uname at runtime: the SDK
# container's arch follows the HOST (aarch64 on Apple Silicon, x86_64 on
# Intel), not the target, so runtime probes report the wrong arch whenever
# host and target differ (e.g. an arm64 Mac building for qemux86-64).
case "$AVOCADO_SDK_TARGET" in
*x86-64*|*x86_64*) NARCH=amd64 ;;
*) NARCH=arm64 ;;

@jetm jetm Jul 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This catch-all replaces the old explicit arm64 arm plus a fail-loud *) exit 1. With only set -e (no set -u), an unset or empty AVOCADO_SDK_TARGET falls through to arm64 - so if a CLI version doesn't export it into this compile hook (the sibling app-compile scripts never read AVOCADO_SDK_TARGET, so its presence here is an assumption worth confirming), a qemux86-64 build silently downloads the linux-arm64 nats-server and fails to exec on the x86_64 device at runtime, with no build-time error. That's a robustness regression vs the code being replaced. Suggest guarding with : "${AVOCADO_SDK_TARGET:?AVOCADO_SDK_TARGET not set}" and keeping a hard-fail *) for unrecognized targets (which also stops a future riscv / 32-bit-arm target from silently mapping to arm64). Mapping is otherwise correct for the three supported targets.

esac

VER="$(curl -sfL https://api.github.com/repos/nats-io/nats-server/releases/latest \
| grep -oE '"tag_name":[[:space:]]*"[^"]+"' | head -1 | grep -oE 'v[0-9.]+')"
echo "[broker] nats-server $VER for linux-$NARCH (target $TARGET_ARCH)"
echo "[broker] nats-server $VER for linux-$NARCH (target $AVOCADO_SDK_TARGET)"

mkdir -p broker/bin
curl -sfL "https://github.com/nats-io/nats-server/releases/download/${VER}/nats-server-${VER}-linux-${NARCH}.tar.gz" \
Expand Down