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

[Demi Marie Obenour <demiobenour@gmail.com>]

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