{"thread":{"id":"56573","subject":"[PATCH 0/8] Makefile: generate a hook-list.h, prep for config-based-hooks","startedAt":"2021-09-23T10:31:14Z","lastAt":"2021-11-17T08:39:16Z","messageCount":39,"participants":["Ævar Arnfjörð Bjarmason","Phillip Wood","Junio C Hamano","René Scharfe","Mike Hommey"],"isPatch":true,"patchVersion":1,"patchTotal":8},"messages":[{"id":"436841","messageId":"cover-0.8-00000000000-20210923T095326Z-avarab@gmail.com","threadId":"56573","inReplyTo":null,"subject":"[PATCH 0/8] Makefile: generate a hook-list.h, prep for config-based-hooks","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-09-23T10:29:55Z","receivedAt":"2021-09-23T10:31:14Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This series is an incremental restart of the es/config-based-hooks and\nab/config-based-hooks-base topics, which should both be ejected in\nfavor of this.\n\nThose topics have been lingering for a long time. The v5 of\nab/config-based-hooks-base[1] was re-rolled on September 2nd, and then\non September 9th I re-rolled a v4 of Emily's on-top topic[2] to solve\nconflicts with the \"base\" one as other things in \"seen\" were colliding\nwith its v4 + Emily's older version.\n\nBut in the end the sum total of master..es/config-based-hooks is 43\ncommits and a rather daunting \"44 files changed, 1870 insertions(+),\n611 deletions(-)\" diffstat. I think it's fair to say that anything\napproaching a full picture of these patches only exists in our\nrespective wetwares, and wider reviewer attention is clearly needed.\n\nThat much larger topic also has a conflict Junio's been solving, and\nby its presence has surely been holding up the progression of various\nother submitted and unsubmitted topics.\n\nSo this is step #1 in an (at least) 5 step plan outlined in [3] (start\nreading at \"It seems due to\") to get this greater series into git in a\nmore piecemeal fashion.\n\nHere we're only setting up some build system changes needed for the\neventual master..ab/config-based-hooks-base topic, the range-diff is\nagainst the relevant part of[1]. These changes are able to stand on\ntheir own.\n\nThe only caveat to that is that we end up with a hook.[ch] with just\ntwo functions, which is usually not the point at which we split out\nnew headers. As noted in the commit messages we expect those headers\nto get much larger with the rest of ab/config-based-hooks-base.\n\nI intentionally split this off from the much larger thread starting at\n[4] and pruned the CC list. If anyone feels left out or is interested\na review of this is a most welcome way to be re-added :)\n\nChanges since [1]:\n\n * Resolved a conflict with advice.[ch]-related changes\n * A new commit to skip the \"mv $@+ $@\" dance for\n   $(GENERATED_H). These patches originally pre-dated our use of\n   \".DELETE_ON_ERROR\" in the Makefile.\n * There was a \"while we're at it\" change to a comment in\n   run-command.h as it was being moved, now this is a purely move-only\n   change. I'll either re-incorporate that minor change in re-rolls of\n   ab/config-based-hooks-base, or just drop it.\n\n1. https://lore.kernel.org/git/cover-v5-00.36-00000000000-20210902T125110Z-avarab@gmail.com/#t\n2. https://lore.kernel.org/git/cover-v4-0.5-00000000000-20210909T122802Z-avarab@gmail.com/\n3. https://lore.kernel.org/git/875yut8nns.fsf@evledraar.gmail.com/\n4. https://lore.kernel.org/git/20210311021037.3001235-1-emilyshaffer@google.com/#t\n\nEmily Shaffer (1):\n  hook.c: add a hook_exists() wrapper and use it in bugreport.c\n\nÆvar Arnfjörð Bjarmason (7):\n  Makefile: mark \"check\" target as .PHONY\n  Makefile: stop hardcoding {command,config}-list.h\n  Makefile: don't perform \"mv $@+ $@\" dance for $(GENERATED_H)\n  Makefile: remove an out-of-date comment\n  hook.[ch]: move find_hook() from run-command.c to hook.c\n  hook.c users: use \"hook_exists()\" instead of \"find_hook()\"\n  hook-list.h: add a generated list of hooks, like config-list.h\n\n .gitignore                          |  1 +\n Makefile                            | 29 ++++++++++--------\n builtin/am.c                        |  1 +\n builtin/bugreport.c                 | 46 ++++++-----------------------\n builtin/commit.c                    |  3 +-\n builtin/merge.c                     |  3 +-\n builtin/receive-pack.c              |  3 +-\n builtin/worktree.c                  |  1 +\n compat/vcbuild/README               |  2 +-\n config.mak.uname                    |  6 ++--\n contrib/buildsystems/CMakeLists.txt |  7 +++++\n generate-hooklist.sh                | 18 +++++++++++\n hook.c                              | 42 ++++++++++++++++++++++++++\n hook.h                              | 16 ++++++++++\n refs.c                              |  1 +\n run-command.c                       | 35 +---------------------\n run-command.h                       |  7 -----\n sequencer.c                         |  3 +-\n transport.c                         |  1 +\n 19 files changed, 126 insertions(+), 99 deletions(-)\n create mode 100755 generate-hooklist.sh\n create mode 100644 hook.c\n create mode 100644 hook.h\n\nRange-diff:\n1:  ac419613fdc = 1:  91d0cd09c8e Makefile: mark \"check\" target as .PHONY\n2:  a161b7f0a5c = 2:  804795771c6 Makefile: stop hardcoding {command,config}-list.h\n-:  ----------- > 3:  010701fd784 Makefile: don't perform \"mv $@+ $@\" dance for $(GENERATED_H)\n3:  ffef1d3257e = 4:  b3cce74d259 Makefile: remove an out-of-date comment\n4:  545e16c6f04 ! 5:  7dd874d50ec hook.[ch]: move find_hook() from run-command.c to hook.c\n    @@ hook.c (new)\n     +\t\t\terr = errno;\n     +#endif\n     +\n    -+\t\tif (err == EACCES && advice_ignored_hook) {\n    ++\t\tif (err == EACCES && advice_enabled(ADVICE_IGNORED_HOOK)) {\n     +\t\t\tstatic struct string_list advise_given = STRING_LIST_INIT_DUP;\n     +\n     +\t\t\tif (!string_list_lookup(&advise_given, name)) {\n    @@ hook.h (new)\n     +#ifndef HOOK_H\n     +#define HOOK_H\n     +\n    -+/**\n    ++/*\n     + * Returns the path to the hook file, or NULL if the hook is missing\n     + * or disabled. Note that this points to static storage that will be\n    -+ * overwritten by further calls to find_hook().\n    ++ * overwritten by further calls to find_hook and run_hook_*.\n     + */\n     +const char *find_hook(const char *name);\n     +\n    @@ refs.c\n     \n      ## run-command.c ##\n     @@\n    - #include \"string-list.h\"\n      #include \"quote.h\"\n      #include \"config.h\"\n    + #include \"packfile.h\"\n     +#include \"hook.h\"\n      \n      void child_process_init(struct child_process *child)\n    @@ run-command.c: int async_with_fork(void)\n     -\t\t\terr = errno;\n     -#endif\n     -\n    --\t\tif (err == EACCES && advice_ignored_hook) {\n    +-\t\tif (err == EACCES && advice_enabled(ADVICE_IGNORED_HOOK)) {\n     -\t\t\tstatic struct string_list advise_given = STRING_LIST_INIT_DUP;\n     -\n     -\t\t\tif (!string_list_lookup(&advise_given, name)) {\n5:  a9bc4519e9a = 6:  db8893afee6 hook.c: add a hook_exists() wrapper and use it in bugreport.c\n6:  e99ec2e6f8f = 7:  b61130dee5b hook.c users: use \"hook_exists()\" instead of \"find_hook()\"\n7:  2ffb2332c8a ! 8:  80aae4d5c13 hook-list.h: add a generated list of hooks, like config-list.h\n    @@ Makefile: git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)\n      \t'-DGIT_MAN_PATH=\"$(mandir_relative_SQ)\"' \\\n     @@ Makefile: command-list.h: $(wildcard Documentation/git*.txt)\n      \t\t$(patsubst %,--exclude-program %,$(EXCLUDED_PROGRAMS)) \\\n    - \t\tcommand-list.txt >$@+ && mv $@+ $@\n    + \t\tcommand-list.txt >$@\n      \n     +hook-list.h: generate-hooklist.sh Documentation/githooks.txt\n    -+\t$(QUIET_GEN)$(SHELL_PATH) ./generate-hooklist.sh \\\n    -+\t\t>$@+ && mv $@+ $@\n    ++\t$(QUIET_GEN)$(SHELL_PATH) ./generate-hooklist.sh >$@\n     +\n      SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\\\n      \t$(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\\\n-- \n2.33.0.1229.g0a86d28df49\n\n"},{"id":"436842","messageId":"patch-1.8-91d0cd09c8e-20210923T095326Z-avarab@gmail.com","threadId":"56573","inReplyTo":"cover-0.8-00000000000-20210923T095326Z-avarab@gmail.com","subject":"[PATCH 1/8] Makefile: mark \"check\" target as .PHONY","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-09-23T10:29:56Z","receivedAt":"2021-09-23T10:31:15Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a bug in 44c9e8594e (Fix up header file dependencies and add\nsparse checking rules, 2005-07-03), we never marked the phony \"check\"\ntarget as such.\n\nPerhaps we should just remove it, since as of a combination of\n912f9980d2 (Makefile: help people who run 'make check' by mistake,\n2008-11-11) 0bcd9ae85d (sparse: Fix errors due to missing\ntarget-specific variables, 2011-04-21) we've been suggesting the user\nrun \"make sparse\" directly.\n\nBut under that mode it still does something, as well as directing the\nuser to run \"make test\" under non-sparse. So let's punt that and\nnarrowly fix the PHONY bug.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n Makefile | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/Makefile b/Makefile\nindex 9df565f27bb..e85a8e07d3e 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2932,6 +2932,7 @@ hdr-check: $(HCO)\n style:\n \tgit clang-format --style file --diff --extensions c,h\n \n+.PHONY: check\n check: config-list.h command-list.h\n \t@if sparse; \\\n \tthen \\\n-- \n2.33.0.1229.g0a86d28df49\n\n"},{"id":"436843","messageId":"patch-2.8-804795771c6-20210923T095326Z-avarab@gmail.com","threadId":"56573","inReplyTo":"cover-0.8-00000000000-20210923T095326Z-avarab@gmail.com","subject":"[PATCH 2/8] Makefile: stop hardcoding {command,config}-list.h","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-09-23T10:29:57Z","receivedAt":"2021-09-23T10:31:18Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change various places that hardcode the names of these two files to\nrefer to either $(GENERATED_H), or to a new generated-hdrs\ntarget. That target is consistent with the *-objs targets I recently\nadded in 029bac01a8 (Makefile: add {program,xdiff,test,git,fuzz}-objs\n& objects targets, 2021-02-23).\n\nA subsequent commit will add a new generated hook-list.h. By doing\nthis refactoring we'll only need to add the new file to the\nGENERATED_H variable, not EXCEPT_HDRS, the vcbuild/README etc.\n\nHardcoding command-list.h there seems to have been a case of\ncopy/paste programming in 976aaedca0 (msvc: add a Makefile target to\npre-generate the Visual Studio solution, 2019-07-29). The\nconfig-list.h was added later in 709df95b78 (help: move\nlist_config_help to builtin/help, 2020-04-16).\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n Makefile              | 6 ++++--\n compat/vcbuild/README | 2 +-\n config.mak.uname      | 6 +++---\n 3 files changed, 8 insertions(+), 6 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex e85a8e07d3e..eb5780264a4 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -817,6 +817,8 @@ XDIFF_LIB = xdiff/lib.a\n \n GENERATED_H += command-list.h\n GENERATED_H += config-list.h\n+.PHONY: generated-hdrs\n+generated-hdrs: $(GENERATED_H)\n \n LIB_H := $(sort $(patsubst ./%,%,$(shell git ls-files '*.h' ':!t/' ':!Documentation/' 2>/dev/null || \\\n \t$(FIND) . \\\n@@ -2910,7 +2912,7 @@ $(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE\n .PHONY: sparse $(SP_OBJ)\n sparse: $(SP_OBJ)\n \n-EXCEPT_HDRS := command-list.h config-list.h unicode-width.h compat/% xdiff/%\n+EXCEPT_HDRS := $(GENERATED_H) unicode-width.h compat/% xdiff/%\n ifndef GCRYPT_SHA256\n \tEXCEPT_HDRS += sha256/gcrypt.h\n endif\n@@ -2933,7 +2935,7 @@ style:\n \tgit clang-format --style file --diff --extensions c,h\n \n .PHONY: check\n-check: config-list.h command-list.h\n+check: $(GENERATED_H)\n \t@if sparse; \\\n \tthen \\\n \t\techo >&2 \"Use 'make sparse' instead\"; \\\ndiff --git a/compat/vcbuild/README b/compat/vcbuild/README\nindex 51fb083dbbe..29ec1d0f104 100644\n--- a/compat/vcbuild/README\n+++ b/compat/vcbuild/README\n@@ -92,7 +92,7 @@ The Steps of Build Git with VS2008\n    the git operations.\n \n 3. Inside Git's directory run the command:\n-       make command-list.h config-list.h\n+       make generated-hdrs\n    to generate the header file needed to compile git.\n \n 4. Then either build Git with the GNU Make Makefile in the Git projects\ndiff --git a/config.mak.uname b/config.mak.uname\nindex 76516aaa9a5..8aac06eb094 100644\n--- a/config.mak.uname\n+++ b/config.mak.uname\n@@ -735,9 +735,9 @@ vcxproj:\n \t echo '</Project>') >git-remote-http/LinkOrCopyRemoteHttp.targets\n \tgit add -f git/LinkOrCopyBuiltins.targets git-remote-http/LinkOrCopyRemoteHttp.targets\n \n-\t# Add command-list.h and config-list.h\n-\t$(MAKE) MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 config-list.h command-list.h\n-\tgit add -f config-list.h command-list.h\n+\t# Add generated headers\n+\t$(MAKE) MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 $(GENERATED_H)\n+\tgit add -f $(GENERATED_H)\n \n \t# Add scripts\n \trm -f perl/perl.mak\n-- \n2.33.0.1229.g0a86d28df49\n\n"},{"id":"436844","messageId":"patch-3.8-010701fd784-20210923T095326Z-avarab@gmail.com","threadId":"56573","inReplyTo":"cover-0.8-00000000000-20210923T095326Z-avarab@gmail.com","subject":"[PATCH 3/8] Makefile: don't perform \"mv $@+ $@\" dance for $(GENERATED_H)","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-09-23T10:29:58Z","receivedAt":"2021-09-23T10:31:20Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the \"cmd.sh > $@+ && mv $@+ $@\" pattern used for generating the\nconfig-list.h and command-list.h to just \"cmd.sh >$@\". This was needed\nas a guard to ensure that we don't have an empty file if the script\nfailed, but since 7b76d6bf221 (Makefile: add and use the\n\".DELETE_ON_ERROR\" flag, 2021-06-29) GNU make ensures that doesn't\nhappen.\n\nThere's still a lot of other places in the Makefile where we\nneedlessly use this pattern, but I'm just changing these because I'm\nabout to add a new $(GENERATED_H) target, let's have them all look and\nact the same way.\n\nEven with \".DELETE_ON_ERROR\" there is still a point to using the \"mv\n$@+ $@\" pattern in some cases, e.g. to ensure that you have a working\nbinary during recompilation (see [1] for the start of a long\ndiscussion about that), but that doesn't apply here. Nothing external\nuses $(GENERATED_H) directly, it's only ever used in the context of\nthe Makefile's own dependency (re-)generation.\n\n1. https://lore.kernel.org/git/8735t93h0u.fsf@evledraar.gmail.com/\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n Makefile | 5 ++---\n 1 file changed, 2 insertions(+), 3 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex eb5780264a4..c529f283dcc 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2238,15 +2238,14 @@ $(BUILT_INS): git$X\n config-list.h: generate-configlist.sh\n \n config-list.h: Documentation/*config.txt Documentation/config/*.txt\n-\t$(QUIET_GEN)$(SHELL_PATH) ./generate-configlist.sh \\\n-\t\t>$@+ && mv $@+ $@\n+\t$(QUIET_GEN)$(SHELL_PATH) ./generate-configlist.sh >$@\n \n command-list.h: generate-cmdlist.sh command-list.txt\n \n command-list.h: $(wildcard Documentation/git*.txt)\n \t$(QUIET_GEN)$(SHELL_PATH) ./generate-cmdlist.sh \\\n \t\t$(patsubst %,--exclude-program %,$(EXCLUDED_PROGRAMS)) \\\n-\t\tcommand-list.txt >$@+ && mv $@+ $@\n+\t\tcommand-list.txt >$@\n \n SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\\\n \t$(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\\\n-- \n2.33.0.1229.g0a86d28df49\n\n"},{"id":"436845","messageId":"patch-4.8-b3cce74d259-20210923T095326Z-avarab@gmail.com","threadId":"56573","inReplyTo":"cover-0.8-00000000000-20210923T095326Z-avarab@gmail.com","subject":"[PATCH 4/8] Makefile: remove an out-of-date comment","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-09-23T10:29:59Z","receivedAt":"2021-09-23T10:31:21Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This comment added in dfea575017 (Makefile: lazily compute header\ndependencies, 2010-01-26) has been out of date since\n92b88eba9f (Makefile: use `git ls-files` to list header files, if\npossible, 2019-03-04), when we did exactly what it tells us not to do\nand added $(GENERATED_H) to $(OBJECTS) dependencies.\n\nThe rest of it was also somewhere between inaccurate and outdated,\nsince as of b8ba629264 (Makefile: fold MISC_H into LIB_H, 2012-06-20)\nit's not followed by a list of header files, that got moved earlier in\nthe file into LIB_H in 60d24dd255 (Makefile: fold XDIFF_H and VCSSVN_H\ninto LIB_H, 2012-07-06).\n\nLet's just remove it entirely, to the extent that we have anything\nuseful to say here the comment on the\n\"USE_COMPUTED_HEADER_DEPENDENCIES\" variable a few lines above this\nchange does the job for us.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n Makefile | 7 -------\n 1 file changed, 7 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex c529f283dcc..c50115cd565 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2508,13 +2508,6 @@ ifneq ($(dep_files_present),)\n include $(dep_files_present)\n endif\n else\n-# Dependencies on header files, for platforms that do not support\n-# the gcc -MMD option.\n-#\n-# Dependencies on automatically generated headers such as command-list.h\n-# should _not_ be included here, since they are necessary even when\n-# building an object for the first time.\n-\n $(OBJECTS): $(LIB_H) $(GENERATED_H)\n endif\n \n-- \n2.33.0.1229.g0a86d28df49\n\n"},{"id":"436846","messageId":"patch-5.8-7dd874d50ec-20210923T095326Z-avarab@gmail.com","threadId":"56573","inReplyTo":"cover-0.8-00000000000-20210923T095326Z-avarab@gmail.com","subject":"[PATCH 5/8] hook.[ch]: move find_hook() from run-command.c to hook.c","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-09-23T10:30:00Z","receivedAt":"2021-09-23T10:31:24Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Move the find_hook() function from run-command.c to a new hook.c\nlibrary. This change establishes a stub library that's pretty\npointless right now, but will see much wider use with Emily Shaffer's\nupcoming \"configuration-based hooks\" series.\n\nEventually all the hook related code will live in hook.[ch]. Let's\nstart that process by moving the simple find_hook() function over\nas-is.\n\nSigned-off-by: Emily Shaffer <emilyshaffer@google.com>\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n Makefile               |  1 +\n builtin/am.c           |  1 +\n builtin/bugreport.c    |  2 +-\n builtin/commit.c       |  1 +\n builtin/merge.c        |  1 +\n builtin/receive-pack.c |  1 +\n builtin/worktree.c     |  1 +\n hook.c                 | 37 +++++++++++++++++++++++++++++++++++++\n hook.h                 | 11 +++++++++++\n refs.c                 |  1 +\n run-command.c          | 35 +----------------------------------\n run-command.h          |  7 -------\n sequencer.c            |  1 +\n transport.c            |  1 +\n 14 files changed, 59 insertions(+), 42 deletions(-)\n create mode 100644 hook.c\n create mode 100644 hook.h\n\ndiff --git a/Makefile b/Makefile\nindex c50115cd565..ad2aeff68f0 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -904,6 +904,7 @@ LIB_OBJS += hash-lookup.o\n LIB_OBJS += hashmap.o\n LIB_OBJS += help.o\n LIB_OBJS += hex.o\n+LIB_OBJS += hook.o\n LIB_OBJS += ident.o\n LIB_OBJS += json-writer.o\n LIB_OBJS += kwset.o\ndiff --git a/builtin/am.c b/builtin/am.c\nindex 60f3737f99c..85e3d92376b 100644\n--- a/builtin/am.c\n+++ b/builtin/am.c\n@@ -11,6 +11,7 @@\n #include \"parse-options.h\"\n #include \"dir.h\"\n #include \"run-command.h\"\n+#include \"hook.h\"\n #include \"quote.h\"\n #include \"tempfile.h\"\n #include \"lockfile.h\"\ndiff --git a/builtin/bugreport.c b/builtin/bugreport.c\nindex 06ed10dc92d..c30a360d695 100644\n--- a/builtin/bugreport.c\n+++ b/builtin/bugreport.c\n@@ -3,7 +3,7 @@\n #include \"strbuf.h\"\n #include \"help.h\"\n #include \"compat/compiler.h\"\n-#include \"run-command.h\"\n+#include \"hook.h\"\n \n \n static void get_system_info(struct strbuf *sys_info)\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex e7320f66f95..5359d961d22 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -19,6 +19,7 @@\n #include \"revision.h\"\n #include \"wt-status.h\"\n #include \"run-command.h\"\n+#include \"hook.h\"\n #include \"refs.h\"\n #include \"log-tree.h\"\n #include \"strbuf.h\"\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 3fbdacc7db4..fe664f6a863 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -13,6 +13,7 @@\n #include \"builtin.h\"\n #include \"lockfile.h\"\n #include \"run-command.h\"\n+#include \"hook.h\"\n #include \"diff.h\"\n #include \"diff-merges.h\"\n #include \"refs.h\"\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 48960a9575b..e3895aec622 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -7,6 +7,7 @@\n #include \"pkt-line.h\"\n #include \"sideband.h\"\n #include \"run-command.h\"\n+#include \"hook.h\"\n #include \"exec-cmd.h\"\n #include \"commit.h\"\n #include \"object.h\"\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 0d0a80da61f..d22ece93e1a 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -8,6 +8,7 @@\n #include \"branch.h\"\n #include \"refs.h\"\n #include \"run-command.h\"\n+#include \"hook.h\"\n #include \"sigchain.h\"\n #include \"submodule.h\"\n #include \"utf8.h\"\ndiff --git a/hook.c b/hook.c\nnew file mode 100644\nindex 00000000000..ba70b314718\n--- /dev/null\n+++ b/hook.c\n@@ -0,0 +1,37 @@\n+#include \"cache.h\"\n+#include \"hook.h\"\n+#include \"run-command.h\"\n+\n+const char *find_hook(const char *name)\n+{\n+\tstatic struct strbuf path = STRBUF_INIT;\n+\n+\tstrbuf_reset(&path);\n+\tstrbuf_git_path(&path, \"hooks/%s\", name);\n+\tif (access(path.buf, X_OK) < 0) {\n+\t\tint err = errno;\n+\n+#ifdef STRIP_EXTENSION\n+\t\tstrbuf_addstr(&path, STRIP_EXTENSION);\n+\t\tif (access(path.buf, X_OK) >= 0)\n+\t\t\treturn path.buf;\n+\t\tif (errno == EACCES)\n+\t\t\terr = errno;\n+#endif\n+\n+\t\tif (err == EACCES && advice_enabled(ADVICE_IGNORED_HOOK)) {\n+\t\t\tstatic struct string_list advise_given = STRING_LIST_INIT_DUP;\n+\n+\t\t\tif (!string_list_lookup(&advise_given, name)) {\n+\t\t\t\tstring_list_insert(&advise_given, name);\n+\t\t\t\tadvise(_(\"The '%s' hook was ignored because \"\n+\t\t\t\t\t \"it's not set as executable.\\n\"\n+\t\t\t\t\t \"You can disable this warning with \"\n+\t\t\t\t\t \"`git config advice.ignoredHook false`.\"),\n+\t\t\t\t       path.buf);\n+\t\t\t}\n+\t\t}\n+\t\treturn NULL;\n+\t}\n+\treturn path.buf;\n+}\ndiff --git a/hook.h b/hook.h\nnew file mode 100644\nindex 00000000000..68624f16059\n--- /dev/null\n+++ b/hook.h\n@@ -0,0 +1,11 @@\n+#ifndef HOOK_H\n+#define HOOK_H\n+\n+/*\n+ * Returns the path to the hook file, or NULL if the hook is missing\n+ * or disabled. Note that this points to static storage that will be\n+ * overwritten by further calls to find_hook and run_hook_*.\n+ */\n+const char *find_hook(const char *name);\n+\n+#endif\ndiff --git a/refs.c b/refs.c\nindex 8b9f7c3a80a..6211692eaae 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -10,6 +10,7 @@\n #include \"refs.h\"\n #include \"refs/refs-internal.h\"\n #include \"run-command.h\"\n+#include \"hook.h\"\n #include \"object-store.h\"\n #include \"object.h\"\n #include \"tag.h\"\ndiff --git a/run-command.c b/run-command.c\nindex 821245672d6..222a48e1400 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -9,6 +9,7 @@\n #include \"quote.h\"\n #include \"config.h\"\n #include \"packfile.h\"\n+#include \"hook.h\"\n \n void child_process_init(struct child_process *child)\n {\n@@ -1322,40 +1323,6 @@ int async_with_fork(void)\n #endif\n }\n \n-const char *find_hook(const char *name)\n-{\n-\tstatic struct strbuf path = STRBUF_INIT;\n-\n-\tstrbuf_reset(&path);\n-\tstrbuf_git_path(&path, \"hooks/%s\", name);\n-\tif (access(path.buf, X_OK) < 0) {\n-\t\tint err = errno;\n-\n-#ifdef STRIP_EXTENSION\n-\t\tstrbuf_addstr(&path, STRIP_EXTENSION);\n-\t\tif (access(path.buf, X_OK) >= 0)\n-\t\t\treturn path.buf;\n-\t\tif (errno == EACCES)\n-\t\t\terr = errno;\n-#endif\n-\n-\t\tif (err == EACCES && advice_enabled(ADVICE_IGNORED_HOOK)) {\n-\t\t\tstatic struct string_list advise_given = STRING_LIST_INIT_DUP;\n-\n-\t\t\tif (!string_list_lookup(&advise_given, name)) {\n-\t\t\t\tstring_list_insert(&advise_given, name);\n-\t\t\t\tadvise(_(\"The '%s' hook was ignored because \"\n-\t\t\t\t\t \"it's not set as executable.\\n\"\n-\t\t\t\t\t \"You can disable this warning with \"\n-\t\t\t\t\t \"`git config advice.ignoredHook false`.\"),\n-\t\t\t\t       path.buf);\n-\t\t\t}\n-\t\t}\n-\t\treturn NULL;\n-\t}\n-\treturn path.buf;\n-}\n-\n int run_hook_ve(const char *const *env, const char *name, va_list args)\n {\n \tstruct child_process hook = CHILD_PROCESS_INIT;\ndiff --git a/run-command.h b/run-command.h\nindex ad207daced7..a9fd2d24f00 100644\n--- a/run-command.h\n+++ b/run-command.h\n@@ -212,13 +212,6 @@ int finish_command_in_signal(struct child_process *);\n  */\n int run_command(struct child_process *);\n \n-/*\n- * Returns the path to the hook file, or NULL if the hook is missing\n- * or disabled. Note that this points to static storage that will be\n- * overwritten by further calls to find_hook and run_hook_*.\n- */\n-const char *find_hook(const char *name);\n-\n /**\n  * Run a hook.\n  * The first argument is a pathname to an index file, or NULL\ndiff --git a/sequencer.c b/sequencer.c\nindex 614d56f5e21..8ee6c4ac240 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -8,6 +8,7 @@\n #include \"sequencer.h\"\n #include \"tag.h\"\n #include \"run-command.h\"\n+#include \"hook.h\"\n #include \"exec-cmd.h\"\n #include \"utf8.h\"\n #include \"cache-tree.h\"\ndiff --git a/transport.c b/transport.c\nindex b37664ba870..5d31f7cc131 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -2,6 +2,7 @@\n #include \"config.h\"\n #include \"transport.h\"\n #include \"run-command.h\"\n+#include \"hook.h\"\n #include \"pkt-line.h\"\n #include \"fetch-pack.h\"\n #include \"remote.h\"\n-- \n2.33.0.1229.g0a86d28df49\n\n"},{"id":"436847","messageId":"patch-6.8-db8893afee6-20210923T095326Z-avarab@gmail.com","threadId":"56573","inReplyTo":"cover-0.8-00000000000-20210923T095326Z-avarab@gmail.com","subject":"[PATCH 6/8] hook.c: add a hook_exists() wrapper and use it in bugreport.c","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-09-23T10:30:01Z","receivedAt":"2021-09-23T10:31:28Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"From: Emily Shaffer <emilyshaffer@google.com>\n\nAdd a boolean version of the find_hook() function for those callers\nwho are only interested in checking whether the hook exists, not what\nthe path to it is.\n\nSigned-off-by: Emily Shaffer <emilyshaffer@google.com>\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/bugreport.c | 2 +-\n hook.c              | 5 +++++\n hook.h              | 5 +++++\n 3 files changed, 11 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/bugreport.c b/builtin/bugreport.c\nindex c30a360d695..a02c2540bb1 100644\n--- a/builtin/bugreport.c\n+++ b/builtin/bugreport.c\n@@ -82,7 +82,7 @@ static void get_populated_hooks(struct strbuf *hook_info, int nongit)\n \t}\n \n \tfor (i = 0; i < ARRAY_SIZE(hook); i++)\n-\t\tif (find_hook(hook[i]))\n+\t\tif (hook_exists(hook[i]))\n \t\t\tstrbuf_addf(hook_info, \"%s\\n\", hook[i]);\n }\n \ndiff --git a/hook.c b/hook.c\nindex ba70b314718..55e1145a4b7 100644\n--- a/hook.c\n+++ b/hook.c\n@@ -35,3 +35,8 @@ const char *find_hook(const char *name)\n \t}\n \treturn path.buf;\n }\n+\n+int hook_exists(const char *name)\n+{\n+\treturn !!find_hook(name);\n+}\ndiff --git a/hook.h b/hook.h\nindex 68624f16059..6aa36fc7ff9 100644\n--- a/hook.h\n+++ b/hook.h\n@@ -8,4 +8,9 @@\n  */\n const char *find_hook(const char *name);\n \n+/**\n+ * A boolean version of find_hook()\n+ */\n+int hook_exists(const char *hookname);\n+\n #endif\n-- \n2.33.0.1229.g0a86d28df49\n\n"},{"id":"436848","messageId":"patch-7.8-b61130dee5b-20210923T095326Z-avarab@gmail.com","threadId":"56573","inReplyTo":"cover-0.8-00000000000-20210923T095326Z-avarab@gmail.com","subject":"[PATCH 7/8] hook.c users: use \"hook_exists()\" instead of \"find_hook()\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-09-23T10:30:02Z","receivedAt":"2021-09-23T10:31:29Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Use the new hook_exists() function instead of find_hook() where the\nlatter was called in boolean contexts. This make subsequent changes in\na series where we further refactor the hook API clearer, as we won't\nconflate wanting to get the path of the hook with checking for its\nexistence.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/commit.c       | 2 +-\n builtin/merge.c        | 2 +-\n builtin/receive-pack.c | 2 +-\n sequencer.c            | 2 +-\n 4 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 5359d961d22..883c16256c8 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1052,7 +1052,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\treturn 0;\n \t}\n \n-\tif (!no_verify && find_hook(\"pre-commit\")) {\n+\tif (!no_verify && hook_exists(\"pre-commit\")) {\n \t\t/*\n \t\t * Re-read the index as pre-commit hook could have updated it,\n \t\t * and write it out as a tree.  We must do this before we invoke\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex fe664f6a863..956b6259f21 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -849,7 +849,7 @@ static void prepare_to_commit(struct commit_list *remoteheads)\n \t * and write it out as a tree.  We must do this before we invoke\n \t * the editor and after we invoke run_status above.\n \t */\n-\tif (find_hook(\"pre-merge-commit\"))\n+\tif (hook_exists(\"pre-merge-commit\"))\n \t\tdiscard_cache();\n \tread_cache_from(index_file);\n \tstrbuf_addbuf(&msg, &merge_msg);\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex e3895aec622..25cc0c907e1 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -1464,7 +1464,7 @@ static const char *update_worktree(unsigned char *sha1, const struct worktree *w\n \n \tstrvec_pushf(&env, \"GIT_DIR=%s\", absolute_path(git_dir));\n \n-\tif (!find_hook(push_to_checkout_hook))\n+\tif (!hook_exists(push_to_checkout_hook))\n \t\tretval = push_to_deploy(sha1, &env, work_tree);\n \telse\n \t\tretval = push_to_checkout(sha1, &env, work_tree);\ndiff --git a/sequencer.c b/sequencer.c\nindex 8ee6c4ac240..e501945796d 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -1460,7 +1460,7 @@ static int try_to_commit(struct repository *r,\n \t\t}\n \t}\n \n-\tif (find_hook(\"prepare-commit-msg\")) {\n+\tif (hook_exists(\"prepare-commit-msg\")) {\n \t\tres = run_prepare_commit_msg_hook(r, msg, hook_commit);\n \t\tif (res)\n \t\t\tgoto out;\n-- \n2.33.0.1229.g0a86d28df49\n\n"},{"id":"436849","messageId":"patch-8.8-80aae4d5c13-20210923T095326Z-avarab@gmail.com","threadId":"56573","inReplyTo":"cover-0.8-00000000000-20210923T095326Z-avarab@gmail.com","subject":"[PATCH 8/8] hook-list.h: add a generated list of hooks, like config-list.h","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-09-23T10:30:03Z","receivedAt":"2021-09-23T10:31:33Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Make githooks(5) the source of truth for what hooks git supports, and\npunt out early on hooks we don't know about in find_hook(). This\nensures that the documentation and the C code's idea about existing\nhooks doesn't diverge.\n\nWe still have Perl and Python code running its own hooks, but that'll\nbe addressed by Emily Shaffer's upcoming \"git hook run\" command.\n\nThis resolves a long-standing TODO item in bugreport.c of there being\nno centralized listing of hooks, and fixes a bug with the bugreport\nlisting only knowing about 1/4 of the p4 hooks. It didn't know about\nthe recent \"reference-transaction\" hook either.\n\nWe could make the find_hook() function die() or BUG() out if the new\nknown_hook() returned 0, but let's make it return NULL just as it does\nwhen it can't find a hook of a known type. Making it die() is overly\nanal, and unlikely to be what we need in catching stupid typos in the\nname of some new hook hardcoded in git.git's sources. By making this\nbe tolerant of unknown hook names, changes in a later series to make\n\"git hook run\" run arbitrary user-configured hook names will be easier\nto implement.\n\nI have not been able to directly test the CMake change being made\nhere. Since 4c2c38e800 (ci: modification of main.yml to use cmake for\nvs-build job, 2020-06-26) some of the Windows CI has a hard dependency\non CMake, this change works there, and is to my eyes an obviously\ncorrect use of a pattern established in previous CMake changes,\nnamely:\n\n - 061c2240b1 (Introduce CMake support for configuring Git,\n    2020-06-12)\n - 709df95b78 (help: move list_config_help to builtin/help,\n    2020-04-16)\n - 976aaedca0 (msvc: add a Makefile target to pre-generate the Visual\n   Studio solution, 2019-07-29)\n\nThe LC_ALL=C is needed because at least in my locale the dash (\"-\") is\nignored for the purposes of sorting, which results in a different\norder. I'm not aware of anything in git that has a hard dependency on\nthe order, but e.g. the bugreport output would end up using whatever\nlocale was in effect when git was compiled.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\nHelped-by: René Scharfe <l.s.r@web.de>\n---\n .gitignore                          |  1 +\n Makefile                            |  9 +++++-\n builtin/bugreport.c                 | 44 ++++++-----------------------\n contrib/buildsystems/CMakeLists.txt |  7 +++++\n generate-hooklist.sh                | 18 ++++++++++++\n 5 files changed, 42 insertions(+), 37 deletions(-)\n create mode 100755 generate-hooklist.sh\n\ndiff --git a/.gitignore b/.gitignore\nindex 311841f9bed..6be9de41ae8 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -190,6 +190,7 @@\n /gitweb/static/gitweb.min.*\n /config-list.h\n /command-list.h\n+/hook-list.h\n *.tar.gz\n *.dsc\n *.deb\ndiff --git a/Makefile b/Makefile\nindex ad2aeff68f0..f883657fa26 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -817,6 +817,8 @@ XDIFF_LIB = xdiff/lib.a\n \n GENERATED_H += command-list.h\n GENERATED_H += config-list.h\n+GENERATED_H += hook-list.h\n+\n .PHONY: generated-hdrs\n generated-hdrs: $(GENERATED_H)\n \n@@ -2216,7 +2218,9 @@ git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)\n \n help.sp help.s help.o: command-list.h\n \n-builtin/help.sp builtin/help.s builtin/help.o: config-list.h GIT-PREFIX\n+hook.sp hook.s hook.o: hook-list.h\n+\n+builtin/help.sp builtin/help.s builtin/help.o: config-list.h hook-list.h GIT-PREFIX\n builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \\\n \t'-DGIT_HTML_PATH=\"$(htmldir_relative_SQ)\"' \\\n \t'-DGIT_MAN_PATH=\"$(mandir_relative_SQ)\"' \\\n@@ -2248,6 +2252,9 @@ command-list.h: $(wildcard Documentation/git*.txt)\n \t\t$(patsubst %,--exclude-program %,$(EXCLUDED_PROGRAMS)) \\\n \t\tcommand-list.txt >$@\n \n+hook-list.h: generate-hooklist.sh Documentation/githooks.txt\n+\t$(QUIET_GEN)$(SHELL_PATH) ./generate-hooklist.sh >$@\n+\n SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\\\n \t$(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\\\n \t$(gitwebdir_SQ):$(PERL_PATH_SQ):$(SANE_TEXT_GREP):$(PAGER_ENV):\\\ndiff --git a/builtin/bugreport.c b/builtin/bugreport.c\nindex a02c2540bb1..9de32bc96e7 100644\n--- a/builtin/bugreport.c\n+++ b/builtin/bugreport.c\n@@ -4,6 +4,7 @@\n #include \"help.h\"\n #include \"compat/compiler.h\"\n #include \"hook.h\"\n+#include \"hook-list.h\"\n \n \n static void get_system_info(struct strbuf *sys_info)\n@@ -41,39 +42,7 @@ static void get_system_info(struct strbuf *sys_info)\n \n static void get_populated_hooks(struct strbuf *hook_info, int nongit)\n {\n-\t/*\n-\t * NEEDSWORK: Doesn't look like there is a list of all possible hooks;\n-\t * so below is a transcription of `git help hooks`. Later, this should\n-\t * be replaced with some programmatically generated list (generated from\n-\t * doc or else taken from some library which tells us about all the\n-\t * hooks)\n-\t */\n-\tstatic const char *hook[] = {\n-\t\t\"applypatch-msg\",\n-\t\t\"pre-applypatch\",\n-\t\t\"post-applypatch\",\n-\t\t\"pre-commit\",\n-\t\t\"pre-merge-commit\",\n-\t\t\"prepare-commit-msg\",\n-\t\t\"commit-msg\",\n-\t\t\"post-commit\",\n-\t\t\"pre-rebase\",\n-\t\t\"post-checkout\",\n-\t\t\"post-merge\",\n-\t\t\"pre-push\",\n-\t\t\"pre-receive\",\n-\t\t\"update\",\n-\t\t\"post-receive\",\n-\t\t\"post-update\",\n-\t\t\"push-to-checkout\",\n-\t\t\"pre-auto-gc\",\n-\t\t\"post-rewrite\",\n-\t\t\"sendemail-validate\",\n-\t\t\"fsmonitor-watchman\",\n-\t\t\"p4-pre-submit\",\n-\t\t\"post-index-change\",\n-\t};\n-\tint i;\n+\tconst char **p;\n \n \tif (nongit) {\n \t\tstrbuf_addstr(hook_info,\n@@ -81,9 +50,12 @@ static void get_populated_hooks(struct strbuf *hook_info, int nongit)\n \t\treturn;\n \t}\n \n-\tfor (i = 0; i < ARRAY_SIZE(hook); i++)\n-\t\tif (hook_exists(hook[i]))\n-\t\t\tstrbuf_addf(hook_info, \"%s\\n\", hook[i]);\n+\tfor (p = hook_name_list; *p; p++) {\n+\t\tconst char *hook = *p;\n+\n+\t\tif (hook_exists(hook))\n+\t\t\tstrbuf_addf(hook_info, \"%s\\n\", hook);\n+\t}\n }\n \n static const char * const bugreport_usage[] = {\ndiff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\nindex 171b4124afe..fd1399c440f 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -624,6 +624,13 @@ if(NOT EXISTS ${CMAKE_BINARY_DIR}/config-list.h)\n \t\t\tOUTPUT_FILE ${CMAKE_BINARY_DIR}/config-list.h)\n endif()\n \n+if(NOT EXISTS ${CMAKE_BINARY_DIR}/hook-list.h)\n+\tmessage(\"Generating hook-list.h\")\n+\texecute_process(COMMAND ${SH_EXE} ${CMAKE_SOURCE_DIR}/generate-hooklist.sh\n+\t\t\tWORKING_DIRECTORY ${CMAKE_SOURCE_DIR}\n+\t\t\tOUTPUT_FILE ${CMAKE_BINARY_DIR}/hook-list.h)\n+endif()\n+\n include_directories(${CMAKE_BINARY_DIR})\n \n #build\ndiff --git a/generate-hooklist.sh b/generate-hooklist.sh\nnew file mode 100755\nindex 00000000000..6d4e56d1a31\n--- /dev/null\n+++ b/generate-hooklist.sh\n@@ -0,0 +1,18 @@\n+#!/bin/sh\n+#\n+# Usage: ./generate-hooklist.sh >hook-list.h\n+\n+cat <<EOF\n+/* Automatically generated by generate-hooklist.sh */\n+\n+static const char *hook_name_list[] = {\n+EOF\n+\n+sed -n -e '/^~~~~*$/ {x; s/^.*$/\t\"&\",/; p;}; x' \\\n+\t<Documentation/githooks.txt |\n+\tLC_ALL=C sort\n+\n+cat <<EOF\n+\tNULL,\n+};\n+EOF\n-- \n2.33.0.1229.g0a86d28df49\n\n"},{"id":"436939","messageId":"92471ff9-7573-c3e4-e9fd-63a5cbf5738f@gmail.com","threadId":"56573","inReplyTo":"patch-8.8-80aae4d5c13-20210923T095326Z-avarab@gmail.com","subject":"Re: [PATCH 8/8] hook-list.h: add a generated list of hooks, like config-list.h","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2021-09-24T10:19:02Z","receivedAt":"2021-09-24T10:19:10Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"Hi Ævar\nOn 23/09/2021 11:30, Ævar Arnfjörð Bjarmason wrote:\n> diff --git a/generate-hooklist.sh b/generate-hooklist.sh\n> new file mode 100755\n> index 00000000000..6d4e56d1a31\n> --- /dev/null\n> +++ b/generate-hooklist.sh\n> @@ -0,0 +1,18 @@\n> +#!/bin/sh\n> +#\n> +# Usage: ./generate-hooklist.sh >hook-list.h\n> +\n> +cat <<EOF\n> +/* Automatically generated by generate-hooklist.sh */\n> +\n> +static const char *hook_name_list[] = {\n> +EOF\n> +\n> +sed -n -e '/^~~~~*$/ {x; s/^.*$/\t\"&\",/; p;}; x' \\\n\nPOSIX does not support using a semicolon after a closing brace [1], \ngrepping our code base with\n\tgit grep 'sed .*};' '*.sh'\ndoes not give any matches so I don't think we're using that pattern any \nwhere else. Replacing the semicolon with ' -e' would fix it.\n\nBest Wishes\n\nPhillip\n\n[1] \"Editing commands other that {...}, a, b, c, i, r, t, w, : and # can \nbe followed by a <semicolon>\" from \nhttps://pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html\n"},{"id":"436969","messageId":"xmqqr1devuoc.fsf@gitster.g","threadId":"56573","inReplyTo":"92471ff9-7573-c3e4-e9fd-63a5cbf5738f@gmail.com","subject":"Re: [PATCH 8/8] hook-list.h: add a generated list of hooks, like config-list.h","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-09-24T15:51:15Z","receivedAt":"2021-09-24T15:51:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Phillip Wood <phillip.wood123@gmail.com> writes:\n\n>> +sed -n -e '/^~~~~*$/ {x; s/^.*$/\t\"&\",/; p;}; x' \\\n>\n> POSIX does not support using a semicolon after a closing brace [1],\n> ...\n> [1] \"Editing commands other that {...}, a, b, c, i, r, t, w, : and #\n> can be followed by a <semicolon>\" from \n> https://pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html\n\nWould sed implementation of BSD ancestry fail reliably to be a good\ncoalmine canary?\n"},{"id":"436973","messageId":"d5f297d4-9c64-1ff9-8422-054979bf8cfa@web.de","threadId":"56573","inReplyTo":"xmqqr1devuoc.fsf@gitster.g","subject":"Re: [PATCH 8/8] hook-list.h: add a generated list of hooks, like config-list.h","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2021-09-24T16:39:15Z","receivedAt":"2021-09-24T16:39:36Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 24.09.21 um 17:51 schrieb Junio C Hamano:\n> Phillip Wood <phillip.wood123@gmail.com> writes:\n>\n>>> +sed -n -e '/^~~~~*$/ {x; s/^.*$/\t\"&\",/; p;}; x' \\\n>>\n>> POSIX does not support using a semicolon after a closing brace [1],\n>> ...\n>> [1] \"Editing commands other that {...}, a, b, c, i, r, t, w, : and #\n>> can be followed by a <semicolon>\" from\n>> https://pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html\n>\n> Would sed implementation of BSD ancestry fail reliably to be a good\n> coalmine canary?\n\nI doubt it.  https://man.openbsd.org/sed says:\n\n   \"Multiple commands may be specified separated by newlines or\n    semicolons\"\n\n   \"The a, c, i, r, and w functions cannot be followed by another\n    command separated with a semicolon.\"\n\n   \"Following the b, t, or : commands with a semicolon and another\n    command is an extension to the specification.\"\n\n   \"The use of newlines to separate multiple commands on the command\n    line is non-portable\"\n\nhttps://www.freebsd.org/cgi/man.cgi?query=sed doesn't mention semicolons\nat all.\n\nOn macOS 11.6 (FreeBSD-based userland) the semicolon is accepted:\n\n   $ echo a | sed -n -e '{p;}; p'\n   a\n   a\n\nRené\n"},{"id":"437022","messageId":"87fstt3gzd.fsf@evledraar.gmail.com","threadId":"56573","inReplyTo":"92471ff9-7573-c3e4-e9fd-63a5cbf5738f@gmail.com","subject":"Re: [PATCH 8/8] hook-list.h: add a generated list of hooks, like config-list.h","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-09-24T19:30:04Z","receivedAt":"2021-09-24T19:34:35Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Sep 24 2021, Phillip Wood wrote:\n\n> Hi Ævar\n> On 23/09/2021 11:30, Ævar Arnfjörð Bjarmason wrote:\n>> diff --git a/generate-hooklist.sh b/generate-hooklist.sh\n>> new file mode 100755\n>> index 00000000000..6d4e56d1a31\n>> --- /dev/null\n>> +++ b/generate-hooklist.sh\n>> @@ -0,0 +1,18 @@\n>> +#!/bin/sh\n>> +#\n>> +# Usage: ./generate-hooklist.sh >hook-list.h\n>> +\n>> +cat <<EOF\n>> +/* Automatically generated by generate-hooklist.sh */\n>> +\n>> +static const char *hook_name_list[] = {\n>> +EOF\n>> +\n>> +sed -n -e '/^~~~~*$/ {x; s/^.*$/\t\"&\",/; p;}; x' \\\n>\n> POSIX does not support using a semicolon after a closing brace [1],\n> grepping our code base with\n> \tgit grep 'sed .*};' '*.sh'\n> does not give any matches so I don't think we're using that pattern\n> any where else. Replacing the semicolon with ' -e' would fix it.\n>\n> Best Wishes\n>\n> Phillip\n\nDoes this fail on any system you're aware of? If so what OS/version (and\npreferably version of \"sed\").\n\nRené's downthread <d5f297d4-9c64-1ff9-8422-054979bf8cfa@web.de> seems to\nsuggest that this is fine.\n\nBoth beforehand and just now I've tested this on AIX, Solaris,\n{Open,Net,Free}BSD, HP/UX, OSX and Linux (a few distros/versions).\n\nAll of them are able to generate the same hook-list.h using this version\nof the patch.\n"},{"id":"437026","messageId":"8c7aaea3-afbc-1980-ac31-6c324eef5a94@web.de","threadId":"56573","inReplyTo":"87fstt3gzd.fsf@evledraar.gmail.com","subject":"Re: [PATCH 8/8] hook-list.h: add a generated list of hooks, like config-list.h","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2021-09-24T19:56:29Z","receivedAt":"2021-09-24T19:56:52Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 24.09.21 um 21:30 schrieb Ævar Arnfjörð Bjarmason:\n>\n> On Fri, Sep 24 2021, Phillip Wood wrote:\n>\n>> Hi Ævar\n>> On 23/09/2021 11:30, Ævar Arnfjörð Bjarmason wrote:\n>>> diff --git a/generate-hooklist.sh b/generate-hooklist.sh\n>>> new file mode 100755\n>>> index 00000000000..6d4e56d1a31\n>>> --- /dev/null\n>>> +++ b/generate-hooklist.sh\n>>> @@ -0,0 +1,18 @@\n>>> +#!/bin/sh\n>>> +#\n>>> +# Usage: ./generate-hooklist.sh >hook-list.h\n>>> +\n>>> +cat <<EOF\n>>> +/* Automatically generated by generate-hooklist.sh */\n>>> +\n>>> +static const char *hook_name_list[] = {\n>>> +EOF\n>>> +\n>>> +sed -n -e '/^~~~~*$/ {x; s/^.*$/\t\"&\",/; p;}; x' \\\n>>\n>> POSIX does not support using a semicolon after a closing brace [1],\n>> grepping our code base with\n>> \tgit grep 'sed .*};' '*.sh'\n>> does not give any matches so I don't think we're using that pattern\n>> any where else. Replacing the semicolon with ' -e' would fix it.\n>>\n>> Best Wishes\n>>\n>> Phillip\n>\n> Does this fail on any system you're aware of? If so what OS/version (and\n> preferably version of \"sed\").\n\nI wasn't aware of that restriction and my gut feeling is that enforcing\nit in a sed implementation would be slightly harder than allowing \"};\".\n\n> René's downthread <d5f297d4-9c64-1ff9-8422-054979bf8cfa@web.de> seems to\n> suggest that this is fine.\n\nThat said, I didn't mean to suggest that we keep using this construct,\njust that I couldn't find an implementation which would reject it.\n\n> Both beforehand and just now I've tested this on AIX, Solaris,\n> {Open,Net,Free}BSD, HP/UX, OSX and Linux (a few distros/versions).\n>\n> All of them are able to generate the same hook-list.h using this version\n> of the patch.\n\nImpressive list, but still it's probably better to move the last \"x\" to\nits own -e, to appease the POSIX spirits.  Small change..\n\nRené\n"},{"id":"437039","messageId":"877df53f4q.fsf@evledraar.gmail.com","threadId":"56573","inReplyTo":"8c7aaea3-afbc-1980-ac31-6c324eef5a94@web.de","subject":"Re: [PATCH 8/8] hook-list.h: add a generated list of hooks, like config-list.h","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-09-24T20:09:35Z","receivedAt":"2021-09-24T20:14:34Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Fri, Sep 24 2021, René Scharfe wrote:\n\n> Am 24.09.21 um 21:30 schrieb Ævar Arnfjörð Bjarmason:\n>>\n>> On Fri, Sep 24 2021, Phillip Wood wrote:\n>>\n>>> Hi Ævar\n>>> On 23/09/2021 11:30, Ævar Arnfjörð Bjarmason wrote:\n>>>> diff --git a/generate-hooklist.sh b/generate-hooklist.sh\n>>>> new file mode 100755\n>>>> index 00000000000..6d4e56d1a31\n>>>> --- /dev/null\n>>>> +++ b/generate-hooklist.sh\n>>>> @@ -0,0 +1,18 @@\n>>>> +#!/bin/sh\n>>>> +#\n>>>> +# Usage: ./generate-hooklist.sh >hook-list.h\n>>>> +\n>>>> +cat <<EOF\n>>>> +/* Automatically generated by generate-hooklist.sh */\n>>>> +\n>>>> +static const char *hook_name_list[] = {\n>>>> +EOF\n>>>> +\n>>>> +sed -n -e '/^~~~~*$/ {x; s/^.*$/\t\"&\",/; p;}; x' \\\n>>>\n>>> POSIX does not support using a semicolon after a closing brace [1],\n>>> grepping our code base with\n>>> \tgit grep 'sed .*};' '*.sh'\n>>> does not give any matches so I don't think we're using that pattern\n>>> any where else. Replacing the semicolon with ' -e' would fix it.\n>>>\n>>> Best Wishes\n>>>\n>>> Phillip\n>>\n>> Does this fail on any system you're aware of? If so what OS/version (and\n>> preferably version of \"sed\").\n>\n> I wasn't aware of that restriction and my gut feeling is that enforcing\n> it in a sed implementation would be slightly harder than allowing \"};\".\n>\n>> René's downthread <d5f297d4-9c64-1ff9-8422-054979bf8cfa@web.de> seems to\n>> suggest that this is fine.\n>\n> That said, I didn't mean to suggest that we keep using this construct,\n> just that I couldn't find an implementation which would reject it.\n\nI'll re-test without \"};\", my (seemingly incorrect) quick read of your\nE-Mail + my own testing was that semicolons were fine there, but maybe\nthey're just accepted by most...\n\n>> Both beforehand and just now I've tested this on AIX, Solaris,\n>> {Open,Net,Free}BSD, HP/UX, OSX and Linux (a few distros/versions).\n>>\n>> All of them are able to generate the same hook-list.h using this version\n>> of the patch.\n>\n> Impressive list.\n\nFWIW that's just the GCC Farm which is fairly easy to get access to for\nfree software devlopment: https://cfarm.tetaneutral.net/machines/list/\n\nThe HP/UX box is a similar \"private\" setup mainly for Perl5 core\ndevelopment, I'm getting to use it to get git working there.\n\n> but still it's probably better to move the last \"x\" to its own -e, to\n> appease the POSIX spirits.  Small change..\n\n*nod*, will test with some version of that.\n"},{"id":"437087","messageId":"cover-v2-0.8-00000000000-20210926T185800Z-avarab@gmail.com","threadId":"56573","inReplyTo":"cover-0.8-00000000000-20210923T095326Z-avarab@gmail.com","subject":"[PATCH v2 0/8] Makefile: generate a hook-list.h, prep for config-based-hooks","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-09-26T19:03:21Z","receivedAt":"2021-09-26T19:03:35Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This series is an incremental restart of the now-ejected\nes/config-based-hooks and ab/config-based-hooks-base topics. See [1]\nfor a summary of the plan and progression.\n\nIn v2 the \"sed\" invocation that generates the new hook-list.h has been\nchanged to be portable under POSIX. See the thread starting at\nhttps://lore.kernel.org/git/92471ff9-7573-c3e4-e9fd-63a5cbf5738f@gmail.com/;\n\nThe portability issue is AFAICT theoretical in that any \"sed\" command\nI've tried accepts the old version (I tried the large list of OS's\nlisted in [2]), but better safe than sorry.\n\nOther changes:\n\n * I noticed that the run-command.h inclusion in transport.c become\n   redundant, I removed that and validated the other ones that have\n   the new hook.h, they all still need run-command.h.\n\n * A whitespace change in v1 in a change to the Makefile makes the\n   diff for 8/8 easier to read.\n\n1. http://lore.kernel.org/git/cover-0.8-00000000000-20210923T095326Z-avarab@gmail.com\n2. https://lore.kernel.org/git/87fstt3gzd.fsf@evledraar.gmail.com/\n\nEmily Shaffer (1):\n  hook.c: add a hook_exists() wrapper and use it in bugreport.c\n\nÆvar Arnfjörð Bjarmason (7):\n  Makefile: mark \"check\" target as .PHONY\n  Makefile: stop hardcoding {command,config}-list.h\n  Makefile: don't perform \"mv $@+ $@\" dance for $(GENERATED_H)\n  Makefile: remove an out-of-date comment\n  hook.[ch]: move find_hook() from run-command.c to hook.c\n  hook.c users: use \"hook_exists()\" instead of \"find_hook()\"\n  hook-list.h: add a generated list of hooks, like config-list.h\n\n .gitignore                          |  1 +\n Makefile                            | 28 ++++++++++--------\n builtin/am.c                        |  1 +\n builtin/bugreport.c                 | 46 ++++++-----------------------\n builtin/commit.c                    |  3 +-\n builtin/merge.c                     |  3 +-\n builtin/receive-pack.c              |  3 +-\n builtin/worktree.c                  |  1 +\n compat/vcbuild/README               |  2 +-\n config.mak.uname                    |  6 ++--\n contrib/buildsystems/CMakeLists.txt |  7 +++++\n generate-hooklist.sh                | 20 +++++++++++++\n hook.c                              | 42 ++++++++++++++++++++++++++\n hook.h                              | 16 ++++++++++\n refs.c                              |  1 +\n run-command.c                       | 35 +---------------------\n run-command.h                       |  7 -----\n sequencer.c                         |  3 +-\n transport.c                         |  2 +-\n 19 files changed, 127 insertions(+), 100 deletions(-)\n create mode 100755 generate-hooklist.sh\n create mode 100644 hook.c\n create mode 100644 hook.h\n\nRange-diff against v1:\n1:  91d0cd09c8e = 1:  095ef66c29e Makefile: mark \"check\" target as .PHONY\n2:  804795771c6 = 2:  520e4baede6 Makefile: stop hardcoding {command,config}-list.h\n3:  010701fd784 = 3:  312b353c651 Makefile: don't perform \"mv $@+ $@\" dance for $(GENERATED_H)\n4:  b3cce74d259 = 4:  aea4868f988 Makefile: remove an out-of-date comment\n5:  7dd874d50ec ! 5:  64f3af94161 hook.[ch]: move find_hook() from run-command.c to hook.c\n    @@ sequencer.c\n     \n      ## transport.c ##\n     @@\n    + #include \"cache.h\"\n      #include \"config.h\"\n      #include \"transport.h\"\n    - #include \"run-command.h\"\n    +-#include \"run-command.h\"\n     +#include \"hook.h\"\n      #include \"pkt-line.h\"\n      #include \"fetch-pack.h\"\n6:  db8893afee6 = 6:  4eabe7ca39f hook.c: add a hook_exists() wrapper and use it in bugreport.c\n7:  b61130dee5b = 7:  6237a1a5549 hook.c users: use \"hook_exists()\" instead of \"find_hook()\"\n8:  80aae4d5c13 ! 8:  7420267ce09 hook-list.h: add a generated list of hooks, like config-list.h\n    @@ Makefile: XDIFF_LIB = xdiff/lib.a\n      generated-hdrs: $(GENERATED_H)\n      \n     @@ Makefile: git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)\n    + \t\t$(filter %.o,$^) $(LIBS)\n      \n      help.sp help.s help.o: command-list.h\n    ++hook.sp hook.s hook.o: hook-list.h\n      \n     -builtin/help.sp builtin/help.s builtin/help.o: config-list.h GIT-PREFIX\n    -+hook.sp hook.s hook.o: hook-list.h\n    -+\n     +builtin/help.sp builtin/help.s builtin/help.o: config-list.h hook-list.h GIT-PREFIX\n      builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \\\n      \t'-DGIT_HTML_PATH=\"$(htmldir_relative_SQ)\"' \\\n    @@ generate-hooklist.sh (new)\n     +static const char *hook_name_list[] = {\n     +EOF\n     +\n    -+sed -n -e '/^~~~~*$/ {x; s/^.*$/\t\"&\",/; p;}; x' \\\n    ++sed -n \\\n    ++\t-e '/^~~~~*$/ {x; s/^.*$/\t\"&\",/; p;}' \\\n    ++\t-e 'x' \\\n     +\t<Documentation/githooks.txt |\n     +\tLC_ALL=C sort\n     +\n-- \n2.33.0.1291.g8857a6a91ac\n\n"},{"id":"437088","messageId":"patch-v2-1.8-095ef66c29e-20210926T185800Z-avarab@gmail.com","threadId":"56573","inReplyTo":"cover-v2-0.8-00000000000-20210926T185800Z-avarab@gmail.com","subject":"[PATCH v2 1/8] Makefile: mark \"check\" target as .PHONY","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-09-26T19:03:22Z","receivedAt":"2021-09-26T19:03:39Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Fix a bug in 44c9e8594e (Fix up header file dependencies and add\nsparse checking rules, 2005-07-03), we never marked the phony \"check\"\ntarget as such.\n\nPerhaps we should just remove it, since as of a combination of\n912f9980d2 (Makefile: help people who run 'make check' by mistake,\n2008-11-11) 0bcd9ae85d (sparse: Fix errors due to missing\ntarget-specific variables, 2011-04-21) we've been suggesting the user\nrun \"make sparse\" directly.\n\nBut under that mode it still does something, as well as directing the\nuser to run \"make test\" under non-sparse. So let's punt that and\nnarrowly fix the PHONY bug.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n Makefile | 1 +\n 1 file changed, 1 insertion(+)\n\ndiff --git a/Makefile b/Makefile\nindex a9f9b689f0c..2777284b0f1 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2925,6 +2925,7 @@ hdr-check: $(HCO)\n style:\n \tgit clang-format --style file --diff --extensions c,h\n \n+.PHONY: check\n check: config-list.h command-list.h\n \t@if sparse; \\\n \tthen \\\n-- \n2.33.0.1291.g8857a6a91ac\n\n"},{"id":"437089","messageId":"patch-v2-2.8-520e4baede6-20210926T185800Z-avarab@gmail.com","threadId":"56573","inReplyTo":"cover-v2-0.8-00000000000-20210926T185800Z-avarab@gmail.com","subject":"[PATCH v2 2/8] Makefile: stop hardcoding {command,config}-list.h","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-09-26T19:03:23Z","receivedAt":"2021-09-26T19:03:41Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change various places that hardcode the names of these two files to\nrefer to either $(GENERATED_H), or to a new generated-hdrs\ntarget. That target is consistent with the *-objs targets I recently\nadded in 029bac01a8 (Makefile: add {program,xdiff,test,git,fuzz}-objs\n& objects targets, 2021-02-23).\n\nA subsequent commit will add a new generated hook-list.h. By doing\nthis refactoring we'll only need to add the new file to the\nGENERATED_H variable, not EXCEPT_HDRS, the vcbuild/README etc.\n\nHardcoding command-list.h there seems to have been a case of\ncopy/paste programming in 976aaedca0 (msvc: add a Makefile target to\npre-generate the Visual Studio solution, 2019-07-29). The\nconfig-list.h was added later in 709df95b78 (help: move\nlist_config_help to builtin/help, 2020-04-16).\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n Makefile              | 6 ++++--\n compat/vcbuild/README | 2 +-\n config.mak.uname      | 6 +++---\n 3 files changed, 8 insertions(+), 6 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 2777284b0f1..47e79a6252a 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -817,6 +817,8 @@ XDIFF_LIB = xdiff/lib.a\n \n GENERATED_H += command-list.h\n GENERATED_H += config-list.h\n+.PHONY: generated-hdrs\n+generated-hdrs: $(GENERATED_H)\n \n LIB_H := $(sort $(patsubst ./%,%,$(shell git ls-files '*.h' ':!t/' ':!Documentation/' 2>/dev/null || \\\n \t$(FIND) . \\\n@@ -2903,7 +2905,7 @@ $(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE\n .PHONY: sparse $(SP_OBJ)\n sparse: $(SP_OBJ)\n \n-EXCEPT_HDRS := command-list.h config-list.h unicode-width.h compat/% xdiff/%\n+EXCEPT_HDRS := $(GENERATED_H) unicode-width.h compat/% xdiff/%\n ifndef GCRYPT_SHA256\n \tEXCEPT_HDRS += sha256/gcrypt.h\n endif\n@@ -2926,7 +2928,7 @@ style:\n \tgit clang-format --style file --diff --extensions c,h\n \n .PHONY: check\n-check: config-list.h command-list.h\n+check: $(GENERATED_H)\n \t@if sparse; \\\n \tthen \\\n \t\techo >&2 \"Use 'make sparse' instead\"; \\\ndiff --git a/compat/vcbuild/README b/compat/vcbuild/README\nindex 51fb083dbbe..29ec1d0f104 100644\n--- a/compat/vcbuild/README\n+++ b/compat/vcbuild/README\n@@ -92,7 +92,7 @@ The Steps of Build Git with VS2008\n    the git operations.\n \n 3. Inside Git's directory run the command:\n-       make command-list.h config-list.h\n+       make generated-hdrs\n    to generate the header file needed to compile git.\n \n 4. Then either build Git with the GNU Make Makefile in the Git projects\ndiff --git a/config.mak.uname b/config.mak.uname\nindex 76516aaa9a5..8aac06eb094 100644\n--- a/config.mak.uname\n+++ b/config.mak.uname\n@@ -735,9 +735,9 @@ vcxproj:\n \t echo '</Project>') >git-remote-http/LinkOrCopyRemoteHttp.targets\n \tgit add -f git/LinkOrCopyBuiltins.targets git-remote-http/LinkOrCopyRemoteHttp.targets\n \n-\t# Add command-list.h and config-list.h\n-\t$(MAKE) MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 config-list.h command-list.h\n-\tgit add -f config-list.h command-list.h\n+\t# Add generated headers\n+\t$(MAKE) MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 $(GENERATED_H)\n+\tgit add -f $(GENERATED_H)\n \n \t# Add scripts\n \trm -f perl/perl.mak\n-- \n2.33.0.1291.g8857a6a91ac\n\n"},{"id":"437090","messageId":"patch-v2-3.8-312b353c651-20210926T185800Z-avarab@gmail.com","threadId":"56573","inReplyTo":"cover-v2-0.8-00000000000-20210926T185800Z-avarab@gmail.com","subject":"[PATCH v2 3/8] Makefile: don't perform \"mv $@+ $@\" dance for $(GENERATED_H)","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-09-26T19:03:24Z","receivedAt":"2021-09-26T19:03:42Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Change the \"cmd.sh > $@+ && mv $@+ $@\" pattern used for generating the\nconfig-list.h and command-list.h to just \"cmd.sh >$@\". This was needed\nas a guard to ensure that we don't have an empty file if the script\nfailed, but since 7b76d6bf221 (Makefile: add and use the\n\".DELETE_ON_ERROR\" flag, 2021-06-29) GNU make ensures that doesn't\nhappen.\n\nThere's still a lot of other places in the Makefile where we\nneedlessly use this pattern, but I'm just changing these because I'm\nabout to add a new $(GENERATED_H) target, let's have them all look and\nact the same way.\n\nEven with \".DELETE_ON_ERROR\" there is still a point to using the \"mv\n$@+ $@\" pattern in some cases, e.g. to ensure that you have a working\nbinary during recompilation (see [1] for the start of a long\ndiscussion about that), but that doesn't apply here. Nothing external\nuses $(GENERATED_H) directly, it's only ever used in the context of\nthe Makefile's own dependency (re-)generation.\n\n1. https://lore.kernel.org/git/8735t93h0u.fsf@evledraar.gmail.com/\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n Makefile | 5 ++---\n 1 file changed, 2 insertions(+), 3 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex 47e79a6252a..e6b8d00f221 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2231,15 +2231,14 @@ $(BUILT_INS): git$X\n config-list.h: generate-configlist.sh\n \n config-list.h: Documentation/*config.txt Documentation/config/*.txt\n-\t$(QUIET_GEN)$(SHELL_PATH) ./generate-configlist.sh \\\n-\t\t>$@+ && mv $@+ $@\n+\t$(QUIET_GEN)$(SHELL_PATH) ./generate-configlist.sh >$@\n \n command-list.h: generate-cmdlist.sh command-list.txt\n \n command-list.h: $(wildcard Documentation/git*.txt)\n \t$(QUIET_GEN)$(SHELL_PATH) ./generate-cmdlist.sh \\\n \t\t$(patsubst %,--exclude-program %,$(EXCLUDED_PROGRAMS)) \\\n-\t\tcommand-list.txt >$@+ && mv $@+ $@\n+\t\tcommand-list.txt >$@\n \n SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\\\n \t$(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\\\n-- \n2.33.0.1291.g8857a6a91ac\n\n"},{"id":"437091","messageId":"patch-v2-4.8-aea4868f988-20210926T185800Z-avarab@gmail.com","threadId":"56573","inReplyTo":"cover-v2-0.8-00000000000-20210926T185800Z-avarab@gmail.com","subject":"[PATCH v2 4/8] Makefile: remove an out-of-date comment","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-09-26T19:03:25Z","receivedAt":"2021-09-26T19:03:44Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This comment added in dfea575017 (Makefile: lazily compute header\ndependencies, 2010-01-26) has been out of date since\n92b88eba9f (Makefile: use `git ls-files` to list header files, if\npossible, 2019-03-04), when we did exactly what it tells us not to do\nand added $(GENERATED_H) to $(OBJECTS) dependencies.\n\nThe rest of it was also somewhere between inaccurate and outdated,\nsince as of b8ba629264 (Makefile: fold MISC_H into LIB_H, 2012-06-20)\nit's not followed by a list of header files, that got moved earlier in\nthe file into LIB_H in 60d24dd255 (Makefile: fold XDIFF_H and VCSSVN_H\ninto LIB_H, 2012-07-06).\n\nLet's just remove it entirely, to the extent that we have anything\nuseful to say here the comment on the\n\"USE_COMPUTED_HEADER_DEPENDENCIES\" variable a few lines above this\nchange does the job for us.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n Makefile | 7 -------\n 1 file changed, 7 deletions(-)\n\ndiff --git a/Makefile b/Makefile\nindex e6b8d00f221..b9bf13239d0 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -2501,13 +2501,6 @@ ifneq ($(dep_files_present),)\n include $(dep_files_present)\n endif\n else\n-# Dependencies on header files, for platforms that do not support\n-# the gcc -MMD option.\n-#\n-# Dependencies on automatically generated headers such as command-list.h\n-# should _not_ be included here, since they are necessary even when\n-# building an object for the first time.\n-\n $(OBJECTS): $(LIB_H) $(GENERATED_H)\n endif\n \n-- \n2.33.0.1291.g8857a6a91ac\n\n"},{"id":"437092","messageId":"patch-v2-5.8-64f3af94161-20210926T185800Z-avarab@gmail.com","threadId":"56573","inReplyTo":"cover-v2-0.8-00000000000-20210926T185800Z-avarab@gmail.com","subject":"[PATCH v2 5/8] hook.[ch]: move find_hook() from run-command.c to hook.c","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-09-26T19:03:26Z","receivedAt":"2021-09-26T19:03:45Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Move the find_hook() function from run-command.c to a new hook.c\nlibrary. This change establishes a stub library that's pretty\npointless right now, but will see much wider use with Emily Shaffer's\nupcoming \"configuration-based hooks\" series.\n\nEventually all the hook related code will live in hook.[ch]. Let's\nstart that process by moving the simple find_hook() function over\nas-is.\n\nSigned-off-by: Emily Shaffer <emilyshaffer@google.com>\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n Makefile               |  1 +\n builtin/am.c           |  1 +\n builtin/bugreport.c    |  2 +-\n builtin/commit.c       |  1 +\n builtin/merge.c        |  1 +\n builtin/receive-pack.c |  1 +\n builtin/worktree.c     |  1 +\n hook.c                 | 37 +++++++++++++++++++++++++++++++++++++\n hook.h                 | 11 +++++++++++\n refs.c                 |  1 +\n run-command.c          | 35 +----------------------------------\n run-command.h          |  7 -------\n sequencer.c            |  1 +\n transport.c            |  2 +-\n 14 files changed, 59 insertions(+), 43 deletions(-)\n create mode 100644 hook.c\n create mode 100644 hook.h\n\ndiff --git a/Makefile b/Makefile\nindex b9bf13239d0..1f2b0205da5 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -904,6 +904,7 @@ LIB_OBJS += hash-lookup.o\n LIB_OBJS += hashmap.o\n LIB_OBJS += help.o\n LIB_OBJS += hex.o\n+LIB_OBJS += hook.o\n LIB_OBJS += ident.o\n LIB_OBJS += json-writer.o\n LIB_OBJS += kwset.o\ndiff --git a/builtin/am.c b/builtin/am.c\nindex e4a0ff9cd7c..3527945a467 100644\n--- a/builtin/am.c\n+++ b/builtin/am.c\n@@ -11,6 +11,7 @@\n #include \"parse-options.h\"\n #include \"dir.h\"\n #include \"run-command.h\"\n+#include \"hook.h\"\n #include \"quote.h\"\n #include \"tempfile.h\"\n #include \"lockfile.h\"\ndiff --git a/builtin/bugreport.c b/builtin/bugreport.c\nindex 06ed10dc92d..c30a360d695 100644\n--- a/builtin/bugreport.c\n+++ b/builtin/bugreport.c\n@@ -3,7 +3,7 @@\n #include \"strbuf.h\"\n #include \"help.h\"\n #include \"compat/compiler.h\"\n-#include \"run-command.h\"\n+#include \"hook.h\"\n \n \n static void get_system_info(struct strbuf *sys_info)\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex e7320f66f95..5359d961d22 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -19,6 +19,7 @@\n #include \"revision.h\"\n #include \"wt-status.h\"\n #include \"run-command.h\"\n+#include \"hook.h\"\n #include \"refs.h\"\n #include \"log-tree.h\"\n #include \"strbuf.h\"\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex 3fbdacc7db4..fe664f6a863 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -13,6 +13,7 @@\n #include \"builtin.h\"\n #include \"lockfile.h\"\n #include \"run-command.h\"\n+#include \"hook.h\"\n #include \"diff.h\"\n #include \"diff-merges.h\"\n #include \"refs.h\"\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex 48960a9575b..e3895aec622 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -7,6 +7,7 @@\n #include \"pkt-line.h\"\n #include \"sideband.h\"\n #include \"run-command.h\"\n+#include \"hook.h\"\n #include \"exec-cmd.h\"\n #include \"commit.h\"\n #include \"object.h\"\ndiff --git a/builtin/worktree.c b/builtin/worktree.c\nindex 0d0a80da61f..d22ece93e1a 100644\n--- a/builtin/worktree.c\n+++ b/builtin/worktree.c\n@@ -8,6 +8,7 @@\n #include \"branch.h\"\n #include \"refs.h\"\n #include \"run-command.h\"\n+#include \"hook.h\"\n #include \"sigchain.h\"\n #include \"submodule.h\"\n #include \"utf8.h\"\ndiff --git a/hook.c b/hook.c\nnew file mode 100644\nindex 00000000000..ba70b314718\n--- /dev/null\n+++ b/hook.c\n@@ -0,0 +1,37 @@\n+#include \"cache.h\"\n+#include \"hook.h\"\n+#include \"run-command.h\"\n+\n+const char *find_hook(const char *name)\n+{\n+\tstatic struct strbuf path = STRBUF_INIT;\n+\n+\tstrbuf_reset(&path);\n+\tstrbuf_git_path(&path, \"hooks/%s\", name);\n+\tif (access(path.buf, X_OK) < 0) {\n+\t\tint err = errno;\n+\n+#ifdef STRIP_EXTENSION\n+\t\tstrbuf_addstr(&path, STRIP_EXTENSION);\n+\t\tif (access(path.buf, X_OK) >= 0)\n+\t\t\treturn path.buf;\n+\t\tif (errno == EACCES)\n+\t\t\terr = errno;\n+#endif\n+\n+\t\tif (err == EACCES && advice_enabled(ADVICE_IGNORED_HOOK)) {\n+\t\t\tstatic struct string_list advise_given = STRING_LIST_INIT_DUP;\n+\n+\t\t\tif (!string_list_lookup(&advise_given, name)) {\n+\t\t\t\tstring_list_insert(&advise_given, name);\n+\t\t\t\tadvise(_(\"The '%s' hook was ignored because \"\n+\t\t\t\t\t \"it's not set as executable.\\n\"\n+\t\t\t\t\t \"You can disable this warning with \"\n+\t\t\t\t\t \"`git config advice.ignoredHook false`.\"),\n+\t\t\t\t       path.buf);\n+\t\t\t}\n+\t\t}\n+\t\treturn NULL;\n+\t}\n+\treturn path.buf;\n+}\ndiff --git a/hook.h b/hook.h\nnew file mode 100644\nindex 00000000000..68624f16059\n--- /dev/null\n+++ b/hook.h\n@@ -0,0 +1,11 @@\n+#ifndef HOOK_H\n+#define HOOK_H\n+\n+/*\n+ * Returns the path to the hook file, or NULL if the hook is missing\n+ * or disabled. Note that this points to static storage that will be\n+ * overwritten by further calls to find_hook and run_hook_*.\n+ */\n+const char *find_hook(const char *name);\n+\n+#endif\ndiff --git a/refs.c b/refs.c\nindex 8b9f7c3a80a..6211692eaae 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -10,6 +10,7 @@\n #include \"refs.h\"\n #include \"refs/refs-internal.h\"\n #include \"run-command.h\"\n+#include \"hook.h\"\n #include \"object-store.h\"\n #include \"object.h\"\n #include \"tag.h\"\ndiff --git a/run-command.c b/run-command.c\nindex 04a07e6366e..e4862b8f46d 100644\n--- a/run-command.c\n+++ b/run-command.c\n@@ -9,6 +9,7 @@\n #include \"quote.h\"\n #include \"config.h\"\n #include \"packfile.h\"\n+#include \"hook.h\"\n \n void child_process_init(struct child_process *child)\n {\n@@ -1322,40 +1323,6 @@ int async_with_fork(void)\n #endif\n }\n \n-const char *find_hook(const char *name)\n-{\n-\tstatic struct strbuf path = STRBUF_INIT;\n-\n-\tstrbuf_reset(&path);\n-\tstrbuf_git_path(&path, \"hooks/%s\", name);\n-\tif (access(path.buf, X_OK) < 0) {\n-\t\tint err = errno;\n-\n-#ifdef STRIP_EXTENSION\n-\t\tstrbuf_addstr(&path, STRIP_EXTENSION);\n-\t\tif (access(path.buf, X_OK) >= 0)\n-\t\t\treturn path.buf;\n-\t\tif (errno == EACCES)\n-\t\t\terr = errno;\n-#endif\n-\n-\t\tif (err == EACCES && advice_enabled(ADVICE_IGNORED_HOOK)) {\n-\t\t\tstatic struct string_list advise_given = STRING_LIST_INIT_DUP;\n-\n-\t\t\tif (!string_list_lookup(&advise_given, name)) {\n-\t\t\t\tstring_list_insert(&advise_given, name);\n-\t\t\t\tadvise(_(\"The '%s' hook was ignored because \"\n-\t\t\t\t\t \"it's not set as executable.\\n\"\n-\t\t\t\t\t \"You can disable this warning with \"\n-\t\t\t\t\t \"`git config advice.ignoredHook false`.\"),\n-\t\t\t\t       path.buf);\n-\t\t\t}\n-\t\t}\n-\t\treturn NULL;\n-\t}\n-\treturn path.buf;\n-}\n-\n int run_hook_ve(const char *const *env, const char *name, va_list args)\n {\n \tstruct child_process hook = CHILD_PROCESS_INIT;\ndiff --git a/run-command.h b/run-command.h\nindex b9aff74914d..5e544acf4bc 100644\n--- a/run-command.h\n+++ b/run-command.h\n@@ -224,13 +224,6 @@ int finish_command_in_signal(struct child_process *);\n  */\n int run_command(struct child_process *);\n \n-/*\n- * Returns the path to the hook file, or NULL if the hook is missing\n- * or disabled. Note that this points to static storage that will be\n- * overwritten by further calls to find_hook and run_hook_*.\n- */\n-const char *find_hook(const char *name);\n-\n /**\n  * Run a hook.\n  * The first argument is a pathname to an index file, or NULL\ndiff --git a/sequencer.c b/sequencer.c\nindex 614d56f5e21..8ee6c4ac240 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -8,6 +8,7 @@\n #include \"sequencer.h\"\n #include \"tag.h\"\n #include \"run-command.h\"\n+#include \"hook.h\"\n #include \"exec-cmd.h\"\n #include \"utf8.h\"\n #include \"cache-tree.h\"\ndiff --git a/transport.c b/transport.c\nindex b37664ba870..e4f1decae20 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -1,7 +1,7 @@\n #include \"cache.h\"\n #include \"config.h\"\n #include \"transport.h\"\n-#include \"run-command.h\"\n+#include \"hook.h\"\n #include \"pkt-line.h\"\n #include \"fetch-pack.h\"\n #include \"remote.h\"\n-- \n2.33.0.1291.g8857a6a91ac\n\n"},{"id":"437093","messageId":"patch-v2-6.8-4eabe7ca39f-20210926T185800Z-avarab@gmail.com","threadId":"56573","inReplyTo":"cover-v2-0.8-00000000000-20210926T185800Z-avarab@gmail.com","subject":"[PATCH v2 6/8] hook.c: add a hook_exists() wrapper and use it in bugreport.c","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-09-26T19:03:27Z","receivedAt":"2021-09-26T19:03:46Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"From: Emily Shaffer <emilyshaffer@google.com>\n\nAdd a boolean version of the find_hook() function for those callers\nwho are only interested in checking whether the hook exists, not what\nthe path to it is.\n\nSigned-off-by: Emily Shaffer <emilyshaffer@google.com>\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/bugreport.c | 2 +-\n hook.c              | 5 +++++\n hook.h              | 5 +++++\n 3 files changed, 11 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/bugreport.c b/builtin/bugreport.c\nindex c30a360d695..a02c2540bb1 100644\n--- a/builtin/bugreport.c\n+++ b/builtin/bugreport.c\n@@ -82,7 +82,7 @@ static void get_populated_hooks(struct strbuf *hook_info, int nongit)\n \t}\n \n \tfor (i = 0; i < ARRAY_SIZE(hook); i++)\n-\t\tif (find_hook(hook[i]))\n+\t\tif (hook_exists(hook[i]))\n \t\t\tstrbuf_addf(hook_info, \"%s\\n\", hook[i]);\n }\n \ndiff --git a/hook.c b/hook.c\nindex ba70b314718..55e1145a4b7 100644\n--- a/hook.c\n+++ b/hook.c\n@@ -35,3 +35,8 @@ const char *find_hook(const char *name)\n \t}\n \treturn path.buf;\n }\n+\n+int hook_exists(const char *name)\n+{\n+\treturn !!find_hook(name);\n+}\ndiff --git a/hook.h b/hook.h\nindex 68624f16059..6aa36fc7ff9 100644\n--- a/hook.h\n+++ b/hook.h\n@@ -8,4 +8,9 @@\n  */\n const char *find_hook(const char *name);\n \n+/**\n+ * A boolean version of find_hook()\n+ */\n+int hook_exists(const char *hookname);\n+\n #endif\n-- \n2.33.0.1291.g8857a6a91ac\n\n"},{"id":"437094","messageId":"patch-v2-7.8-6237a1a5549-20210926T185800Z-avarab@gmail.com","threadId":"56573","inReplyTo":"cover-v2-0.8-00000000000-20210926T185800Z-avarab@gmail.com","subject":"[PATCH v2 7/8] hook.c users: use \"hook_exists()\" instead of \"find_hook()\"","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-09-26T19:03:28Z","receivedAt":"2021-09-26T19:03:49Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Use the new hook_exists() function instead of find_hook() where the\nlatter was called in boolean contexts. This make subsequent changes in\na series where we further refactor the hook API clearer, as we won't\nconflate wanting to get the path of the hook with checking for its\nexistence.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n builtin/commit.c       | 2 +-\n builtin/merge.c        | 2 +-\n builtin/receive-pack.c | 2 +-\n sequencer.c            | 2 +-\n 4 files changed, 4 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/commit.c b/builtin/commit.c\nindex 5359d961d22..883c16256c8 100644\n--- a/builtin/commit.c\n+++ b/builtin/commit.c\n@@ -1052,7 +1052,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix,\n \t\treturn 0;\n \t}\n \n-\tif (!no_verify && find_hook(\"pre-commit\")) {\n+\tif (!no_verify && hook_exists(\"pre-commit\")) {\n \t\t/*\n \t\t * Re-read the index as pre-commit hook could have updated it,\n \t\t * and write it out as a tree.  We must do this before we invoke\ndiff --git a/builtin/merge.c b/builtin/merge.c\nindex fe664f6a863..956b6259f21 100644\n--- a/builtin/merge.c\n+++ b/builtin/merge.c\n@@ -849,7 +849,7 @@ static void prepare_to_commit(struct commit_list *remoteheads)\n \t * and write it out as a tree.  We must do this before we invoke\n \t * the editor and after we invoke run_status above.\n \t */\n-\tif (find_hook(\"pre-merge-commit\"))\n+\tif (hook_exists(\"pre-merge-commit\"))\n \t\tdiscard_cache();\n \tread_cache_from(index_file);\n \tstrbuf_addbuf(&msg, &merge_msg);\ndiff --git a/builtin/receive-pack.c b/builtin/receive-pack.c\nindex e3895aec622..25cc0c907e1 100644\n--- a/builtin/receive-pack.c\n+++ b/builtin/receive-pack.c\n@@ -1464,7 +1464,7 @@ static const char *update_worktree(unsigned char *sha1, const struct worktree *w\n \n \tstrvec_pushf(&env, \"GIT_DIR=%s\", absolute_path(git_dir));\n \n-\tif (!find_hook(push_to_checkout_hook))\n+\tif (!hook_exists(push_to_checkout_hook))\n \t\tretval = push_to_deploy(sha1, &env, work_tree);\n \telse\n \t\tretval = push_to_checkout(sha1, &env, work_tree);\ndiff --git a/sequencer.c b/sequencer.c\nindex 8ee6c4ac240..e501945796d 100644\n--- a/sequencer.c\n+++ b/sequencer.c\n@@ -1460,7 +1460,7 @@ static int try_to_commit(struct repository *r,\n \t\t}\n \t}\n \n-\tif (find_hook(\"prepare-commit-msg\")) {\n+\tif (hook_exists(\"prepare-commit-msg\")) {\n \t\tres = run_prepare_commit_msg_hook(r, msg, hook_commit);\n \t\tif (res)\n \t\t\tgoto out;\n-- \n2.33.0.1291.g8857a6a91ac\n\n"},{"id":"437095","messageId":"patch-v2-8.8-7420267ce09-20210926T185800Z-avarab@gmail.com","threadId":"56573","inReplyTo":"cover-v2-0.8-00000000000-20210926T185800Z-avarab@gmail.com","subject":"[PATCH v2 8/8] hook-list.h: add a generated list of hooks, like config-list.h","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-09-26T19:03:29Z","receivedAt":"2021-09-26T19:03:50Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Make githooks(5) the source of truth for what hooks git supports, and\npunt out early on hooks we don't know about in find_hook(). This\nensures that the documentation and the C code's idea about existing\nhooks doesn't diverge.\n\nWe still have Perl and Python code running its own hooks, but that'll\nbe addressed by Emily Shaffer's upcoming \"git hook run\" command.\n\nThis resolves a long-standing TODO item in bugreport.c of there being\nno centralized listing of hooks, and fixes a bug with the bugreport\nlisting only knowing about 1/4 of the p4 hooks. It didn't know about\nthe recent \"reference-transaction\" hook either.\n\nWe could make the find_hook() function die() or BUG() out if the new\nknown_hook() returned 0, but let's make it return NULL just as it does\nwhen it can't find a hook of a known type. Making it die() is overly\nanal, and unlikely to be what we need in catching stupid typos in the\nname of some new hook hardcoded in git.git's sources. By making this\nbe tolerant of unknown hook names, changes in a later series to make\n\"git hook run\" run arbitrary user-configured hook names will be easier\nto implement.\n\nI have not been able to directly test the CMake change being made\nhere. Since 4c2c38e800 (ci: modification of main.yml to use cmake for\nvs-build job, 2020-06-26) some of the Windows CI has a hard dependency\non CMake, this change works there, and is to my eyes an obviously\ncorrect use of a pattern established in previous CMake changes,\nnamely:\n\n - 061c2240b1 (Introduce CMake support for configuring Git,\n    2020-06-12)\n - 709df95b78 (help: move list_config_help to builtin/help,\n    2020-04-16)\n - 976aaedca0 (msvc: add a Makefile target to pre-generate the Visual\n   Studio solution, 2019-07-29)\n\nThe LC_ALL=C is needed because at least in my locale the dash (\"-\") is\nignored for the purposes of sorting, which results in a different\norder. I'm not aware of anything in git that has a hard dependency on\nthe order, but e.g. the bugreport output would end up using whatever\nlocale was in effect when git was compiled.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\nHelped-by: René Scharfe <l.s.r@web.de>\n---\n .gitignore                          |  1 +\n Makefile                            |  8 +++++-\n builtin/bugreport.c                 | 44 ++++++-----------------------\n contrib/buildsystems/CMakeLists.txt |  7 +++++\n generate-hooklist.sh                | 20 +++++++++++++\n 5 files changed, 43 insertions(+), 37 deletions(-)\n create mode 100755 generate-hooklist.sh\n\ndiff --git a/.gitignore b/.gitignore\nindex 311841f9bed..6be9de41ae8 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -190,6 +190,7 @@\n /gitweb/static/gitweb.min.*\n /config-list.h\n /command-list.h\n+/hook-list.h\n *.tar.gz\n *.dsc\n *.deb\ndiff --git a/Makefile b/Makefile\nindex 1f2b0205da5..21e2bcc21e3 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -817,6 +817,8 @@ XDIFF_LIB = xdiff/lib.a\n \n GENERATED_H += command-list.h\n GENERATED_H += config-list.h\n+GENERATED_H += hook-list.h\n+\n .PHONY: generated-hdrs\n generated-hdrs: $(GENERATED_H)\n \n@@ -2208,8 +2210,9 @@ git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)\n \t\t$(filter %.o,$^) $(LIBS)\n \n help.sp help.s help.o: command-list.h\n+hook.sp hook.s hook.o: hook-list.h\n \n-builtin/help.sp builtin/help.s builtin/help.o: config-list.h GIT-PREFIX\n+builtin/help.sp builtin/help.s builtin/help.o: config-list.h hook-list.h GIT-PREFIX\n builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \\\n \t'-DGIT_HTML_PATH=\"$(htmldir_relative_SQ)\"' \\\n \t'-DGIT_MAN_PATH=\"$(mandir_relative_SQ)\"' \\\n@@ -2241,6 +2244,9 @@ command-list.h: $(wildcard Documentation/git*.txt)\n \t\t$(patsubst %,--exclude-program %,$(EXCLUDED_PROGRAMS)) \\\n \t\tcommand-list.txt >$@\n \n+hook-list.h: generate-hooklist.sh Documentation/githooks.txt\n+\t$(QUIET_GEN)$(SHELL_PATH) ./generate-hooklist.sh >$@\n+\n SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\\\n \t$(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\\\n \t$(gitwebdir_SQ):$(PERL_PATH_SQ):$(SANE_TEXT_GREP):$(PAGER_ENV):\\\ndiff --git a/builtin/bugreport.c b/builtin/bugreport.c\nindex a02c2540bb1..9de32bc96e7 100644\n--- a/builtin/bugreport.c\n+++ b/builtin/bugreport.c\n@@ -4,6 +4,7 @@\n #include \"help.h\"\n #include \"compat/compiler.h\"\n #include \"hook.h\"\n+#include \"hook-list.h\"\n \n \n static void get_system_info(struct strbuf *sys_info)\n@@ -41,39 +42,7 @@ static void get_system_info(struct strbuf *sys_info)\n \n static void get_populated_hooks(struct strbuf *hook_info, int nongit)\n {\n-\t/*\n-\t * NEEDSWORK: Doesn't look like there is a list of all possible hooks;\n-\t * so below is a transcription of `git help hooks`. Later, this should\n-\t * be replaced with some programmatically generated list (generated from\n-\t * doc or else taken from some library which tells us about all the\n-\t * hooks)\n-\t */\n-\tstatic const char *hook[] = {\n-\t\t\"applypatch-msg\",\n-\t\t\"pre-applypatch\",\n-\t\t\"post-applypatch\",\n-\t\t\"pre-commit\",\n-\t\t\"pre-merge-commit\",\n-\t\t\"prepare-commit-msg\",\n-\t\t\"commit-msg\",\n-\t\t\"post-commit\",\n-\t\t\"pre-rebase\",\n-\t\t\"post-checkout\",\n-\t\t\"post-merge\",\n-\t\t\"pre-push\",\n-\t\t\"pre-receive\",\n-\t\t\"update\",\n-\t\t\"post-receive\",\n-\t\t\"post-update\",\n-\t\t\"push-to-checkout\",\n-\t\t\"pre-auto-gc\",\n-\t\t\"post-rewrite\",\n-\t\t\"sendemail-validate\",\n-\t\t\"fsmonitor-watchman\",\n-\t\t\"p4-pre-submit\",\n-\t\t\"post-index-change\",\n-\t};\n-\tint i;\n+\tconst char **p;\n \n \tif (nongit) {\n \t\tstrbuf_addstr(hook_info,\n@@ -81,9 +50,12 @@ static void get_populated_hooks(struct strbuf *hook_info, int nongit)\n \t\treturn;\n \t}\n \n-\tfor (i = 0; i < ARRAY_SIZE(hook); i++)\n-\t\tif (hook_exists(hook[i]))\n-\t\t\tstrbuf_addf(hook_info, \"%s\\n\", hook[i]);\n+\tfor (p = hook_name_list; *p; p++) {\n+\t\tconst char *hook = *p;\n+\n+\t\tif (hook_exists(hook))\n+\t\t\tstrbuf_addf(hook_info, \"%s\\n\", hook);\n+\t}\n }\n \n static const char * const bugreport_usage[] = {\ndiff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt\nindex 171b4124afe..fd1399c440f 100644\n--- a/contrib/buildsystems/CMakeLists.txt\n+++ b/contrib/buildsystems/CMakeLists.txt\n@@ -624,6 +624,13 @@ if(NOT EXISTS ${CMAKE_BINARY_DIR}/config-list.h)\n \t\t\tOUTPUT_FILE ${CMAKE_BINARY_DIR}/config-list.h)\n endif()\n \n+if(NOT EXISTS ${CMAKE_BINARY_DIR}/hook-list.h)\n+\tmessage(\"Generating hook-list.h\")\n+\texecute_process(COMMAND ${SH_EXE} ${CMAKE_SOURCE_DIR}/generate-hooklist.sh\n+\t\t\tWORKING_DIRECTORY ${CMAKE_SOURCE_DIR}\n+\t\t\tOUTPUT_FILE ${CMAKE_BINARY_DIR}/hook-list.h)\n+endif()\n+\n include_directories(${CMAKE_BINARY_DIR})\n \n #build\ndiff --git a/generate-hooklist.sh b/generate-hooklist.sh\nnew file mode 100755\nindex 00000000000..2f9f54eb545\n--- /dev/null\n+++ b/generate-hooklist.sh\n@@ -0,0 +1,20 @@\n+#!/bin/sh\n+#\n+# Usage: ./generate-hooklist.sh >hook-list.h\n+\n+cat <<EOF\n+/* Automatically generated by generate-hooklist.sh */\n+\n+static const char *hook_name_list[] = {\n+EOF\n+\n+sed -n \\\n+\t-e '/^~~~~*$/ {x; s/^.*$/\t\"&\",/; p;}' \\\n+\t-e 'x' \\\n+\t<Documentation/githooks.txt |\n+\tLC_ALL=C sort\n+\n+cat <<EOF\n+\tNULL,\n+};\n+EOF\n-- \n2.33.0.1291.g8857a6a91ac\n\n"},{"id":"437121","messageId":"e0feb87e-4f8e-04c7-5533-3b10d82403e4@gmail.com","threadId":"56573","inReplyTo":"87fstt3gzd.fsf@evledraar.gmail.com","subject":"Re: [PATCH 8/8] hook-list.h: add a generated list of hooks, like config-list.h","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2021-09-27T09:24:28Z","receivedAt":"2021-09-27T09:24:35Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 24/09/2021 20:30, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Fri, Sep 24 2021, Phillip Wood wrote:\n> \n>> Hi Ævar\n>> On 23/09/2021 11:30, Ævar Arnfjörð Bjarmason wrote:\n>>> diff --git a/generate-hooklist.sh b/generate-hooklist.sh\n>>> new file mode 100755\n>>> index 00000000000..6d4e56d1a31\n>>> --- /dev/null\n>>> +++ b/generate-hooklist.sh\n>>> @@ -0,0 +1,18 @@\n>>> +#!/bin/sh\n>>> +#\n>>> +# Usage: ./generate-hooklist.sh >hook-list.h\n>>> +\n>>> +cat <<EOF\n>>> +/* Automatically generated by generate-hooklist.sh */\n>>> +\n>>> +static const char *hook_name_list[] = {\n>>> +EOF\n>>> +\n>>> +sed -n -e '/^~~~~*$/ {x; s/^.*$/\t\"&\",/; p;}; x' \\\n>>\n>> POSIX does not support using a semicolon after a closing brace [1],\n>> grepping our code base with\n>> \tgit grep 'sed .*};' '*.sh'\n>> does not give any matches so I don't think we're using that pattern\n>> any where else. Replacing the semicolon with ' -e' would fix it.\n>>\n>> Best Wishes\n>>\n>> Phillip\n> \n> Does this fail on any system you're aware of? If so what OS/version (and\n> preferably version of \"sed\").\n\nI'm not aware of any such system but I rarely use anything other than \nlinux. As this departure from POSIX is not already in the code base I \nthought it was worth flagging it. I did wonder if it would be supported \nby the various BSDs but you testing shows it is actually quite widely \nsupported.\n\nBest Wishes\n\nPhillip\n\n> René's downthread <d5f297d4-9c64-1ff9-8422-054979bf8cfa@web.de> seems to\n> suggest that this is fine.\n> \n> Both beforehand and just now I've tested this on AIX, Solaris,\n> {Open,Net,Free}BSD, HP/UX, OSX and Linux (a few distros/versions).\n> \n> All of them are able to generate the same hook-list.h using this version\n> of the patch.\n> \n"},{"id":"437122","messageId":"8d2ccf02-09ad-3876-9c9f-2dd79f9221bc@gmail.com","threadId":"56573","inReplyTo":"cover-v2-0.8-00000000000-20210926T185800Z-avarab@gmail.com","subject":"Re: [PATCH v2 0/8] Makefile: generate a hook-list.h, prep for config-based-hooks","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2021-09-27T09:30:58Z","receivedAt":"2021-09-27T09:31:06Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 26/09/2021 20:03, Ævar Arnfjörð Bjarmason wrote:\n> This series is an incremental restart of the now-ejected\n> es/config-based-hooks and ab/config-based-hooks-base topics. See [1]\n> for a summary of the plan and progression.\n> \n> In v2 the \"sed\" invocation that generates the new hook-list.h has been\n> changed to be portable under POSIX. See the thread starting at\n> https://lore.kernel.org/git/92471ff9-7573-c3e4-e9fd-63a5cbf5738f@gmail.com/;\n> \n> The portability issue is AFAICT theoretical in that any \"sed\" command\n> I've tried accepts the old version (I tried the large list of OS's\n> listed in [2]), but better safe than sorry.\n> \n> Other changes:\n> \n>   * I noticed that the run-command.h inclusion in transport.c become\n>     redundant, I removed that and validated the other ones that have\n>     the new hook.h, they all still need run-command.h.\n> \n>   * A whitespace change in v1 in a change to the Makefile makes the\n>     diff for 8/8 easier to read.\n> \n> 1. http://lore.kernel.org/git/cover-0.8-00000000000-20210923T095326Z-avarab@gmail.com\n> 2. https://lore.kernel.org/git/87fstt3gzd.fsf@evledraar.gmail.com/\n > [...]\n> 8:  80aae4d5c13 ! 8:  7420267ce09 hook-list.h: add a generated list of hooks, like config-list.h\n>      @@ Makefile: XDIFF_LIB = xdiff/lib.a\n>        generated-hdrs: $(GENERATED_H)\n>        \n>       @@ Makefile: git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)\n>      + \t\t$(filter %.o,$^) $(LIBS)\n>        \n>        help.sp help.s help.o: command-list.h\n>      ++hook.sp hook.s hook.o: hook-list.h\n>        \n>       -builtin/help.sp builtin/help.s builtin/help.o: config-list.h GIT-PREFIX\n>      -+hook.sp hook.s hook.o: hook-list.h\n>      -+\n>       +builtin/help.sp builtin/help.s builtin/help.o: config-list.h hook-list.h GIT-PREFIX\n\nThis is billed as a whitespace change above but this line has actually \nchanged since the last version - was that intentional?\n\n>        builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \\\n>        \t'-DGIT_HTML_PATH=\"$(htmldir_relative_SQ)\"' \\\n>      @@ generate-hooklist.sh (new)\n>       +static const char *hook_name_list[] = {\n>       +EOF\n>       +\n>      -+sed -n -e '/^~~~~*$/ {x; s/^.*$/\t\"&\",/; p;}; x' \\\n>      ++sed -n \\\n>      ++\t-e '/^~~~~*$/ {x; s/^.*$/\t\"&\",/; p;}' \\\n>      ++\t-e 'x' \\\n>       +\t<Documentation/githooks.txt |\n>       +\tLC_ALL=C sort\n>       +\n\nThe sed change looks good\n\nThanks\n\nPhillip\n"},{"id":"437125","messageId":"87tui61ez0.fsf@evledraar.gmail.com","threadId":"56573","inReplyTo":"e0feb87e-4f8e-04c7-5533-3b10d82403e4@gmail.com","subject":"Re: [PATCH 8/8] hook-list.h: add a generated list of hooks, like config-list.h","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-09-27T10:36:32Z","receivedAt":"2021-09-27T10:37:44Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Sep 27 2021, Phillip Wood wrote:\n\n> On 24/09/2021 20:30, Ævar Arnfjörð Bjarmason wrote:\n>> On Fri, Sep 24 2021, Phillip Wood wrote:\n>> \n>>> Hi Ævar\n>>> On 23/09/2021 11:30, Ævar Arnfjörð Bjarmason wrote:\n>>>> diff --git a/generate-hooklist.sh b/generate-hooklist.sh\n>>>> new file mode 100755\n>>>> index 00000000000..6d4e56d1a31\n>>>> --- /dev/null\n>>>> +++ b/generate-hooklist.sh\n>>>> @@ -0,0 +1,18 @@\n>>>> +#!/bin/sh\n>>>> +#\n>>>> +# Usage: ./generate-hooklist.sh >hook-list.h\n>>>> +\n>>>> +cat <<EOF\n>>>> +/* Automatically generated by generate-hooklist.sh */\n>>>> +\n>>>> +static const char *hook_name_list[] = {\n>>>> +EOF\n>>>> +\n>>>> +sed -n -e '/^~~~~*$/ {x; s/^.*$/\t\"&\",/; p;}; x' \\\n>>>\n>>> POSIX does not support using a semicolon after a closing brace [1],\n>>> grepping our code base with\n>>> \tgit grep 'sed .*};' '*.sh'\n>>> does not give any matches so I don't think we're using that pattern\n>>> any where else. Replacing the semicolon with ' -e' would fix it.\n>>>\n>>> Best Wishes\n>>>\n>>> Phillip\n>> Does this fail on any system you're aware of? If so what OS/version\n>> (and\n>> preferably version of \"sed\").\n>\n> I'm not aware of any such system but I rarely use anything other than\n> linux. As this departure from POSIX is not already in the code base I \n> thought it was worth flagging it. I did wonder if it would be\n> supported by the various BSDs but you testing shows it is actually\n> quite widely supported.\n>\n> Best Wishes\n>\n> Phillip\n\nThanks, I completely agree that it's worth changing either way (as I did\nin my v2). I just wondered if it was a careful reading of the standard\nor having run into it on a system in the wild, in that case I'd try to\ntest on that system in the future.\n\nGood eyes :)\n"},{"id":"437128","messageId":"87pmsu1eao.fsf@evledraar.gmail.com","threadId":"56573","inReplyTo":"8d2ccf02-09ad-3876-9c9f-2dd79f9221bc@gmail.com","subject":"Re: [PATCH v2 0/8] Makefile: generate a hook-list.h, prep for config-based-hooks","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-09-27T10:38:22Z","receivedAt":"2021-09-27T10:52:19Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Mon, Sep 27 2021, Phillip Wood wrote:\n\n> On 26/09/2021 20:03, Ævar Arnfjörð Bjarmason wrote:\n>> This series is an incremental restart of the now-ejected\n>> es/config-based-hooks and ab/config-based-hooks-base topics. See [1]\n>> for a summary of the plan and progression.\n>> In v2 the \"sed\" invocation that generates the new hook-list.h has\n>> been\n>> changed to be portable under POSIX. See the thread starting at\n>> https://lore.kernel.org/git/92471ff9-7573-c3e4-e9fd-63a5cbf5738f@gmail.com/;\n>> The portability issue is AFAICT theoretical in that any \"sed\"\n>> command\n>> I've tried accepts the old version (I tried the large list of OS's\n>> listed in [2]), but better safe than sorry.\n>> Other changes:\n>>   * I noticed that the run-command.h inclusion in transport.c become\n>>     redundant, I removed that and validated the other ones that have\n>>     the new hook.h, they all still need run-command.h.\n>>   * A whitespace change in v1 in a change to the Makefile makes the\n>>     diff for 8/8 easier to read.\n>> 1. http://lore.kernel.org/git/cover-0.8-00000000000-20210923T095326Z-avarab@gmail.com\n>> 2. https://lore.kernel.org/git/87fstt3gzd.fsf@evledraar.gmail.com/\n>> [...]\n>> 8:  80aae4d5c13 ! 8:  7420267ce09 hook-list.h: add a generated list of hooks, like config-list.h\n>>      @@ Makefile: XDIFF_LIB = xdiff/lib.a\n>>        generated-hdrs: $(GENERATED_H)\n>>              @@ Makefile: git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS)\n>> $(GITLIBS)\n>>      + \t\t$(filter %.o,$^) $(LIBS)\n>>               help.sp help.s help.o: command-list.h\n>>      ++hook.sp hook.s hook.o: hook-list.h\n>>              -builtin/help.sp builtin/help.s builtin/help.o:\n>> config-list.h GIT-PREFIX\n>>      -+hook.sp hook.s hook.o: hook-list.h\n>>      -+\n>>       +builtin/help.sp builtin/help.s builtin/help.o: config-list.h hook-list.h GIT-PREFIX\n>\n> This is billed as a whitespace change above but this line has actually\n> changed since the last version - was that intentional?\n\nI think you're mistaken here, it is a whitespace-only change to the\nend-state, but the diff and range-diff are confusing. If I diff the two\nMakefiles I end up with when applying v1 and v2 I get:\n\n@@ -2217,7 +2210,6 @@ git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)\n \t\t$(filter %.o,$^) $(LIBS)\n \n help.sp help.s help.o: command-list.h\n-\n hook.sp hook.s hook.o: hook-list.h\n \n builtin/help.sp builtin/help.s builtin/help.o: config-list.h hook-list.h GIT-PREFIX\n\nI.e. only the line between the old command-list.h and new hook-list.h\nline is gone.\n\nBut the diff for v1 is D/A/A/A and for v2 A/D/A (D = Deletion, A =\nAddition).\n\nI.e. it's one of those times when \"git diff\" produces a valid diff, and\none that's actually smaller than couldu have been produced with a\nv2-like diff given the change in v1.\n\nAs an aside I've sometimes wished we had a --diff-algorithm=maximal or\nsomething, i.e. there's a lot of cases where by just adding one line to\nthe diff you can produce a bigger but IMO less confusing one.\n\nIn any case, the end state looks better & the diff for v2 is more\nintuitive to look at.\n\n>>        builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \\\n>>        \t'-DGIT_HTML_PATH=\"$(htmldir_relative_SQ)\"' \\\n>>      @@ generate-hooklist.sh (new)\n>>       +static const char *hook_name_list[] = {\n>>       +EOF\n>>       +\n>>      -+sed -n -e '/^~~~~*$/ {x; s/^.*$/\t\"&\",/; p;}; x' \\\n>>      ++sed -n \\\n>>      ++\t-e '/^~~~~*$/ {x; s/^.*$/\t\"&\",/; p;}' \\\n>>      ++\t-e 'x' \\\n>>       +\t<Documentation/githooks.txt |\n>>       +\tLC_ALL=C sort\n>>       +\n>\n> The sed change looks good\n\nThanks for confirming.\n"},{"id":"437165","messageId":"xmqqfstqq80j.fsf@gitster.g","threadId":"56573","inReplyTo":"patch-v2-8.8-7420267ce09-20210926T185800Z-avarab@gmail.com","subject":"Re: [PATCH v2 8/8] hook-list.h: add a generated list of hooks, like config-list.h","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-09-27T16:48:44Z","receivedAt":"2021-09-27T16:48:49Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:\n\n> +sed -n \\\n> +\t-e '/^~~~~*$/ {x; s/^.*$/\t\"&\",/; p;}' \\\n> +\t-e 'x' \\\n> +\t<Documentation/githooks.txt |\n> +\tLC_ALL=C sort\n\nThanks.  It is not wrong per-se, but if you are willing to do a\nmulti-line script for readability, wouldn't a much more readable\n\"single long and multi-line string\" approach, i.e.\n\n    sed -ne '\n\t/^~~~~*$/ {\n\t\tx\n\t\ts/^.*$/ \"&\",/\n\t\tp\n\t}\n\tx\n    ' Documentation/githooks.txt |\n    LC_ALL=C sort\n\nwork better?\n"},{"id":"437172","messageId":"0e07cee4-079a-af12-880f-d4a99300df28@web.de","threadId":"56573","inReplyTo":"xmqqfstqq80j.fsf@gitster.g","subject":"Re: [PATCH v2 8/8] hook-list.h: add a generated list of hooks, like config-list.h","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2021-09-27T18:00:49Z","receivedAt":"2021-09-27T18:01:23Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"Am 27.09.21 um 18:48 schrieb Junio C Hamano:\n> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:\n>\n>> +sed -n \\\n>> +\t-e '/^~~~~*$/ {x; s/^.*$/\t\"&\",/; p;}' \\\n>> +\t-e 'x' \\\n>> +\t<Documentation/githooks.txt |\n>> +\tLC_ALL=C sort\n>\n> Thanks.  It is not wrong per-se, but if you are willing to do a\n> multi-line script for readability, wouldn't a much more readable\n> \"single long and multi-line string\" approach, i.e.\n>\n>     sed -ne '\n> \t/^~~~~*$/ {\n> \t\tx\n> \t\ts/^.*$/ \"&\",/\n> \t\tp\n> \t}\n> \tx\n>     ' Documentation/githooks.txt |\n>     LC_ALL=C sort\n>\n> work better?\n>\n\nIt is more readable, but according to OpenBSD's sed(1) manpage it would\nalso be non-portable (https://man.openbsd.org/sed#STANDARDS).  That note\nwas added in 2006 (https://github.com/openbsd/src/commit/24ce9718),\nthough, so perhaps it needs an update.\n\nFWIW, generate-cmdlist.sh uses such a multi-line sed script in its\nfunction get_synopsis.\n\nRené\n"},{"id":"437173","messageId":"6e7d5945-d64f-9511-9668-b453c20c086c@gmail.com","threadId":"56573","inReplyTo":"87pmsu1eao.fsf@evledraar.gmail.com","subject":"Re: [PATCH v2 0/8] Makefile: generate a hook-list.h, prep for config-based-hooks","fromName":"Phillip Wood","fromEmail":"phillip.wood123@gmail.com","sentAt":"2021-09-27T18:01:59Z","receivedAt":"2021-09-27T18:02:04Z","isPatch":true,"sender":{"key":"phillip.wood@dunelm.org.uk","avatar":null},"body":"On 27/09/2021 11:38, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Mon, Sep 27 2021, Phillip Wood wrote:\n> \n>> On 26/09/2021 20:03, Ævar Arnfjörð Bjarmason wrote:\n>>> This series is an incremental restart of the now-ejected\n>>> es/config-based-hooks and ab/config-based-hooks-base topics. See [1]\n>>> for a summary of the plan and progression.\n>>> In v2 the \"sed\" invocation that generates the new hook-list.h has\n>>> been\n>>> changed to be portable under POSIX. See the thread starting at\n>>> https://lore.kernel.org/git/92471ff9-7573-c3e4-e9fd-63a5cbf5738f@gmail.com/;\n>>> The portability issue is AFAICT theoretical in that any \"sed\"\n>>> command\n>>> I've tried accepts the old version (I tried the large list of OS's\n>>> listed in [2]), but better safe than sorry.\n>>> Other changes:\n>>>    * I noticed that the run-command.h inclusion in transport.c become\n>>>      redundant, I removed that and validated the other ones that have\n>>>      the new hook.h, they all still need run-command.h.\n>>>    * A whitespace change in v1 in a change to the Makefile makes the\n>>>      diff for 8/8 easier to read.\n>>> 1. http://lore.kernel.org/git/cover-0.8-00000000000-20210923T095326Z-avarab@gmail.com\n>>> 2. https://lore.kernel.org/git/87fstt3gzd.fsf@evledraar.gmail.com/\n>>> [...]\n>>> 8:  80aae4d5c13 ! 8:  7420267ce09 hook-list.h: add a generated list of hooks, like config-list.h\n>>>       @@ Makefile: XDIFF_LIB = xdiff/lib.a\n>>>         generated-hdrs: $(GENERATED_H)\n>>>               @@ Makefile: git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS)\n>>> $(GITLIBS)\n>>>       + \t\t$(filter %.o,$^) $(LIBS)\n>>>                help.sp help.s help.o: command-list.h\n>>>       ++hook.sp hook.s hook.o: hook-list.h\n>>>               -builtin/help.sp builtin/help.s builtin/help.o:\n>>> config-list.h GIT-PREFIX\n>>>       -+hook.sp hook.s hook.o: hook-list.h\n>>>       -+\n>>>        +builtin/help.sp builtin/help.s builtin/help.o: config-list.h hook-list.h GIT-PREFIX\n>>\n>> This is billed as a whitespace change above but this line has actually\n>> changed since the last version - was that intentional?\n> \n> I think you're mistaken here,\n\nYes you're right, looking more closely it is a context line in the range \ndiff that has changed, sorry.\n\n> it is a whitespace-only change to the\n> end-state, but the diff and range-diff are confusing. If I diff the two\n> Makefiles I end up with when applying v1 and v2 I get:\n> \n> @@ -2217,7 +2210,6 @@ git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS)\n>   \t\t$(filter %.o,$^) $(LIBS)\n>   \n>   help.sp help.s help.o: command-list.h\n> -\n>   hook.sp hook.s hook.o: hook-list.h\n>   \n>   builtin/help.sp builtin/help.s builtin/help.o: config-list.h hook-list.h GIT-PREFIX\n> \n> I.e. only the line between the old command-list.h and new hook-list.h\n> line is gone.\n> \n> But the diff for v1 is D/A/A/A and for v2 A/D/A (D = Deletion, A =\n> Addition).\n> \n> I.e. it's one of those times when \"git diff\" produces a valid diff, and\n> one that's actually smaller than couldu have been produced with a\n> v2-like diff given the change in v1.\n> \n> As an aside I've sometimes wished we had a --diff-algorithm=maximal or\n> something, i.e. there's a lot of cases where by just adding one line to\n> the diff you can produce a bigger but IMO less confusing one.\n\nYes especially when it splits the diff just to keep an empty line or \nclosing brace as context even though the line itself is not really \ninteresting. Python's diff-lib has an option to ignore \"junk\" when \ndetermining which lines are unchanged, I keep meaning to see how it \naffects the diff but have never got round to it (the library uses a \ndifferent slower algorithm for calculating the diff as well).\n\nBest Wishes\n\nPhillip\n\n> \n> In any case, the end state looks better & the diff for v2 is more\n> intuitive to look at.\n> \n>>>         builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \\\n>>>         \t'-DGIT_HTML_PATH=\"$(htmldir_relative_SQ)\"' \\\n>>>       @@ generate-hooklist.sh (new)\n>>>        +static const char *hook_name_list[] = {\n>>>        +EOF\n>>>        +\n>>>       -+sed -n -e '/^~~~~*$/ {x; s/^.*$/\t\"&\",/; p;}; x' \\\n>>>       ++sed -n \\\n>>>       ++\t-e '/^~~~~*$/ {x; s/^.*$/\t\"&\",/; p;}' \\\n>>>       ++\t-e 'x' \\\n>>>        +\t<Documentation/githooks.txt |\n>>>        +\tLC_ALL=C sort\n>>>        +\n>>\n>> The sed change looks good\n> \n> Thanks for confirming.\n> \n\n"},{"id":"437184","messageId":"xmqqk0j1py2a.fsf@gitster.g","threadId":"56573","inReplyTo":"0e07cee4-079a-af12-880f-d4a99300df28@web.de","subject":"Re: [PATCH v2 8/8] hook-list.h: add a generated list of hooks, like config-list.h","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-09-27T20:23:41Z","receivedAt":"2021-09-27T20:23:47Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe <l.s.r@web.de> writes:\n\n> Am 27.09.21 um 18:48 schrieb Junio C Hamano:\n>> Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:\n>>\n>>> +sed -n \\\n>>> +\t-e '/^~~~~*$/ {x; s/^.*$/\t\"&\",/; p;}' \\\n>>> +\t-e 'x' \\\n>>> +\t<Documentation/githooks.txt |\n>>> +\tLC_ALL=C sort\n>>\n>> Thanks.  It is not wrong per-se, but if you are willing to do a\n>> multi-line script for readability, wouldn't a much more readable\n>> \"single long and multi-line string\" approach, i.e.\n>>\n>>     sed -ne '\n>> \t/^~~~~*$/ {\n>> \t\tx\n>> \t\ts/^.*$/ \"&\",/\n>> \t\tp\n>> \t}\n>> \tx\n>>     ' Documentation/githooks.txt |\n>>     LC_ALL=C sort\n>>\n>> work better?\n>>\n>\n> It is more readable, but according to OpenBSD's sed(1) manpage it would\n> also be non-portable (https://man.openbsd.org/sed#STANDARDS).  That note\n> was added in 2006 (https://github.com/openbsd/src/commit/24ce9718),\n> though, so perhaps it needs an update.\n>\n> FWIW, generate-cmdlist.sh uses such a multi-line sed script in its\n> function get_synopsis.\n\nI wonder who wrote the invocation in generate-cmdlist.sh ;-)\n\nFWIW, the informative part of POSIX.1 sed manpage\n\nhttps://pubs.opengroup.org/onlinepubs/9699919799/utilities/sed.html#tag_20_116_17\n\nhas such a single long and multi-line string script given on the\ncommand line, but \"non-portable\" can refer to \"some implementations\nmay not implement all of what POSIX expects\", so ... ;-)\n\n"},{"id":"441201","messageId":"211115.86k0h9qcf5.gmgdl@evledraar.gmail.com","threadId":"56573","inReplyTo":"20211115220455.xse7mhbwabrheej4@glandium.org","subject":"Re: [PATCH 8/8] hook-list.h: add a generated list of hooks, like config-list.h","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-15T22:26:36Z","receivedAt":"2021-11-15T23:36:32Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Nov 16 2021, Mike Hommey wrote:\n\n> On Thu, Sep 23, 2021 at 12:30:03PM +0200, Ævar Arnfjörð Bjarmason wrote:\n>> -builtin/help.sp builtin/help.s builtin/help.o: config-list.h GIT-PREFIX\n>> +hook.sp hook.s hook.o: hook-list.h\n>> +\n>> +builtin/help.sp builtin/help.s builtin/help.o: config-list.h hook-list.h GIT-PREFIX\n>\n> hook-list.h is only included from buitin/bugreport.c, so\n> builtin/bugreport.o should be the one with the hook-list.h dependency,\n> shouldn't it?\n\nWell spotted, yes. This is a mistake. I think from some earlier WIP\nversion of the series.\n\nIn practice we don't really miss dependencies due to these sorts of\nmistakes since we use the .depends files, i.e. GCC & Clang figure this\nout for us:\n\n    $ grep hook-list .depend/* */.depend/*\n    builtin/.depend/bugreport.o.d: compat/compiler.h git-compat-util.h hook.h hook-list.h\n    builtin/.depend/bugreport.o.d:hook-list.h:\n\nBut we do over-depend a bit, if you touch hook-list.h and make\nbuiltin/help.o we'll re-generate it due to this line.\n\nIf you or anyone else is more generally interested in the Makefile I'd\nreally like to get some reviews over at:\nhttps://lore.kernel.org/git/cover-v2-00.18-00000000000-20211112T214150Z-avarab@gmail.com/\n\nI've got some follow-up work that solve these dependencies more\ngenerally, e.g. in this case we should really not need to slavishly\nmaintain these fallback dependency lists by hand, or have automated ways\nof validating their correctness.\n"},{"id":"441202","messageId":"20211115224015.vzfevpvptysxzpaa@glandium.org","threadId":"56573","inReplyTo":"211115.86k0h9qcf5.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 8/8] hook-list.h: add a generated list of hooks, like config-list.h","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2021-11-15T22:40:15Z","receivedAt":"2021-11-15T23:39:45Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Mon, Nov 15, 2021 at 11:26:36PM +0100, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Tue, Nov 16 2021, Mike Hommey wrote:\n> \n> > On Thu, Sep 23, 2021 at 12:30:03PM +0200, Ævar Arnfjörð Bjarmason wrote:\n> >> -builtin/help.sp builtin/help.s builtin/help.o: config-list.h GIT-PREFIX\n> >> +hook.sp hook.s hook.o: hook-list.h\n> >> +\n> >> +builtin/help.sp builtin/help.s builtin/help.o: config-list.h hook-list.h GIT-PREFIX\n> >\n> > hook-list.h is only included from buitin/bugreport.c, so\n> > builtin/bugreport.o should be the one with the hook-list.h dependency,\n> > shouldn't it?\n> \n> Well spotted, yes. This is a mistake. I think from some earlier WIP\n> version of the series.\n> \n> In practice we don't really miss dependencies due to these sorts of\n> mistakes since we use the .depends files, i.e. GCC & Clang figure this\n> out for us:\n> \n>     $ grep hook-list .depend/* */.depend/*\n>     builtin/.depend/bugreport.o.d: compat/compiler.h git-compat-util.h hook.h hook-list.h\n>     builtin/.depend/bugreport.o.d:hook-list.h:\n\nBut aren't those .depends files are only created when compiling object\nfiles, such that builtin/.depend/bugreport.o.d wouldn't exist until\nbugreport.c is compiled, which would fail if hook-list.h wasn't created\nbefore that?\n\nMike\n"},{"id":"441203","messageId":"20211115220455.xse7mhbwabrheej4@glandium.org","threadId":"56573","inReplyTo":"patch-8.8-80aae4d5c13-20210923T095326Z-avarab@gmail.com","subject":"Re: [PATCH 8/8] hook-list.h: add a generated list of hooks, like config-list.h","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2021-11-15T22:04:55Z","receivedAt":"2021-11-15T23:43:01Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Thu, Sep 23, 2021 at 12:30:03PM +0200, Ævar Arnfjörð Bjarmason wrote:\n> -builtin/help.sp builtin/help.s builtin/help.o: config-list.h GIT-PREFIX\n> +hook.sp hook.s hook.o: hook-list.h\n> +\n> +builtin/help.sp builtin/help.s builtin/help.o: config-list.h hook-list.h GIT-PREFIX\n\nhook-list.h is only included from buitin/bugreport.c, so\nbuiltin/bugreport.o should be the one with the hook-list.h dependency,\nshouldn't it?\n\nMike\n"},{"id":"441206","messageId":"211115.86bl2lqbbt.gmgdl@evledraar.gmail.com","threadId":"56573","inReplyTo":"20211115224015.vzfevpvptysxzpaa@glandium.org","subject":"Re: [PATCH 8/8] hook-list.h: add a generated list of hooks, like config-list.h","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-15T22:49:31Z","receivedAt":"2021-11-16T00:16:41Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Nov 16 2021, Mike Hommey wrote:\n\n> On Mon, Nov 15, 2021 at 11:26:36PM +0100, Ævar Arnfjörð Bjarmason wrote:\n>> \n>> On Tue, Nov 16 2021, Mike Hommey wrote:\n>> \n>> > On Thu, Sep 23, 2021 at 12:30:03PM +0200, Ævar Arnfjörð Bjarmason wrote:\n>> >> -builtin/help.sp builtin/help.s builtin/help.o: config-list.h GIT-PREFIX\n>> >> +hook.sp hook.s hook.o: hook-list.h\n>> >> +\n>> >> +builtin/help.sp builtin/help.s builtin/help.o: config-list.h hook-list.h GIT-PREFIX\n>> >\n>> > hook-list.h is only included from buitin/bugreport.c, so\n>> > builtin/bugreport.o should be the one with the hook-list.h dependency,\n>> > shouldn't it?\n>> \n>> Well spotted, yes. This is a mistake. I think from some earlier WIP\n>> version of the series.\n>> \n>> In practice we don't really miss dependencies due to these sorts of\n>> mistakes since we use the .depends files, i.e. GCC & Clang figure this\n>> out for us:\n>> \n>>     $ grep hook-list .depend/* */.depend/*\n>>     builtin/.depend/bugreport.o.d: compat/compiler.h git-compat-util.h hook.h hook-list.h\n>>     builtin/.depend/bugreport.o.d:hook-list.h:\n>\n> But aren't those .depends files are only created when compiling object\n> files, such that builtin/.depend/bugreport.o.d wouldn't exist until\n> bugreport.c is compiled, which would fail if hook-list.h wasn't created\n> before that?\n\nFail how?\n\nI don't think it could fail, because the purpose of these dependency\nrelationships is to avoid needless *re*builds. So if you're building for\nthe first time it doesn't matter, your compiler will find the relevant\nthings to include for you. It doesn't need what's in the Makefile to do\nthat.\n\nSee [1], what I said about LIB_H there applies more generally for the\n.depends files.\n\nIt will only fail in the sense that it over-depends, i.e. if you do:\n\n    git clean -dxf; make builtin/help.o\n\nWe'll generate the hook list, as opposed to for buildin/add.o or\nwhatever, but I'd say that generally doesn't matter all that much, what\nmatters is that we don't miss dependency relationships.\n\nAnyway, I'll try to loop around to fixing this, just wanted to gather\nsome interest in the more general set of fixes I've got. I've got some\nfixes queued up after that that fix/improve the *.sp and *.s parts of\nthe line you quoted (and other similar lines). I'd prefer just to fix\nthis along with those so the two don't textually conflict.\n\n1. https://lore.kernel.org/git/211110.86h7cki0uo.gmgdl@evledraar.gmail.com/\n"},{"id":"441217","messageId":"20211115230032.3or3qqlsdnxjtrol@glandium.org","threadId":"56573","inReplyTo":"211115.86bl2lqbbt.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 8/8] hook-list.h: add a generated list of hooks, like config-list.h","fromName":"Mike Hommey","fromEmail":"mh@glandium.org","sentAt":"2021-11-15T23:00:32Z","receivedAt":"2021-11-16T00:47:15Z","isPatch":true,"sender":{"key":"mh@glandium.org","avatar":"https://avatars.githubusercontent.com/u/1038527?v=4"},"body":"On Mon, Nov 15, 2021 at 11:49:31PM +0100, Ævar Arnfjörð Bjarmason wrote:\n> \n> On Tue, Nov 16 2021, Mike Hommey wrote:\n> \n> > On Mon, Nov 15, 2021 at 11:26:36PM +0100, Ævar Arnfjörð Bjarmason wrote:\n> >> \n> >> On Tue, Nov 16 2021, Mike Hommey wrote:\n> >> \n> >> > On Thu, Sep 23, 2021 at 12:30:03PM +0200, Ævar Arnfjörð Bjarmason wrote:\n> >> >> -builtin/help.sp builtin/help.s builtin/help.o: config-list.h GIT-PREFIX\n> >> >> +hook.sp hook.s hook.o: hook-list.h\n> >> >> +\n> >> >> +builtin/help.sp builtin/help.s builtin/help.o: config-list.h hook-list.h GIT-PREFIX\n> >> >\n> >> > hook-list.h is only included from buitin/bugreport.c, so\n> >> > builtin/bugreport.o should be the one with the hook-list.h dependency,\n> >> > shouldn't it?\n> >> \n> >> Well spotted, yes. This is a mistake. I think from some earlier WIP\n> >> version of the series.\n> >> \n> >> In practice we don't really miss dependencies due to these sorts of\n> >> mistakes since we use the .depends files, i.e. GCC & Clang figure this\n> >> out for us:\n> >> \n> >>     $ grep hook-list .depend/* */.depend/*\n> >>     builtin/.depend/bugreport.o.d: compat/compiler.h git-compat-util.h hook.h hook-list.h\n> >>     builtin/.depend/bugreport.o.d:hook-list.h:\n> >\n> > But aren't those .depends files are only created when compiling object\n> > files, such that builtin/.depend/bugreport.o.d wouldn't exist until\n> > bugreport.c is compiled, which would fail if hook-list.h wasn't created\n> > before that?\n> \n> Fail how?\n> \n> I don't think it could fail, because the purpose of these dependency\n> relationships is to avoid needless *re*builds. So if you're building for\n> the first time it doesn't matter, your compiler will find the relevant\n> things to include for you. It doesn't need what's in the Makefile to do\n> that.\n> \n> See [1], what I said about LIB_H there applies more generally for the\n> .depends files.\n> \n> It will only fail in the sense that it over-depends, i.e. if you do:\n> \n>     git clean -dxf; make builtin/help.o\n\nTry\n\n    git clean -dxf; make builtin/bugreport.o\n\nIt fails with:\n\n      CC builtin/bugreport.o\n  builtin/bugreport.c:7:10: fatal error: hook-list.h: そのようなファイルやディレクトリはありません\n      7 | #include \"hook-list.h\"\n        |          ^~~~~~~~~~~~~\n  compilation terminated.\n  make: *** [Makefile:2500: builtin/bugreport.o] エラー 1\n\nThe only reason I can see why it builds at all normally is that hook.o\nis built soon enough that by the time builtin/bugreport.o is built\nhook-list.h has already been generated.\n\nMike\n\n"},{"id":"441320","messageId":"211116.86y25opapy.gmgdl@evledraar.gmail.com","threadId":"56573","inReplyTo":"20211115230032.3or3qqlsdnxjtrol@glandium.org","subject":"Re: [PATCH 8/8] hook-list.h: add a generated list of hooks, like config-list.h","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-11-16T12:01:00Z","receivedAt":"2021-11-16T12:13:39Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Tue, Nov 16 2021, Mike Hommey wrote:\n\n> On Mon, Nov 15, 2021 at 11:49:31PM +0100, Ævar Arnfjörð Bjarmason wrote:\n>> \n>> On Tue, Nov 16 2021, Mike Hommey wrote:\n>> \n>> > On Mon, Nov 15, 2021 at 11:26:36PM +0100, Ævar Arnfjörð Bjarmason wrote:\n>> >> \n>> >> On Tue, Nov 16 2021, Mike Hommey wrote:\n>> >> \n>> >> > On Thu, Sep 23, 2021 at 12:30:03PM +0200, Ævar Arnfjörð Bjarmason wrote:\n>> >> >> -builtin/help.sp builtin/help.s builtin/help.o: config-list.h GIT-PREFIX\n>> >> >> +hook.sp hook.s hook.o: hook-list.h\n>> >> >> +\n>> >> >> +builtin/help.sp builtin/help.s builtin/help.o: config-list.h hook-list.h GIT-PREFIX\n>> >> >\n>> >> > hook-list.h is only included from buitin/bugreport.c, so\n>> >> > builtin/bugreport.o should be the one with the hook-list.h dependency,\n>> >> > shouldn't it?\n>> >> \n>> >> Well spotted, yes. This is a mistake. I think from some earlier WIP\n>> >> version of the series.\n>> >> \n>> >> In practice we don't really miss dependencies due to these sorts of\n>> >> mistakes since we use the .depends files, i.e. GCC & Clang figure this\n>> >> out for us:\n>> >> \n>> >>     $ grep hook-list .depend/* */.depend/*\n>> >>     builtin/.depend/bugreport.o.d: compat/compiler.h git-compat-util.h hook.h hook-list.h\n>> >>     builtin/.depend/bugreport.o.d:hook-list.h:\n>> >\n>> > But aren't those .depends files are only created when compiling object\n>> > files, such that builtin/.depend/bugreport.o.d wouldn't exist until\n>> > bugreport.c is compiled, which would fail if hook-list.h wasn't created\n>> > before that?\n>> \n>> Fail how?\n>> \n>> I don't think it could fail, because the purpose of these dependency\n>> relationships is to avoid needless *re*builds. So if you're building for\n>> the first time it doesn't matter, your compiler will find the relevant\n>> things to include for you. It doesn't need what's in the Makefile to do\n>> that.\n>> \n>> See [1], what I said about LIB_H there applies more generally for the\n>> .depends files.\n>> \n>> It will only fail in the sense that it over-depends, i.e. if you do:\n>> \n>>     git clean -dxf; make builtin/help.o\n>\n> Try\n>\n>     git clean -dxf; make builtin/bugreport.o\n>\n> It fails with:\n>\n>       CC builtin/bugreport.o\n>   builtin/bugreport.c:7:10: fatal error: hook-list.h: そのようなファイルやディレクトリはありません\n>       7 | #include \"hook-list.h\"\n>         |          ^~~~~~~~~~~~~\n>   compilation terminated.\n>   make: *** [Makefile:2500: builtin/bugreport.o] エラー 1\n>\n> The only reason I can see why it builds at all normally is that hook.o\n> is built soon enough that by the time builtin/bugreport.o is built\n> hook-list.h has already been generated.\n\nAh, you're obviously right. I don't know what I was thinking yesterday.\n\nI submitted a re-roll of the greater dependency fix-up & optimization\nseries I've got kicking around, which includes a fix for this\nissue. Thank you for the report:\nhttps://lore.kernel.org/git/cover-v3-00.23-00000000000-20211116T114334Z-avarab@gmail.com\n\n"},{"id":"441415","messageId":"xmqq8rxn41n5.fsf@gitster.g","threadId":"56573","inReplyTo":"211116.86y25opapy.gmgdl@evledraar.gmail.com","subject":"Re: [PATCH 8/8] hook-list.h: add a generated list of hooks, like config-list.h","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-11-17T08:39:10Z","receivedAt":"2021-11-17T08:39:16Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:\n\n> I submitted a re-roll of the greater dependency fix-up & optimization\n> series I've got kicking around, which includes a fix for this\n> issue. Thank you for the report:\n\nMakes me wonder what other breakages are introduced in such a big\nseries, in exchange for correcting this single issue X-<.  We'll see\nsoon enough, I guess ;-)\n"}]}