Skip to content

Commit cfd457f

Browse files
shai-almogclaude
andcommitted
Reject an Xcode whose xcodebuild cannot exit cleanly, and retire the probe
The probe answered what it was for, on the real image: Xcode_27_Release_Candidate.app -> prints "Xcode 27.0", exit 134 Xcode_27.0.0.app / Xcode_27.0.app / Xcode.app -> 27.0, exit 0 Two traps in one line, not one. The crashing app is FIRST in glob order, so without `|| true` the failing status propagates out of the assignment and `set -e` kills selection on candidate one -- before any working Xcode is tried, and with no output at all, because the crash text went to /dev/null. That much the previous commit fixed. But it also prints a perfectly good version BEFORE aborting, so testing the string alone accepts an Xcode whose xcodebuild dies. On this image that was survivable only because all four report 27.0 and the tie-break keeps the last one, which is luck: had the crashing app sorted last, it would have been chosen and every build on it would have failed somewhere less obvious. Candidates are now judged on their exit status too. A/B against a fake /Applications holding a crasher that sorts first and prints a version, plus a working Xcode: master exits 134 and selects nothing; this selects the working one and exits 0. The probe workflow goes, as its header said it would. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 4c92f9f commit cfd457f

2 files changed

Lines changed: 24 additions & 86 deletions

File tree

‎.github/workflows/ios-xcode27-probe.yml‎

Lines changed: 0 additions & 77 deletions
This file was deleted.

‎scripts/lib/xcode.sh‎

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -58,17 +58,32 @@ cn1_select_xcode() {
5858
local app version major best_version="" seen=""
5959
for app in /Applications/Xcode*.app; do
6060
[ -x "$app/Contents/Developer/usr/bin/xcodebuild" ] || continue
61-
# `|| true` is load-bearing. A caller running under `set -e` -- which
62-
# every CI step here does -- is killed by this assignment the moment
63-
# one candidate's xcodebuild exits non-zero, before the emptiness
64-
# check below ever runs. Measured on the GitHub xcode-27 image, where
65-
# /Applications holds the real Xcode plus three symlinks to it and one
66-
# of those invocations aborts (SIGABRT, status 134): selection died
67-
# with no output at all, because the crash text goes to /dev/null and
68-
# the status goes to `set -e`. A candidate that cannot report its
69-
# version is one to SKIP, which is what the next line is for.
61+
# Run it, then judge it on BOTH its output and its exit status, and
62+
# let neither kill the caller.
63+
#
64+
# Measured on the GitHub xcode-27 image, where /Applications holds the
65+
# real Xcode plus three symlinks to it:
66+
#
67+
# Xcode_27_Release_Candidate.app -> prints "Xcode 27.0", exit 134
68+
# Xcode_27.0.0.app / Xcode_27.0.app / Xcode.app -> 27.0, exit 0
69+
#
70+
# Two separate traps in that one line. The crashing app is FIRST in
71+
# glob order, and without the `|| true` the failing status propagates
72+
# out of the assignment and `set -e` -- which every CI step here runs
73+
# with -- kills selection on candidate one, before any working Xcode
74+
# is tried, with no output at all because the crash text went to
75+
# /dev/null. And it prints a perfectly good version BEFORE aborting,
76+
# so checking the string alone accepts an Xcode whose xcodebuild dies;
77+
# here that was survivable only because all four report 27.0 and the
78+
# tie-break happens to keep the last, which is luck rather than logic.
79+
#
80+
# An xcodebuild that cannot exit cleanly is not one to build with.
7081
version="$("$app/Contents/Developer/usr/bin/xcodebuild" -version 2>/dev/null \
7182
| awk '/^Xcode /{print $2; exit}' || true)"
83+
if ! "$app/Contents/Developer/usr/bin/xcodebuild" -version >/dev/null 2>&1; then
84+
seen="$seen ${version:-?}($app, unusable)"
85+
continue
86+
fi
7287
[ -n "$version" ] || continue
7388
seen="$seen $version($app)"
7489
major="${version%%.*}"

0 commit comments

Comments
 (0)