From e0785d66d83c82e6d1c821e191c5a86d70081924 Mon Sep 17 00:00:00 2001 From: Bernard Ladenthin Date: Sun, 11 Oct 2026 21:14:37 +0200 Subject: [PATCH] Resolve sdkmanager by its known layouts, and say what is there when it is not My fix in #519 was off by one. It searched with find -maxdepth 2 under cmdline-tools, but the file sits at cmdline-tools/latest/bin/sdkmanager, three levels down -- so the search stopped at the bin directory and reported the file missing while it was there. Run 38164077038 said so in one line, which is what that PR's other half bought: the message named the right cause, so the real defect took one look instead of another suspicion about dl.google.com. This replaces the find with the three known layouts in order -- cmdline-tools/latest/bin, cmdline-tools//bin and the old tools/bin -- which is both correct and readable, and cannot be off by one again. The outer loop over ANDROID_HOME, ANDROID_SDK_ROOT and the image path now stops once one answers: the inner break leaves only the inner loop, so a later root could otherwise overwrite the found path. That guard is an if rather than a trailing test-and-break, which under set -e would end the step the moment the test is false. And the part #519 still lacked: when nothing is found, the step now lists what IS under each root. Naming only the places searched leaves three different causes -- no directory, no version subdirectory, no file -- behind one message, and a reader cannot tell them apart. The listing loop deliberately has no early break, so it reports every root. Validated locally against five faked layouts: latest (what CI has), a version directory without latest, the old tools/bin, an empty SDK that must fail, and a first root that is empty with the second carrying the file. Both copies of the step are fixed identically; check-run-scripts.py parses all 161 scripts, the release gate and the natives list are unchanged. A trap recorded in TODO.md for whoever tests such a step on this checkout: chmod +x does not stick on a plain file here, so [ -x ] answers false and the first run of that probe called all five layouts broken -- including the one that demonstrably works in CI. The fixtures needed a shebang; it was the harness, not the logic. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01E2h8gXyyE5UeimkL9vQv9G --- .github/workflows/publish.yml | 46 ++++++++++++++++++++++++++++++++--- TODO.md | 27 +++++++++++++++----- 2 files changed, 63 insertions(+), 10 deletions(-) diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 6f749a69..a58422e9 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -1188,12 +1188,31 @@ jobs: if [ -z "$SDKMANAGER" ]; then for root in "${ANDROID_HOME:-}" "${ANDROID_SDK_ROOT:-}" /usr/local/lib/android/sdk; do [ -n "$root" ] || continue - candidate="$(find "$root/cmdline-tools" -maxdepth 2 -name sdkmanager -type f 2>/dev/null | sort | tail -1)" - if [ -n "$candidate" ]; then SDKMANAGER="$candidate"; break; fi + # Keep the first root that answers: the inner break leaves only the inner + # loop, so without this a later root would overwrite the found path. Written + # as an if rather than "test && break", which under set -e would end the + # step the moment the test is false. + if [ -n "$SDKMANAGER" ]; then break; fi + # The known layouts, newest first, rather than one find: cmdline-tools//bin + # sits three levels below cmdline-tools, which an earlier -maxdepth 2 never reached -- + # it stopped at the bin directory and reported 'not found' for a file that was there. + for candidate in "$root/cmdline-tools/latest/bin/sdkmanager" "$root/cmdline-tools"/*/bin/sdkmanager "$root/tools/bin/sdkmanager"; do + if [ -x "$candidate" ]; then SDKMANAGER="$candidate"; break; fi + done done fi if [ -z "$SDKMANAGER" ]; then echo "::error::sdkmanager not found -- looked on PATH and under ANDROID_HOME='${ANDROID_HOME:-}', ANDROID_SDK_ROOT='${ANDROID_SDK_ROOT:-}' and /usr/local/lib/android/sdk. This is a runner-image change, not a download problem." + # Name what IS there. The first version said only where it had looked, which left + # the reader to guess whether the directory, the version subdirectory or the file + # was missing -- three causes behind one message. + for root in "${ANDROID_HOME:-}" "${ANDROID_SDK_ROOT:-}" /usr/local/lib/android/sdk; do + [ -n "$root" ] || continue + echo "contents of ${root}:" + ls -la "$root" 2>&1 | sed "s/^/ /" || true + echo "contents of ${root}/cmdline-tools:" + ls -la "$root/cmdline-tools" 2>&1 | sed "s/^/ /" || true + done exit 1 fi echo "using sdkmanager: $SDKMANAGER" @@ -1378,12 +1397,31 @@ jobs: if [ -z "$SDKMANAGER" ]; then for root in "${ANDROID_HOME:-}" "${ANDROID_SDK_ROOT:-}" /usr/local/lib/android/sdk; do [ -n "$root" ] || continue - candidate="$(find "$root/cmdline-tools" -maxdepth 2 -name sdkmanager -type f 2>/dev/null | sort | tail -1)" - if [ -n "$candidate" ]; then SDKMANAGER="$candidate"; break; fi + # Keep the first root that answers: the inner break leaves only the inner + # loop, so without this a later root would overwrite the found path. Written + # as an if rather than "test && break", which under set -e would end the + # step the moment the test is false. + if [ -n "$SDKMANAGER" ]; then break; fi + # The known layouts, newest first, rather than one find: cmdline-tools//bin + # sits three levels below cmdline-tools, which an earlier -maxdepth 2 never reached -- + # it stopped at the bin directory and reported 'not found' for a file that was there. + for candidate in "$root/cmdline-tools/latest/bin/sdkmanager" "$root/cmdline-tools"/*/bin/sdkmanager "$root/tools/bin/sdkmanager"; do + if [ -x "$candidate" ]; then SDKMANAGER="$candidate"; break; fi + done done fi if [ -z "$SDKMANAGER" ]; then echo "::error::sdkmanager not found -- looked on PATH and under ANDROID_HOME='${ANDROID_HOME:-}', ANDROID_SDK_ROOT='${ANDROID_SDK_ROOT:-}' and /usr/local/lib/android/sdk. This is a runner-image change, not a download problem." + # Name what IS there. The first version said only where it had looked, which left + # the reader to guess whether the directory, the version subdirectory or the file + # was missing -- three causes behind one message. + for root in "${ANDROID_HOME:-}" "${ANDROID_SDK_ROOT:-}" /usr/local/lib/android/sdk; do + [ -n "$root" ] || continue + echo "contents of ${root}:" + ls -la "$root" 2>&1 | sed "s/^/ /" || true + echo "contents of ${root}/cmdline-tools:" + ls -la "$root/cmdline-tools" 2>&1 | sed "s/^/ /" || true + done exit 1 fi echo "using sdkmanager: $SDKMANAGER" diff --git a/TODO.md b/TODO.md index 6aa65506..9e7b107f 100644 --- a/TODO.md +++ b/TODO.md @@ -1046,12 +1046,27 @@ for everything else. that was only occasionally flaky into one that is always red** (runs 38146809329, 38148401395, 38149667195), and the misleading message is what kept that invisible: a reader sees a download problem at Google and waits for it to pass. - Fixed by resolving the command (PATH, then `$ANDROID_HOME`, `$ANDROID_SDK_ROOT` and the image's - `/usr/local/lib/android/sdk`, newest `cmdline-tools` version last) and by **keeping the two failure - modes apart**: "not found" now says it is a runner-image change and not a download problem. Both - copies of the step are fixed identically. **Still to verify in CI** -- the emulator jobs have not - been green since the step was added, so the thing the step was built to guard against (the corrupt - archive of run 38069595101) also remains unproven. + Fixed in two steps, and the first one was wrong in a way the second could only find *because* it + was wrong. The first (#519) kept the two failure modes apart -- "not found" says it is a + runner-image change and not a download problem -- and resolved the command with + `find "$root/cmdline-tools" -maxdepth 2`. Run 38164077038 then reported, in one line, + `sdkmanager not found -- looked ... ANDROID_HOME='/usr/local/lib/android/sdk'`: the directory is + there, and the file sits at `cmdline-tools/latest/bin/sdkmanager`, which is **three** levels below + `cmdline-tools` -- the search stopped at the `bin` directory. An off-by-one, mine; what the + message rewrite bought was seeing it at a glance instead of suspecting Google again. + The second (#526) drops the find for the three known layouts in order (`cmdline-tools/latest/bin`, + `cmdline-tools//bin`, the old `tools/bin`), guards the outer loop so a later root cannot + overwrite the found path, and -- the part the first version lacked -- **lists what is actually + under each root** when nothing is found, because naming only the places searched leaves three + causes (missing directory, missing version subdirectory, missing file) behind one message. + Validated against five faked layouts locally, including the failure case. **Still to verify in CI** + -- the emulator jobs have not been green since the step was added, so the thing it was built to + guard against (the corrupt archive of run 38069595101) also remains unproven. + One trap worth keeping for whoever tests such a step locally: on this Windows checkout `chmod +x` + does not stick on a plain file (`ls -l` still shows `rw-r--r--`), so `[ -x ]` answers *false* and + every probe "failed" until the fixtures were given a `#!/bin/sh` line. The first run of that probe + reported all five layouts broken, including the one that demonstrably works in CI -- the harness, + not the logic. - **Everything behind `package` has never run on the modular natives.** Run 38069595101 skipped `package` (it `needs` the Windows clang test job, red with the AMX crash); this branch's run 38112603436 reaches it and fails on the Windows OpenVINO module jar (the blocker at the top of