git@vger.kernel.org mailing list mirror (one of many)
 help / Atom feed
From: Damien Marié <damien@dam.io>
To: git@vger.kernel.org
Subject: [PATCH v2] run-command: add hint when a hook is ignored
Date: Thu, 5 Oct 2017 20:53:07 +0000
Message-ID: <0102015eee503678-52126849-7580-4832-9d46-0f5ac7a52336-000000@eu-west-1.amazonses.com> (raw)
In-Reply-To: <0102015ee1e41f17-927a8da1-ac14-4399-8424-fee8a82c2b0a-000000@eu-west-1.amazonses.com>

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>
---
 Documentation/config.txt               |  3 ++
 advice.c                               |  2 +
 advice.h                               |  1 +
 contrib/completion/git-completion.bash |  1 +
 run-command.c                          | 10 +++++
 t/t7519-ignored-hook-warning.sh        | 67 ++++++++++++++++++++++++++++++++++
 6 files changed, 84 insertions(+)
 create mode 100755 t/t7519-ignored-hook-warning.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 1ac0ae6adb046..9abca499f725c 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -351,6 +351,9 @@ advice.*::
 	addEmbeddedRepo::
 		Advice on what to do when you've accidentally added one
 		git repo inside of another.
+	ignoredHook::
+		Advice shown if an hook is ignored because the it's not
+		set as executable.
 --
 
 core.fileMode::
diff --git a/advice.c b/advice.c
index d81e1cb7425b0..970bd2b71bf53 100644
--- a/advice.c
+++ b/advice.c
@@ -17,6 +17,7 @@ int advice_set_upstream_failure = 1;
 int advice_object_name_warning = 1;
 int advice_rm_hints = 1;
 int advice_add_embedded_repo = 1;
+int advice_ignored_hook = 1;
 
 static struct {
 	const char *name;
@@ -38,6 +39,7 @@ static struct {
 	{ "objectnamewarning", &advice_object_name_warning },
 	{ "rmhints", &advice_rm_hints },
 	{ "addembeddedrepo", &advice_add_embedded_repo },
+	{ "ignoredhook", &advice_ignored_hook},
 
 	/* make this an alias for backward compatibility */
 	{ "pushnonfastforward", &advice_push_update_rejected }
diff --git a/advice.h b/advice.h
index c84a44531c7d8..f525d6f89cb44 100644
--- a/advice.h
+++ b/advice.h
@@ -19,6 +19,7 @@ extern int advice_set_upstream_failure;
 extern int advice_object_name_warning;
 extern int advice_rm_hints;
 extern int advice_add_embedded_repo;
+extern int advice_ignored_hook;
 
 int git_default_advice_config(const char *var, const char *value);
 __attribute__((format (printf, 1, 2)))
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index d93441747523a..a331ccc556b01 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -2350,6 +2350,7 @@ _git_config ()
 		advice.rmHints
 		advice.statusHints
 		advice.statusUoption
+		advice.ignoredHook
 		alias.
 		am.keepcr
 		am.threeWay
diff --git a/run-command.c b/run-command.c
index b5e6eb37c0eb3..9456355595cc1 100644
--- a/run-command.c
+++ b/run-command.c
@@ -1169,11 +1169,21 @@ const char *find_hook(const char *name)
 	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;
+		else if (errno == EACCES)
+			err = errno;
 #endif
+		if (err == EACCES && advice_ignored_hook) {
+			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/t/t7519-ignored-hook-warning.sh b/t/t7519-ignored-hook-warning.sh
new file mode 100755
index 0000000000000..59052a4429111
--- /dev/null
+++ b/t/t7519-ignored-hook-warning.sh
@@ -0,0 +1,67 @@
+#!/bin/sh
+
+test_description='ignored hook warning'
+
+. ./test-lib.sh
+
+# install hook that always succeeds
+HOOKDIR="$(git rev-parse --git-dir)/hooks"
+HOOK="$HOOKDIR/pre-commit"
+mkdir -p "$HOOKDIR"
+cat > "$HOOK" <<EOF
+#!/bin/sh
+exit 0
+EOF
+
+chmod +x "$HOOK"
+
+test_expect_success 'no warning if proper hook' '
+
+    if git commit -m "more" 2>&1 >/dev/null | grep hint
+    then
+        false
+    else
+        true
+    fi
+
+'
+
+chmod -x "$HOOK"
+
+test_expect_success POSIXPERM 'warning if hook not set as executable' '
+
+    if git commit -m "more" 2>&1 >/dev/null | grep hint
+    then
+        true
+    else
+        false
+    fi
+'
+
+test_expect_success 'no warning if advice.ignoredHook set to false' '
+
+    git config advice.ignoredHook false &&
+    if git commit -m "more" 2>&1 >/dev/null | grep hint
+    then
+        false
+    else
+        true
+    fi
+'
+
+rm "$HOOK"
+
+test_expect_success 'no warning if unset advice.ignoredHook and hook removed' '
+
+    git config --unset advice.ignoredHook &&
+    if git commit -m "more" 2>&1 >/dev/null | grep hint
+    then
+        false
+    else
+        true
+    fi
+'
+git commit -m "more" 2>&1 >/dev/null
+
+
+test_done
\ No newline at end of file

--
https://github.com/git/git/pull/411

  parent reply index

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-03 10:59 [PATCH] run-command.c: add hint when hook is not executable Damien
2017-10-04  4:40 ` Junio C Hamano
2017-10-05 20:48   ` Damien
2017-10-05 20:53 ` Damien Marié [this message]
2017-10-06  4:52   ` [PATCH v2] run-command: add hint when a hook is ignored Junio C Hamano
2017-10-06  5:25     ` Junio C Hamano
2017-10-06  5:43     ` Junio C Hamano
2017-10-06  5:53     ` Junio C Hamano
2017-10-06  8:04       ` Damien
2017-10-06  8:07       ` [PATCH v3] " Damien Marié
2017-10-10  4:21         ` Junio C Hamano
2017-10-11  6:26           ` Junio C Hamano
2018-01-03  8:31             ` Jeff King
2018-01-05 19:36               ` Junio C Hamano
2018-03-03  7:16                 ` Jeff King
2018-03-05 21:19                   ` Junio C Hamano

Reply instructions:

You may reply publically to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=0102015eee503678-52126849-7580-4832-9d46-0f5ac7a52336-000000@eu-west-1.amazonses.com \
    --to=damien@dam.io \
    --cc=git@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

git@vger.kernel.org mailing list mirror (one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/
       or Tor2web: https://www.tor2web.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox