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

From: "SZEDER Gábor" <szeder.dev@gmail.com>
To: Derrick Stolee <stolee@gmail.com>
Cc: Derrick Stolee via GitGitGadget <gitgitgadget@gmail.com>,
	git@vger.kernel.org, ukshah2@illinois.edu,
	Kevin.Willford@microsoft.com,
	Derrick Stolee <dstolee@microsoft.com>,
	Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH v2 4/8] t3030-merge-recursive.sh: disable fsmonitor when tweaking GIT_WORK_TREE
Date: Tue, 10 Dec 2019 16:07:45 +0100	[thread overview]
Message-ID: <20191210150745.GH6527@szeder.dev> (raw)
In-Reply-To: <b3e8ad07-b2cb-e024-405e-27d9f065f5fc@gmail.com>

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.