Re: [PATCH v2 4/8] t3030-merge-recursive.sh: disable fsmonitor when tweaking GIT_WORK_TREE

[Derrick Stolee <stolee@gmail.com>]

On 12/10/2019 10:07 AM, SZEDER Gábor wrote:
> On Tue, Dec 10, 2019 at 08:45:27AM -0500, Derrick Stolee wrote:
>>>> Worktrees use a ".git" _file_ instead of a folder to point to
>>>> the base repo's .git directory and the proper worktree HEAD. The
>>>> fsmonitor hook tries to create a JSON file inside the ".git" folder
>>>> which violates the expectation here.
>>>
>>> Yeah, there are a couple hardcoded paths in there, e.g.:
>>>
>>>   open ($fh, ">", ".git/watchman-response.json");
>>>
>>> and, worse, not only in the test helper hook in
>>> 't/t7519/fsmonitor-watchman' but in the sample hook template
>>> 'templates/hooks--fsmonitor-watchman.sample' as well.
>>>
>>>> It would be better to properly
>>>> find a safe folder for storing this JSON file.
>>>
>>>   git rev-parse --git-path ''
>>>
>>> gives us the right directory prefix to use and we could then append
>>> the various filenames that must be accessed in there.
>>
>> Adding another git process inside the hook is hopefully not
>> the only way to achieve something like this. The performance
>> hit (mostly on Windows) would be a non-starter for me.
> 
> Oh, hang on, it seems that we could simply use $GIT_DIR.
> 
> I added
> 
>   echo >&2 "GIT_DIR in the fsmonitor hook: '$GIT_DIR'"
> 
> to 't/t7519/fsmonitor-all', and then run the test:
> 
>   test_expect_success 'test' '
>           echo 1 >file &&
>           git add file &&
>           git commit -m first &&
>   
>           git worktree add --detach WT &&
>           cd WT &&
>           echo 2 >file &&
>           git add -u
>   '
> 
> with 'GIT_TEST_FSMONITOR=$(pwd)/t7519/fsmonitor-all', and in the
> verbose output got lines like:
> 
>   GIT_DIR in the fsmonitor hook: ''
>   GIT_DIR in the fsmonitor hook: ''
>   GIT_DIR in the fsmonitor hook: '/home/szeder/src/git/t/trash directory.t9999-test/.git/worktrees/WT'
>   GIT_DIR in the fsmonitor hook: '/home/szeder/src/git/t/trash directory.t9999-test/.git/worktrees/WT'
> 
> I'm not sure why $GIT_DIR is not exported to the hook script while in
> the main working tree.  Anyway, as it is now, if $GIT_DIR is
> unset/empty, then the hook should write to ".git/<whatever>", and if
> it is set, then to "$GIT_DIR/<whatever>", so no git process is needed
> in the hook, only a getenv() and a condition.

Thanks for this. It helps that also the test hooks were using the
.git directory only for debug information, and that was commented-out
in the v2 version of the hook.

Thanks,
-Stolee