Subject: Re: [PATCH 31/31] hook-list.h: add a generated list of hooks, like config-list.h

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Tue, 01 Jun 2021 03:00:04 +0200

On Fri, May 28 2021, Ævar Arnfjörð Bjarmason wrote:

> Resolve a long-standing TODO item in bugreport.c of there being no
> centralized listing of hooks, this fixes a bug with the bugreport
> listing only knowing about 1/4 of the p4 hooks. It didn't know about
> the "reference-transaction" hook either.
>
> We can now make sure this is kept up-to-date, as the hook.c library
> will die if asked to find a hook we don't know about yet. The only
> (undocumented) exception is the artificial "test-hook" used in our own
> test suite. Move some of the tests away from the "does-not-exist"
> pseudo-hook, and test for the new behavior.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <[email-masked]>
> ---
>  .gitignore           |  1 +
>  Makefile             | 14 +++++++++++---
>  builtin/bugreport.c  | 44 ++++++++------------------------------------
>  generate-hooklist.sh | 20 ++++++++++++++++++++
>  hook.c               | 22 ++++++++++++++++++++++
>  t/t1800-hook.sh      | 14 +++++++++++---
>  6 files changed, 73 insertions(+), 42 deletions(-)
>  create mode 100755 generate-hooklist.sh
>
> diff --git a/.gitignore b/.gitignore
> index de39dc9961b..66189ca3cdc 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -191,6 +191,7 @@
>  /gitweb/static/gitweb.min.*
>  /config-list.h
>  /command-list.h
> +/hook-list.h
>  *.tar.gz
>  *.dsc
>  *.deb
> diff --git a/Makefile b/Makefile
> index a6b71a0fbed..d0532f3c744 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -817,6 +817,7 @@ XDIFF_LIB = xdiff/lib.a
>  
>  GENERATED_H += command-list.h
>  GENERATED_H += config-list.h
> +GENERATED_H += hook-list.h
>  
>  LIB_H := $(sort $(patsubst ./%,%,$(shell git ls-files '*.h' ':!t/' ':!Documentation/' 2>/dev/null || \
>  	$(FIND) . \
> @@ -2207,7 +2208,9 @@ git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)
>  
>  help.sp help.s help.o: command-list.h
>  
> -builtin/help.sp builtin/help.s builtin/help.o: config-list.h GIT-PREFIX
> +hook.sp hook.s hook.o: hook-list.h
> +
> +builtin/help.sp builtin/help.s builtin/help.o: config-list.h hook-list.h GIT-PREFIX
>  builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \
>  	'-DGIT_HTML_PATH="$(htmldir_relative_SQ)"' \
>  	'-DGIT_MAN_PATH="$(mandir_relative_SQ)"' \
> @@ -2240,6 +2243,11 @@ command-list.h: $(wildcard Documentation/git*.txt)
>  		$(patsubst %,--exclude-program %,$(EXCLUDED_PROGRAMS)) \
>  		command-list.txt >$@+ && mv $@+ $@
>  
> +hook-list.h: generate-hooklist.sh
> +hook-list.h: Documentation/githooks.txt
> +	$(QUIET_GEN)$(SHELL_PATH) ./generate-hooklist.sh \
> +		>$@+ && mv $@+ $@
> +
>  SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\
>  	$(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\
>  	$(gitwebdir_SQ):$(PERL_PATH_SQ):$(SANE_TEXT_GREP):$(PAGER_ENV):\
> @@ -2890,7 +2898,7 @@ $(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE
>  .PHONY: sparse $(SP_OBJ)
>  sparse: $(SP_OBJ)
>  
> -EXCEPT_HDRS := command-list.h config-list.h unicode-width.h compat/% xdiff/%
> +EXCEPT_HDRS := command-list.h config-list.h hook-list.h unicode-width.h compat/% xdiff/%
>  ifndef GCRYPT_SHA256
>  	EXCEPT_HDRS += sha256/gcrypt.h
>  endif
> @@ -2912,7 +2920,7 @@ hdr-check: $(HCO)
>  style:
>  	git clang-format --style file --diff --extensions c,h
>  
> -check: config-list.h command-list.h
> +check: config-list.h command-list.h hook-list.h
>  	@if sparse; \
>  	then \
>  		echo >&2 "Use 'make sparse' instead"; \
> diff --git a/builtin/bugreport.c b/builtin/bugreport.c
> index 941c8d5e270..a7a1fcb8a7a 100644
> --- a/builtin/bugreport.c
> +++ b/builtin/bugreport.c
> @@ -4,6 +4,7 @@
>  #include "help.h"
>  #include "compat/compiler.h"
>  #include "hook.h"
> +#include "hook-list.h"
>  
>  
>  static void get_system_info(struct strbuf *sys_info)
> @@ -41,39 +42,7 @@ static void get_system_info(struct strbuf *sys_info)
>  
>  static void get_populated_hooks(struct strbuf *hook_info, int nongit)
>  {
> -	/*
> -	 * NEEDSWORK: Doesn't look like there is a list of all possible hooks;
> -	 * so below is a transcription of `git help hooks`. Later, this should
> -	 * be replaced with some programmatically generated list (generated from
> -	 * doc or else taken from some library which tells us about all the
> -	 * hooks)
> -	 */
> -	static const char *hook[] = {
> -		"applypatch-msg",
> -		"pre-applypatch",
> -		"post-applypatch",
> -		"pre-commit",
> -		"pre-merge-commit",
> -		"prepare-commit-msg",
> -		"commit-msg",
> -		"post-commit",
> -		"pre-rebase",
> -		"post-checkout",
> -		"post-merge",
> -		"pre-push",
> -		"pre-receive",
> -		"update",
> -		"post-receive",
> -		"post-update",
> -		"push-to-checkout",
> -		"pre-auto-gc",
> -		"post-rewrite",
> -		"sendemail-validate",
> -		"fsmonitor-watchman",
> -		"p4-pre-submit",
> -		"post-index-change",
> -	};
> -	int i;
> +	const char **p;
>  
>  	if (nongit) {
>  		strbuf_addstr(hook_info,
> @@ -81,9 +50,12 @@ static void get_populated_hooks(struct strbuf *hook_info, int nongit)
>  		return;
>  	}
>  
> -	for (i = 0; i < ARRAY_SIZE(hook); i++)
> -		if (hook_exists(hook[i]))
> -			strbuf_addf(hook_info, "%s\n", hook[i]);
> +	for (p = hook_name_list; *p; p++) {
> +		const char *hook = *p;
> +
> +		if (hook_exists(hook))
> +			strbuf_addf(hook_info, "%s\n", hook);
> +	}
>  }
>  
>  static const char * const bugreport_usage[] = {
> diff --git a/generate-hooklist.sh b/generate-hooklist.sh
> new file mode 100755
> index 00000000000..25a7207b276
> --- /dev/null
> +++ b/generate-hooklist.sh
> @@ -0,0 +1,20 @@
> +#!/bin/sh
> +
> +echo "/* Automatically generated by generate-hooklist.sh */"
> +
> +print_hook_list () {
> +	cat <<EOF
> +static const char *hook_name_list[] = {
> +EOF
> +	grep -C1 -h '^~~~' Documentation/githooks.txt |
> +	grep '^[a-z0-9][a-z0-9-]*$' |
> +	sort |
> +	sed 's/^.*$/	"&",/'
> +	cat <<EOF
> +	NULL,
> +};
> +EOF
> +}
> +

I found that grep -C isn't portable, so (pending any feedback on this
series) I've locally replaced it with:

diff --git a/generate-hooklist.sh b/generate-hooklist.sh
new file mode 100755
index 0000000000..5a3f7f849c
--- /dev/null
+++ b/generate-hooklist.sh
@@ -0,0 +1,24 @@
+#!/bin/sh
+
+echo "/* Automatically generated by generate-hooklist.sh */"
+
+print_hook_list () {
+	cat <<EOF
+static const char *hook_name_list[] = {
+EOF
+	perl -ne '
+		chomp;
+		@l[$.] = $_;
+		push @h => $l[$. - 1] if /^~~~+$/s;
+		END {
+			print qq[\t"$_",\n] for sort @h;
+		}
+	' <Documentation/githooks.txt
+	cat <<EOF
+	NULL,
+};
+EOF
+}
+
+echo
+print_hook_list