Re: [PATCH 2/2] worktree: add: change to new worktree directory before running hook
- Date: Mon, 12 Feb 2018 11:37:43 -0800
- From: Junio C Hamano <gitster@xxxxxxxxx>
- Subject: Re: [PATCH 2/2] worktree: add: change to new worktree directory before running hook
Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes:
> Although "git worktree add" learned to run the 'post-checkout' hook in
> ade546be47 (worktree: invoke post-checkout hook, 2017-12-07), it
> neglects to change to the directory of the newly-created worktree
> before running the hook. Instead, the hook is run within the directory
> from which the "git worktree add" command itself was invoked, which
> effectively neuters the hook since it knows nothing about the new
> worktree directory.
> 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.
I like the approach taken by this replacement better. Just to make
sure I understand the basic idea, let me rephrase what these two
patches are doing:
- "path" that is made absolute in this step is where the new
worktree is created, i.e. the top-level of the working tree in
the new worktree. We chdir there and then run the hook script.
- Even though we often see hooks executed inside .git/ directory,
for post-checkout, the top-level of the working tree is the right
place, as that is where the hook is run by "git checkout" (which
does the "cd to the toplevel" thing upfront and then runs hooks
without doing anything special) and "git clone" (which goes to
the newly created repository's working tree by calling
setup.c::setup_work_tree() before builtin/clone.c::checkout(),
which may call post-checkout hook).
I wonder if we need to clear existing GIT_DIR/GIT_WORK_TREE from the
environment, though. When a user with a funny configuration (where
these two environment variables are pointing at unusual places) uses
"git worktree add" to create another worktree for the repository, it
would not be sufficient to chdir to defeat them that are appropriate
for the original, and not for the new, worktree, would it?