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

[Alyssa Ross <hi@alyssa.is>]

[-- 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 --]