diff --git a/COVERAGE.md b/COVERAGE.md index 4d51d81..aff62f3 100644 --- a/COVERAGE.md +++ b/COVERAGE.md @@ -12,6 +12,25 @@ Pull request #1 merged on 2026-09-26 (merge commit e79be43) and brought down, and is only ever raised. The sections that follow record the state before the merge. +## 2026-09-27: the unit loop and where it runs + +`tests/coverage.sh` now runs two suites and counts the checks of both: + +| Suite | Checks | What it measures | +|-------|--------|------------------| +| tests/source-date-epoch.sh | 19 of 19 | the `SOURCE_DATE_EPOCH` handling | +| tests/units.sh | 38 of 38 | the unit loop, `UNITS` and the position of the units in `root.patched` | + +Total 57 of 57, 100 percent. The gate stays at 100. + +`tests/units.sh` does not dry run. It builds a fake product with three +units against this checkout's `product.mk` with the four fab tools replaced +by stubs that log their arguments, and asserts on the log: what was +applied, in which order, and what a failing tool does. That is why it can +tell that a unit whose `conf` is not executable is skipped, which a +`make -n` of the same recipe cannot, since the `[ -x ]` guard is resolved +by the shell and not by make. + ## Baseline before the merge: 0 percent, nothing measured `tests/` holds `regtest.sh` (77 lines), `override.sh`, `parseopts.py` and diff --git a/share/product.mk b/share/product.mk index c89c5d3..1e28c85 100644 --- a/share/product.mk +++ b/share/product.mk @@ -104,6 +104,14 @@ ifeq ($(wildcard $(REMOVELIST)),) endif UNIT_DIRS ?= unit.d + +# The units this build applies, as directory paths. Every unit under +# UNIT_DIRS by default. A build layered on a parent image overrides it with +# the units the parent has not applied yet, because a unit's conf script +# runs once and is not written to run twice; the plans of every unit are +# still resolved (root.spec below), since the plan only names packages. +UNITS ?= $(sort $(wildcard $(UNIT_DIRS)/*)) + CONF_SCRIPTS ?= conf.d PATCHES_PATH ?= patches.d @@ -191,6 +199,7 @@ define help/body @echo ' PLAN $(value PLAN)' @echo ' REMOVELIST $(value REMOVELIST)' @echo ' UNIT_DIRS $(value UNIT_DIRS)/' + @echo ' UNITS $(value UNITS)' @echo ' ROOT_OVERLAY $(value ROOT_OVERLAY)/' @echo ' CONF_SCRIPTS $(value CONF_SCRIPTS)/' @echo ' PATCHES_PATH $(value PATCHES_PATH)/' @@ -282,6 +291,10 @@ endef # target: root.spec root.spec/deps ?= $(STAMPS_DIR)/bootstrap $(wildcard plan/*) +# every unit's plan is resolved, including the plans of the units UNITS +# leaves out: a plan names packages, and a package the parent image already +# carries resolves to the version that is there rather than being installed +# a second time define root.spec/body unit_plans="$(wildcard $(UNIT_DIRS)/*/plan)"; \ fab-plan-resolve $(PLAN) $$unit_plans $(EXTRA_PLAN) --bootstrap=$(BOOTSTRAP) --output=$O/root.spec $(foreach var,$(_CONF_VARS_BUILTIN),-D '$(var)=$($(var))') @@ -375,23 +388,31 @@ define root.patched/body fi ) - # apply the common removelists - $(foreach removelist,$(_COMMON_REMOVELISTS), - fab-apply-removelist $(removelist) $O/root.patched; \ - ) - - # apply the product-local units - $(foreach unit,$(wildcard $(UNIT_DIRS)/*), \ + # apply the unit overlays, in unit name order and all of them before any + # unit conf script, the order the common overlays and the common conf + # scripts above are applied in: a unit is a component of the same kind, + # and a recipe that composes several must not depend on which one of + # them fab happens to reach first + $(foreach unit,$(UNITS), \ if [ -d $(unit)/overlay ]; then \ echo fab-apply-overlay $(unit)/overlay $O/root.patched; \ - fab-apply-overlay $(unit)/overlay $O/root.patched; \ + fab-apply-overlay $(unit)/overlay $O/root.patched || exit; \ fi; \ + ) + + # run the unit configuration scripts + $(foreach unit,$(UNITS), \ if [ -x $(unit)/conf ]; then \ echo fab-chroot $O/root.patched --script $(unit)/conf; \ - fab-chroot $O/root.patched --script $(unit)/conf; \ + fab-chroot $O/root.patched --script $(unit)/conf || exit; \ fi; \ ) + # apply the common removelists, after the units, so that a removelist + # can remove a file a unit brought in + $(foreach removelist,$(_COMMON_REMOVELISTS), + fab-apply-removelist $(removelist) $O/root.patched; \ + ) # apply the product-local root overlay if [ -d $(ROOT_OVERLAY) ]; then \ diff --git a/tests/coverage.sh b/tests/coverage.sh index 9c62427..80e006b 100755 --- a/tests/coverage.sh +++ b/tests/coverage.sh @@ -1,10 +1,14 @@ #!/bin/bash -# Coverage of the SOURCE_DATE_EPOCH code in share/product.mk. +# Coverage of the project-authored code in share/product.mk: +# tests/source-date-epoch.sh the SOURCE_DATE_EPOCH handling +# tests/units.sh the unit loop and UNITS # # make has no line coverage tool, so the number here is the share of the -# checks in tests/source-date-epoch.sh that pass: one check per effect of -# the touched code (both arms of the ifneq, both xorriso recipes, isohybrid, -# FAB_CHROOT_ENV, fab-plan-resolve and the untouched mksquashfs options). +# checks in those scripts that pass: one check per effect of the touched +# code (both arms of the ifneq, both xorriso recipes, isohybrid, +# FAB_CHROOT_ENV, fab-plan-resolve and the untouched mksquashfs options for +# the first; each unit input, each position in root.patched, each value of +# UNITS and each failure path for the second). # Exits 1 when that share is below the threshold (default 95). # # tests/coverage.sh [THRESHOLD] (or COVERAGE_THRESHOLD in the environment) @@ -13,13 +17,21 @@ set -euo pipefail here="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" threshold="${1:-${COVERAGE_THRESHOLD:-95}}" -tap="$("$here/source-date-epoch.sh" || true)" -printf '%s\n' "$tap" -total="$(grep -cE '^(not )?ok [0-9]+ ' <<<"$tap")" -passed="$(grep -cE '^ok [0-9]+ ' <<<"$tap")" +suites=(source-date-epoch units) + +total=0 +passed=0 +for suite in "${suites[@]}"; do + echo "# $suite" + tap="$("$here/$suite.sh" || true)" + printf '%s\n' "$tap" + total=$((total + $(grep -cE '^(not )?ok [0-9]+ ' <<<"$tap"))) + passed=$((passed + $(grep -cE '^ok [0-9]+ ' <<<"$tap"))) +done + percent="$(awk -v p="$passed" -v t="$total" 'BEGIN { printf "%.2f", (t ? 100 * p / t : 0) }')" -echo "product.mk SOURCE_DATE_EPOCH: $percent percent ($passed of $total checks) covered, threshold $threshold" +echo "product.mk: $percent percent ($passed of $total checks) covered, threshold $threshold" if ! awk -v p="$percent" -v t="$threshold" 'BEGIN { exit !(p + 0 >= t + 0) }'; then echo "coverage below threshold" >&2 exit 1 diff --git a/tests/units.sh b/tests/units.sh new file mode 100755 index 0000000..1c1d4b8 --- /dev/null +++ b/tests/units.sh @@ -0,0 +1,310 @@ +#!/bin/bash +# make-level test for the unit loop of share/product.mk: which units a build +# applies (UNITS) and where in root.patched they are applied. +# +# A minimal fake product with three units is built under a scratch FAB_PATH +# against the product.mk of this checkout. The four fab tools the recipe +# calls (fab-apply-overlay, fab-chroot, fab-apply-removelist, +# fab-plan-resolve) are stubs that append their arguments to a log, and the +# init step that would deck a real filesystem is overridden away, so the +# recipe runs for real and the log says what it did and in which order. +# Output is TAP; the exit status is the number of failed checks. +# +# product.mk includes /tmp/.build_env, written by share/load_env; the test +# runs load_env from the fake product, so that file is overwritten for the +# duration of the run and restored afterwards. Nothing else outside the +# scratch directory is touched and no real fab tool is executed. +# +# tests/units.sh + +set -uo pipefail + +here="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +share="$(cd "$here/.." && pwd)/share" +work="$(mktemp -d)" +log="$work/calls.log" + +restore_build_env() { + if [[ -e "$work/build_env.saved" ]]; then + cp -p "$work/build_env.saved" /tmp/.build_env + else + rm -f /tmp/.build_env + fi +} +cleanup() { + restore_build_env + rm -rf "$work" +} +trap cleanup EXIT +[[ ! -e /tmp/.build_env ]] || cp -p /tmp/.build_env "$work/build_env.saved" + +# --- stub fab tools ----------------------------------------------------------- + +mkdir -p "$work/stubs" +for tool in fab-apply-overlay fab-chroot fab-apply-removelist fab-plan-resolve \ + fab-apply-patch; do + cat > "$work/stubs/$tool" <> "$log" +if [ -n "\${STUB_FAIL:-}" ] && echo "$tool \$*" | grep -qE "\$STUB_FAIL"; then + echo "$tool: refusing on purpose" >&2 + exit 1 +fi +exit 0 +EOF + chmod 755 "$work/stubs/$tool" +done +export PATH="$work/stubs:$PATH" + +# --- fake product with three units --------------------------------------------- + +export FAB_PATH="$work/fab" +export FAB_SHARE_PATH="$share" +export RELEASE=debian/trixie +export FAB_ARCH=amd64 +export NO_PROXY=true +unset SOURCE_DATE_EPOCH BOOTSTRAP O ISOLABEL CONF_VARS CHROOT_ONLY UNITS STUB_FAIL + +product="$work/fakeprod" +mkdir -p "$FAB_PATH/bootstraps/trixie-amd64" \ + "$FAB_PATH/common/overlays/shared" \ + "$FAB_PATH/common/conf" \ + "$FAB_PATH/common/removelists" \ + "$product/plan" "$product/overlay" +: > "$product/plan/main" +: > "$FAB_PATH/common/conf/shared" +: > "$FAB_PATH/common/removelists/shared" + +# unit "alpha" carries all of plan, overlay and conf; "beta" carries a plan +# and an overlay only; "gamma" carries a conf script that is not executable +for unit in alpha beta gamma; do + mkdir -p "$product/unit.d/$unit/overlay" + : > "$product/unit.d/$unit/plan" +done +printf '#!/bin/sh\ntrue\n' > "$product/unit.d/alpha/conf" +chmod 755 "$product/unit.d/alpha/conf" +printf '#!/bin/sh\ntrue\n' > "$product/unit.d/gamma/conf" +chmod 644 "$product/unit.d/gamma/conf" + +cat > "$product/Makefile" <<'EOF' +COMMON_OVERLAYS = shared +COMMON_CONF = shared +COMMON_REMOVELISTS = shared +include $(FAB_SHARE_PATH)/product.mk +EOF + +# The stamped rule is
   ; only the body is
+# under test, and root.patched/init would deck a real filesystem.
+neutral=(
+    root.patched/init=
+    root.patched/deps=
+    root.spec/deps=
+)
+
+# build TARGET [VAR=VALUE ...]: run the recipe, leaving its calls in $log
+build() {
+    rm -f "$log"
+    rm -rf "$product/build"
+    (
+        cd "$product" || exit 1
+        "$share/load_env" 2>/dev/null
+        make "$@" "${neutral[@]}" 2>&1
+    ) > "$work/make.out"
+    local rc=$?
+    touch "$log"
+    return $rc
+}
+
+# make_var VAR: the value make computes for VAR
+make_var() {
+    (
+        cd "$product" || exit 1
+        "$share/load_env" 2>/dev/null
+        make debug V="$1" 2>&1 >/dev/null
+    ) | sed -n "s|^.*: $1 = ||p" | head -n 1
+}
+
+# --- TAP helpers ------------------------------------------------------------
+
+count=0
+failed=0
+
+ok() {
+    count=$((count + 1))
+    echo "ok $count - $1"
+}
+
+not_ok() {
+    count=$((count + 1))
+    failed=$((failed + 1))
+    echo "not ok $count - $1"
+    printf '%s\n' "$2" | sed 's/^/# /'
+}
+
+is() {
+    if [[ "$2" == "$3" ]]; then
+        ok "$1"
+    else
+        not_ok "$1" "got:  $2
+want: $3"
+    fi
+}
+
+# builds DESC TARGET [VAR=VALUE ...]: the recipe runs to the end
+builds() {
+    local desc=$1
+    shift
+    if build "$@"; then
+        ok "$desc"
+    else
+        not_ok "$desc" "$(cat "$work/make.out")"
+    fi
+}
+
+# build_fails DESC TARGET [VAR=VALUE ...]: the recipe stops with an error
+build_fails() {
+    local desc=$1
+    shift
+    if build "$@"; then
+        not_ok "$desc" "make returned 0
+$(cat "$work/make.out")"
+    else
+        ok "$desc"
+    fi
+}
+
+# called DESC REGEX: the log has a line matching REGEX
+called() {
+    if grep -qE -e "$2" "$log"; then
+        ok "$1"
+    else
+        not_ok "$1" "no call matches: $2
+$(cat "$log")"
+    fi
+}
+
+# not_called DESC REGEX: no line of the log matches REGEX
+not_called() {
+    if grep -qE -e "$2" "$log"; then
+        not_ok "$1" "$(grep -E -e "$2" "$log")"
+    else
+        ok "$1"
+    fi
+}
+
+# before DESC REGEX_A REGEX_B: the first call matching A precedes the first
+# matching B, and both are there
+before() {
+    local a b
+    a=$(grep -nE -e "$2" "$log" | head -n 1 | cut -d : -f 1)
+    b=$(grep -nE -e "$3" "$log" | head -n 1 | cut -d : -f 1)
+    if [[ -z "$a" || -z "$b" ]]; then
+        not_ok "$1" "not both called: $2 and $3
+$(cat "$log")"
+    elif [[ "$a" -lt "$b" ]]; then
+        ok "$1"
+    else
+        not_ok "$1" "line $a is not before line $b
+$(cat "$log")"
+    fi
+}
+
+alpha_overlay='^fab-apply-overlay unit\.d/alpha/overlay build/root\.patched$'
+beta_overlay='^fab-apply-overlay unit\.d/beta/overlay build/root\.patched$'
+gamma_overlay='^fab-apply-overlay unit\.d/gamma/overlay build/root\.patched$'
+alpha_conf='^fab-chroot build/root\.patched --script unit\.d/alpha/conf$'
+common_overlay='^fab-apply-overlay .*/common/overlays/shared '
+common_conf='^fab-chroot build/root\.patched --script .*/common/conf/shared$'
+common_removelist='^fab-apply-removelist .*/common/removelists/shared '
+local_overlay='^fab-apply-overlay overlay build/root\.patched$'
+
+# --- every unit is applied by default -----------------------------------------
+
+is "UNITS defaults to every unit directory, sorted" \
+    "$(make_var UNITS)" "unit.d/alpha unit.d/beta unit.d/gamma"
+
+builds "the default build succeeds" build/stamps/root.patched
+called "the overlay of a unit is applied" "$alpha_overlay"
+called "the conf script of a unit is run" "$alpha_conf"
+called "a unit without a conf script contributes its overlay" "$beta_overlay"
+not_called "a unit conf script that is not executable is not run" \
+    'script unit\.d/gamma/conf'
+called "a unit whose conf is not executable still contributes its overlay" \
+    "$gamma_overlay"
+
+# --- the position in root.patched ---------------------------------------------
+
+before "unit overlays come after the common overlays" \
+    "$common_overlay" "$alpha_overlay"
+before "unit overlays come after the common conf scripts" \
+    "$common_conf" "$alpha_overlay"
+before "every unit overlay comes before any unit conf script" \
+    "$gamma_overlay" "$alpha_conf"
+before "unit conf scripts come before the common removelists" \
+    "$alpha_conf" "$common_removelist"
+before "unit overlays come before the product-local overlay" \
+    "$alpha_overlay" "$local_overlay"
+before "the common removelists come before the product-local overlay" \
+    "$common_removelist" "$local_overlay"
+
+# --- a failing unit stops the build ---------------------------------------------
+# Without the "|| exit" the loop is one shell line per phase, so a unit that
+# fails in the middle of it is swallowed and the layer ships half applied.
+
+export STUB_FAIL='overlay unit\.d/alpha/overlay'
+build_fails "a unit overlay that fails stops the build" \
+    build/stamps/root.patched
+not_called "nothing after a failed unit overlay runs" "$local_overlay"
+
+export STUB_FAIL='script unit\.d/alpha/conf'
+build_fails "a unit conf script that fails stops the build" \
+    build/stamps/root.patched
+not_called "nothing after a failed unit conf script runs" "$local_overlay"
+unset STUB_FAIL
+
+# --- a layered build applies only the units the parent has not applied ---------
+
+builds "a build with a selected unit succeeds" \
+    build/stamps/root.patched UNITS=unit.d/beta
+called "a selected unit is applied" "$beta_overlay"
+not_called "a unit left out of UNITS is not applied" 'unit\.d/alpha/overlay'
+not_called "the conf script of a unit left out of UNITS is not run" \
+    'unit\.d/alpha/conf'
+called "the common removelists are applied with a selected unit" \
+    "$common_removelist"
+
+builds "a build with no unit selected succeeds" build/stamps/root.patched UNITS=
+not_called "an empty UNITS applies no unit overlay" 'unit\.d/[a-z]+/overlay'
+not_called "an empty UNITS runs no unit conf script" 'unit\.d/[a-z]+/conf'
+called "an empty UNITS still applies the common removelists" \
+    "$common_removelist"
+called "an empty UNITS still applies the product-local overlay" "$local_overlay"
+
+# --- the plan of every unit is resolved, whatever UNITS says --------------------
+
+builds "root.spec is resolved with every unit" build/stamps/root.spec
+called "every unit plan reaches fab-plan-resolve" \
+    'fab-plan-resolve plan/main unit\.d/alpha/plan unit\.d/beta/plan unit\.d/gamma/plan '
+
+builds "root.spec is resolved with one unit selected" \
+    build/stamps/root.spec UNITS=unit.d/beta
+called "the plan of a unit left out of UNITS is still resolved" \
+    'fab-plan-resolve plan/main unit\.d/alpha/plan unit\.d/beta/plan unit\.d/gamma/plan '
+
+builds "root.spec is resolved with no unit selected" \
+    build/stamps/root.spec UNITS=
+called "an empty UNITS still resolves every unit plan" \
+    'fab-plan-resolve plan/main unit\.d/alpha/plan unit\.d/beta/plan unit\.d/gamma/plan '
+
+# --- a product without units ----------------------------------------------------
+
+rm -rf "$product/unit.d"
+is "UNITS is empty without a unit.d" "$(make_var UNITS)" ""
+builds "a product without unit.d builds" build/stamps/root.patched
+not_called "a product without unit.d applies no unit" 'unit\.d'
+called "a product without unit.d applies the common overlays" "$common_overlay"
+called "a product without unit.d applies the common removelists" \
+    "$common_removelist"
+
+echo "1..$count"
+exit "$failed"