Re: [PATCH v5] Generate file lists from a script

Demi Marie Obenour
2025-09-27T08:42:14Z

[-- Attachment #1.1.1: Type: text/plain, Size: 7397 bytes --]

On 9/27/25 04:19, Alyssa Ross wrote:
> Demi Marie Obenour <demiobenour@gmail.com> writes:
>
>> Right now, the makefiles in host/rootfs, vm/sys/net, and img/app have
>> manually-maintained lists of files and symlinks. These duplicate the
>> information in the git repository and can easily get out of sync or
>> cause unnecessary merge conflicts. Fix all of these issues by having
>> the git repository be the source of truth, and using a script to
>> generate the file lists. Developers can regenerate the lists before
>> every commit, or even add a git hook to do that.
>>
>> Signed-off-by: Demi Marie Obenour <demiobenour@gmail.com>
>> ---
>> This actually reduces the amount of code that has to be written by hand.
>> ---
>
> This is so close. Was almost at the point where I could have just fixed
> it up myself and committed it, but there's one thing I want to check
> with you.

Thank you for being careful!

>> Changes in v5:
>> - Use 'print ""' instead of 'print' in awk to print a newline. 'print'
>> with no arguments implicitly prints $0 instead. This caused the
>> generated makefiles to be incorrect.
>> - Use S6_RC_FILES instead of VM_S6_RC_FILES in vm/sys/net/Makefile and
>> img/app/Makefile. This prevented the image from being built.
>> - Do not check for git repository being in a directory with a name
>> ending in a newline.
>> - Use shell redirection instead of awk redirection.
>> - Do not include trailing DONE line in input to awk.
>> - Link to v4: https://lore.kernel.org/r/20250921-genfiles-v4-1-4375bda78707@gmail.com
>>
>> Changes in v4:
>> - Use /bin/sh instead of bash.
>> - Do not assume that negated awk character classes match all bytes.
>> - Do not check the mode of license files.
>> - Use implicit awk variable initialization.
>> - Use 'git rev-parse --show-toplevel' to find the repository root.
>> - Remove wrongly added copyright header.
>> - Improve documentation.
>> - Remove git hooks.
>> - Add missing copyright header.
>> - Avoid non-portable /usr/bin/env -S.
>> - Avoid assuming that awk is GNU awk.
>> - Avoid non-portable awk -E.
>> - Do not check for git bugs.
>> - Fix link in v3 changelog.
>> - Link to v3: https://spectrum-os.org/lists/archives/spectrum-devel/20250920-genfiles-v3-1-d6c2b6767b42@gmail.com
>>
>> Changes in v3:
>> - Only include the file list generator. Move the rest to separate patch
>> series.
>> - Remove the update-file-list make targets from img/app/Makefile and
>> vm/sys/net/Makefile.
>> - Link to v2: https://spectrum-os.org/lists/archives/spectrum-devel/20250910-genfiles-v2-0-37ebe07a3cdc@gmail.com
>>
>> Changes in v2:
>> - Drop the last patch (switching to /etc/s6-rc/compiled) as it is
>> controversial and should be reviewed separately.
>> - Add missing copyright notices.
>> - Use a wrapper shell script to make the awk code easier to read.
>> - Improve documentation.
>> - Add helper scripts for use in git hooks and rebasing.
>> - Link to v1: https://spectrum-os.org/lists/archives/spectrum-devel/20250903-genfiles-v1-0-cc993fcb1e4c@gmail.com/
>> ---
>> Documentation/development/built-in-vms.adoc | 7 ++
>> host/rootfs/Makefile | 102 +---------------------------
>> host/rootfs/file-list.mk | 99 +++++++++++++++++++++++++++
>> img/app/Makefile | 82 ++++------------------
>> img/app/file-list.mk | 65 ++++++++++++++++++
>> scripts/genfiles.awk | 77 +++++++++++++++++++++
>> scripts/genfiles.sh | 24 +++++++
>> vm/sys/net/Makefile | 52 +++-----------
>> vm/sys/net/file-list.mk | 42 ++++++++++++
>> 9 files changed, 337 insertions(+), 213 deletions(-)
>>
>> diff --git a/host/rootfs/file-list.mk b/host/rootfs/file-list.mk
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..58cda39f85f720ab46f025bc72f1a98f108f1c25
>> --- /dev/null
>> +++ b/host/rootfs/file-list.mk
>> @@ -0,0 +1,99 @@
>> +# SPDX-License-Identifier: EUPL-1.2+
>> +# SPDX-FileCopyrightText: 2021-2024 Alyssa Ross <hi@alyssa.is>
>
> Only 2021. The only thing I think is even arguably copyrightable is the
> comment. And following that principle let's actually put your copyright
> in here too.

I'd rather just drop the comment. It can be phrased better anyway,
and it is much cleaner for the generated file to only contain data.

>> +# Generated by scripts/genfile.sh. Any changes will be overwritten.
>
> Let's have a blank line before and after this comment, for readability.
>
>> diff --git a/scripts/genfiles.awk b/scripts/genfiles.awk
>> new file mode 100644
>> index 0000000000000000000000000000000000000000..935eebbdc7f0e2aa07b0b6439ab53d1f50940929
>> --- /dev/null
>> +++ b/scripts/genfiles.awk
>> @@ -0,0 +1,77 @@
>> +# SPDX-License-Identifier: EUPL-1.2+
>> +# SPDX-FileCopyrightText: 2021-2024 Alyssa Ross <hi@alyssa.is>
>
> I wouldn't include this one.
See above.

>> +# SPDX-FileCopyrightText: 2025 Demi Marie Obenour <demiobenour@gmail.com>
>> +BEGIN {
>> + RS = "\n";
>> + FS = "\t";
>> + modes["120000"] = "symlink";
>> + modes["100644"] = "regular";
>> + modes["100755"] = "regular";
>> +}
>> +
>> +function fail(msg) {
>> + exit_code = 1;
>> + print msg > "/dev/stderr";
>> + exit 1;
>> +}
>> +
>> +# Extract data from built-in variables.
>> +{
>> + filename = $2;
>> + raw_mode = $1;
>> + # awk autocreates empty string entries if the key is invalid,
>> + # but the code exits in this case so that is okay.
>> + mode = modes[raw_mode];
>> +}
>> +
>> +filename !~ /^[[:alnum:]_./-]+$/ {
>> + fail("filename '" filename "' has forbidden characters");
>> +}
>> +
>> +# Skip license files
>> +/\.license$/ { next }
>> +
>> +filename ~ /^image\/etc\/s6-rc\// {
>> + if (mode != "regular") {
>> + fail("s6-rc-compile input '" filename "' isn't a regular file");
>> + }
>> + rc_files[rc_count++] = filename;
>> + next;
>> +}
>> +
>> +mode == "symlink" {
>> + symlinks[symlink_count++] = filename;
>> + next;
>> +}
>> +
>> +mode == "regular" {
>> + files[file_count++] = filename;
>> + next;
>> +}
>> +
>> +{ fail("File '" filename "' is not regular file or symlink (mode " raw_mode ")"); }
>> +
>> +END {
>> + if (exit_code) {
>> + exit exit_code;
>> + }
>
> I don't think this ever happens?
>
>> + printf ("# SPDX-License-Identifier: EUPL-1.2+\n" \
>> +"# SPDX-FileCopyrightText: 2021-2024 Alyssa Ross <hi@alyssa.is>\n" \
>> +"# Generated by scripts/genfile.sh. Any changes will be overwritten.\n" \
>> +"FILES =");
>> + for (array_index = 0; array_index < file_count; array_index += 1) {
>> + printf " \\\n\t%s", files[array_index];
>> + }
>> + printf ("\n\n" \
>> +"# These are separate because they need to be included, but putting\n" \
>> +"# them as make dependencies would confuse make.\n" \
>> +"LINKS =");
>> + for (array_index = 0; array_index < symlink_count; array_index += 1) {
>> + printf " \\\n\t%s", symlinks[array_index];
>> + }
>> + printf "\n\nS6_RC_FILES =";
>> + for (array_index = 0; array_index < rc_count; array_index += 1) {
>> + printf " \\\n\t%s", rc_files[array_index];
>> + }
>> + print "";
>> +}

--
Sincerely,
Demi Marie Obenour (she/her/hers)

[-- Attachment #1.1.2: OpenPGP public key --]
[-- Type: application/pgp-keys, Size: 7253 bytes --]

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 833 bytes --]