Re: Weird output of git status in pre-commit hook when providing a pathspec on commit

[Jeff King <peff@peff.net>]

On Tue, Feb 03, 2015 at 08:14:06AM -0500, Michaël Fortin wrote:

> Repo1 has the following pre-commit hook:
> 
> #!/bin/bash
> git -C "../Repo2" status --porcelain
> 
> I then commit in Repo1 using the following (this is actually ran by a
> GUI, I have no control over the commands themselves):
> 
> git add --force -- MyNewFile
> git commit -m "My message" -o -- MyNewFile

Because the commit command uses "-o", git must use a temporary index
file to stage the commit. It sets GIT_INDEX_FILE in the environment, and
then runs your hook.

When the hook runs "git -C" it moves into another repository, but the
GIT_INDEX_FILE in the environment takes precedence over the local index
found in that repository. So you see a HEAD and working tree from Repo2,
but the index from Repo1.

This is a specific case of a more general problem. If you were to run
git like:

  git --git-dir=/path/to/.git commit ...

you would have similar problems. You would have $GIT_DIR set in the
environment, and would see the HEAD from Repo1, but the working tree
from Repo2.

In other words, moving into another git directory from inside a hook is
not as easy as just going there. You also need to clear any state from
the environment. E.g., by adding this to the top of your hook:

  unset $(git rev-parse --local-env-vars)

Should "-C" (or "--git-dir") make this easier by doing it for you? I'm
not sure it is a good idea. It is right now valid to do:

  GIT_INDEX_FILE=/path/to/index.tmp git -C /path/to/repo ...

which would break if we cleared git variables. OTOH, maybe using "-C"
(instead of chdir-ing yourself) is a good indication that you want that
cleared. I dunno. It is probably too late at this point, as we would be
subtly breaking backwards compatibility. Perhaps it would make more
sense to add a new option, so you could do:

  git --reset-git-vars -C ../Repo2 status --porcelain

But this is a fairly obscure use case, and the rev-parse invocation
above is not so bad. I think it is more about knowing that you need to
use it, rather than the pain of typing it. And I am not sure how to make
that more clear. Is there a specific place in the documentation you
looked and might have been warned? Maybe the description for "-C" should
cover this.

-Peff