run-command: add hint when a hook is ignored

When an hook is present but the file is not set as executable then git will ignore the hook. For now this is silent which can be confusing. This commit adds this warning to improve the situation: hint: The 'pre-commit' hook was ignored because it's not set as executable. hint: You can disable this warning with `git config advice.ignoredHook false` To allow the old use-case of enabling/disabling hooks via the executable flag a new setting is introduced: advice.ignoredHook. Signed-off-by: Damien Marié <damien@dam.io> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Damien Marié committed Oct 6, 2017 at 08:07 UTC f805a00a396ee91599902cebe55620b2a4c813b9
6 files changed +66
Documentation/config.txt
+3
@@ -351,6 +351,9 @@ advice.*::
351 addEmbeddedRepo::
352 Advice on what to do when you've accidentally added one
353 git repo inside of another.
354 + ignoredHook::
355 + Advice shown if an hook is ignored because the hook is not
356 + set as executable.
357 --
358
359 core.fileMode::
advice.c
+2
@@ -17,6 +17,7 @@ int advice_set_upstream_failure = 1;
17 int advice_object_name_warning = 1;
18 int advice_rm_hints = 1;
19 int advice_add_embedded_repo = 1;
20 +int advice_ignored_hook = 1;
21
22 static struct {
23 const char *name;
@@ -38,6 +39,7 @@ static struct {
39 { "objectnamewarning", &advice_object_name_warning },
40 { "rmhints", &advice_rm_hints },
41 { "addembeddedrepo", &advice_add_embedded_repo },
42 + { "ignoredhook", &advice_ignored_hook },
43
44 /* make this an alias for backward compatibility */
45 { "pushnonfastforward", &advice_push_update_rejected }
advice.h
+1
@@ -19,6 +19,7 @@ extern int advice_set_upstream_failure;
19 extern int advice_object_name_warning;
20 extern int advice_rm_hints;
21 extern int advice_add_embedded_repo;
22 +extern int advice_ignored_hook;
23
24 int git_default_advice_config(const char *var, const char *value);
25 __attribute__((format (printf, 1, 2)))
contrib/completion/git-completion.bash
+1
@@ -2350,6 +2350,7 @@ _git_config ()
2350 advice.rmHints
2351 advice.statusHints
2352 advice.statusUoption
2353 + advice.ignoredHook
2354 alias.
2355 am.keepcr
2356 am.threeWay
run-command.c
+18
@@ -5,6 +5,7 @@
5 #include "argv-array.h"
6 #include "thread-utils.h"
7 #include "strbuf.h"
8 +#include "string-list.h"
9
10 void child_process_init(struct child_process *child)
11 {
@@ -1169,11 +1170,28 @@ const char *find_hook(const char *name)
1170 strbuf_reset(&path);
1171 strbuf_git_path(&path, "hooks/%s", name);
1172 if (access(path.buf, X_OK) < 0) {
1173 + int err = errno;
1174 +
1175 #ifdef STRIP_EXTENSION
1176 strbuf_addstr(&path, STRIP_EXTENSION);
1177 if (access(path.buf, X_OK) >= 0)
1178 return path.buf;
1179 + if (errno == EACCES)
1180 + err = errno;
1181 #endif
1182 +
1183 + if (err == EACCES && advice_ignored_hook) {
1184 + static struct string_list advise_given = STRING_LIST_INIT_DUP;
1185 +
1186 + if (!string_list_lookup(&advise_given, name)) {
1187 + string_list_insert(&advise_given, name);
1188 + advise(_("The '%s' hook was ignored because "
1189 + "it's not set as executable.\n"
1190 + "You can disable this warning with "
1191 + "`git config advice.ignoredHook false`."),
1192 + path.buf);
1193 + }
1194 + }
1195 return NULL;
1196 }
1197 return path.buf;
t/t7520-ignored-hook-warning.sh new
+41
@@ -0,0 +1,41 @@
1 +#!/bin/sh
2 +
3 +test_description='ignored hook warning'
4 +
5 +. ./test-lib.sh
6 +
7 +test_expect_success setup '
8 + hookdir="$(git rev-parse --git-dir)/hooks" &&
9 + hook="$hookdir/pre-commit" &&
10 + mkdir -p "$hookdir" &&
11 + write_script "$hook" <<-\EOF
12 + exit 0
13 + EOF
14 +'
15 +
16 +test_expect_success 'no warning if hook is not ignored' '
17 + git commit --allow-empty -m "more" 2>message &&
18 + test_i18ngrep ! -e "hook was ignored" message
19 +'
20 +
21 +test_expect_success POSIXPERM 'warning if hook is ignored' '
22 + chmod -x "$hook" &&
23 + git commit --allow-empty -m "even more" 2>message &&
24 + test_i18ngrep -e "hook was ignored" message
25 +'
26 +
27 +test_expect_success POSIXPERM 'no warning if advice.ignoredHook set to false' '
28 + test_config advice.ignoredHook false &&
29 + chmod -x "$hook" &&
30 + git commit --allow-empty -m "even more" 2>message &&
31 + test_i18ngrep ! -e "hook was ignored" message
32 +'
33 +
34 +test_expect_success 'no warning if unset advice.ignoredHook and hook removed' '
35 + rm -f "$hook" &&
36 + test_unconfig advice.ignoredHook &&
37 + git commit --allow-empty -m "even more" 2>message &&
38 + test_i18ngrep ! -e "hook was ignored" message
39 +'
40 +
41 +test_done