From 29a94623f48b584020538579f9eb81d653a40344 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcos=20M=C3=A9ndez?= Date: Sun, 27 Sep 2026 03:46:09 +0000 Subject: [PATCH] Let a unit carry a removelist and name the variables its conf script reads The unit loop read overlay/ and conf and nothing else, so two thirds of what a component is could be expressed as a unit and the last third could not. A removelist. A common component goes through three phases: its overlay, its conf script, its removelist. A unit had no slot for the third, so a component with a non-empty removelist could not leave the shared tree. The loop now has that phase, after the unit conf scripts and before the common removelists, where the common removelists sit relative to the common conf scripts. Only a regular file counts, as for the product-local REMOVELIST. CONF_VARS. _CONF_VARS is settled before any unit is looked at, so a unit had no way to ask for a build-time variable: mk/turnkey/mysql.mk line 1 is "CONF_VARS += MYSQL_PASS" and the unit form of that line did not exist. A unit may now carry conf-vars, naming one variable per line with # starting a comment, and those names join CONF_VARS. The values still come from the environment or the product Makefile, exactly as a makefile fragment naming a variable leaves them. The names are read from UNIT_DIRS and not from UNITS, so the chroot environment of a layered build does not change with the set of units that build applies; an entry that is not a variable name stops make and names the file it came from, rather than reaching export as junk. UNIT_DIRS moves above the CONF_VARS block, since _CONF_VARS now needs it. tests/units.sh grows to 56 checks: the removelist applied, its position, a removelist that is a directory, a unit without one, a unit left out of UNITS, a failing removelist, and for conf-vars a set variable reaching the chroot, an unset one left out, several units contributing, the set not moving with UNITS, and the error path. tests/coverage.sh: 75 of 75, 100 percent. --- COVERAGE.md | 4 ++-- share/product.mk | 32 ++++++++++++++++++++++--- tests/units.sh | 62 ++++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 93 insertions(+), 5 deletions(-) diff --git a/COVERAGE.md b/COVERAGE.md index aff62f3..9b238a9 100644 --- a/COVERAGE.md +++ b/COVERAGE.md @@ -19,9 +19,9 @@ before the merge. | 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` | +| tests/units.sh | 56 of 56 | the unit loop, `UNITS`, the position of the units in `root.patched`, the per-unit removelist and `UNIT_CONF_VARS` | -Total 57 of 57, 100 percent. The gate stays at 100. +Total 75 of 75, 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 diff --git a/share/product.mk b/share/product.mk index 1e28c85..b2dd580 100644 --- a/share/product.mk +++ b/share/product.mk @@ -23,14 +23,32 @@ endif COMMON_PATCHES := turnkey.d $(COMMON_PATCHES) +# defined here, before _CONF_VARS, because a unit may contribute to it +UNIT_DIRS ?= unit.d + CONF_VARS_BUILTIN ?= FAB_ARCH HOST_ARCH FAB_HTTP_PROXY AMD64 ARM64 RELEASE DISTRO CODENAME DEBIAN UBUNTU KERNEL DEBUG CHROOT_ONLY DI_LIVE_DEBUG SOURCE_DATE_EPOCH +# The build-time variables a unit's conf script reads, named one per line in +# the unit's conf-vars file, # starting a comment: the unit form of a +# "CONF_VARS += NAME" line in a makefile fragment, and the only way a unit can +# ask for one, since _CONF_VARS is settled before any unit is looked at. +# Read from every unit under UNIT_DIRS and not from UNITS, so that the chroot +# environment of a layered build does not change with the units that build +# happens to apply. +UNIT_CONF_VARS_FILES = $(wildcard $(UNIT_DIRS)/*/conf-vars) +UNIT_CONF_VARS_NAMED = $(shell cat $(UNIT_CONF_VARS_FILES) /dev/null | sed 's/#.*//') +UNIT_CONF_VARS = $(shell printf '%s\n' $(UNIT_CONF_VARS_NAMED) | grep -xE '[A-Za-z_][A-Za-z0-9_]*') +UNIT_CONF_VARS_BAD = $(filter-out $(UNIT_CONF_VARS),$(UNIT_CONF_VARS_NAMED)) +ifneq ($(UNIT_CONF_VARS_BAD),) + $(error not a variable name in $(UNIT_CONF_VARS_FILES): $(UNIT_CONF_VARS_BAD)) +endif + define filter-undefined-vars $(foreach var,$1,$(if $($(var)), $(var))) endef _CONF_VARS_BUILTIN = $(call filter-undefined-vars,$(CONF_VARS_BUILTIN)) -_CONF_VARS = $(_CONF_VARS_BUILTIN) $(call filter-undefined-vars,$(CONF_VARS)) +_CONF_VARS = $(_CONF_VARS_BUILTIN) $(call filter-undefined-vars,$(CONF_VARS) $(UNIT_CONF_VARS)) export $(_CONF_VARS) export FAB_CHROOT_ENV = $(shell echo $(_CONF_VARS) | sed 's/ \+/:/g') @@ -103,8 +121,6 @@ ifeq ($(wildcard $(REMOVELIST)),) 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 @@ -200,6 +216,7 @@ define help/body @echo ' REMOVELIST $(value REMOVELIST)' @echo ' UNIT_DIRS $(value UNIT_DIRS)/' @echo ' UNITS $(value UNITS)' + @echo ' UNIT_CONF_VARS $(UNIT_CONF_VARS)' @echo ' ROOT_OVERLAY $(value ROOT_OVERLAY)/' @echo ' CONF_SCRIPTS $(value CONF_SCRIPTS)/' @echo ' PATCHES_PATH $(value PATCHES_PATH)/' @@ -408,6 +425,15 @@ define root.patched/body fi; \ ) + # apply the unit removelists, the third phase a common component goes + # through, and a component that carries one cannot be a unit without it + $(foreach unit,$(UNITS), \ + if [ -f $(unit)/removelist ]; then \ + echo fab-apply-removelist $(unit)/removelist $O/root.patched; \ + fab-apply-removelist $(unit)/removelist $O/root.patched || 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), diff --git a/tests/units.sh b/tests/units.sh index 1c1d4b8..0b25bc9 100755 --- a/tests/units.sh +++ b/tests/units.sh @@ -85,6 +85,8 @@ 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" +echo '/usr/local/src/alpha' > "$product/unit.d/alpha/removelist" +mkdir -p "$product/unit.d/gamma/removelist" cat > "$product/Makefile" <<'EOF' COMMON_OVERLAYS = shared @@ -213,6 +215,7 @@ 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$' +alpha_removelist='^fab-apply-removelist unit\.d/alpha/removelist build/root\.patched$' 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 ' @@ -240,6 +243,15 @@ 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" +called "the removelist of a unit is applied" "$alpha_removelist" +not_called "a unit whose removelist is a directory contributes none" \ + 'unit\.d/gamma/removelist' +not_called "a unit without a removelist contributes none" \ + 'unit\.d/beta/removelist' +before "unit removelists come after the unit conf scripts" \ + "$alpha_conf" "$alpha_removelist" +before "unit removelists come before the common removelists" \ + "$alpha_removelist" "$common_removelist" before "unit conf scripts come before the common removelists" \ "$alpha_conf" "$common_removelist" before "unit overlays come before the product-local overlay" \ @@ -260,6 +272,11 @@ 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" + +export STUB_FAIL='removelist unit\.d/alpha/removelist' +build_fails "a unit removelist that fails stops the build" \ + build/stamps/root.patched +not_called "nothing after a failed unit removelist runs" "$local_overlay" unset STUB_FAIL # --- a layered build applies only the units the parent has not applied --------- @@ -270,6 +287,8 @@ 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' +not_called "the removelist of a unit left out of UNITS is not applied" \ + 'unit\.d/alpha/removelist' called "the common removelists are applied with a selected unit" \ "$common_removelist" @@ -296,6 +315,49 @@ builds "root.spec is resolved with no unit selected" \ 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 unit contributes a CONF_VARS entry --------------------------------------- +# _CONF_VARS is settled before any unit is looked at, so a conf-vars file is +# the only way a unit can ask for a build-time variable. mk/turnkey/mysql.mk +# line 1 is "CONF_VARS += MYSQL_PASS"; this is that line, in unit form. + +printf '# the database password\nMYSQL_PASS\n\n' \ + > "$product/unit.d/alpha/conf-vars" +printf 'BETA_ONE\nBETA_TWO\n' > "$product/unit.d/beta/conf-vars" + +is "a variable a unit names but nobody sets is left out" \ + "$(make_var UNIT_CONF_VARS)" "MYSQL_PASS BETA_ONE BETA_TWO" +is "an unset unit variable stays out of the chroot environment" \ + "$(make_var FAB_CHROOT_ENV | grep -c MYSQL_PASS)" "0" + +export MYSQL_PASS=secret BETA_TWO=2 +is "a unit variable that is set reaches the chroot environment" \ + "$(make_var FAB_CHROOT_ENV | tr : '\n' | grep -cE '^(MYSQL_PASS|BETA_TWO)$')" "2" +is "the unit variable nobody set is still left out" \ + "$(make_var FAB_CHROOT_ENV | tr : '\n' | grep -c '^BETA_ONE$')" "0" +is "a unit variable does not change with UNITS" \ + "$(cd "$product" && "$share/load_env" 2>/dev/null; make debug V=FAB_CHROOT_ENV UNITS= 2>&1 >/dev/null \ + | sed -n 's|^.*: FAB_CHROOT_ENV = ||p' | head -n 1 | tr : '\n' | grep -cE '^(MYSQL_PASS|BETA_TWO)$')" "2" + +builds "a build with unit variables succeeds" build/stamps/root.patched +called "the unit conf script still runs" "$alpha_conf" +unset MYSQL_PASS BETA_TWO + +printf 'MYSQL-PASS\n' > "$product/unit.d/gamma/conf-vars" +if build build/stamps/root.patched; then + not_ok "a conf-vars entry that is not a variable name stops make" \ + "make returned 0" +else + ok "a conf-vars entry that is not a variable name stops make" +fi +if grep -q 'not a variable name in .*unit\.d/gamma/conf-vars: MYSQL-PASS' "$work/make.out"; then + ok "the error names the offending file and entry" +else + not_ok "the error names the offending file and entry" "$(cat "$work/make.out")" +fi +rm "$product/unit.d/gamma/conf-vars" "$product/unit.d/alpha/conf-vars" \ + "$product/unit.d/beta/conf-vars" +is "no conf-vars file means no unit variable" "$(make_var UNIT_CONF_VARS)" "" + # --- a product without units ---------------------------------------------------- rm -rf "$product/unit.d"