From ea47e59fe321241848e0367de9fe3289290e8324 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Thu, 23 Sep 2021 12:29:56 +0200 Subject: [PATCH 1/8] Makefile: mark "check" target as .PHONY MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fix a bug in 44c9e8594e (Fix up header file dependencies and add sparse checking rules, 2005-07-03), we never marked the phony "check" target as such. Perhaps we should just remove it, since as of a combination of 912f9980d2 (Makefile: help people who run 'make check' by mistake, 2008-11-11) 0bcd9ae85d (sparse: Fix errors due to missing target-specific variables, 2011-04-21) we've been suggesting the user run "make sparse" directly. But under that mode it still does something, as well as directing the user to run "make test" under non-sparse. So let's punt that and narrowly fix the PHONY bug. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- Makefile | 1 + 1 file changed, 1 insertion(+) diff --git a/Makefile b/Makefile index a9f9b689f0..2777284b0f 100644 --- a/Makefile +++ b/Makefile @@ -2925,6 +2925,7 @@ hdr-check: $(HCO) style: git clang-format --style file --diff --extensions c,h +.PHONY: check check: config-list.h command-list.h @if sparse; \ then \ From 7c3c0a99cc22279f7060ff2e73befbcd7d449896 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Thu, 23 Sep 2021 12:29:57 +0200 Subject: [PATCH 2/8] Makefile: stop hardcoding {command,config}-list.h MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change various places that hardcode the names of these two files to refer to either $(GENERATED_H), or to a new generated-hdrs target. That target is consistent with the *-objs targets I recently added in 029bac01a8 (Makefile: add {program,xdiff,test,git,fuzz}-objs & objects targets, 2021-02-23). A subsequent commit will add a new generated hook-list.h. By doing this refactoring we'll only need to add the new file to the GENERATED_H variable, not EXCEPT_HDRS, the vcbuild/README etc. Hardcoding command-list.h there seems to have been a case of copy/paste programming in 976aaedca0 (msvc: add a Makefile target to pre-generate the Visual Studio solution, 2019-07-29). The config-list.h was added later in 709df95b78 (help: move list_config_help to builtin/help, 2020-04-16). Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- Makefile | 6 ++++-- compat/vcbuild/README | 2 +- config.mak.uname | 6 +++--- 3 files changed, 8 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index 2777284b0f..47e79a6252 100644 --- a/Makefile +++ b/Makefile @@ -817,6 +817,8 @@ XDIFF_LIB = xdiff/lib.a GENERATED_H += command-list.h GENERATED_H += config-list.h +.PHONY: generated-hdrs +generated-hdrs: $(GENERATED_H) LIB_H := $(sort $(patsubst ./%,%,$(shell git ls-files '*.h' ':!t/' ':!Documentation/' 2>/dev/null || \ $(FIND) . \ @@ -2903,7 +2905,7 @@ $(SP_OBJ): %.sp: %.c GIT-CFLAGS FORCE .PHONY: sparse $(SP_OBJ) sparse: $(SP_OBJ) -EXCEPT_HDRS := command-list.h config-list.h unicode-width.h compat/% xdiff/% +EXCEPT_HDRS := $(GENERATED_H) unicode-width.h compat/% xdiff/% ifndef GCRYPT_SHA256 EXCEPT_HDRS += sha256/gcrypt.h endif @@ -2926,7 +2928,7 @@ style: git clang-format --style file --diff --extensions c,h .PHONY: check -check: config-list.h command-list.h +check: $(GENERATED_H) @if sparse; \ then \ echo >&2 "Use 'make sparse' instead"; \ diff --git a/compat/vcbuild/README b/compat/vcbuild/README index 51fb083dbb..29ec1d0f10 100644 --- a/compat/vcbuild/README +++ b/compat/vcbuild/README @@ -92,7 +92,7 @@ The Steps of Build Git with VS2008 the git operations. 3. Inside Git's directory run the command: - make command-list.h config-list.h + make generated-hdrs to generate the header file needed to compile git. 4. Then either build Git with the GNU Make Makefile in the Git projects diff --git a/config.mak.uname b/config.mak.uname index 76516aaa9a..8aac06eb09 100644 --- a/config.mak.uname +++ b/config.mak.uname @@ -735,9 +735,9 @@ vcxproj: echo '') >git-remote-http/LinkOrCopyRemoteHttp.targets git add -f git/LinkOrCopyBuiltins.targets git-remote-http/LinkOrCopyRemoteHttp.targets - # Add command-list.h and config-list.h - $(MAKE) MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 config-list.h command-list.h - git add -f config-list.h command-list.h + # Add generated headers + $(MAKE) MSVC=1 SKIP_VCPKG=1 prefix=/mingw64 $(GENERATED_H) + git add -f $(GENERATED_H) # Add scripts rm -f perl/perl.mak From 7c812953826549bd21c119a0b533d03973c52443 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Thu, 23 Sep 2021 12:29:58 +0200 Subject: [PATCH 3/8] Makefile: don't perform "mv $@+ $@" dance for $(GENERATED_H) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Change the "cmd.sh > $@+ && mv $@+ $@" pattern used for generating the config-list.h and command-list.h to just "cmd.sh >$@". This was needed as a guard to ensure that we don't have an empty file if the script failed, but since 7b76d6bf221 (Makefile: add and use the ".DELETE_ON_ERROR" flag, 2021-06-29) GNU make ensures that doesn't happen. There's still a lot of other places in the Makefile where we needlessly use this pattern, but I'm just changing these because I'm about to add a new $(GENERATED_H) target, let's have them all look and act the same way. Even with ".DELETE_ON_ERROR" there is still a point to using the "mv $@+ $@" pattern in some cases, e.g. to ensure that you have a working binary during recompilation (see [1] for the start of a long discussion about that), but that doesn't apply here. Nothing external uses $(GENERATED_H) directly, it's only ever used in the context of the Makefile's own dependency (re-)generation. 1. https://lore.kernel.org/git/8735t93h0u.fsf@evledraar.gmail.com/ Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- Makefile | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 47e79a6252..e6b8d00f22 100644 --- a/Makefile +++ b/Makefile @@ -2231,15 +2231,14 @@ $(BUILT_INS): git$X config-list.h: generate-configlist.sh config-list.h: Documentation/*config.txt Documentation/config/*.txt - $(QUIET_GEN)$(SHELL_PATH) ./generate-configlist.sh \ - >$@+ && mv $@+ $@ + $(QUIET_GEN)$(SHELL_PATH) ./generate-configlist.sh >$@ command-list.h: generate-cmdlist.sh command-list.txt command-list.h: $(wildcard Documentation/git*.txt) $(QUIET_GEN)$(SHELL_PATH) ./generate-cmdlist.sh \ $(patsubst %,--exclude-program %,$(EXCLUDED_PROGRAMS)) \ - command-list.txt >$@+ && mv $@+ $@ + command-list.txt >$@ SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\ $(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\ From f53df0bdf6d5ea9da12b97fc7f87b1dd3681b278 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Thu, 23 Sep 2021 12:29:59 +0200 Subject: [PATCH 4/8] Makefile: remove an out-of-date comment MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This comment added in dfea575017 (Makefile: lazily compute header dependencies, 2010-01-26) has been out of date since 92b88eba9f (Makefile: use `git ls-files` to list header files, if possible, 2019-03-04), when we did exactly what it tells us not to do and added $(GENERATED_H) to $(OBJECTS) dependencies. The rest of it was also somewhere between inaccurate and outdated, since as of b8ba629264 (Makefile: fold MISC_H into LIB_H, 2012-06-20) it's not followed by a list of header files, that got moved earlier in the file into LIB_H in 60d24dd255 (Makefile: fold XDIFF_H and VCSSVN_H into LIB_H, 2012-07-06). Let's just remove it entirely, to the extent that we have anything useful to say here the comment on the "USE_COMPUTED_HEADER_DEPENDENCIES" variable a few lines above this change does the job for us. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- Makefile | 7 ------- 1 file changed, 7 deletions(-) diff --git a/Makefile b/Makefile index e6b8d00f22..b9bf13239d 100644 --- a/Makefile +++ b/Makefile @@ -2501,13 +2501,6 @@ ifneq ($(dep_files_present),) include $(dep_files_present) endif else -# Dependencies on header files, for platforms that do not support -# the gcc -MMD option. -# -# Dependencies on automatically generated headers such as command-list.h -# should _not_ be included here, since they are necessary even when -# building an object for the first time. - $(OBJECTS): $(LIB_H) $(GENERATED_H) endif From 5e3aba33da26803e48b0099c9dabfd327f7f8b8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Sun, 26 Sep 2021 21:03:26 +0200 Subject: [PATCH 5/8] hook.[ch]: move find_hook() from run-command.c to hook.c MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Move the find_hook() function from run-command.c to a new hook.c library. This change establishes a stub library that's pretty pointless right now, but will see much wider use with Emily Shaffer's upcoming "configuration-based hooks" series. Eventually all the hook related code will live in hook.[ch]. Let's start that process by moving the simple find_hook() function over as-is. Signed-off-by: Emily Shaffer Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- Makefile | 1 + builtin/am.c | 1 + builtin/bugreport.c | 2 +- builtin/commit.c | 1 + builtin/merge.c | 1 + builtin/receive-pack.c | 1 + builtin/worktree.c | 1 + hook.c | 37 +++++++++++++++++++++++++++++++++++++ hook.h | 11 +++++++++++ refs.c | 1 + run-command.c | 35 +---------------------------------- run-command.h | 7 ------- sequencer.c | 1 + transport.c | 2 +- 14 files changed, 59 insertions(+), 43 deletions(-) create mode 100644 hook.c create mode 100644 hook.h diff --git a/Makefile b/Makefile index b9bf13239d..1f2b0205da 100644 --- a/Makefile +++ b/Makefile @@ -904,6 +904,7 @@ LIB_OBJS += hash-lookup.o LIB_OBJS += hashmap.o LIB_OBJS += help.o LIB_OBJS += hex.o +LIB_OBJS += hook.o LIB_OBJS += ident.o LIB_OBJS += json-writer.o LIB_OBJS += kwset.o diff --git a/builtin/am.c b/builtin/am.c index e4a0ff9cd7..3527945a46 100644 --- a/builtin/am.c +++ b/builtin/am.c @@ -11,6 +11,7 @@ #include "parse-options.h" #include "dir.h" #include "run-command.h" +#include "hook.h" #include "quote.h" #include "tempfile.h" #include "lockfile.h" diff --git a/builtin/bugreport.c b/builtin/bugreport.c index 06ed10dc92..c30a360d69 100644 --- a/builtin/bugreport.c +++ b/builtin/bugreport.c @@ -3,7 +3,7 @@ #include "strbuf.h" #include "help.h" #include "compat/compiler.h" -#include "run-command.h" +#include "hook.h" static void get_system_info(struct strbuf *sys_info) diff --git a/builtin/commit.c b/builtin/commit.c index e7320f66f9..5359d961d2 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -19,6 +19,7 @@ #include "revision.h" #include "wt-status.h" #include "run-command.h" +#include "hook.h" #include "refs.h" #include "log-tree.h" #include "strbuf.h" diff --git a/builtin/merge.c b/builtin/merge.c index 3fbdacc7db..fe664f6a86 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -13,6 +13,7 @@ #include "builtin.h" #include "lockfile.h" #include "run-command.h" +#include "hook.h" #include "diff.h" #include "diff-merges.h" #include "refs.h" diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index 48960a9575..e3895aec62 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -7,6 +7,7 @@ #include "pkt-line.h" #include "sideband.h" #include "run-command.h" +#include "hook.h" #include "exec-cmd.h" #include "commit.h" #include "object.h" diff --git a/builtin/worktree.c b/builtin/worktree.c index 0d0a80da61..d22ece93e1 100644 --- a/builtin/worktree.c +++ b/builtin/worktree.c @@ -8,6 +8,7 @@ #include "branch.h" #include "refs.h" #include "run-command.h" +#include "hook.h" #include "sigchain.h" #include "submodule.h" #include "utf8.h" diff --git a/hook.c b/hook.c new file mode 100644 index 0000000000..ba70b31471 --- /dev/null +++ b/hook.c @@ -0,0 +1,37 @@ +#include "cache.h" +#include "hook.h" +#include "run-command.h" + +const char *find_hook(const char *name) +{ + static struct strbuf path = STRBUF_INIT; + + strbuf_reset(&path); + strbuf_git_path(&path, "hooks/%s", name); + if (access(path.buf, X_OK) < 0) { + int err = errno; + +#ifdef STRIP_EXTENSION + strbuf_addstr(&path, STRIP_EXTENSION); + if (access(path.buf, X_OK) >= 0) + return path.buf; + if (errno == EACCES) + err = errno; +#endif + + if (err == EACCES && advice_enabled(ADVICE_IGNORED_HOOK)) { + static struct string_list advise_given = STRING_LIST_INIT_DUP; + + if (!string_list_lookup(&advise_given, name)) { + string_list_insert(&advise_given, name); + advise(_("The '%s' hook was ignored because " + "it's not set as executable.\n" + "You can disable this warning with " + "`git config advice.ignoredHook false`."), + path.buf); + } + } + return NULL; + } + return path.buf; +} diff --git a/hook.h b/hook.h new file mode 100644 index 0000000000..68624f1605 --- /dev/null +++ b/hook.h @@ -0,0 +1,11 @@ +#ifndef HOOK_H +#define HOOK_H + +/* + * Returns the path to the hook file, or NULL if the hook is missing + * or disabled. Note that this points to static storage that will be + * overwritten by further calls to find_hook and run_hook_*. + */ +const char *find_hook(const char *name); + +#endif diff --git a/refs.c b/refs.c index 8b9f7c3a80..6211692eaa 100644 --- a/refs.c +++ b/refs.c @@ -10,6 +10,7 @@ #include "refs.h" #include "refs/refs-internal.h" #include "run-command.h" +#include "hook.h" #include "object-store.h" #include "object.h" #include "tag.h" diff --git a/run-command.c b/run-command.c index 04a07e6366..e4862b8f46 100644 --- a/run-command.c +++ b/run-command.c @@ -9,6 +9,7 @@ #include "quote.h" #include "config.h" #include "packfile.h" +#include "hook.h" void child_process_init(struct child_process *child) { @@ -1322,40 +1323,6 @@ int async_with_fork(void) #endif } -const char *find_hook(const char *name) -{ - static struct strbuf path = STRBUF_INIT; - - strbuf_reset(&path); - strbuf_git_path(&path, "hooks/%s", name); - if (access(path.buf, X_OK) < 0) { - int err = errno; - -#ifdef STRIP_EXTENSION - strbuf_addstr(&path, STRIP_EXTENSION); - if (access(path.buf, X_OK) >= 0) - return path.buf; - if (errno == EACCES) - err = errno; -#endif - - if (err == EACCES && advice_enabled(ADVICE_IGNORED_HOOK)) { - static struct string_list advise_given = STRING_LIST_INIT_DUP; - - if (!string_list_lookup(&advise_given, name)) { - string_list_insert(&advise_given, name); - advise(_("The '%s' hook was ignored because " - "it's not set as executable.\n" - "You can disable this warning with " - "`git config advice.ignoredHook false`."), - path.buf); - } - } - return NULL; - } - return path.buf; -} - int run_hook_ve(const char *const *env, const char *name, va_list args) { struct child_process hook = CHILD_PROCESS_INIT; diff --git a/run-command.h b/run-command.h index b9aff74914..5e544acf4b 100644 --- a/run-command.h +++ b/run-command.h @@ -224,13 +224,6 @@ int finish_command_in_signal(struct child_process *); */ int run_command(struct child_process *); -/* - * Returns the path to the hook file, or NULL if the hook is missing - * or disabled. Note that this points to static storage that will be - * overwritten by further calls to find_hook and run_hook_*. - */ -const char *find_hook(const char *name); - /** * Run a hook. * The first argument is a pathname to an index file, or NULL diff --git a/sequencer.c b/sequencer.c index 614d56f5e2..8ee6c4ac24 100644 --- a/sequencer.c +++ b/sequencer.c @@ -8,6 +8,7 @@ #include "sequencer.h" #include "tag.h" #include "run-command.h" +#include "hook.h" #include "exec-cmd.h" #include "utf8.h" #include "cache-tree.h" diff --git a/transport.c b/transport.c index b37664ba87..e4f1decae2 100644 --- a/transport.c +++ b/transport.c @@ -1,7 +1,7 @@ #include "cache.h" #include "config.h" #include "transport.h" -#include "run-command.h" +#include "hook.h" #include "pkt-line.h" #include "fetch-pack.h" #include "remote.h" From 330155ed8af3b2e050ac74554993ba68d303e8c3 Mon Sep 17 00:00:00 2001 From: Emily Shaffer Date: Sun, 26 Sep 2021 21:03:27 +0200 Subject: [PATCH 6/8] hook.c: add a hook_exists() wrapper and use it in bugreport.c MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add a boolean version of the find_hook() function for those callers who are only interested in checking whether the hook exists, not what the path to it is. Signed-off-by: Emily Shaffer Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/bugreport.c | 2 +- hook.c | 5 +++++ hook.h | 5 +++++ 3 files changed, 11 insertions(+), 1 deletion(-) diff --git a/builtin/bugreport.c b/builtin/bugreport.c index c30a360d69..a02c2540bb 100644 --- a/builtin/bugreport.c +++ b/builtin/bugreport.c @@ -82,7 +82,7 @@ static void get_populated_hooks(struct strbuf *hook_info, int nongit) } for (i = 0; i < ARRAY_SIZE(hook); i++) - if (find_hook(hook[i])) + if (hook_exists(hook[i])) strbuf_addf(hook_info, "%s\n", hook[i]); } diff --git a/hook.c b/hook.c index ba70b31471..55e1145a4b 100644 --- a/hook.c +++ b/hook.c @@ -35,3 +35,8 @@ const char *find_hook(const char *name) } return path.buf; } + +int hook_exists(const char *name) +{ + return !!find_hook(name); +} diff --git a/hook.h b/hook.h index 68624f1605..6aa36fc7ff 100644 --- a/hook.h +++ b/hook.h @@ -8,4 +8,9 @@ */ const char *find_hook(const char *name); +/** + * A boolean version of find_hook() + */ +int hook_exists(const char *hookname); + #endif From 07a348e7461afe9411004a0501034cb3ff1cdee8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Sun, 26 Sep 2021 21:03:28 +0200 Subject: [PATCH 7/8] hook.c users: use "hook_exists()" instead of "find_hook()" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use the new hook_exists() function instead of find_hook() where the latter was called in boolean contexts. This make subsequent changes in a series where we further refactor the hook API clearer, as we won't conflate wanting to get the path of the hook with checking for its existence. Signed-off-by: Ævar Arnfjörð Bjarmason Signed-off-by: Junio C Hamano --- builtin/commit.c | 2 +- builtin/merge.c | 2 +- builtin/receive-pack.c | 2 +- sequencer.c | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/builtin/commit.c b/builtin/commit.c index 5359d961d2..883c16256c 100644 --- a/builtin/commit.c +++ b/builtin/commit.c @@ -1052,7 +1052,7 @@ static int prepare_to_commit(const char *index_file, const char *prefix, return 0; } - if (!no_verify && find_hook("pre-commit")) { + if (!no_verify && hook_exists("pre-commit")) { /* * Re-read the index as pre-commit hook could have updated it, * and write it out as a tree. We must do this before we invoke diff --git a/builtin/merge.c b/builtin/merge.c index fe664f6a86..956b6259f2 100644 --- a/builtin/merge.c +++ b/builtin/merge.c @@ -849,7 +849,7 @@ static void prepare_to_commit(struct commit_list *remoteheads) * and write it out as a tree. We must do this before we invoke * the editor and after we invoke run_status above. */ - if (find_hook("pre-merge-commit")) + if (hook_exists("pre-merge-commit")) discard_cache(); read_cache_from(index_file); strbuf_addbuf(&msg, &merge_msg); diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index e3895aec62..25cc0c907e 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -1464,7 +1464,7 @@ static const char *update_worktree(unsigned char *sha1, const struct worktree *w strvec_pushf(&env, "GIT_DIR=%s", absolute_path(git_dir)); - if (!find_hook(push_to_checkout_hook)) + if (!hook_exists(push_to_checkout_hook)) retval = push_to_deploy(sha1, &env, work_tree); else retval = push_to_checkout(sha1, &env, work_tree); diff --git a/sequencer.c b/sequencer.c index 8ee6c4ac24..e501945796 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1460,7 +1460,7 @@ static int try_to_commit(struct repository *r, } } - if (find_hook("prepare-commit-msg")) { + if (hook_exists("prepare-commit-msg")) { res = run_prepare_commit_msg_hook(r, msg, hook_commit); if (res) goto out; From cfe853e66be56b4a035739b0f21ba409dfca695f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=86var=20Arnfj=C3=B6r=C3=B0=20Bjarmason?= Date: Sun, 26 Sep 2021 21:03:29 +0200 Subject: [PATCH 8/8] hook-list.h: add a generated list of hooks, like config-list.h MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Make githooks(5) the source of truth for what hooks git supports, and punt out early on hooks we don't know about in find_hook(). This ensures that the documentation and the C code's idea about existing hooks doesn't diverge. We still have Perl and Python code running its own hooks, but that'll be addressed by Emily Shaffer's upcoming "git hook run" command. This resolves a long-standing TODO item in bugreport.c of there being no centralized listing of hooks, and fixes a bug with the bugreport listing only knowing about 1/4 of the p4 hooks. It didn't know about the recent "reference-transaction" hook either. We could make the find_hook() function die() or BUG() out if the new known_hook() returned 0, but let's make it return NULL just as it does when it can't find a hook of a known type. Making it die() is overly anal, and unlikely to be what we need in catching stupid typos in the name of some new hook hardcoded in git.git's sources. By making this be tolerant of unknown hook names, changes in a later series to make "git hook run" run arbitrary user-configured hook names will be easier to implement. I have not been able to directly test the CMake change being made here. Since 4c2c38e800 (ci: modification of main.yml to use cmake for vs-build job, 2020-06-26) some of the Windows CI has a hard dependency on CMake, this change works there, and is to my eyes an obviously correct use of a pattern established in previous CMake changes, namely: - 061c2240b1 (Introduce CMake support for configuring Git, 2020-06-12) - 709df95b78 (help: move list_config_help to builtin/help, 2020-04-16) - 976aaedca0 (msvc: add a Makefile target to pre-generate the Visual Studio solution, 2019-07-29) The LC_ALL=C is needed because at least in my locale the dash ("-") is ignored for the purposes of sorting, which results in a different order. I'm not aware of anything in git that has a hard dependency on the order, but e.g. the bugreport output would end up using whatever locale was in effect when git was compiled. Signed-off-by: Ævar Arnfjörð Bjarmason Helped-by: René Scharfe Signed-off-by: Junio C Hamano --- .gitignore | 1 + Makefile | 8 +++++- builtin/bugreport.c | 44 ++++++----------------------- contrib/buildsystems/CMakeLists.txt | 7 +++++ generate-hooklist.sh | 20 +++++++++++++ 5 files changed, 43 insertions(+), 37 deletions(-) create mode 100755 generate-hooklist.sh diff --git a/.gitignore b/.gitignore index 311841f9be..6be9de41ae 100644 --- a/.gitignore +++ b/.gitignore @@ -190,6 +190,7 @@ /gitweb/static/gitweb.min.* /config-list.h /command-list.h +/hook-list.h *.tar.gz *.dsc *.deb diff --git a/Makefile b/Makefile index 1f2b0205da..21e2bcc21e 100644 --- a/Makefile +++ b/Makefile @@ -817,6 +817,8 @@ XDIFF_LIB = xdiff/lib.a GENERATED_H += command-list.h GENERATED_H += config-list.h +GENERATED_H += hook-list.h + .PHONY: generated-hdrs generated-hdrs: $(GENERATED_H) @@ -2208,8 +2210,9 @@ git$X: git.o GIT-LDFLAGS $(BUILTIN_OBJS) $(GITLIBS) $(filter %.o,$^) $(LIBS) help.sp help.s help.o: command-list.h +hook.sp hook.s hook.o: hook-list.h -builtin/help.sp builtin/help.s builtin/help.o: config-list.h GIT-PREFIX +builtin/help.sp builtin/help.s builtin/help.o: config-list.h hook-list.h GIT-PREFIX builtin/help.sp builtin/help.s builtin/help.o: EXTRA_CPPFLAGS = \ '-DGIT_HTML_PATH="$(htmldir_relative_SQ)"' \ '-DGIT_MAN_PATH="$(mandir_relative_SQ)"' \ @@ -2241,6 +2244,9 @@ command-list.h: $(wildcard Documentation/git*.txt) $(patsubst %,--exclude-program %,$(EXCLUDED_PROGRAMS)) \ command-list.txt >$@ +hook-list.h: generate-hooklist.sh Documentation/githooks.txt + $(QUIET_GEN)$(SHELL_PATH) ./generate-hooklist.sh >$@ + SCRIPT_DEFINES = $(SHELL_PATH_SQ):$(DIFF_SQ):$(GIT_VERSION):\ $(localedir_SQ):$(NO_CURL):$(USE_GETTEXT_SCHEME):$(SANE_TOOL_PATH_SQ):\ $(gitwebdir_SQ):$(PERL_PATH_SQ):$(SANE_TEXT_GREP):$(PAGER_ENV):\ diff --git a/builtin/bugreport.c b/builtin/bugreport.c index a02c2540bb..9de32bc96e 100644 --- a/builtin/bugreport.c +++ b/builtin/bugreport.c @@ -4,6 +4,7 @@ #include "help.h" #include "compat/compiler.h" #include "hook.h" +#include "hook-list.h" static void get_system_info(struct strbuf *sys_info) @@ -41,39 +42,7 @@ static void get_system_info(struct strbuf *sys_info) static void get_populated_hooks(struct strbuf *hook_info, int nongit) { - /* - * NEEDSWORK: Doesn't look like there is a list of all possible hooks; - * so below is a transcription of `git help hooks`. Later, this should - * be replaced with some programmatically generated list (generated from - * doc or else taken from some library which tells us about all the - * hooks) - */ - static const char *hook[] = { - "applypatch-msg", - "pre-applypatch", - "post-applypatch", - "pre-commit", - "pre-merge-commit", - "prepare-commit-msg", - "commit-msg", - "post-commit", - "pre-rebase", - "post-checkout", - "post-merge", - "pre-push", - "pre-receive", - "update", - "post-receive", - "post-update", - "push-to-checkout", - "pre-auto-gc", - "post-rewrite", - "sendemail-validate", - "fsmonitor-watchman", - "p4-pre-submit", - "post-index-change", - }; - int i; + const char **p; if (nongit) { strbuf_addstr(hook_info, @@ -81,9 +50,12 @@ static void get_populated_hooks(struct strbuf *hook_info, int nongit) return; } - for (i = 0; i < ARRAY_SIZE(hook); i++) - if (hook_exists(hook[i])) - strbuf_addf(hook_info, "%s\n", hook[i]); + for (p = hook_name_list; *p; p++) { + const char *hook = *p; + + if (hook_exists(hook)) + strbuf_addf(hook_info, "%s\n", hook); + } } static const char * const bugreport_usage[] = { diff --git a/contrib/buildsystems/CMakeLists.txt b/contrib/buildsystems/CMakeLists.txt index 171b4124af..fd1399c440 100644 --- a/contrib/buildsystems/CMakeLists.txt +++ b/contrib/buildsystems/CMakeLists.txt @@ -624,6 +624,13 @@ if(NOT EXISTS ${CMAKE_BINARY_DIR}/config-list.h) OUTPUT_FILE ${CMAKE_BINARY_DIR}/config-list.h) endif() +if(NOT EXISTS ${CMAKE_BINARY_DIR}/hook-list.h) + message("Generating hook-list.h") + execute_process(COMMAND ${SH_EXE} ${CMAKE_SOURCE_DIR}/generate-hooklist.sh + WORKING_DIRECTORY ${CMAKE_SOURCE_DIR} + OUTPUT_FILE ${CMAKE_BINARY_DIR}/hook-list.h) +endif() + include_directories(${CMAKE_BINARY_DIR}) #build diff --git a/generate-hooklist.sh b/generate-hooklist.sh new file mode 100755 index 0000000000..2f9f54eb54 --- /dev/null +++ b/generate-hooklist.sh @@ -0,0 +1,20 @@ +#!/bin/sh +# +# Usage: ./generate-hooklist.sh >hook-list.h + +cat <