On 11/13/25 06:46, Alyssa Ross wrote: > Demi Marie Obenour writes: > >> Avoid redundant rebuilds of the rootfs verity superblock and roothash. >> Remove duplicate code. Clean up Makefile to avoid temporary files. >> >> Signed-off-by: Demi Marie Obenour >> --- >> host/initramfs/Makefile | 26 +++++--------------------- >> host/initramfs/shell.nix | 4 +++- >> host/rootfs/Makefile | 45 +++++++++++++++++++++------------------------ >> host/rootfs/default.nix | 2 +- >> host/rootfs/shell.nix | 2 +- >> release/live/Makefile | 26 +++++--------------------- >> release/live/default.nix | 4 +++- >> 7 files changed, 39 insertions(+), 70 deletions(-) > > Looking good! Thank you! >> diff --git a/host/initramfs/Makefile b/host/initramfs/Makefile >> index cb13fbb35f065b67d291d4a35591d6f12720060c..102870ecba4456303414e2531ea592473ddfc1cf 100644 >> --- a/host/initramfs/Makefile >> +++ b/host/initramfs/Makefile >> @@ -35,26 +35,10 @@ build/mountpoints: >> cd build/mountpoints && mkdir -p $(MOUNTPOINTS) >> find build/mountpoints -mindepth 1 -exec touch -d @0 {} ';' >> >> -# veritysetup format produces two files, but Make only (portably) >> -# supports one output per rule, so we combine the two outputs then >> -# define two more rules to separate them again. >> -build/rootfs.verity: $(ROOT_FS) >> - mkdir -p build >> - $(VERITYSETUP) format $(ROOT_FS) build/rootfs.verity.superblock.tmp \ >> - | awk -F ':[[:blank:]]*' '$$1 == "Root hash" {print $$2; exit}' \ >> - > build/rootfs.verity.roothash.tmp >> - cat build/rootfs.verity.roothash.tmp build/rootfs.verity.superblock.tmp \ >> - > $@ >> - rm build/rootfs.verity.roothash.tmp build/rootfs.verity.superblock.tmp >> -build/rootfs.verity.roothash: build/rootfs.verity >> - head -n 1 build/rootfs.verity > $@ >> -build/rootfs.verity.superblock: build/rootfs.verity >> - tail -n +2 build/rootfs.verity > $@ >> - >> -build/live.img: ../../scripts/format-uuid.sh ../../scripts/make-gpt.sh ../../scripts/sfdisk-field.awk build/rootfs.verity.superblock build/rootfs.verity.roothash $(ROOT_FS) >> +build/live.img: ../../scripts/format-uuid.sh ../../scripts/make-gpt.sh ../../scripts/sfdisk-field.awk $(ROOT_FS_VERITY) $(ROOT_FS_VERITY_ROOTHASH) $(ROOT_FS) >> ../../scripts/make-gpt.sh $@.tmp \ >> - build/rootfs.verity.superblock:verity:$$(../../scripts/format-uuid.sh "$$(dd if=build/rootfs.verity.roothash bs=32 skip=1 count=1 status=none)") \ >> - $(ROOT_FS):root:$$(../../scripts/format-uuid.sh "$$(head -c 32 build/rootfs.verity.roothash)") >> + "$$ROOT_FS_VERITY:verity:$$(../../scripts/format-uuid.sh "$$(dd "if=$$ROOT_FS_VERITY_ROOTHASH" bs=32 skip=1 count=1 status=none)")" \ > > Indentation got messed up here. > > Given rootfs has a well-defined output structure, maybe we could just > write $(ROOT_FS)/rootfs.verity.roothash, so we don't need to define lots > of different environment variables in each component that uses the > verity data. > > I think we should consistently use Make variable expansion rather than > shell variable expansion when we're using the variable in a Make > dependency line too, to avoid the possibility of them being different. Make expansion followed by shell expansion and just Make expansion aren't even consistent with each other. >> diff --git a/host/rootfs/Makefile b/host/rootfs/Makefile >> index f107ca44fcf1ad85af5788d87f6c772910d40072..d7764d9b796f1773b4bebd0d50eec52b9be29e42 100644 >> --- a/host/rootfs/Makefile >> +++ b/host/rootfs/Makefile >> @@ -6,7 +6,7 @@ >> include ../../lib/common.mk >> include file-list.mk >> >> -dest = build/rootfs.erofs >> +dest = build >> >> DIRS = \ >> dev \ >> @@ -46,15 +46,27 @@ FIFOS = etc/s6-linux-init/run-image/service/s6-svscan-log/fifo >> >> BUILD_FILES = build/etc/s6-rc >> >> -$(dest): ../../scripts/make-erofs.sh $(PACKAGES_FILE) $(FILES) $(BUILD_FILES) build/empty build/fifo file-list.mk >> - set -euo pipefail; \ >> +# This rule produces three files but Make only (portably) >> +# supports one output per rule. Instead of resorting to temporary >> +# files, a timestamp file is created as the last step. The actual >> +# outputs are produced as side-effects. >> +$(dest)/timestamp: ../../scripts/make-erofs.sh $(PACKAGES_FILE) $(FILES) $(BUILD_FILES) build/empty build/fifo file-list.mk $(dest) > > Semes a bit odd to install the timestamp in $(dest). > >> { \ >> cat $(PACKAGES_FILE) ;\ >> for file in $(FILES) $(LINKS); do printf '%s\n%s\n' $$file "$${file#image/}"; done ;\ >> for file in $(BUILD_FILES); do printf '%s\n%s\n' $$file $${file#build/}; done ;\ >> printf 'build/empty\n%s\n' $(DIRS) ;\ >> printf 'build/fifo\n%s\n' $(FIFOS) ;\ >> - } | ../../scripts/make-erofs.sh $@ >> + } | ../../scripts/make-erofs.sh $(dest)/rootfs >> + $(VERITYSETUP) format \ >> + --root-hash-file $(dest)/rootfs.verity.roothash \ >> + -- $(dest)/rootfs $(dest)/rootfs.verity.superblock >> + # Add trailing newline >> + echo >> $(dest)/rootfs.verity.roothash >> + touch -- $(dest)/timestamp > > I prefer smaller independent Make rules where possible. Can't the > verity data stay in a separate rule from make-erofs.sh? > > Either way, change deserves a copyright header IMO. :) > >> + >> +$(dest): >> + mkdir -p $(dest) > > Would rather this was inlined into every target in $(dest), because > directories can be a bit confusing to me as Make targets. "Does the > target just create the directory, or does it create the directory and > everything in it?" sort of thing. Will fix. >> build/fifo: >> mkdir -p build >> @@ -83,25 +95,10 @@ clean: >> rm -rf build >> .PHONY: clean >> >> -# veritysetup format produces two files, but Make only (portably) >> -# supports one output per rule, so we combine the two outputs then >> -# define two more rules to separate them again. >> -build/rootfs.verity: $(dest) >> - $(VERITYSETUP) format $(dest) build/rootfs.verity.superblock.tmp \ >> - | awk -F ':[[:blank:]]*' '$$1 == "Root hash" {print $$2; exit}' \ >> - > build/rootfs.verity.roothash.tmp >> - cat build/rootfs.verity.roothash.tmp build/rootfs.verity.superblock.tmp \ >> - > $@ >> - rm build/rootfs.verity.roothash.tmp build/rootfs.verity.superblock.tmp >> -build/rootfs.verity.roothash: build/rootfs.verity >> - head -n 1 build/rootfs.verity > $@ >> -build/rootfs.verity.superblock: build/rootfs.verity >> - tail -n +2 build/rootfs.verity > $@ >> - >> -build/live.img: ../../scripts/format-uuid.sh ../../scripts/make-gpt.sh ../../scripts/sfdisk-field.awk build/rootfs.verity.superblock build/rootfs.verity.roothash $(dest) >> +build/live.img: ../../scripts/format-uuid.sh ../../scripts/make-gpt.sh ../../scripts/sfdisk-field.awk $(dest)/timestamp >> ../../scripts/make-gpt.sh $@.tmp \ >> - build/rootfs.verity.superblock:verity:$$(../../scripts/format-uuid.sh "$$(dd if=build/rootfs.verity.roothash bs=32 skip=1 count=1 status=none)") \ >> - $(dest):root:$$(../../scripts/format-uuid.sh "$$(head -c 32 build/rootfs.verity.roothash)") >> + $(dest)/rootfs.verity.superblock:verity:$$(../../scripts/format-uuid.sh "$$(dd if=$(dest)/rootfs.verity.roothash bs=32 skip=1 count=1 status=none)") \ >> + $(dest)/rootfs:root:$$(../../scripts/format-uuid.sh "$$(head -c 32 $(dest)/rootfs.verity.roothash)") >> mv $@.tmp $@ >> >> debug: >> @@ -111,7 +108,7 @@ debug: >> $(VMLINUX) >> .PHONY: debug >> >> -run: build/live.img $(EXT_FS) build/rootfs.verity.roothash >> +run: build/live.img > > What happened to $(EXT_FS)? Since commit 12b64009d9cde56b5629a832086d2c2311908ebe ("host/initramfs/extfs.nix: remove") it has been unused. I deleted it while changing other stuff in this line. -- Sincerely, Demi Marie Obenour (she/her/hers)