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

Alyssa Ross
2025-09-27T08:19:55Z

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

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.

> 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.

> +# 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.

> +# 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 "";
> +}

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