patch, 2 partsDocumentation/githooks: mention pwd, $GIT_PREFIX

A couple of patches to document and test that hooks are run from the top-level directory and that GIT_PREFIX is set to the subdirectory that Git was run from (like !aliases).

The new documentation is mostly lifted from the documentation for alias.*. I don't think the new documentation is perfectly clear, but it's a start. In particular, what does Git do for hook pwd and GIT_PREFIX when it is run from within a bare repository? Or from within .git? Or if GIT_WORK_TREE (--work-tree) and/or GIT_DIR (--git-dir) are set? Many of these same questions apply to !aliases, so the documentation for alias.* should also be shored up.

-Richard

Richard Hansen (2):
  Documentation/githooks: mention pwd, $GIT_PREFIX
  t1020-subdirectory.sh: check hook pwd, $GIT_PREFIX

 Documentation/githooks.txt |  6 ++++++
 t/t1020-subdirectory.sh    | 34 ++++++++++++++++++++++++++++++++++
 2 files changed, 40 insertions(+)

Document that hooks are run from the top-level directory and that GIT_PREFIX is set to the name of the original subdirectory (relative to the top-level directory).

Signed-off-by: Richard Hansen <rhansen@bbn.com>
---
 Documentation/githooks.txt | 6 ++++++
 1 file changed, 6 insertions(+)

Make sure hooks are executed at the top-level directory and that GIT_PREFIX is set (as documented).

Signed-off-by: Richard Hansen <rhansen@bbn.com>
---
 t/t1020-subdirectory.sh | 34 ++++++++++++++++++++++++++++++++++
 1 file changed, 34 insertions(+)

Am 10.01.2015 um 07:49 schrieb Richard Hansen:

What is the purpose of this test?

Use write_script() to construct a shell script.

This cleanup would be skipped if the checkout fails for some reason. Use test_when_finished.

The outer sub-shell us unnecessary, isn't it?

If 'git checkout' runs the hook from the wrong directory, there would not exist a file 'actual' at this point because it was rm -f'd earlier, and the test would fail. Perhaps it would make sense to document this failure case by inserting

	test_path_is_file actual &&

before the test_cmp?

Which makes me think: Would the test for existence of 'actual' be sufficient? Then the test_cmp could be omitted. The advantage is that we do not depend on how the `pwd` is formatted: With or without symbolic links in any leading path or c:/foo vs. /c/foo on Windows. (I anticipate that the test as written fails on Windows because 'expect' is in c:/foo form and 'actual' is in /c/foo form.)

The comments about the sub-shell, write_script, and clean-up apply here, too.

-- Hannes

patch 1/2 is the same as v1 patch 2/2 has been reworked to incorporate Hannes's feedback (thank you!)

-Richard

Richard Hansen (2):
  Documentation/githooks: mention pwd, $GIT_PREFIX
  t1020-subdirectory.sh: check hook pwd, $GIT_PREFIX

 Documentation/githooks.txt |  6 ++++++
 t/t1020-subdirectory.sh    | 23 +++++++++++++++++++++++
 2 files changed, 29 insertions(+)

Document that hooks are run from the top-level directory and that GIT_PREFIX is set to the name of the original subdirectory (relative to the top-level directory).

Signed-off-by: Richard Hansen <rhansen@bbn.com>
---
 Documentation/githooks.txt | 6 ++++++
 1 file changed, 6 insertions(+)

Richard Hansen <rhansen@bbn.com> writes:

I agree that it is a good idea to describe how the hook writers can go to the top-level directory and how the hook writers can discover where the hooked operation started, but these two lines cannot be the whole story---what happens when there is no top-level directory (i.e. a bare repository)?

Is this universal to all hooks, or just the ones you examined? I ask this because I know we do not go through a single interface to call out to hooks that says "cd to the root and then run the hook given as an argument".

Is this also universal, or is it set only for some but not all hooks? What happens in a bare repository? What is given if you are in a non-bare repository and are already at the root level?

Make sure hooks are executed at the top-level directory and that GIT_PREFIX is set (as documented).

Signed-off-by: Richard Hansen <rhansen@bbn.com>
---
 t/t1020-subdirectory.sh | 23 +++++++++++++++++++++++
 1 file changed, 23 insertions(+)

Richard Hansen <rhansen@bbn.com> writes:

The same comment as the one for 1/2 applies here. If we substitute 'hook' everywhere with 'post-checkout hook' in this patch, it makes perfect sense to me, but otherwise this is far from "check _hook_" in general.

Cute, but it is misleading to use "pwd" there, because the contents of the file does not matter for this test, even though the test is about the current directory. It forces the reader to look for the place where you are comparing the contents of that file with expected path to the current directory, and no such code exists.

"date >actual", "echo >actual", or even just a redirection without command, i.e. ">actual", woudl have been easier to see what is going on (I would have used the last form if I were doing this patch).

It is not wrong per-se, but the same cute trick could have been used, i.e.

	write_script ... post-checkout <<-\EOF &&
        >"$GIT_PREFIX/actual"
        EOF
        ...
        test_path_is_file dir/actual