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

From: Jeff King <peff@peff.net>
Date: Feb 3, 2015, 16:32 UTC
Message-ID: <20150203163235.GA9325@peff.net>
In-Reply-To: <80E24BA2-84FE-47FC-A5C0-D291E3C63BD5@gmail.com>

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

Show 10 quoted lines

> 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