[PATCH v4 0/4] githooks.txt improvements + core.hooksDirectory

[PATCH v4 0/4] githooks.txt improvements + core.hooksDirectory

[Ævar Arnfjörð Bjarmason]

In this version of the series I've hopefully addressed all the
comments that came up on the list after the last one, and a few fixes
I noticed myself, e.g. a couple of grammar errors and a broken
asciidoc syntax.

I've combined both the githooks.txt documentation improvements and the
core.hooksDirectory patch into one series. Although they're logically
different things I think it makes more sense to combine them for ease
of reading, since the core.hooksDirectory documentation refers to some
documentation I fixed earlier in the series.

Ævar Arnfjörð Bjarmason (4):
  githooks.txt: Improve the intro section
  githooks.txt: Amend dangerous advice about 'update' hook ACL
  githooks.txt: Minor improvements to the grammar & phrasing
  hooks: Add ability to specify where the hook directory is

 Documentation/config.txt     | 18 +++++++++++
 Documentation/git-init.txt   |  7 ++++-
 Documentation/githooks.txt   | 74 ++++++++++++++++++++++++++------------------
 cache.h                      |  1 +
 config.c                     |  3 ++
 environment.c                |  1 +
 run-command.c                |  5 ++-
 t/t1350-config-hooks-path.sh | 31 +++++++++++++++++++
 8 files changed, 108 insertions(+), 32 deletions(-)
 create mode 100755 t/t1350-config-hooks-path.sh

-- 
2.1.3

[PATCH v4 1/4] githooks.txt: Improve the intro section

[Ævar Arnfjörð Bjarmason]

Change the documentation so that:

 * We don't talk about "little scripts". Hooks can be as big as you
   want, and don't have to be scripts, just call them "programs".

 * We note what happens with chdir() before a hook is called, nothing
   documented this explicitly, but the current behavior is
   predictable. It helps a lot to know what directory these hooks will
   be executed from.

 * We don't make claims about the example hooks which may not be true
   depending on the configuration of 'init.templateDir'. Clarify that
   we're talking about the default settings of git-init in those cases,
   and move some of this documentation into git-init's documentation
   about the default templates.

 * We briefly note in the intro that hooks can get their arguments in
   various different ways, and that how exactly is described below for
   each hook.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/git-init.txt |  7 ++++++-
 Documentation/githooks.txt | 32 ++++++++++++++++++++------------
 2 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/Documentation/git-init.txt b/Documentation/git-init.txt
index 8174d27..6364e5d 100644
--- a/Documentation/git-init.txt
+++ b/Documentation/git-init.txt
@@ -130,7 +130,12 @@ The template directory will be one of the following (in order):
  - the default template directory: `/usr/share/git-core/templates`.
 
 The default template directory includes some directory structure, suggested
-"exclude patterns" (see linkgit:gitignore[5]), and sample hook files (see linkgit:githooks[5]).
+"exclude patterns" (see linkgit:gitignore[5]), and sample hook files.
+
+The sample hooks are all disabled by default, To enable one of the
+sample hooks rename it by removing its `.sample` suffix.
+
+See linkgit:githooks[5] for more general info on hook execution.
 
 EXAMPLES
 --------
diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index a2f59b1..7660b95 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -13,18 +13,26 @@ $GIT_DIR/hooks/*
 DESCRIPTION
 -----------
 
-Hooks are little scripts you can place in `$GIT_DIR/hooks`
-directory to trigger action at certain points.  When
-'git init' is run, a handful of example hooks are copied into the
-`hooks` directory of the new repository, but by default they are
-all disabled.  To enable a hook, rename it by removing its `.sample`
-suffix.
-
-NOTE: It is also a requirement for a given hook to be executable.
-However - in a freshly initialized repository - the `.sample` files are
-executable by default.
-
-This document describes the currently defined hooks.
+Hooks are programs you can place in the `$GIT_DIR/hooks` directory to
+trigger actions at certain points in git's execution. Hooks that don't
+have the executable bit set are ignored.
+
+When a hook is invoked, it is run at the root of the working tree in a
+non-bare repository, or in the $GIT_DIR in a bare
+repository. I.e. hooks don't need to worry about the user's current
+working directory.
+
+Hooks can get their arguments via the environment, command-line
+arguments, and stdin. See the documentation for each hook below for
+details.
+
+When 'git init' is run it may, depending on its configuration, copy
+hooks to the new repository, see the the "TEMPLATE DIRECTORY" section
+in linkgit:git-init[1] for details. When the rest of this document
+refers to "default hooks" it's talking about the default template
+shipped with Git.
+
+The currently supported hooks are described below.
 
 HOOKS
 -----
-- 
2.1.3

Re: [PATCH v4 1/4] githooks.txt: Improve the intro section

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

>  * We note what happens with chdir() before a hook is called, nothing
>    documented this explicitly, but the current behavior is
>    predictable. It helps a lot to know what directory these hooks will
>    be executed from.

I _think_ this item is what you meant by "changed the wording for
all of this to hopefully be more clear in my v4.", but I do not
think it addresses "find it somehow and then chdir()" confusion.

It does say "it is run at", meaning that chdir() happens before it
is run.  That is not sufficient to let the reader guess taht chdir()
happens before the hooks directory is checked relative to that
directory to find the hook to run.

> +When 'git init' is run it may, depending on its configuration, copy
> +hooks to the new repository, see the the "TEMPLATE DIRECTORY" section

"... is run, it may, depending on its configuration, copy hooks..."

If that is too much sentence fragments, you could reorder them, e.g.

	'git init' may copy hooks to the new repository, depending
	on its configuration.  See the "TEMPLATE DIRECTORY" section

That would fix "the the" as a side effect ;-).

> +in linkgit:git-init[1] for details. When the rest of this document
> +refers to "default hooks" it's talking about the default template
> +shipped with Git.
> +
> +The currently supported hooks are described below.
>  
>  HOOKS
>  -----

[PATCH v4 2/4] githooks.txt: Amend dangerous advice about 'update' hook ACL

[Ævar Arnfjörð Bjarmason]

Any ACL you implement via an 'update' hook isn't actual access control
if the user has login access to the machine running git, because they
can trivially just built their own git version which doesn't run the
hook.

Change the documentation to take this dangerous edge case into account,
and remove the mention of the advice originating on the mailing list,
the users reading this don't care where the idea came up.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/githooks.txt | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index 7660b95..9051584 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -275,9 +275,11 @@ does not know the entire set of branches, so it would end up
 firing one e-mail per ref when used naively, though.  The
 <<post-receive,'post-receive'>> hook is more suited to that.
 
-Another use suggested on the mailing list is to use this hook to
-implement access control which is finer grained than the one
-based on filesystem group.
+In an environment that restricts the users' access only to git
+commands over the wire, this hook can be used to implement access
+control without relying on filesystem ownership and group
+membership. See linkgit:git-shell[1] for how you might use the login
+shell to restrict the user's access to only git commands.
 
 Both standard output and standard error output are forwarded to
 'git send-pack' on the other end, so you can simply `echo` messages
-- 
2.1.3

Re: [PATCH v4 2/4] githooks.txt: Amend dangerous advice about 'update' hook ACL

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> Any ACL you implement via an 'update' hook isn't actual access control
> if the user has login access to the machine running git, because they
> can trivially just built their own git version which doesn't run the

s/built their own git version/build their own version of git/;

> hook.
>
> Change the documentation to take this dangerous edge case into account,
> and remove the mention of the advice originating on the mailing list,
> the users reading this don't care where the idea came up.
>
> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
> ---
>  Documentation/githooks.txt | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
> index 7660b95..9051584 100644
> --- a/Documentation/githooks.txt
> +++ b/Documentation/githooks.txt
> @@ -275,9 +275,11 @@ does not know the entire set of branches, so it would end up
>  firing one e-mail per ref when used naively, though.  The
>  <<post-receive,'post-receive'>> hook is more suited to that.
>  
> -Another use suggested on the mailing list is to use this hook to
> -implement access control which is finer grained than the one
> -based on filesystem group.
> +In an environment that restricts the users' access only to git
> +commands over the wire, this hook can be used to implement access
> +control without relying on filesystem ownership and group
> +membership. See linkgit:git-shell[1] for how you might use the login
> +shell to restrict the user's access to only git commands.
>  
>  Both standard output and standard error output are forwarded to
>  'git send-pack' on the other end, so you can simply `echo` messages

[PATCH v4 3/4] githooks.txt: Minor improvements to the grammar & phrasing

[Ævar Arnfjörð Bjarmason]

Change:

 * Sentences that needed "the" or "a" to either add those or change them
   so they don't need them.

 * The little tangent about "You can use this to do X (if your project
   wants to do X)" can just be shortened to "e.g. if you want to do X".

 * s/parameter/parameters/ when the plural made more sense.

Most of this goes all the way back to the initial introduction of
hooks.txt in v0.99.5-76-g6d35cc7 by Junio.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/githooks.txt | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index 9051584..a214284 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -40,15 +40,15 @@ HOOKS
 applypatch-msg
 ~~~~~~~~~~~~~~
 
-This hook is invoked by 'git am' script.  It takes a single
+This hook is invoked by 'git am'.  It takes a single
 parameter, the name of the file that holds the proposed commit
-log message.  Exiting with non-zero status causes
-'git am' to abort before applying the patch.
+log message.  Exiting with a non-zero status causes 'git am' to abort
+before applying the patch.
 
 The hook is allowed to edit the message file in place, and can
 be used to normalize the message into some project standard
-format (if the project has one). It can also be used to refuse
-the commit after inspecting the message file.
+format. It can also be used to refuse the commit after inspecting
+the message file.
 
 The default 'applypatch-msg' hook, when enabled, runs the
 'commit-msg' hook, if the latter is enabled.
@@ -81,10 +81,10 @@ pre-commit
 ~~~~~~~~~~
 
 This hook is invoked by 'git commit', and can be bypassed
-with `--no-verify` option.  It takes no parameter, and is
+with the `--no-verify` option.  It takes no parameters, and is
 invoked before obtaining the proposed commit log message and
-making a commit.  Exiting with non-zero status from this script
-causes the 'git commit' to abort.
+making a commit.  Exiting with a non-zero status from this script
+causes the 'git commit' command to abort before creating a commit.
 
 The default 'pre-commit' hook, when enabled, catches introduction
 of lines with trailing whitespaces and aborts the commit when
@@ -123,15 +123,15 @@ commit-msg
 ~~~~~~~~~~
 
 This hook is invoked by 'git commit', and can be bypassed
-with `--no-verify` option.  It takes a single parameter, the
+with the `--no-verify` option.  It takes a single parameter, the
 name of the file that holds the proposed commit log message.
-Exiting with non-zero status causes the 'git commit' to
+Exiting with a non-zero status causes the 'git commit' to
 abort.
 
-The hook is allowed to edit the message file in place, and can
-be used to normalize the message into some project standard
-format (if the project has one). It can also be used to refuse
-the commit after inspecting the message file.
+The hook is allowed to edit the message file in place, and can be used
+to normalize the message into some project standard format. It
+can also be used to refuse the commit after inspecting the message
+file.
 
 The default 'commit-msg' hook, when enabled, detects duplicate
 "Signed-off-by" lines, and aborts the commit if one is found.
@@ -139,8 +139,8 @@ The default 'commit-msg' hook, when enabled, detects duplicate
 post-commit
 ~~~~~~~~~~~
 
-This hook is invoked by 'git commit'.  It takes no
-parameter, and is invoked after a commit is made.
+This hook is invoked by 'git commit'. It takes no parameters, and is
+invoked after a commit is made.
 
 This hook is meant primarily for notification, and cannot affect
 the outcome of 'git commit'.
-- 
2.1.3

[PATCH v4 4/4] hooks: Add ability to specify where the hook directory is

[Ævar Arnfjörð Bjarmason]

Change the hardcoded lookup for .git/hooks/* to optionally lookup in
$(git config core.hooksPath)/* instead.

This is essentially a more intrusive version of the git-init ability to
specify hooks on init time via init templates.

The difference between that facility and this feature is that this can
be set up after the fact via e.g. ~/.gitconfig or /etc/gitconfig to
apply for all your personal repositories, or all repositories on the
system.

I plan on using this on a centralized Git server where users can create
arbitrary repositories under /gitroot, but I'd like to manage all the
hooks that should be run centrally via a unified dispatch mechanism.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/config.txt     | 18 ++++++++++++++++++
 Documentation/githooks.txt   | 12 ++++++++----
 cache.h                      |  1 +
 config.c                     |  3 +++
 environment.c                |  1 +
 run-command.c                |  5 ++++-
 t/t1350-config-hooks-path.sh | 31 +++++++++++++++++++++++++++++++
 7 files changed, 66 insertions(+), 5 deletions(-)
 create mode 100755 t/t1350-config-hooks-path.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 42d2b50..c007b12 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -618,6 +618,24 @@ core.attributesFile::
 	$XDG_CONFIG_HOME/git/attributes. If $XDG_CONFIG_HOME is either not
 	set or empty, $HOME/.config/git/attributes is used instead.
 
+core.hooksPath::
+	By default Git will look for your hooks in the
+	'$GIT_DIR/hooks' directory. Set this to different path,
+	e.g. '/etc/git/hooks', and Git will try to find your hooks in
+	that directory, e.g. '/etc/git/hooks/pre-receive' instead of
+	in '$GIT_DIR/hooks/pre-receive'.
++
+The path can either be absolute or relative. When specifying a
+relative path see the discussion in the "DESCRIPTION" section of
+linkgit:githooks[5] for what the relative relative path will be
+relative to.
++
+This configuration is useful in cases where you'd like to
+e.g. centrally configure your Git hooks instead of configuring them on
+a per-repository basis, or as a more flexible and centralized
+alterantive to having an `init.templateDir` where you've changed the
+'hooks' directory.
+
 core.editor::
 	Commands such as `commit` and `tag` that lets you edit
 	messages by launching an editor uses the value of this
diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index a214284..97ae78d 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -7,15 +7,19 @@ githooks - Hooks used by Git
 
 SYNOPSIS
 --------
-$GIT_DIR/hooks/*
+$GIT_DIR/hooks/* (or \`git config core.hooksPath`/*)
 
 
 DESCRIPTION
 -----------
 
-Hooks are programs you can place in the `$GIT_DIR/hooks` directory to
-trigger actions at certain points in git's execution. Hooks that don't
-have the executable bit set are ignored.
+Hooks are programs you can place in a hooks directory to trigger
+actions at certain points in git's execution. Hooks that don't have
+the executable bit set are ignored.
+
+By default the hooks directory is `$GIT_DIR/hooks`, but that can be
+changed via the `core.hooksPath` configuration variable (see
+linkgit:git-config[1]).
 
 When a hook is invoked, it is run at the root of the working tree in a
 non-bare repository, or in the $GIT_DIR in a bare
diff --git a/cache.h b/cache.h
index 2711048..4f7d222 100644
--- a/cache.h
+++ b/cache.h
@@ -654,6 +654,7 @@ extern int warn_on_object_refname_ambiguity;
 extern const char *apply_default_whitespace;
 extern const char *apply_default_ignorewhitespace;
 extern const char *git_attributes_file;
+extern const char *git_hooks_path;
 extern int zlib_compression_level;
 extern int core_compression_level;
 extern int core_compression_seen;
diff --git a/config.c b/config.c
index 10b5c95..51f80e4 100644
--- a/config.c
+++ b/config.c
@@ -717,6 +717,9 @@ static int git_default_core_config(const char *var, const char *value)
 	if (!strcmp(var, "core.attributesfile"))
 		return git_config_pathname(&git_attributes_file, var, value);
 
+	if (!strcmp(var, "core.hookspath"))
+		return git_config_pathname(&git_hooks_path, var, value);
+
 	if (!strcmp(var, "core.bare")) {
 		is_bare_repository_cfg = git_config_bool(var, value);
 		return 0;
diff --git a/environment.c b/environment.c
index 57acb2f..2857e3f 100644
--- a/environment.c
+++ b/environment.c
@@ -31,6 +31,7 @@ const char *git_log_output_encoding;
 const char *apply_default_whitespace;
 const char *apply_default_ignorewhitespace;
 const char *git_attributes_file;
+const char *git_hooks_path;
 int zlib_compression_level = Z_BEST_SPEED;
 int core_compression_level;
 int core_compression_seen;
diff --git a/run-command.c b/run-command.c
index 8c7115a..39d7237 100644
--- a/run-command.c
+++ b/run-command.c
@@ -815,7 +815,10 @@ const char *find_hook(const char *name)
 	static struct strbuf path = STRBUF_INIT;
 
 	strbuf_reset(&path);
-	strbuf_git_path(&path, "hooks/%s", name);
+	if (git_hooks_path)
+		strbuf_addf(&path, "%s/%s", git_hooks_path, name);
+	else
+		strbuf_git_path(&path, "hooks/%s", name);
 	if (access(path.buf, X_OK) < 0)
 		return NULL;
 	return path.buf;
diff --git a/t/t1350-config-hooks-path.sh b/t/t1350-config-hooks-path.sh
new file mode 100755
index 0000000..f2f0aa9
--- /dev/null
+++ b/t/t1350-config-hooks-path.sh
@@ -0,0 +1,31 @@
+#!/bin/sh
+
+test_description='Test the core.hooksPath configuration variable'
+
+. ./test-lib.sh
+
+test_expect_success 'set up a pre-commit hook in core.hooksPath' '
+	mkdir -p .git/custom-hooks .git/hooks &&
+	write_script .git/custom-hooks/pre-commit <<-\EOF &&
+printf "%s" "CUST" >>.git/PRE-COMMIT-HOOK-WAS-CALLED
+EOF
+	write_script .git/hooks/pre-commit <<-\EOF
+printf "%s" "NORM" >>.git/PRE-COMMIT-HOOK-WAS-CALLED
+EOF
+'
+
+test_expect_success 'Check that various forms of specifying core.hooksPath work' '
+	test_commit no_custom_hook &&
+	git config core.hooksPath .git/custom-hooks &&
+	test_commit have_custom_hook &&
+	git config core.hooksPath .git/custom-hooks/ &&
+	test_commit have_custom_hook_trailing_slash &&
+	git config core.hooksPath "$PWD/.git/custom-hooks" &&
+	test_commit have_custom_hook_abs_path &&
+	git config core.hooksPath "$PWD/.git/custom-hooks/" &&
+	test_commit have_custom_hook_abs_path_trailing_slash &&
+	printf "%s" "NORMCUSTCUSTCUSTCUST" >.git/PRE-COMMIT-HOOK-WAS-CALLED.expect &&
+	test_cmp .git/PRE-COMMIT-HOOK-WAS-CALLED.expect .git/PRE-COMMIT-HOOK-WAS-CALLED
+'
+
+test_done
-- 
2.1.3

Re: [PATCH v4 4/4] hooks: Add ability to specify where the hook directory is

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> +core.hooksPath::
> +	By default Git will look for your hooks in the
> +	'$GIT_DIR/hooks' directory. Set this to different path,
> +	e.g. '/etc/git/hooks', and Git will try to find your hooks in
> +	that directory, e.g. '/etc/git/hooks/pre-receive' instead of
> +	in '$GIT_DIR/hooks/pre-receive'.
> ++
> +The path can either be absolute or relative. When specifying a
> +relative path see the discussion in the "DESCRIPTION" section of
> +linkgit:githooks[5] for what the relative relative path will be
> +relative to.

    The path can be either absolute or relative.  A relative path is
    taken as relative to the directory where the hooks are run (see
    linkgit:githooks[5]).

perhaps?

> +This configuration is useful in cases where you'd like to
> +e.g. centrally configure your Git hooks instead of configuring them on

s/e.g. //;

I thought "you can do X (if you want to do X)" were getting
rewritten to "you can do X", not "you can do e.g. X".

> diff --git a/t/t1350-config-hooks-path.sh b/t/t1350-config-hooks-path.sh
> new file mode 100755
> index 0000000..f2f0aa9
> --- /dev/null
> +++ b/t/t1350-config-hooks-path.sh
> @@ -0,0 +1,31 @@
> +#!/bin/sh
> +
> +test_description='Test the core.hooksPath configuration variable'
> +
> +. ./test-lib.sh
> +
> +test_expect_success 'set up a pre-commit hook in core.hooksPath' '
> +	mkdir -p .git/custom-hooks .git/hooks &&
> +	write_script .git/custom-hooks/pre-commit <<-\EOF &&
> +printf "%s" "CUST" >>.git/PRE-COMMIT-HOOK-WAS-CALLED
> +EOF

	write_script .git/custom-hooks/pre-commit <<-\EOF &&
	printf "%s" "CUST" >>.git/PRE-COMMIT-HOOK-WAS-CALLED
	EOF

I however wonder why this is not "echo CUST" (or even "echo
CUSTOM").

> +	write_script .git/hooks/pre-commit <<-\EOF
> +printf "%s" "NORM" >>.git/PRE-COMMIT-HOOK-WAS-CALLED
> +EOF
> +'

Likewise.

> +test_expect_success 'Check that various forms of specifying core.hooksPath work' '
> +	test_commit no_custom_hook &&
> +	git config core.hooksPath .git/custom-hooks &&
> +	test_commit have_custom_hook &&
> +	git config core.hooksPath .git/custom-hooks/ &&
> +	test_commit have_custom_hook_trailing_slash &&
> +	git config core.hooksPath "$PWD/.git/custom-hooks" &&
> +	test_commit have_custom_hook_abs_path &&
> +	git config core.hooksPath "$PWD/.git/custom-hooks/" &&
> +	test_commit have_custom_hook_abs_path_trailing_slash &&
> +	printf "%s" "NORMCUSTCUSTCUSTCUST" >.git/PRE-COMMIT-HOOK-WAS-CALLED.expect &&

And this will become

	test_write_lines NORM CUST CUST CUST CUST >expect &&

> +	test_cmp .git/PRE-COMMIT-HOOK-WAS-CALLED.expect .git/PRE-COMMIT-HOOK-WAS-CALLED

If you did one-line-at-a-time with 'echo', test_cmp would show
line-wise diff, which may make spotting the difference easier.

> +'
> +
> +test_done

[PATCH v5 0/4] githooks.txt improvements + core.hooksDirectory

[Ævar Arnfjörð Bjarmason]

This is hopefully the last version of this series. I've hopefully
addressed the comments that came up and fixed a couple of other minor
things.

Changes since v4:

 * Changed wording to config.txt's core.hooksPath documentation as
   suggested.

 * Ditto for githooks.txt, and I tried to make the whole "how we
   chdir()" bit less confusing.

 * Use more obvious test idioms, e.g. indent when using <<-\EOF, use a
   multi-line test_cmp file, and use "expect" and "actual" like other
   test files instead of needlessly verbose alternatives.

 * Fixed a grammar error in a commit message.

For convenience & ease of review a diff between the bits of v4 and v5
that I changed follows below.

Ævar Arnfjörð Bjarmason (4):
  githooks.txt: Improve the intro section
  githooks.txt: Amend dangerous advice about 'update' hook ACL
  githooks.txt: Minor improvements to the grammar & phrasing
  hooks: Add ability to specify where the hook directory is

 Documentation/config.txt     | 17 ++++++++++
 Documentation/git-init.txt   |  7 ++++-
 Documentation/githooks.txt   | 75 ++++++++++++++++++++++++++------------------
 cache.h                      |  1 +
 config.c                     |  3 ++
 environment.c                |  1 +
 run-command.c                |  5 ++-
 t/t1350-config-hooks-path.sh | 37 ++++++++++++++++++++++
 8 files changed, 114 insertions(+), 32 deletions(-)
 create mode 100755 t/t1350-config-hooks-path.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index c007b12..9a74acd 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -625,16 +625,15 @@ core.hooksPath::
 	that directory, e.g. '/etc/git/hooks/pre-receive' instead of
 	in '$GIT_DIR/hooks/pre-receive'.
 +
-The path can either be absolute or relative. When specifying a
-relative path see the discussion in the "DESCRIPTION" section of
-linkgit:githooks[5] for what the relative relative path will be
-relative to.
-+
-This configuration is useful in cases where you'd like to
-e.g. centrally configure your Git hooks instead of configuring them on
-a per-repository basis, or as a more flexible and centralized
-alterantive to having an `init.templateDir` where you've changed the
-'hooks' directory.
+The path can be either absolute or relative. A relative path is
+taken as relative to the directory where the hooks are run (see
+the "DESCRIPTION" section of linkgit:githooks[5]).
++
+This configuration variable is useful in cases where you'd like to
+centrally configure your Git hooks instead of configuring them on a
+per-repository basis, or as a more flexible and centralized
+alterantive to having an `init.templateDir` where you've changed
+default hooks.
 
 core.editor::
 	Commands such as `commit` and `tag` that lets you edit
diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index 97ae78d..6f562cd 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -21,20 +21,21 @@ By default the hooks directory is `$GIT_DIR/hooks`, but that can be
 changed via the `core.hooksPath` configuration variable (see
 linkgit:git-config[1]).
 
-When a hook is invoked, it is run at the root of the working tree in a
-non-bare repository, or in the $GIT_DIR in a bare
-repository. I.e. hooks don't need to worry about the user's current
-working directory.
+Hooks don't need to worry about the user's current working
+directory. Before Git invokes a hook it changes its working
+directory. The working directory will be the root of the working tree
+in a non-bare repository, or in the $GIT_DIR directory in a bare
+repository.
 
 Hooks can get their arguments via the environment, command-line
 arguments, and stdin. See the documentation for each hook below for
 details.
 
-When 'git init' is run it may, depending on its configuration, copy
-hooks to the new repository, see the the "TEMPLATE DIRECTORY" section
-in linkgit:git-init[1] for details. When the rest of this document
-refers to "default hooks" it's talking about the default template
-shipped with Git.
+'git init' may copy hooks to the new repository, depending on its
+configuration. See the "TEMPLATE DIRECTORY" section in
+linkgit:git-init[1] for details. When the rest of this document refers
+to "default hooks" it's talking about the default template shipped
+with Git.
 
 The currently supported hooks are described below.
 
diff --git a/t/t1350-config-hooks-path.sh b/t/t1350-config-hooks-path.sh
index f2f0aa9..6e4586a 100755
--- a/t/t1350-config-hooks-path.sh
+++ b/t/t1350-config-hooks-path.sh
@@ -7,10 +7,10 @@ test_description='Test the core.hooksPath configuration variable'
 test_expect_success 'set up a pre-commit hook in core.hooksPath' '
 	mkdir -p .git/custom-hooks .git/hooks &&
 	write_script .git/custom-hooks/pre-commit <<-\EOF &&
-printf "%s" "CUST" >>.git/PRE-COMMIT-HOOK-WAS-CALLED
+	echo CUSTOM >>actual
 EOF
 	write_script .git/hooks/pre-commit <<-\EOF
-printf "%s" "NORM" >>.git/PRE-COMMIT-HOOK-WAS-CALLED
+	echo NORMAL >>actual
 EOF
 '
 
@@ -24,8 +24,14 @@ test_expect_success 'Check that various forms of specifying core.hooksPath work'
 	test_commit have_custom_hook_abs_path &&
 	git config core.hooksPath "$PWD/.git/custom-hooks/" &&
 	test_commit have_custom_hook_abs_path_trailing_slash &&
-	printf "%s" "NORMCUSTCUSTCUSTCUST" >.git/PRE-COMMIT-HOOK-WAS-CALLED.expect &&
-	test_cmp .git/PRE-COMMIT-HOOK-WAS-CALLED.expect .git/PRE-COMMIT-HOOK-WAS-CALLED
+	cat >expect <<-\EOF &&
+	NORMAL
+	CUSTOM
+	CUSTOM
+	CUSTOM
+	CUSTOM
+EOF
+	test_cmp expect actual
 '
 
 test_done


-- 
2.1.3

[PATCH v5 1/4] githooks.txt: Improve the intro section

[Ævar Arnfjörð Bjarmason]

Change the documentation so that:

 * We don't talk about "little scripts". Hooks can be as big as you
   want, and don't have to be scripts, just call them "programs".

 * We note that we change the working directory before a hook is called,
   nothing documented this explicitly, but the current behavior is
   predictable. It helps a lot to know what directory these hooks will
   be executed from.

 * We don't make claims about the example hooks which may not be true
   depending on the configuration of 'init.templateDir'. Clarify that
   we're talking about the default settings of git-init in those cases,
   and move some of this documentation into git-init's documentation
   about the default templates.

 * We briefly note in the intro that hooks can get their arguments in
   various different ways, and that how exactly is described below for
   each hook.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/git-init.txt |  7 ++++++-
 Documentation/githooks.txt | 33 +++++++++++++++++++++------------
 2 files changed, 27 insertions(+), 13 deletions(-)

diff --git a/Documentation/git-init.txt b/Documentation/git-init.txt
index 8174d27..6364e5d 100644
--- a/Documentation/git-init.txt
+++ b/Documentation/git-init.txt
@@ -130,7 +130,12 @@ The template directory will be one of the following (in order):
  - the default template directory: `/usr/share/git-core/templates`.
 
 The default template directory includes some directory structure, suggested
-"exclude patterns" (see linkgit:gitignore[5]), and sample hook files (see linkgit:githooks[5]).
+"exclude patterns" (see linkgit:gitignore[5]), and sample hook files.
+
+The sample hooks are all disabled by default, To enable one of the
+sample hooks rename it by removing its `.sample` suffix.
+
+See linkgit:githooks[5] for more general info on hook execution.
 
 EXAMPLES
 --------
diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index a2f59b1..cc082d2 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -13,18 +13,27 @@ $GIT_DIR/hooks/*
 DESCRIPTION
 -----------
 
-Hooks are little scripts you can place in `$GIT_DIR/hooks`
-directory to trigger action at certain points.  When
-'git init' is run, a handful of example hooks are copied into the
-`hooks` directory of the new repository, but by default they are
-all disabled.  To enable a hook, rename it by removing its `.sample`
-suffix.
-
-NOTE: It is also a requirement for a given hook to be executable.
-However - in a freshly initialized repository - the `.sample` files are
-executable by default.
-
-This document describes the currently defined hooks.
+Hooks are programs you can place in the `$GIT_DIR/hooks` directory to
+trigger actions at certain points in git's execution. Hooks that don't
+have the executable bit set are ignored.
+
+Hooks don't need to worry about the user's current working
+directory. Before Git invokes a hook it changes its working
+directory. The working directory will be the root of the working tree
+in a non-bare repository, or in the $GIT_DIR directory in a bare
+repository.
+
+Hooks can get their arguments via the environment, command-line
+arguments, and stdin. See the documentation for each hook below for
+details.
+
+'git init' may copy hooks to the new repository, depending on its
+configuration. See the "TEMPLATE DIRECTORY" section in
+linkgit:git-init[1] for details. When the rest of this document refers
+to "default hooks" it's talking about the default template shipped
+with Git.
+
+The currently supported hooks are described below.
 
 HOOKS
 -----
-- 
2.1.3

[PATCH v5 2/4] githooks.txt: Amend dangerous advice about 'update' hook ACL

[Ævar Arnfjörð Bjarmason]

Any ACL you implement via an 'update' hook isn't actual access control
if the user has login access to the machine running git, because they
can trivially just build their own git version which doesn't run the
hook.

Change the documentation to take this dangerous edge case into account,
and remove the mention of the advice originating on the mailing list,
the users reading this don't care where the idea came up.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/githooks.txt | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index cc082d2..d28dc4c 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -276,9 +276,11 @@ does not know the entire set of branches, so it would end up
 firing one e-mail per ref when used naively, though.  The
 <<post-receive,'post-receive'>> hook is more suited to that.
 
-Another use suggested on the mailing list is to use this hook to
-implement access control which is finer grained than the one
-based on filesystem group.
+In an environment that restricts the users' access only to git
+commands over the wire, this hook can be used to implement access
+control without relying on filesystem ownership and group
+membership. See linkgit:git-shell[1] for how you might use the login
+shell to restrict the user's access to only git commands.
 
 Both standard output and standard error output are forwarded to
 'git send-pack' on the other end, so you can simply `echo` messages
-- 
2.1.3

[PATCH v5 3/4] githooks.txt: Minor improvements to the grammar & phrasing

[Ævar Arnfjörð Bjarmason]

Change:

 * Sentences that needed "the" or "a" to either add those or change them
   so they don't need them.

 * The little tangent about "You can use this to do X (if your project
   wants to do X)" can just be shortened to "e.g. if you want to do X".

 * s/parameter/parameters/ when the plural made more sense.

Most of this goes all the way back to the initial introduction of
hooks.txt in v0.99.5-76-g6d35cc7 by Junio.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/githooks.txt | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index d28dc4c..13f9a18 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -41,15 +41,15 @@ HOOKS
 applypatch-msg
 ~~~~~~~~~~~~~~
 
-This hook is invoked by 'git am' script.  It takes a single
+This hook is invoked by 'git am'.  It takes a single
 parameter, the name of the file that holds the proposed commit
-log message.  Exiting with non-zero status causes
-'git am' to abort before applying the patch.
+log message.  Exiting with a non-zero status causes 'git am' to abort
+before applying the patch.
 
 The hook is allowed to edit the message file in place, and can
 be used to normalize the message into some project standard
-format (if the project has one). It can also be used to refuse
-the commit after inspecting the message file.
+format. It can also be used to refuse the commit after inspecting
+the message file.
 
 The default 'applypatch-msg' hook, when enabled, runs the
 'commit-msg' hook, if the latter is enabled.
@@ -82,10 +82,10 @@ pre-commit
 ~~~~~~~~~~
 
 This hook is invoked by 'git commit', and can be bypassed
-with `--no-verify` option.  It takes no parameter, and is
+with the `--no-verify` option.  It takes no parameters, and is
 invoked before obtaining the proposed commit log message and
-making a commit.  Exiting with non-zero status from this script
-causes the 'git commit' to abort.
+making a commit.  Exiting with a non-zero status from this script
+causes the 'git commit' command to abort before creating a commit.
 
 The default 'pre-commit' hook, when enabled, catches introduction
 of lines with trailing whitespaces and aborts the commit when
@@ -124,15 +124,15 @@ commit-msg
 ~~~~~~~~~~
 
 This hook is invoked by 'git commit', and can be bypassed
-with `--no-verify` option.  It takes a single parameter, the
+with the `--no-verify` option.  It takes a single parameter, the
 name of the file that holds the proposed commit log message.
-Exiting with non-zero status causes the 'git commit' to
+Exiting with a non-zero status causes the 'git commit' to
 abort.
 
-The hook is allowed to edit the message file in place, and can
-be used to normalize the message into some project standard
-format (if the project has one). It can also be used to refuse
-the commit after inspecting the message file.
+The hook is allowed to edit the message file in place, and can be used
+to normalize the message into some project standard format. It
+can also be used to refuse the commit after inspecting the message
+file.
 
 The default 'commit-msg' hook, when enabled, detects duplicate
 "Signed-off-by" lines, and aborts the commit if one is found.
@@ -140,8 +140,8 @@ The default 'commit-msg' hook, when enabled, detects duplicate
 post-commit
 ~~~~~~~~~~~
 
-This hook is invoked by 'git commit'.  It takes no
-parameter, and is invoked after a commit is made.
+This hook is invoked by 'git commit'. It takes no parameters, and is
+invoked after a commit is made.
 
 This hook is meant primarily for notification, and cannot affect
 the outcome of 'git commit'.
-- 
2.1.3

[PATCH v5 4/4] hooks: Add ability to specify where the hook directory is

[Ævar Arnfjörð Bjarmason]

Change the hardcoded lookup for .git/hooks/* to optionally lookup in
$(git config core.hooksPath)/* instead.

This is essentially a more intrusive version of the git-init ability to
specify hooks on init time via init templates.

The difference between that facility and this feature is that this can
be set up after the fact via e.g. ~/.gitconfig or /etc/gitconfig to
apply for all your personal repositories, or all repositories on the
system.

I plan on using this on a centralized Git server where users can create
arbitrary repositories under /gitroot, but I'd like to manage all the
hooks that should be run centrally via a unified dispatch mechanism.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/config.txt     | 17 +++++++++++++++++
 Documentation/githooks.txt   | 12 ++++++++----
 cache.h                      |  1 +
 config.c                     |  3 +++
 environment.c                |  1 +
 run-command.c                |  5 ++++-
 t/t1350-config-hooks-path.sh | 37 +++++++++++++++++++++++++++++++++++++
 7 files changed, 71 insertions(+), 5 deletions(-)
 create mode 100755 t/t1350-config-hooks-path.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 42d2b50..9a74acd 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -618,6 +618,23 @@ core.attributesFile::
 	$XDG_CONFIG_HOME/git/attributes. If $XDG_CONFIG_HOME is either not
 	set or empty, $HOME/.config/git/attributes is used instead.
 
+core.hooksPath::
+	By default Git will look for your hooks in the
+	'$GIT_DIR/hooks' directory. Set this to different path,
+	e.g. '/etc/git/hooks', and Git will try to find your hooks in
+	that directory, e.g. '/etc/git/hooks/pre-receive' instead of
+	in '$GIT_DIR/hooks/pre-receive'.
++
+The path can be either absolute or relative. A relative path is
+taken as relative to the directory where the hooks are run (see
+the "DESCRIPTION" section of linkgit:githooks[5]).
++
+This configuration variable is useful in cases where you'd like to
+centrally configure your Git hooks instead of configuring them on a
+per-repository basis, or as a more flexible and centralized
+alterantive to having an `init.templateDir` where you've changed
+default hooks.
+
 core.editor::
 	Commands such as `commit` and `tag` that lets you edit
 	messages by launching an editor uses the value of this
diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index 13f9a18..6f562cd 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -7,15 +7,19 @@ githooks - Hooks used by Git
 
 SYNOPSIS
 --------
-$GIT_DIR/hooks/*
+$GIT_DIR/hooks/* (or \`git config core.hooksPath`/*)
 
 
 DESCRIPTION
 -----------
 
-Hooks are programs you can place in the `$GIT_DIR/hooks` directory to
-trigger actions at certain points in git's execution. Hooks that don't
-have the executable bit set are ignored.
+Hooks are programs you can place in a hooks directory to trigger
+actions at certain points in git's execution. Hooks that don't have
+the executable bit set are ignored.
+
+By default the hooks directory is `$GIT_DIR/hooks`, but that can be
+changed via the `core.hooksPath` configuration variable (see
+linkgit:git-config[1]).
 
 Hooks don't need to worry about the user's current working
 directory. Before Git invokes a hook it changes its working
diff --git a/cache.h b/cache.h
index fd728f0..c071f5d 100644
--- a/cache.h
+++ b/cache.h
@@ -654,6 +654,7 @@ extern int warn_on_object_refname_ambiguity;
 extern const char *apply_default_whitespace;
 extern const char *apply_default_ignorewhitespace;
 extern const char *git_attributes_file;
+extern const char *git_hooks_path;
 extern int zlib_compression_level;
 extern int core_compression_level;
 extern int core_compression_seen;
diff --git a/config.c b/config.c
index 10b5c95..51f80e4 100644
--- a/config.c
+++ b/config.c
@@ -717,6 +717,9 @@ static int git_default_core_config(const char *var, const char *value)
 	if (!strcmp(var, "core.attributesfile"))
 		return git_config_pathname(&git_attributes_file, var, value);
 
+	if (!strcmp(var, "core.hookspath"))
+		return git_config_pathname(&git_hooks_path, var, value);
+
 	if (!strcmp(var, "core.bare")) {
 		is_bare_repository_cfg = git_config_bool(var, value);
 		return 0;
diff --git a/environment.c b/environment.c
index 57acb2f..2857e3f 100644
--- a/environment.c
+++ b/environment.c
@@ -31,6 +31,7 @@ const char *git_log_output_encoding;
 const char *apply_default_whitespace;
 const char *apply_default_ignorewhitespace;
 const char *git_attributes_file;
+const char *git_hooks_path;
 int zlib_compression_level = Z_BEST_SPEED;
 int core_compression_level;
 int core_compression_seen;
diff --git a/run-command.c b/run-command.c
index e4593cd..f5c57a5 100644
--- a/run-command.c
+++ b/run-command.c
@@ -825,7 +825,10 @@ const char *find_hook(const char *name)
 	static struct strbuf path = STRBUF_INIT;
 
 	strbuf_reset(&path);
-	strbuf_git_path(&path, "hooks/%s", name);
+	if (git_hooks_path)
+		strbuf_addf(&path, "%s/%s", git_hooks_path, name);
+	else
+		strbuf_git_path(&path, "hooks/%s", name);
 	if (access(path.buf, X_OK) < 0)
 		return NULL;
 	return path.buf;
diff --git a/t/t1350-config-hooks-path.sh b/t/t1350-config-hooks-path.sh
new file mode 100755
index 0000000..6e4586a
--- /dev/null
+++ b/t/t1350-config-hooks-path.sh
@@ -0,0 +1,37 @@
+#!/bin/sh
+
+test_description='Test the core.hooksPath configuration variable'
+
+. ./test-lib.sh
+
+test_expect_success 'set up a pre-commit hook in core.hooksPath' '
+	mkdir -p .git/custom-hooks .git/hooks &&
+	write_script .git/custom-hooks/pre-commit <<-\EOF &&
+	echo CUSTOM >>actual
+EOF
+	write_script .git/hooks/pre-commit <<-\EOF
+	echo NORMAL >>actual
+EOF
+'
+
+test_expect_success 'Check that various forms of specifying core.hooksPath work' '
+	test_commit no_custom_hook &&
+	git config core.hooksPath .git/custom-hooks &&
+	test_commit have_custom_hook &&
+	git config core.hooksPath .git/custom-hooks/ &&
+	test_commit have_custom_hook_trailing_slash &&
+	git config core.hooksPath "$PWD/.git/custom-hooks" &&
+	test_commit have_custom_hook_abs_path &&
+	git config core.hooksPath "$PWD/.git/custom-hooks/" &&
+	test_commit have_custom_hook_abs_path_trailing_slash &&
+	cat >expect <<-\EOF &&
+	NORMAL
+	CUSTOM
+	CUSTOM
+	CUSTOM
+	CUSTOM
+EOF
+	test_cmp expect actual
+'
+
+test_done
-- 
2.1.3

Re: [PATCH v5 0/4] githooks.txt improvements + core.hooksDirectory

[Junio C Hamano]

Ævar Arnfjörð Bjarmason  <avarab@gmail.com> writes:

> For convenience & ease of review a diff between the bits of v4 and v5
> that I changed follows below.

Thanks.

> diff --git a/Documentation/config.txt b/Documentation/config.txt
> ...
> +This configuration variable is useful in cases where you'd like to
> +centrally configure your Git hooks instead of configuring them on a
> +per-repository basis, or as a more flexible and centralized
> +alterantive to having an `init.templateDir` where you've changed

"alternative" (can be locally amended).

> diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
> ...
> +Hooks don't need to worry about the user's current working
> +directory. Before Git invokes a hook it changes its working
> +directory. The working directory will be the root of the working tree
> +in a non-bare repository, or in the $GIT_DIR directory in a bare
> +repository.

I suspect that the first sentence does more harm than good.  When
you say "don't need to worry about", somebody is bound to ask "that
is good that I do not need to, but what if I _want_ to know?", and
you do not have a good answer to that question.

	Before Git invokes a hook, it changes its working directory
	to either the root of the working tree in a non-bare
	repository, or to the $GIT_DIR in a bare repository.

perhaps?

> diff --git a/t/t1350-config-hooks-path.sh b/t/t1350-config-hooks-path.sh
> index f2f0aa9..6e4586a 100755
> --- a/t/t1350-config-hooks-path.sh
> +++ b/t/t1350-config-hooks-path.sh
> @@ -7,10 +7,10 @@ test_description='Test the core.hooksPath configuration variable'
>  test_expect_success 'set up a pre-commit hook in core.hooksPath' '
>  	mkdir -p .git/custom-hooks .git/hooks &&
>  	write_script .git/custom-hooks/pre-commit <<-\EOF &&
> -printf "%s" "CUST" >>.git/PRE-COMMIT-HOOK-WAS-CALLED
> +	echo CUSTOM >>actual
>  EOF
>  	write_script .git/hooks/pre-commit <<-\EOF
> -printf "%s" "NORM" >>.git/PRE-COMMIT-HOOK-WAS-CALLED
> +	echo NORMAL >>actual
>  EOF
>  '

These closing EOF can and should be indented like the body of the
here-text, i.e.

	cmd <<-\EOF &&
        ...
        EOF

> @@ -24,8 +24,14 @@ test_expect_success 'Check that various forms of specifying core.hooksPath work'
>  	test_commit have_custom_hook_abs_path &&
>  	git config core.hooksPath "$PWD/.git/custom-hooks/" &&
>  	test_commit have_custom_hook_abs_path_trailing_slash &&
> -	printf "%s" "NORMCUSTCUSTCUSTCUST" >.git/PRE-COMMIT-HOOK-WAS-CALLED.expect &&
> -	test_cmp .git/PRE-COMMIT-HOOK-WAS-CALLED.expect .git/PRE-COMMIT-HOOK-WAS-CALLED
> +	cat >expect <<-\EOF &&
> +	NORMAL
> +	CUSTOM
> +	CUSTOM
> +	CUSTOM
> +	CUSTOM
> +EOF

Likewise.

[PATCH v6 0/4] githooks.txt improvements + core.hooksDirectory

[Ævar Arnfjörð Bjarmason]

Changed to reflect Junio's latest comment, diff between the whole
series and the last one here at the end (made with -U0).

Ævar Arnfjörð Bjarmason (4):
  githooks.txt: Improve the intro section
  githooks.txt: Amend dangerous advice about 'update' hook ACL
  githooks.txt: Minor improvements to the grammar & phrasing
  hooks: Add ability to specify where the hook directory is

 Documentation/config.txt     | 17 +++++++++++
 Documentation/git-init.txt   |  7 ++++-
 Documentation/githooks.txt   | 73 ++++++++++++++++++++++++++------------------
 cache.h                      |  1 +
 config.c                     |  3 ++
 environment.c                |  1 +
 run-command.c                |  5 ++-
 t/t1350-config-hooks-path.sh | 37 ++++++++++++++++++++++
 8 files changed, 112 insertions(+), 32 deletions(-)
 create mode 100755 t/t1350-config-hooks-path.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 9a74acd..6cc069f 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -635 +635 @@ per-repository basis, or as a more flexible and centralized
-alterantive to having an `init.templateDir` where you've changed
+alternative to having an `init.templateDir` where you've changed
diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index 6f562cd..d82e912 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -24,5 +24,3 @@ linkgit:git-config[1]).
-Hooks don't need to worry about the user's current working
-directory. Before Git invokes a hook it changes its working
-directory. The working directory will be the root of the working tree
-in a non-bare repository, or in the $GIT_DIR directory in a bare
-repository.
+Before Git invokes a hook, it changes its working directory to either
+the root of the working tree in a non-bare repository, or to the
+$GIT_DIR in a bare repository.
diff --git a/t/t1350-config-hooks-path.sh b/t/t1350-config-hooks-path.sh
index 6e4586a..5e3fb3a 100755
--- a/t/t1350-config-hooks-path.sh
+++ b/t/t1350-config-hooks-path.sh
@@ -11 +11 @@ test_expect_success 'set up a pre-commit hook in core.hooksPath' '
-EOF
+	EOF
@@ -14 +14 @@ EOF
-EOF
+	EOF
@@ -33 +33 @@ test_expect_success 'Check that various forms of specifying core.hooksPath work'
-EOF
+	EOF


-- 
2.1.3

[PATCH v6 1/4] githooks.txt: Improve the intro section

[Ævar Arnfjörð Bjarmason]

Change the documentation so that:

 * We don't talk about "little scripts". Hooks can be as big as you
   want, and don't have to be scripts, just call them "programs".

 * We note that we change the working directory before a hook is called,
   nothing documented this explicitly, but the current behavior is
   predictable. It helps a lot to know what directory these hooks will
   be executed from.

 * We don't make claims about the example hooks which may not be true
   depending on the configuration of 'init.templateDir'. Clarify that
   we're talking about the default settings of git-init in those cases,
   and move some of this documentation into git-init's documentation
   about the default templates.

 * We briefly note in the intro that hooks can get their arguments in
   various different ways, and that how exactly is described below for
   each hook.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/git-init.txt |  7 ++++++-
 Documentation/githooks.txt | 31 +++++++++++++++++++------------
 2 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/Documentation/git-init.txt b/Documentation/git-init.txt
index 8174d27..6364e5d 100644
--- a/Documentation/git-init.txt
+++ b/Documentation/git-init.txt
@@ -130,7 +130,12 @@ The template directory will be one of the following (in order):
  - the default template directory: `/usr/share/git-core/templates`.
 
 The default template directory includes some directory structure, suggested
-"exclude patterns" (see linkgit:gitignore[5]), and sample hook files (see linkgit:githooks[5]).
+"exclude patterns" (see linkgit:gitignore[5]), and sample hook files.
+
+The sample hooks are all disabled by default, To enable one of the
+sample hooks rename it by removing its `.sample` suffix.
+
+See linkgit:githooks[5] for more general info on hook execution.
 
 EXAMPLES
 --------
diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index a2f59b1..0d94337 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -13,18 +13,25 @@ $GIT_DIR/hooks/*
 DESCRIPTION
 -----------
 
-Hooks are little scripts you can place in `$GIT_DIR/hooks`
-directory to trigger action at certain points.  When
-'git init' is run, a handful of example hooks are copied into the
-`hooks` directory of the new repository, but by default they are
-all disabled.  To enable a hook, rename it by removing its `.sample`
-suffix.
-
-NOTE: It is also a requirement for a given hook to be executable.
-However - in a freshly initialized repository - the `.sample` files are
-executable by default.
-
-This document describes the currently defined hooks.
+Hooks are programs you can place in the `$GIT_DIR/hooks` directory to
+trigger actions at certain points in git's execution. Hooks that don't
+have the executable bit set are ignored.
+
+Before Git invokes a hook, it changes its working directory to either
+the root of the working tree in a non-bare repository, or to the
+$GIT_DIR in a bare repository.
+
+Hooks can get their arguments via the environment, command-line
+arguments, and stdin. See the documentation for each hook below for
+details.
+
+'git init' may copy hooks to the new repository, depending on its
+configuration. See the "TEMPLATE DIRECTORY" section in
+linkgit:git-init[1] for details. When the rest of this document refers
+to "default hooks" it's talking about the default template shipped
+with Git.
+
+The currently supported hooks are described below.
 
 HOOKS
 -----
-- 
2.1.3

[PATCH v6 2/4] githooks.txt: Amend dangerous advice about 'update' hook ACL

[Ævar Arnfjörð Bjarmason]

Any ACL you implement via an 'update' hook isn't actual access control
if the user has login access to the machine running git, because they
can trivially just build their own git version which doesn't run the
hook.

Change the documentation to take this dangerous edge case into account,
and remove the mention of the advice originating on the mailing list,
the users reading this don't care where the idea came up.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/githooks.txt | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index 0d94337..0dd618a 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -274,9 +274,11 @@ does not know the entire set of branches, so it would end up
 firing one e-mail per ref when used naively, though.  The
 <<post-receive,'post-receive'>> hook is more suited to that.
 
-Another use suggested on the mailing list is to use this hook to
-implement access control which is finer grained than the one
-based on filesystem group.
+In an environment that restricts the users' access only to git
+commands over the wire, this hook can be used to implement access
+control without relying on filesystem ownership and group
+membership. See linkgit:git-shell[1] for how you might use the login
+shell to restrict the user's access to only git commands.
 
 Both standard output and standard error output are forwarded to
 'git send-pack' on the other end, so you can simply `echo` messages
-- 
2.1.3

[PATCH v6 3/4] githooks.txt: Minor improvements to the grammar & phrasing

[Ævar Arnfjörð Bjarmason]

Change:

 * Sentences that needed "the" or "a" to either add those or change them
   so they don't need them.

 * The little tangent about "You can use this to do X (if your project
   wants to do X)" can just be shortened to "e.g. if you want to do X".

 * s/parameter/parameters/ when the plural made more sense.

Most of this goes all the way back to the initial introduction of
hooks.txt in v0.99.5-76-g6d35cc7 by Junio.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/githooks.txt | 32 ++++++++++++++++----------------
 1 file changed, 16 insertions(+), 16 deletions(-)

diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index 0dd618a..3049574 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -39,15 +39,15 @@ HOOKS
 applypatch-msg
 ~~~~~~~~~~~~~~
 
-This hook is invoked by 'git am' script.  It takes a single
+This hook is invoked by 'git am'.  It takes a single
 parameter, the name of the file that holds the proposed commit
-log message.  Exiting with non-zero status causes
-'git am' to abort before applying the patch.
+log message.  Exiting with a non-zero status causes 'git am' to abort
+before applying the patch.
 
 The hook is allowed to edit the message file in place, and can
 be used to normalize the message into some project standard
-format (if the project has one). It can also be used to refuse
-the commit after inspecting the message file.
+format. It can also be used to refuse the commit after inspecting
+the message file.
 
 The default 'applypatch-msg' hook, when enabled, runs the
 'commit-msg' hook, if the latter is enabled.
@@ -80,10 +80,10 @@ pre-commit
 ~~~~~~~~~~
 
 This hook is invoked by 'git commit', and can be bypassed
-with `--no-verify` option.  It takes no parameter, and is
+with the `--no-verify` option.  It takes no parameters, and is
 invoked before obtaining the proposed commit log message and
-making a commit.  Exiting with non-zero status from this script
-causes the 'git commit' to abort.
+making a commit.  Exiting with a non-zero status from this script
+causes the 'git commit' command to abort before creating a commit.
 
 The default 'pre-commit' hook, when enabled, catches introduction
 of lines with trailing whitespaces and aborts the commit when
@@ -122,15 +122,15 @@ commit-msg
 ~~~~~~~~~~
 
 This hook is invoked by 'git commit', and can be bypassed
-with `--no-verify` option.  It takes a single parameter, the
+with the `--no-verify` option.  It takes a single parameter, the
 name of the file that holds the proposed commit log message.
-Exiting with non-zero status causes the 'git commit' to
+Exiting with a non-zero status causes the 'git commit' to
 abort.
 
-The hook is allowed to edit the message file in place, and can
-be used to normalize the message into some project standard
-format (if the project has one). It can also be used to refuse
-the commit after inspecting the message file.
+The hook is allowed to edit the message file in place, and can be used
+to normalize the message into some project standard format. It
+can also be used to refuse the commit after inspecting the message
+file.
 
 The default 'commit-msg' hook, when enabled, detects duplicate
 "Signed-off-by" lines, and aborts the commit if one is found.
@@ -138,8 +138,8 @@ The default 'commit-msg' hook, when enabled, detects duplicate
 post-commit
 ~~~~~~~~~~~
 
-This hook is invoked by 'git commit'.  It takes no
-parameter, and is invoked after a commit is made.
+This hook is invoked by 'git commit'. It takes no parameters, and is
+invoked after a commit is made.
 
 This hook is meant primarily for notification, and cannot affect
 the outcome of 'git commit'.
-- 
2.1.3

[PATCH v6 4/4] hooks: Add ability to specify where the hook directory is

[Ævar Arnfjörð Bjarmason]

Change the hardcoded lookup for .git/hooks/* to optionally lookup in
$(git config core.hooksPath)/* instead.

This is essentially a more intrusive version of the git-init ability to
specify hooks on init time via init templates.

The difference between that facility and this feature is that this can
be set up after the fact via e.g. ~/.gitconfig or /etc/gitconfig to
apply for all your personal repositories, or all repositories on the
system.

I plan on using this on a centralized Git server where users can create
arbitrary repositories under /gitroot, but I'd like to manage all the
hooks that should be run centrally via a unified dispatch mechanism.

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
---
 Documentation/config.txt     | 17 +++++++++++++++++
 Documentation/githooks.txt   | 12 ++++++++----
 cache.h                      |  1 +
 config.c                     |  3 +++
 environment.c                |  1 +
 run-command.c                |  5 ++++-
 t/t1350-config-hooks-path.sh | 37 +++++++++++++++++++++++++++++++++++++
 7 files changed, 71 insertions(+), 5 deletions(-)
 create mode 100755 t/t1350-config-hooks-path.sh

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 42d2b50..6cc069f 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -618,6 +618,23 @@ core.attributesFile::
 	$XDG_CONFIG_HOME/git/attributes. If $XDG_CONFIG_HOME is either not
 	set or empty, $HOME/.config/git/attributes is used instead.
 
+core.hooksPath::
+	By default Git will look for your hooks in the
+	'$GIT_DIR/hooks' directory. Set this to different path,
+	e.g. '/etc/git/hooks', and Git will try to find your hooks in
+	that directory, e.g. '/etc/git/hooks/pre-receive' instead of
+	in '$GIT_DIR/hooks/pre-receive'.
++
+The path can be either absolute or relative. A relative path is
+taken as relative to the directory where the hooks are run (see
+the "DESCRIPTION" section of linkgit:githooks[5]).
++
+This configuration variable is useful in cases where you'd like to
+centrally configure your Git hooks instead of configuring them on a
+per-repository basis, or as a more flexible and centralized
+alternative to having an `init.templateDir` where you've changed
+default hooks.
+
 core.editor::
 	Commands such as `commit` and `tag` that lets you edit
 	messages by launching an editor uses the value of this
diff --git a/Documentation/githooks.txt b/Documentation/githooks.txt
index 3049574..d82e912 100644
--- a/Documentation/githooks.txt
+++ b/Documentation/githooks.txt
@@ -7,15 +7,19 @@ githooks - Hooks used by Git
 
 SYNOPSIS
 --------
-$GIT_DIR/hooks/*
+$GIT_DIR/hooks/* (or \`git config core.hooksPath`/*)
 
 
 DESCRIPTION
 -----------
 
-Hooks are programs you can place in the `$GIT_DIR/hooks` directory to
-trigger actions at certain points in git's execution. Hooks that don't
-have the executable bit set are ignored.
+Hooks are programs you can place in a hooks directory to trigger
+actions at certain points in git's execution. Hooks that don't have
+the executable bit set are ignored.
+
+By default the hooks directory is `$GIT_DIR/hooks`, but that can be
+changed via the `core.hooksPath` configuration variable (see
+linkgit:git-config[1]).
 
 Before Git invokes a hook, it changes its working directory to either
 the root of the working tree in a non-bare repository, or to the
diff --git a/cache.h b/cache.h
index fd728f0..c071f5d 100644
--- a/cache.h
+++ b/cache.h
@@ -654,6 +654,7 @@ extern int warn_on_object_refname_ambiguity;
 extern const char *apply_default_whitespace;
 extern const char *apply_default_ignorewhitespace;
 extern const char *git_attributes_file;
+extern const char *git_hooks_path;
 extern int zlib_compression_level;
 extern int core_compression_level;
 extern int core_compression_seen;
diff --git a/config.c b/config.c
index 10b5c95..51f80e4 100644
--- a/config.c
+++ b/config.c
@@ -717,6 +717,9 @@ static int git_default_core_config(const char *var, const char *value)
 	if (!strcmp(var, "core.attributesfile"))
 		return git_config_pathname(&git_attributes_file, var, value);
 
+	if (!strcmp(var, "core.hookspath"))
+		return git_config_pathname(&git_hooks_path, var, value);
+
 	if (!strcmp(var, "core.bare")) {
 		is_bare_repository_cfg = git_config_bool(var, value);
 		return 0;
diff --git a/environment.c b/environment.c
index 57acb2f..2857e3f 100644
--- a/environment.c
+++ b/environment.c
@@ -31,6 +31,7 @@ const char *git_log_output_encoding;
 const char *apply_default_whitespace;
 const char *apply_default_ignorewhitespace;
 const char *git_attributes_file;
+const char *git_hooks_path;
 int zlib_compression_level = Z_BEST_SPEED;
 int core_compression_level;
 int core_compression_seen;
diff --git a/run-command.c b/run-command.c
index e4593cd..f5c57a5 100644
--- a/run-command.c
+++ b/run-command.c
@@ -825,7 +825,10 @@ const char *find_hook(const char *name)
 	static struct strbuf path = STRBUF_INIT;
 
 	strbuf_reset(&path);
-	strbuf_git_path(&path, "hooks/%s", name);
+	if (git_hooks_path)
+		strbuf_addf(&path, "%s/%s", git_hooks_path, name);
+	else
+		strbuf_git_path(&path, "hooks/%s", name);
 	if (access(path.buf, X_OK) < 0)
 		return NULL;
 	return path.buf;
diff --git a/t/t1350-config-hooks-path.sh b/t/t1350-config-hooks-path.sh
new file mode 100755
index 0000000..5e3fb3a
--- /dev/null
+++ b/t/t1350-config-hooks-path.sh
@@ -0,0 +1,37 @@
+#!/bin/sh
+
+test_description='Test the core.hooksPath configuration variable'
+
+. ./test-lib.sh
+
+test_expect_success 'set up a pre-commit hook in core.hooksPath' '
+	mkdir -p .git/custom-hooks .git/hooks &&
+	write_script .git/custom-hooks/pre-commit <<-\EOF &&
+	echo CUSTOM >>actual
+	EOF
+	write_script .git/hooks/pre-commit <<-\EOF
+	echo NORMAL >>actual
+	EOF
+'
+
+test_expect_success 'Check that various forms of specifying core.hooksPath work' '
+	test_commit no_custom_hook &&
+	git config core.hooksPath .git/custom-hooks &&
+	test_commit have_custom_hook &&
+	git config core.hooksPath .git/custom-hooks/ &&
+	test_commit have_custom_hook_trailing_slash &&
+	git config core.hooksPath "$PWD/.git/custom-hooks" &&
+	test_commit have_custom_hook_abs_path &&
+	git config core.hooksPath "$PWD/.git/custom-hooks/" &&
+	test_commit have_custom_hook_abs_path_trailing_slash &&
+	cat >expect <<-\EOF &&
+	NORMAL
+	CUSTOM
+	CUSTOM
+	CUSTOM
+	CUSTOM
+	EOF
+	test_cmp expect actual
+'
+
+test_done
-- 
2.1.3

Re: [PATCH v6 0/4] githooks.txt improvements + core.hooksDirectory

[Junio C Hamano]

Thanks, will replace.  Let's start moving this to 'next'.