Re: [PATCH 2/2] worktree: add: change to new worktree directory before running hook

[Eric Sunshine <sunshine@sunshineco.com>]

On Mon, Feb 12, 2018 at 3:01 PM, Lars Schneider
<larsxschneider@gmail.com> wrote:
>> On 12 Feb 2018, at 04:15, Eric Sunshine <sunshine@sunshineco.com> wrote:
>> Fix this by changing to the new worktree's directory before running
>> the hook, and adjust the tests to verify that the hook is indeed run
>> within the correct directory.
>
> Looks good but I think we are not quite there yet. It does not work
> for bare repos. You can test this if you apply the following patch on
> top of your changes.

Thanks for providing a useful test case.

The problem is that Git itself exports GIT_DIR with value "." (which
makes sense since "git worktree add" is invoked within a bare repo)
and that GIT_DIR leaks into the hook's environment. However, since the
hook is running within the worktree, into which we've chdir()'d, the
relative "." is wrong, and setup.c:is_git_directory() (invoked
indirectly by setup_bare_git_dir()) correctly reports that the
worktree itself is not a valid Git directory. As a result, 'rev-parse'
run by the hook fails with "fatal: Not a git repository: '.'". The fix
is either to remove GIT_DIR from the environment or make it absolute
so the chdir() doesn't invalidate it.

However, it turns out that builtin/worktree.c already _does_ export
GIT_DIR and GIT_WORK_TREE for the commands it invokes ('update-ref',
'symbolic-ref', 'reset --hard') to create the new worktree, which
makes perfect sense since these commands need to know the location of
the new worktree. So, a second approach/fix is also to use these
existing exports when running the hook. The (minor) catch is that they
are relative and break upon chdir() but that is easily fixed by making
them absolute.

So, either approach works: removing GIT_DIR or using "worktree add"'s
existing GIT_DIR and GIT_WORK_TREE. I favor the latter since it is
consistent with how "worktree add" invokes other command already and,
especially, because it also addresses the issue Junio raised of
user-defined GIT_DIR/GIT_WORK_TREE potentially polluting the hook's
environment.

> Please note that also '"add" within worktree invokes post-checkout hook'
> seems to fail with my extended test case.

Also fixed by either approach.