Re: [PATCH 3/4] Common make rules for building erofs images

Alyssa Ross
2025-09-09T14:56:22Z

[-- Attachment #1: Type: text/plain, Size: 2476 bytes --]

Demi Marie Obenour <demiobenour@gmail.com> writes:

> On 9/8/25 06:01, Alyssa Ross wrote:
>> Demi Marie Obenour <demiobenour@gmail.com> writes:
>>
>>> Instead of duplicating the logic in three different places, write common
>>> code for all three makefiles to use instead. This will make future
>>> changes much easier.
>>>
>>> Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
>>
>> Makes sense, and we could do this now for the s6-rc directory without
>> having to wait for the generated file list stuff. Does need to be
>> updated for my review comments from the previous patch though (stick to
>> POSIX).
>
> If we are going to stick with POSIX, I think it is best to test with both
> GNU make and BSD make. Otherwise, GNUisms are bound to creep in.

I wish there was something like shellcheck for Makefiles, but I'm not
aware of one. If you can find a good way to ensure it (ideally without
building everything twice), I'm all for it.

>>> diff --git a/lib/erofs.mk b/lib/erofs.mk
>>> new file mode 100644
>>> index 0000000000000000000000000000000000000000..b3fc112f5e793725977cd8c4b2e71d6ed8d888c4
>>> --- /dev/null
>>> +++ b/lib/erofs.mk
>>> @@ -0,0 +1,51 @@
>>> +override basedir ::= $(dir $(lastword $(MAKEFILE_LIST)))/..
>
> This is one part that is not possible in POSIX make. Without
> this, there is no way for the included makefile to know where
> its wrapper scripts are.

Maybe we can just have each Makefile define a "root" variable that
points to the root of the source tree? Not very onerous if it works.

>>> +override BUILD_FILES ::= $(BUILD_FILES) build/etc/s6-rc
>>> +# No override here so that it can be overridden in host/rootfs/default.nix.
>>> +dest ::= build/rootfs.erofs
>>> +
>>> +all:
>>> +.PHONY: all
>>> +$(dest): $(basedir)/scripts/make-erofs.sh $(PACKAGES_FILE) $(addprefix image/,$(FILES)) $(BUILD_FILES) build/empty build/fifo file-list.mk
>>> + set -euo pipefail; ( \
>>> + cat $(PACKAGES_FILE) ;\
>>> + for file in $(FILES) $(LINKS); do printf 'image/%s\n%s\n' "$$file" "$$file"; done ;\
>>> + for file in $(BUILD_FILES); do printf '%s\n%s\n' "$$file" "$${file#build/}"; done ;\
>>> + $(and $(DIRS),printf 'build/empty\n%s\n' $(DIRS);)\
>>> + $(and $(FIFOS),printf 'build/fifo\n%s\n' $(FIFOS);)\
>
> This also has no analog in POSIX make, though it might be possible
> to emulate it with some shell hacks.

Can be a for loop like above I think?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 227 bytes --]