Skip to content

Let a unit carry a removelist and name the variables its conf script reads #5

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
marcos-mendez merged 1 commit into from
Sep 27, 2026
Merged

Let a unit carry a removelist and name the variables its conf script reads #5

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions COVERAGE.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
32 changes: 29 additions & 3 deletions share/product.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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')
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)/'
Expand Down Expand Up @@ -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),
Expand Down
62 changes: 62 additions & 0 deletions tests/units.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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 '
Expand Down Expand Up @@ -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" \
Expand All @@ -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 ---------
Expand All @@ -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"

Expand All @@ -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"
Expand Down
Loading