On 9/27/25 04:19, Alyssa Ross wrote: > Demi Marie Obenour 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 >> --- >> 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 > > 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 > > I wouldn't include this one. See above. >> +# SPDX-FileCopyrightText: 2025 Demi Marie Obenour >> +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 \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)