Skip to content

Commit d94e121

Browse files
committed
Make the bash 3.2 array rule mechanical instead of remembered
This has now bitten twice in the same file: fixed once in "Expand possibly-empty arrays the way macOS's bash 3.2 tolerates", reintroduced with the Rosetta launcher, and both times it cost a full macOS build that finished, validated, and then staged nothing. Deciding case by case which array can be empty is the judgement that failed, so the remaining expansions in the scripts that run on a published host all take the tolerant form -- it means the same thing as the plain one whenever the array has elements -- and a test refuses any that do not. Against the parent of this branch it names lines 161 to 165 of build-host.sh, the ones that broke the nightly. The test is static because it has to be: BASH_COMPAT=32 does not restore the 3.2 behaviour, so nothing running on a Linux runner can reproduce it by executing anything.
1 parent 65d65a0 commit d94e121

6 files changed

Lines changed: 60 additions & 17 deletions

File tree

scripts/ci/build-host.sh

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -197,8 +197,8 @@ build_and_stage() {
197197
targets+=(core-package bootstrap-archive)
198198
fi
199199

200-
cmake "${cmake_args[@]}"
201-
cmake --build build --target "${targets[@]}" --parallel "$(ci_nproc)"
200+
cmake ${cmake_args[@]+"${cmake_args[@]}"}
201+
cmake --build build --target ${targets[@]+"${targets[@]}"} --parallel "$(ci_nproc)"
202202
report_ccache_statistics
203203

204204
# Verified wherever it can be run, which is not the same as natively.
@@ -224,7 +224,7 @@ stage_and_write_provenance() {
224224
fi
225225
local -a names=()
226226
local file
227-
for file in "${produced[@]}"; do
227+
for file in ${produced[@]+"${produced[@]}"}; do
228228
[[ -f $file ]] || continue
229229
cp "$file" "$out_dir/"
230230
names+=("$(basename "$file")")
@@ -271,7 +271,7 @@ build_musl_host() {
271271
# so the container reaches the same cache the runner restored.
272272
docker run --rm \
273273
-v "$PWD":/src -w /src \
274-
"${docker_env[@]}" \
274+
${docker_env[@]+"${docker_env[@]}"} \
275275
alpine:3.20 sh -eux -c '
276276
apk add --no-cache bash build-base cmake git autoconf automake \
277277
libtool libarchive-tools texinfo bison flex pkgconf curl xz \
@@ -322,12 +322,12 @@ install_dependencies() {
322322
i686-w64-mingw32) extra=(g++-mingw-w64-i686) ;;
323323
x86_64-unknown-freebsd | aarch64-unknown-freebsd) extra=(clang lld llvm) ;;
324324
esac
325-
"${sudo_cmd[@]}" apt-get update -qq
326-
"${sudo_cmd[@]}" env DEBIAN_FRONTEND=noninteractive apt-get install -y -qq \
325+
${sudo_cmd[@]+"${sudo_cmd[@]}"} apt-get update -qq
326+
${sudo_cmd[@]+"${sudo_cmd[@]}"} env DEBIAN_FRONTEND=noninteractive apt-get install -y -qq \
327327
cmake cmake-data git build-essential autoconf automake libtool \
328328
texinfo bison flex pkg-config python3 python3-pip curl bzip2 xz-utils \
329329
libarchive-tools ccache \
330-
"${extra[@]}"
330+
${extra[@]+"${extra[@]}"}
331331
pip3 install --quiet cmake==3.31.6
332332
;;
333333
esac
@@ -342,7 +342,7 @@ enable_ccache() {
342342
shims="$(brew --prefix ccache)/libexec"
343343
else
344344
[[ -x /usr/sbin/update-ccache-symlinks ]] &&
345-
"${sudo_cmd[@]}" /usr/sbin/update-ccache-symlinks
345+
${sudo_cmd[@]+"${sudo_cmd[@]}"} /usr/sbin/update-ccache-symlinks
346346
shims=/usr/lib/ccache
347347
fi
348348
[[ -d $shims ]] || {

scripts/create-bootstrap-archive.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ case $host in
3131
bin/include/refresh-repositories.sh)
3232
;;
3333
esac
34-
for relative_path in "${required[@]}"; do
34+
for relative_path in ${required[@]+"${required[@]}"}; do
3535
[[ -e $sdk_root/$relative_path ]] || {
3636
printf 'bootstrap SDK is missing %s\n' "$relative_path" >&2
3737
exit 1

scripts/create-core-package.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ cp -a "$sdk_root/." "$core_root/"
9797
rm -f "$core_root/.PKGINFO" "$core_root/.BUILDINFO" "$core_root/.MTREE" \
9898
"$core_root/etc/pacman.conf"
9999

100-
for path in "${client_paths[@]}"; do
100+
for path in ${client_paths[@]+"${client_paths[@]}"}; do
101101
[[ -e $core_root/$path ]] || {
102102
printf 'the SDK does not carry %s\n' "$path" >&2
103103
exit 1

scripts/create-core-repositories.sh

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -93,13 +93,13 @@ normalize_database() {
9393
}
9494

9595
mapfile -t sorted_architectures < <(printf '%s\n' "${!architectures[@]}" | LC_ALL=C sort)
96-
for architecture in "${sorted_architectures[@]}"; do
96+
for architecture in ${sorted_architectures[@]+"${sorted_architectures[@]}"}; do
9797
read -r -a package_filenames <<<"${architectures[$architecture]}"
9898
packages=()
99-
for package_filename in "${package_filenames[@]}"; do
99+
for package_filename in ${package_filenames[@]+"${package_filenames[@]}"}; do
100100
packages+=("$staging_directory/$package_filename")
101101
done
102-
repo-add "$staging_directory/$architecture.db.tar.gz" "${packages[@]}"
102+
repo-add "$staging_directory/$architecture.db.tar.gz" ${packages[@]+"${packages[@]}"}
103103
normalize_database "$staging_directory/$architecture.db.tar.gz" \
104104
"$temporary_directory/$architecture.db"
105105
normalize_database "$staging_directory/$architecture.files.tar.gz" \
@@ -177,10 +177,10 @@ if [[ -n ${RELEASE_SCHEMA:-}${RELEASE_BUILD_ID:-}${RELEASE_BUILDSCRIPTS_REVISION
177177
}
178178

179179
host_fragments=()
180-
for architecture in "${sorted_architectures[@]}"; do
180+
for architecture in ${sorted_architectures[@]+"${sorted_architectures[@]}"}; do
181181
host_artifacts=("$architecture.db" "$architecture.files")
182182
read -r -a package_filenames <<<"${architectures[$architecture]}"
183-
host_artifacts+=("${package_filenames[@]}")
183+
host_artifacts+=(${package_filenames[@]+"${package_filenames[@]}"})
184184
while IFS= read -r -d '' extra; do
185185
host_artifacts+=("$(basename "$extra")")
186186
done < <(find "$staging_directory" -maxdepth 1 -type f \
@@ -191,7 +191,7 @@ if [[ -n ${RELEASE_SCHEMA:-}${RELEASE_BUILD_ID:-}${RELEASE_BUILDSCRIPTS_REVISION
191191
printf 'no provenance echo found for published host: %s\n' "$architecture" >&2
192192
exit 1
193193
}
194-
host_fragments+=("{\"name\":\"$(json_escape "$architecture")\",\"build_id\":\"$(json_escape "$build_id")\",\"artifacts\":$(json_string_array "${host_artifacts[@]}")}")
194+
host_fragments+=("{\"name\":\"$(json_escape "$architecture")\",\"build_id\":\"$(json_escape "$build_id")\",\"artifacts\":$(json_string_array ${host_artifacts[@]+"${host_artifacts[@]}"})}")
195195
done
196196

197197
hosts_joined=$(

scripts/install-vdpm-bundle.sh

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,7 @@ else
6464
bin/include/refresh-repositories.sh
6565
)
6666
fi
67-
for relative_path in "${required[@]}"; do
67+
for relative_path in ${required[@]+"${required[@]}"}; do
6868
[[ -f $root/$relative_path && ! -L $root/$relative_path ]] || {
6969
printf 'vdpm bundle is missing required regular file: %s\n' "$relative_path" >&2
7070
exit 1
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
#!/usr/bin/env bash
2+
# Arrays that reach a published host must expand the way bash 3.2 tolerates.
3+
#
4+
# macOS runners are the reason. Their /bin/bash is 3.2, where expanding an
5+
# empty array under `set -u` is an unbound-variable error rather than
6+
# nothing, and the failure comes after a complete build: the SDK is
7+
# finalized and validated, then the script dies and stages nothing. It has
8+
# happened twice -- once fixed in "Expand possibly-empty arrays the way
9+
# macOS's bash 3.2 tolerates", then reintroduced with the Rosetta launcher,
10+
# which is empty for every host that runs its own binaries.
11+
#
12+
# The rule is mechanical on purpose. Deciding case by case which array can
13+
# be empty is exactly the judgement that failed: `${name[@]+"${name[@]}"}`
14+
# means the same thing as `"${name[@]}"` whenever the array has elements,
15+
# so requiring it everywhere costs nothing and needs no judgement.
16+
17+
set -euo pipefail
18+
19+
repository_root=$(cd "$(dirname "${BASH_SOURCE[0]}")/../.." && pwd -P)
20+
21+
offenders=$(python3 - "$repository_root" <<'PYEOF'
22+
import glob, os, re, sys
23+
24+
root = sys.argv[1]
25+
# Value expansions only: ${#name[@]} and ${!name[@]} are fine in 3.2.
26+
plain = re.compile(r'"\$\{([A-Za-z_][A-Za-z0-9_]*)\[@\]\}"')
27+
tolerant = re.compile(r'\$\{([A-Za-z_][A-Za-z0-9_]*)\[@\]\+"\$\{\1\[@\]\}"\}')
28+
29+
for path in sorted(glob.glob(os.path.join(root, "scripts", "**", "*.sh"), recursive=True)):
30+
with open(path, encoding="utf-8") as handle:
31+
for number, line in enumerate(handle, 1):
32+
if plain.search(tolerant.sub("", line)):
33+
print(f"{os.path.relpath(path, root)}:{number}:{line.rstrip()}")
34+
PYEOF
35+
)
36+
37+
if [[ -n $offenders ]]; then
38+
printf 'these expansions die on macOS bash 3.2 when the array is empty;\n' >&2
39+
printf 'write ${name[@]+"${name[@]}"} instead:\n\n%s\n' "$offenders" >&2
40+
exit 1
41+
fi
42+
43+
printf 'empty-array expansion: all checks passed\n'

0 commit comments

Comments
 (0)