artifact-helpers: survive bash -e

`restore=$(shopt -p nullglob)` exits 1 when nullglob is off, which it is
in the workflow shell. Under the `bash -e` that GitHub runs steps with,
that ended "Pack artifact" before tar ran: every job on #512 and #550
built fine and then failed with nothing in the log but the command.

The self-test never saw it because `pack_packages ... || fail` suppresses
errexit. Enumerate package files with a loop instead of toggling shell
options, and add a test that calls both helpers under `bash -e` exactly
as the workflows do; it fails against the old helper.

helpers/artifact-helpers.sh

@@ -5,14 +5,24 @@
 # makepkg. So the files ride inside a tar with a plain name and keep their
 # own names untouched: pacman clients and bin/publish-artifact both rely on
 # the filename matching PKGINFO.
+#
+# Both functions run under the workflow's `bash -e`: nothing in them may
+# return non-zero except the final failure.
 
-# pack_packages <dir> <tar>: every *.pkg.tar.zst directly in <dir> into <tar>.
+# package_files <dir>: the *.pkg.tar.zst directly in <dir>, one per line.
-# Signatures and the scratch database next to them stay behind.
+# Signatures and the scratch database next to them are not packages.
+package_files() {
+  local f
+  for f in "$1"/*.pkg.tar.zst; do
+    [[ -e "$f" ]] && printf '%s\n' "$f"
+  done
+  return 0
+}
+
+# pack_packages <dir> <tar>: every package in <dir> into <tar>.
 pack_packages() {
-  local dir=$1 out=$2 restore files=()
+  local dir=$1 out=$2 files=()
-  restore=$(shopt -p nullglob); shopt -s nullglob
+  mapfile -t files < <(package_files "$dir")
-  files=("$dir"/*.pkg.tar.zst)
-  $restore
   (( ${#files[@]} )) || { echo "pack_packages: no *.pkg.tar.zst in $dir" >&2; return 1; }
   tar -cf "$out" -C "$dir" -- "${files[@]##*/}"
 }
@@ -22,15 +32,13 @@ pack_packages() {
 # builds before packing hold the bare files. The bare form can go once
 # those artifacts have expired (7-day retention).
 unpack_packages() {
-  local src=$1 dest=$2 restore files=()
+  local src=$1 dest=$2 files=()
   mkdir -p "$dest"
   if [[ -f "$src/packages.tar" ]]; then
     tar -xf "$src/packages.tar" -C "$dest"
-    return
+    return 0
   fi
-  restore=$(shopt -p nullglob); shopt -s nullglob
+  mapfile -t files < <(package_files "$src")
-  files=("$src"/*.pkg.tar.zst)
-  $restore
   (( ${#files[@]} )) || { echo "unpack_packages: nothing to unpack in $src" >&2; return 1; }
   cp -- "${files[@]}" "$dest/"
 }

tests/artifact-helpers.sh

@@ -31,6 +31,15 @@ mkdir -p "$T/old" "$T/out2"; cp "$T/built"/*.pkg.tar.zst "$T/old/"
 unpack_packages "$T/old" "$T/out2" && [[ "$(ls "$T/out2" | sort | tr '\n' ' ')" == "$PLAIN $EPOCH " ]] \
   && pass "a bare pre-packing artifact still unpacks" || fail "bare artifact"
 
+# The workflows call these bare under `bash -e`, so any non-zero status
+# inside them ends the step. (The first version used `shopt -p nullglob`,
+# which exits 1 when the option is off; the tests above never saw it because
+# `||` suppresses errexit.)
+mkdir -p "$T/out4" "$T/out5"
+bash -e -c "source '$ROOT/helpers/artifact-helpers.sh'; pack_packages '$T/built' '$T/out4/packages.tar'; tar -tf '$T/out4/packages.tar' >/dev/null; unpack_packages '$T/out4' '$T/out5'" \
+  && [[ "$(ls "$T/out5" | sort | tr '\n' ' ')" == "$PLAIN $EPOCH " ]] \
+  && pass "pack and unpack succeed under bash -e, as the workflows call them" || fail "bash -e"
+
 mkdir -p "$T/empty"
 if pack_packages "$T/empty" "$T/x.tar" 2>/dev/null; then fail "packing an empty build dir should fail"; else pass "empty build dir refused"; fi
 if unpack_packages "$T/empty" "$T/out3" 2>/dev/null; then fail "an empty artifact should fail"; else pass "empty artifact refused"; fi