Re: [PATCH v3 4/4] Documentation: add lint-fsck-msgids

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Oct 26, 2022, 02:43 UTC
Message-ID: <221026.867d0ncncu.gmgdl@evledraar.gmail.com>
In-Reply-To: <20221025224224.2352979-5-gitster@pobox.com>

On Tue, Oct 25 2022, Junio C Hamano wrote:

> During the initial development of the fsck-msgids.txt feature, it
> has become apparent that it is very much error prone to make sure
> the description in the documentation file are sorted and correctly
> match what is in the fsck.h header file.

I have local fixes for the same issues in the list of advice in our docs, some of it's missing, wrong, out of date etc.

I tried to quickly adapt the generation script I had for that, which works nicely, and by line count much shorter than the lint :)

Having to exhaustively list every *.c file that uses fsck.h is a bit of a bother, but we have the same with the other generated *.h's, so it shouldn't be too bad.

And this way, if we get it wrong we get a compile error:
	
	$ git -P diff; make
	diff --git a/Documentation/fsck-msgids.txt b/Documentation/fsck-msgids.txt
	index 7af76ff99a5..f0b4308a6e7 100644
	--- a/Documentation/fsck-msgids.txt
	+++ b/Documentation/fsck-msgids.txt
	@@ -1,6 +1,3 @@
	-`badDate`::
	-       (ERROR) Invalid date format in an author/committer line.
	-
	 `badDateOverflow`::
	        (ERROR) Invalid date value in an author/committer line.
	 
	    CC fsck.o
	fsck.c:31:9: error: excess elements in array initializer [-Werror]
	   31 |         { NULL, NULL, NULL, -1 }
	      |         ^
	fsck.c:31:9: note: (near initialization for ‘msg_id_info’)
	fsck.c: In function ‘fsck_ident’:
	fsck.c:810:51: error: ‘FSCK_MSG_BAD_DATE’ undeclared (first use in this function); did you mean ‘FSCK_MSG_BAD_NAME’?
	  810 |                 return report(options, oid, type, FSCK_MSG_BAD_DATE, "invalid author/committer line - bad date");
	      |                                                   ^~~~~~~~~~~~~~~~~
	      |                                                   FSCK_MSG_BAD_NAME
	fsck.c:810:51: note: each undeclared identifier is reported only once for each function it appears in
	cc1: all warnings being treated as errors
	make: *** [Makefile:2617: fsck.o] Error 1

So, if you're interested:

 Makefile               | 12 ++++++++++++
 fsck.h                 |  8 +-------
 generate-fsckmsgids.sh | 42 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 55 insertions(+), 7 deletions(-)

diff --git a/Makefile b/Makefile
index 85f03c6aed1..21bbc3dfb9f 100644
--- a/Makefile
+++ b/Makefile
@@ -860,6 +860,7 @@ REFTABLE_TEST_LIB = reftable/libreftable_test.a
 GENERATED_H += command-list.h
 GENERATED_H += config-list.h
 GENERATED_H += hook-list.h
+GENERATED_H += fsck-msgids.h
 
 .PHONY: generated-hdrs
 generated-hdrs: $(GENERATED_H)
@@ -2289,6 +2290,14 @@ git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)
 	$(QUIET_LINK)$(CC) $(ALL_CFLAGS) -o $@ $(ALL_LDFLAGS) \
 		$(filter %.o,$^) $(LIBS)
 
+# Unfortunately our dependency management of generated headers used
+# from within other headers suck, so we'll need to list every user of
+# fsck.h here, but not too bad...
+FSCK_MSGIDS_H_BUILTINS = fsck index-pack mktag receive-pack unpack-objects
+$(foreach f,$(FSCK_MSGIDS_H_BUILTINS:%=builtin/%),$f.sp $f.s $f.o): fsck-msgids.h
+FSCK_MSGIDS_H_LIBS = fetch-pack fsck
+$(foreach f,$(FSCK_MSGIDS_H_LIBS),$f.sp $f.s $f.o): fsck-msgids.h
+
 help.sp help.s help.o: command-list.h
 builtin/bugreport.sp builtin/bugreport.s builtin/bugreport.o: hook-list.h
 
@@ -2333,6 +2342,9 @@ command-list.h: $(wildcard Documentation/git*.txt)
 hook-list.h: generate-hooklist.sh Documentation/githooks.txt
 	$(QUIET_GEN)$(SHELL_PATH) ./generate-hooklist.sh >$@
 
+fsck-msgids.h: generate-fsckmsgids.sh Documentation/fsck-msgids.txt
+	$(QUIET_GEN)$(SHELL_PATH) ./generate-fsckmsgids.sh >$@
+
 SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):\
 	$(localedir_SQ):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\
 	$(gitwebdir_SQ):$(PERL_PATH_SQ):$(PAGER_ENV):\
diff --git a/fsck.h b/fsck.h
index 6fbce68ad67..1a9d68482ea 100644
--- a/fsck.h
+++ b/fsck.h
@@ -2,6 +2,7 @@
 #define GIT_FSCK_H
 
 #include "oidset.h"
+#include "fsck-msgids.h"
 
 enum fsck_msg_type {
 	/* for internal use only */
@@ -79,13 +80,6 @@ enum fsck_msg_type {
 	/* ignored (elevated when requested) */ \
 	FUNC(EXTRA_HEADER_ENTRY, IGNORE)
 
-#define MSG_ID(id, msg_type) FSCK_MSG_##id,
-enum fsck_msg_id {
-	FOREACH_FSCK_MSG_ID(MSG_ID)
-	FSCK_MSG_MAX
-};
-#undef MSG_ID
-
 struct fsck_options;
 struct object;
 
diff --git a/generate-fsckmsgids.sh b/generate-fsckmsgids.sh
new file mode 100755
index 00000000000..bbf236159aa
--- /dev/null
+++ b/generate-fsckmsgids.sh
@@ -0,0 +1,42 @@
+#!/bin/sh
+
+HT='	'
+
+fsck_list () {
+	sed -n \
+		-e "/::$/ {
+			s/::\$//;
+			s/^\`//;
+			s/\`$//;
+			p;
+		}" \
+	    <Documentation/fsck-msgids.txt
+}
+
+txt2label () {
+	sed \
+		-e 's/\([^_]\)\([[:upper:]]\)/\1_\2/g' \
+		-e 's/^/FSCK_MSG_/' |
+		tr '[:lower:]' '[:upper:]'
+}
+
+fsck_labels () {
+	fsck_list |
+	txt2label
+}
+
+listify () {
+	sed \
+		-e "2,\$s/^/$HT/" \
+		-e 's/$/,/'
+}
+
+cat <<EOF
+/* Automatically generated by generate-fsck-msgids.sh */
+
+enum fsck_msg_id {
+	/* Auto-generated from Documentation/fsck-msgids.txt */
+	$(fsck_labels | listify)
+	FSCK_MSG_MAX
+};
+EOF