Skip to content

build: harden build.sh (verify gate, fail-fast, derived deps list) #834

Description

@Chemaclass

Context

Audit of build.sh (2026-07-19) found one real CI gate hole, one silent-corruption path, and several latent traps. Build speed is a non-issue (full build: 0.32s; verification suite dominates). All findings below were verified by experiment, not just code reading.

Findings

1. --verify does not gate the build (CI hole) — build.sh:24-35

BASHUNIT_BUILD_DIR="$out_dir" "$out" tests ... --stop-on-failure
# shellcheck disable=SC2181
if [[ $? -eq 0 ]]; then
  echo "✅ Build verified ✅"
fi

The if has no else and no exit. A failing suite (inner exit code 1, verified with a forced failing test) still lets build.sh exit 0. .github/workflows/build.yml:27 runs ./build.sh --verify and relies on that exit code — the workflow goes green on a broken artifact.

Fix: fail the script when the run fails:

if ! BASHUNIT_BUILD_DIR="$out_dir" "$out" tests --simple --parallel \
    --log-junit "$out_dir/log-junit.xml" --report-html "$out_dir/report.html" \
    --stop-on-failure; then
  echo "❌ Build verification failed"
  exit 1
fi

2. build::embed_docs corrupts the artifact silently on a missing marker — build.sh:126-149

If # __BASHUNIT_EMBEDDED_DOCS_START__ is ever absent from the generated file, line_num is empty, $((line_num - 1)) becomes -1, BSD head -n -1 errors, and a partial temp_file is still mv'd over the artifact. There is no set -euo pipefail anywhere in build.sh.

Fix: add set -euo pipefail at the top of build.sh, and guard the marker lookup (error + exit 1 when either marker is missing).

3. grep -v '^source' over-strips — build.sh:50

Strips any column-0 line starting with source, which would also eat a future source_dir=... variable line. Fix: anchor as grep -v '^source ' (trailing space). Note: indented runtime sourcing (e.g. src/main.sh:150 bootstrap) must keep surviving — it does today because it is indented; do not "fix" that.

4. No dedup guard in build::process_file recursion — build.sh:59-89

The source graph is a tree today (verified: 28 embedded files, zero duplicates), but one added cross-source between src files would silently embed a file twice (duplicate function defs + double top-level execution). Fix: track visited files in the recursion and skip already-processed ones.

5. eval echo "$sourced_file"build.sh:76

eval on grep'd file content to expand $BASHUNIT_ROOT_DIR. Works, but it is an eval-on-text footgun (word splitting, globbing, command execution on unexpected content). Fix: plain parameter substitution:

sourced_file="${sourced_file/\$BASHUNIT_ROOT_DIR/$BASHUNIT_ROOT_DIR}"

6. No post-build syntax gate

bash -n "$out" is free (<50ms) and catches artifact corruption (e.g. finding 2) instantly, before the expensive verify suite. Fix: run it in build::generate_bin right after build::embed_docs, exit 1 on failure.

7. Hardcoded deps array duplicates the entrypoint — build.sh:91-124

build::dependencies hand-lists 27 files in an order that duplicates the source order already present in the bashunit entrypoint (lines 57-84). tests/unit/build_test.sh (test_build_bundles_every_src_file_sourced_by_entrypoint) exists solely to police that sync, and forgetting the manual step is a recurring gotcha. Fix: derive the list by parsing the entrypoint's ^source lines (skip src/dev/ — dev-only, intentionally excluded from the artifact), preserving order. The sync test can then be simplified to assert derivation instead of policing a manual list.

8. Fixed temp.sh name — build.sh:40

Concurrent builds into the same out dir clobber each other's temp file. Fix: unique suffix (temp.$$.sh) — minor, do last.

Acceptance criteria

  • ./build.sh --verify exits non-zero when the built binary fails the test suite (add a unit/acceptance test that simulates a failing verify run, or test the gate function in isolation)
  • build.sh runs under set -euo pipefail; missing embed markers abort with a clear error and non-zero exit
  • bash -n gate runs on every build; corrupt artifact aborts before verify
  • build::process_file skips already-embedded files (regression test: artifact contains each # <file>.sh marker exactly once)
  • No eval in build.sh
  • Deps list derived from the bashunit entrypoint; src/dev/* excluded; embed order unchanged (byte-identical artifact vs current build, except intentional changes)
  • Built artifact verified: ./build.sh --verify green, bin/bashunit doc equals output hash unchanged (8e93990bbb22cf48c0b4a4fdb9308243015cdcf9)

Constraints

  • build.sh itself runs only in dev/CI, so Bash 4+ is tolerated there today — but keep it Bash 3.2-clean anyway (macOS default bash runs it locally); no declare -A, prefer [ ] in new code where practical
  • Portable across macOS (BSD head/sed) and Linux; CI also builds on Windows (checksum step skips there — keep that)
  • Do not change what gets embedded: src/dev/debug.sh (bashunit::dump/bashunit::dd) stays dev-only
  • TDD: each fix lands with its failing test first where testable (gate, dedup, marker guard, derivation)

Verification

./build.sh --verify                    # must gate
bash -n bin/bashunit                   # clean
bin/bashunit doc equals | shasum       # 8e93990bbb22cf48c0b4a4fdb9308243015cdcf9
./bashunit tests/ && ./bashunit --parallel --simple --strict tests/
make sa && make lint

Metadata

Metadata

Assignees

Labels

enhancementNew feature or request

Type

No type

Fields

No fields configured for issues without a type.

Projects

Status
Done

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions