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

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

[Michaël Fortin]

Hi all,

I'm seeing behavior that I *think* might be a bug in git (I'm running 2.2.2). At least I couldn't find anything about this anywhere, so here goes:

I'm trying to run git commands outside of the current working copy (e.g. inside another repo) from a pre-commit hook. However I'm encountering a very weird issue that occurs when a pathspec is provided when performing a commit.

More precisely, what I'm trying to do is check whether a repository beside the current repository is clean (no uncommitted changes) or not, from a pre-commit hook.

My repositories look like this on disk:

ContainerDirectory
|-- Repo1 (contains one untracked file)
|-- Repo2 (no uncommitted changes)

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

This results in some very puzzling output. I would expect the hook to output nothing because Repo2 contains no changes. However, it returns the following:

D  Repo2-File1.txt
D  Repo2-File2.txt
D  Repo2-File3.txt
?? Repo2-File1.txt
?? Repo2-File2.txt
?? Repo2-File3.txt

As you can see, the files are listed as both deleted and untracked, which makes no sense to me because there are in fact no uncommitted changes in that repo. There are only these three files in the repo. Removing the pathspec ("-- MyNewFile") from the commit command shows the expected output (nothing, because there are no changes in Repo2).

I have reproduced this behavior on OS X and Windows (with the latest git for Windows).

Can you explain what's going on?

Thanks in advance for any info!

Michaël Fortin
www.irradiated.net

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

[Jeff King]

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

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

[Michaël Fortin]

Interesting!

Thanks for the info; unsetting git environment variables as you suggested fixes my issue on OS X (and Linux I suppose). I’m going to try this on Windows soon. Would that work as-is on Windows, or would I need to unset the vars using some other Windows-specific syntax?

Yes, I looked at the documentation for “-C", and “-o". Because this issue is triggered by the use of “-o”, maybe the appropriate place to mention this behaviour would be in the documentation for “-o”? I don’t know.

Thanks again,

Michaël Fortin
www.irradiated.net

> On Feb 3, 2015, at 11:32 AM, Jeff King <peff@peff.net> wrote:
> 
> 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