git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Eric Sunshine <sunshine@sunshineco.com>
Cc: git@vger.kernel.org, Lars Schneider <larsxschneider@gmail.com>,
	matthew.k.gumbel@intel.com
Subject: Re: [PATCH 2/2] worktree: add: change to new worktree directory before running hook
Date: Mon, 12 Feb 2018 11:37:43 -0800	[thread overview]
Message-ID: <xmqq7erit2wo.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <20180212031526.40039-3-sunshine@sunshineco.com> (Eric Sunshine's message of "Sun, 11 Feb 2018 22:15:26 -0500")

Eric Sunshine <sunshine@sunshineco.com> 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?

  reply	other threads:[~2018-02-12 19:37 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-02-10  1:01 [PATCH v1] worktree: set worktree environment in post-checkout hook lars.schneider
2018-02-10  1:28 ` Lars Schneider
2018-02-12  3:15 ` [PATCH 0/2] worktree: change to new worktree dir before running hook(s) Eric Sunshine
2018-02-12  3:15   ` [PATCH 1/2] run-command: teach 'run_hook' about alternate worktrees Eric Sunshine
2018-02-12 20:58     ` Lars Schneider
2018-02-12 21:49       ` Eric Sunshine
2018-02-12  3:15   ` [PATCH 2/2] worktree: add: change to new worktree directory before running hook Eric Sunshine
2018-02-12 19:37     ` Junio C Hamano [this message]
2018-02-12 20:31       ` Eric Sunshine
2018-02-12 20:01     ` Lars Schneider
2018-02-13  4:42       ` Eric Sunshine
2018-02-13  4:48         ` Eric Sunshine
2018-02-13  7:27     ` Johannes Sixt
2018-02-13  7:34       ` Eric Sunshine
2018-02-15 19:18   ` [PATCH v2] worktree: add: fix 'post-checkout' not knowing new worktree location Eric Sunshine
2018-02-15 20:52     ` Junio C Hamano
2018-02-15 21:27       ` Eric Sunshine
2018-02-15 21:36         ` Junio C Hamano
2018-02-15 23:09         ` Eric Sunshine
2018-02-15 23:09     ` [PATCH v3] " Eric Sunshine
2018-02-16 16:55       ` Lars Schneider
2018-02-16 18:27         ` Eric Sunshine
2018-02-16 18:05       ` Junio C Hamano
2018-02-12  3:27 ` [PATCH v1] worktree: set worktree environment in post-checkout hook Eric Sunshine

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqq7erit2wo.fsf@gitster-ct.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=larsxschneider@gmail.com \
    --cc=matthew.k.gumbel@intel.com \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).