On 11/25/25 08:02, Alyssa Ross wrote: > Demi Marie Obenour writes: > >> Generate 4 partition UUIDs instead of just 2. Port >> scripts/format-uuid.sh to awk to make this much easier. > > Would have been nice to see the awk port first with unchanged behaviour, > so it was then easier to see what behaviour was actually getting changed > here. > >> Signed-off-by: Demi Marie Obenour >> --- >> Changes since v2: >> - Split into separate commit. >> --- >> host/initramfs/Makefile | 8 +++++--- >> host/rootfs/Makefile | 6 ++++-- >> release/live/Makefile | 8 +++++--- >> release/live/default.nix | 2 +- >> scripts/format-uuid.awk | 35 +++++++++++++++++++++++++++++++++++ >> scripts/format-uuid.sh | 19 ------------------- >> 6 files changed, 50 insertions(+), 28 deletions(-) > > No blockers in here, but there are improvements I'd like to see made at > some point, even if that's after this has been applied. > >> diff --git a/host/initramfs/Makefile b/host/initramfs/Makefile >> index fd8cbb6c3e775ed27d0a524bf167cb4d3940d799..27a26b46a8110d35ee02a63b12931d6b9c2742e5 100644 >> --- a/host/initramfs/Makefile >> +++ b/host/initramfs/Makefile >> @@ -35,10 +35,12 @@ build/mountpoints: >> cd build/mountpoints && mkdir -p $(MOUNTPOINTS) >> find build/mountpoints -mindepth 1 -exec touch -d @0 {} ';' >> >> -build/live.img: ../../scripts/format-uuid.sh ../../scripts/make-gpt.sh ../../scripts/sfdisk-field.awk $(ROOT_FS_IMAGES) >> +build/live.img: ../../scripts/format-uuid.awk ../../scripts/make-gpt.sh ../../scripts/sfdisk-field.awk build/boot.fat $(ROOT_FS_IMAGES) >> + uuids=$$(awk -f ../../scripts/format-uuid.awk < $(ROOT_FS_VERITY_ROOTHASH)) && \ >> + set -euo pipefail -- $$uuids && \ > > Nice trick — I like it. There's no need to set -eo pipefail on Make > commands that don't use ; or | though. If we did that consistently our > Makefiles would be unreadable because it'd be hard to see past all the > sets to the actual functionality, and seeing this here when it's not set > on every command makes me wonder what's special about this one, which > turns out to be nothing. Will change in v5. >> bash ../../scripts/make-gpt.sh $@.tmp \ >> - $(ROOT_FS_VERITY):verity:$$(../../scripts/format-uuid.sh "$$(dd if=$(ROOT_FS_VERITY_ROOTHASH) bs=32 skip=1 count=1 status=none)") \ >> - $(ROOT_FS):root:$$(../../scripts/format-uuid.sh "$$(head -c 32 $(ROOT_FS_VERITY_ROOTHASH))") >> + $(ROOT_FS_VERITY):verity:$$3 \ >> + $(ROOT_FS):root:$$1 >> mv $@.tmp $@ >> >> build/loop.tar: build/live.img >> diff --git a/host/rootfs/Makefile b/host/rootfs/Makefile >> index fcfbc3e437fdb108252ba77d4d4e8f4f636ffd78..f02bb76371f000e3f65bb7c2a7f217d437845481 100644 >> --- a/host/rootfs/Makefile >> +++ b/host/rootfs/Makefile >> @@ -90,9 +90,11 @@ clean: >> .PHONY: clean >> >> build/live.img: ../../scripts/format-uuid.sh ../../scripts/make-gpt.sh ../../scripts/sfdisk-field.awk $(ROOT_FS_DIR)/verity-timestamp $(ROOT_FS) >> + uuids=$$(awk -f ../../scripts/format-uuid.awk < $(ROOT_FS_VERITY_ROOTHASH)) && \ >> + set -euo pipefail -- $$uuids && \ >> bash ../../scripts/make-gpt.sh $@.tmp \ >> - $(ROOT_FS)/rootfs.verity.superblock:verity:$$(../../scripts/format-uuid.sh "$$(dd if=$(ROOT_FS_VERITY_ROOTHASH) bs=32 skip=1 count=1 status=none)") \ >> - $(ROOT_FS)/rootfs:root:$$(../../scripts/format-uuid.sh "$$(head -c 32 $(ROOT_FS_VERITY_ROOTHASH)") >> + $(ROOT_FS_VERITY):verity:$$3 \ >> + $(ROOT_FS):root:$$1 >> mv $@.tmp $@ >> >> debug: >> diff --git a/release/live/Makefile b/release/live/Makefile >> index a79947c57d562677760bc669c66320953a2b0d2d..78361a48512a37514ba0e57e0cc8b0ec3a71664b 100644 >> --- a/release/live/Makefile >> +++ b/release/live/Makefile >> @@ -9,11 +9,13 @@ DTBS ?= build/empty >> >> dest = build/live.img >> >> -$(dest): ../../scripts/format-uuid.sh ../../scripts/make-gpt.sh ../../scripts/sfdisk-field.awk build/boot.fat $(ROOT_FS_IMAGES) >> +$(dest): ../../scripts/format-uuid.awk ../../scripts/make-gpt.sh ../../scripts/sfdisk-field.awk build/boot.fat $(ROOT_FS_IMAGES) >> + uuids=$$(awk -f ../../scripts/format-uuid.awk < $(ROOT_FS_VERITY_ROOTHASH)) && \ >> + set -euo pipefail -- $$uuids && \ >> bash ../../scripts/make-gpt.sh $@.tmp \ >> build/boot.fat:c12a7328-f81f-11d2-ba4b-00a0c93ec93b \ >> - $(ROOT_FS_VERITY):verity:$$(../../scripts/format-uuid.sh "$$(dd if=$(ROOT_FS_VERITY_ROOTHASH) bs=32 skip=1 count=1 status=none)") \ >> - $(ROOT_FS):root:$$(../../scripts/format-uuid.sh "$$(head -c 32 $(ROOT_FS_VERITY_ROOTHASH))") >> + $(ROOT_FS_VERITY):verity:$$3 \ >> + $(ROOT_FS):root:$$1 >> mv $@.tmp $@ >> >> build/empty: >> diff --git a/release/live/default.nix b/release/live/default.nix >> index ac2d7a55fd4fe0c02108309ecea20e368000af0d..98cb4862e239e3ad9ddbd7b5ace5716f57df683b 100644 >> --- a/release/live/default.nix >> +++ b/release/live/default.nix >> @@ -29,7 +29,7 @@ stdenv.mkDerivation { >> fileset = lib.fileset.intersection src (lib.fileset.unions [ >> ./. >> ../../lib/common.mk >> - ../../scripts/format-uuid.sh >> + ../../scripts/format-uuid.awk >> ../../scripts/make-gpt.sh >> ../../scripts/sfdisk-field.awk >> ]); >> diff --git a/scripts/format-uuid.awk b/scripts/format-uuid.awk >> new file mode 100644 >> index 0000000000000000000000000000000000000000..a5349d68a4d29be5f750650236420c9b5a7257eb >> --- /dev/null >> +++ b/scripts/format-uuid.awk >> @@ -0,0 +1,35 @@ >> +# SPDX-License-Identifier: EUPL-1.2+ >> +# SPDX-FileCopyrightText: 2025 Demi Marie Obenour >> +function format_uuid(arg) { >> + if (arg in found_so_far) { >> + fail("Duplicate UUID, try changing the image (by even 1 bit)"); >> + } > > This doesn't really feel like the right place for this check? If > duplicate UUIDs are broken that should be detected when assembling the > partition table. Will drop in v5. >> + found_so_far[arg] = 1; >> + print (substr(arg, 1, 8) "-" \ >> + substr(arg, 9, 4) "-" \ >> + substr(arg, 13, 4) "-" \ >> + substr(arg, 17, 4) "-" \ >> + substr(arg, 21, 12)); >> +} >> + >> +function fail(msg) { >> + print msg > "/dev/stderr"; >> + exit 1; >> +} >> + >> +BEGIN { >> + FS = ""; >> + RS = "\n"; > > RS is \n by default. Will fix in v5. >> + if ((getline) != 1) > > No need for the inner parens. Will fix in v5. >> + fail("Empty input file"); >> + roothash = $0; >> + if (roothash !~ /^[a-f0-9]{64}$/) >> + fail("Invalid root hash"); >> + if (getline) >> + fail("Junk after root hash"); > > This will only ever be used by our build system. It'll be readily > apparent if it's providing junk even without these validations, and > without them the script would be shorter and easier to fit in my brain. Will fix in v5. >> + found_so_far[""] = ""; > > What does this do? Won't this just cause the script to falsely claim an > empty string is a duplicate UUID if it ever ends up being passed to > format_uuid()? That can't happen :). >> + for (i = 1; i != 49; i += 16) { >> + format_uuid(substr($0, i, 32)); >> + } > > Either unrolling the loop, or using a modulo inside the loop and getting > rid of the format_uuid() call afterwards, would have helped me figure > out what this does a lot quicker, but I see now that we're generating > the two UUIDs we had before, and then two additional UUIDs offset into > the string. I did this because it is easy to implement. > I guess this is something to do with having multiple copies of the OS > installed, but I wish I understood how the offset ones will be used > without having to go hunting in later patches and come back here > afterwards. > >> + format_uuid(substr($0, 49, 16) substr($0, 1, 16)); >> +} These GUIDs aren't booted ever. They are used for the blank backup partitions. They just need to be deterministic and not collide with anything a user might use. The verity hash was a convenient source for this. A fixed GUID would work too. -- Sincerely, Demi Marie Obenour (she/her/hers)