Git development
 help / color / mirror / Atom feed
From: Phillip Wood <phillip.wood123@gmail.com>
To: "Domen Kožar" <domen@cachix.org>, phillip.wood@dunelm.org.uk
Cc: git@vger.kernel.org, "Eric Sunshine" <sunshine@sunshineco.com>,
	"Patrick Steinhardt" <ps@pks.im>,
	"Ævar Arnfjörð Bjarmason" <avarab@gmail.com>,
	"Caleb White" <cdwhite3@pm.me>,
	"Junio C Hamano" <gitster@pobox.com>
Subject: Re: [PATCH v1 0/3] worktree: add post-worktree-add and post-worktree-remove hooks
Date: Mon, 13 Jul 2026 14:19:45 +0100	[thread overview]
Message-ID: <04051fe2-c54c-45e9-8773-92d40e84c765@gmail.com> (raw)
In-Reply-To: <CAMvcdZS=ZYbLmjKaGJvjQ_fWYhVbOzwMvYq+MMENWPYi_RiqvQ@mail.gmail.com>

Hi Domen

Unfortunately it doesn't look like your message appeared on the list, 
sadly I'm not sure it accepts multipart/alternative messages even when 
they contain a plain-text version of the message.

On 10/07/2026 18:20, Domen Kožar wrote:
> Hi Phillip,
> 
> thanks for the quick and careful read.
> 
>  > It is useful for copying across untracked files to the new worktree
>  > like "config.mak".
> 
> That is a nice example, and it needs the hook to also fire for
> --no-checkout and --orphan, which post-checkout does not cover.
> 
>  > Looking at the existing code, if the checkout fails then we remove
>  > the worktree because "is_junk == 1" when remove_junk() is called via
>  > atexit() so I think it is correct to skip the new hook in that case.
> 
> Right, when the checkout itself fails the worktree is removed as junk
> and neither hook runs; no disagreement there. The case I was asking
> about is the post-checkout hook itself failing: that runs after
> is_junk is cleared, so the worktree survives, but post-worktree-add
> is currently skipped and tooling that registers worktrees would miss
> one that exists. I kept the skip because a failing post-checkout
> already signals a broken setup, but I am happy to run
> post-worktree-add whenever the worktree was created, regardless of
> the earlier hook's exit status, if that is preferred.

Oh sorry I'd misunderstood the question. I think I'd lean towards 
running the hook anyway because we've still populated a new worktree, 
even if the post-checkout hooks fails.

>  > The new hook is run after the checkout, but before the post-checkout
>  > hook - we should document their relative order.
> 
> Unless I am misreading my own series, it is the other way around:
> add_worktree() invokes post-checkout first and post-worktree-add
> after it, t2400 has a test pinning that order ('"add" runs
> post-worktree-add after post-checkout'), and githooks.adoc says "It
> runs after the post-checkout hook, and is skipped if that hook
> fails." If that did not come across I am happy to reword the
> documentation.

Oops, when I wrote that I was looking at the wrong branch - I had my 
"add-worktree-hook" checked out and confused it with this patch. It's 
great to see that there is a test and documentation for this.

>  > I'm wondering if either of those is useful if we're running the
>  > hook in the new worktree.
> 
> Strictly they are derivable from inside, --show-toplevel for the path
> and the basename of --git-dir for the id.

Isn't the worktree path the current working directory of the hook script?

> I passed them anyway so
> that one script can serve both hooks: post-worktree-remove has to
> receive them as arguments because the worktree is gone by the time it
> runs, and keeping the two signatures identical makes shared hook code
> simpler. I can drop them from post-worktree-add if the symmetry is
> not considered worth it.

Oh right, as a counter argument I wonder if having a different  argument 
count for the two hooks makes it easier for a script that's shared 
between the two hooks to determine which hook has invoked it. Is the 
worktree id useful for anything apart from accessing on worktree's local 
refs from another worktree?

>  > So the hook knows a worktree was removed but not which one?
> 
> It always gets the worktree id as $2; only the path in $1 can be
> empty, and only for entries whose gitdir file is missing or
> unreadable, where git itself no longer knows the path either. Tooling
> that recorded the id at post-worktree-add time can still match the
> removal.

That answers why you want the id.

One thought I had after I wrote my mail was that worktrees can be 
renamed - do we want a hook for that so that external tools can move any 
services they've started and update their id -> path mapping.

Thanks

Phillip

> Thanks,
> Domen
> 
> On Fri, Jul 10, 2026 at 3:34 AM Phillip Wood <phillip.wood123@gmail.com 
> <mailto:phillip.wood123@gmail.com>> wrote:
> 
>     Hi Domen
> 
>     On 10/07/2026 00:36, Domen Kožar wrote:
>      >
>      > Today there is no reliable trigger to set that up when a worktree
>      > appears: post-checkout does not fire for --no-checkout or --orphan
>      > and cannot be told apart from a plain checkout. Nothing at all fires
>      > when a worktree goes away, so stale databases and services pile up
>      > after "git worktree remove" or a manual rm followed by "git worktree
>      > prune". Wrapping the worktree commands only helps when every tool,
>      > human or agent, goes through the wrapper.
> 
>     I agree a hook that's run after the worktree is added is useful (I have
>     a patch for it that I've never got round to cleaning up and sending so
>     thank you for working on this). It is useful for copying across
>     untracked files to the new worktree like "config.mak".
> 
>      > Patch 1 adds a post-worktree-add hook that fires after the working
>      > tree is fully set up. Patch 2 adds post-worktree-remove for "git
>      > worktree remove". Patch 3 extends it to "git worktree prune" so that
>      > manually deleted worktrees are also observed.
> 
>     I don't have a strong opinion on a hook running when a worktree is
>     removed - an IDE that cares about that could set up a filesystem watch
>     on the directory but I guess adding a hook doesn't do any harm.
>      > Two design points I would especially appreciate feedback on:
>      >
>      >   * post-worktree-add runs after post-checkout and is skipped when
>      >     post-checkout fails. An argument could be made that it should run
>      >     whenever the worktree was created, regardless of the earlier
>      >     hook's exit status, since tooling registering worktrees would
>      >     otherwise miss one that does exist.
> 
>     Looking at the existing code, if the checkout fails then we remove the
>     worktree because "is_junk == 1" when remove_junk() is called via
>     atexit() so I think it is correct to skip the new hook in that case.
> 
>     The new hook is run after the checkout, but before the post-checkout
>     hook - we should document their relative order. I see the hook is
>     run in
>     the new worktree and passed the absolute directory and worktree id. I'm
>     wondering if either of those is useful if we're running the hook in the
>     new worktree.
> 
>      >   * for entries pruned because their gitdir file points to a location
>      >     that no longer exists, the hook receives the recorded path; when
>      >     the path cannot be determined at all (missing or corrupt gitdir
>      >     file) it receives an empty string.
> 
>     So the hook knows a worktree was removed but not which one?
> 
>     Thanks
> 
>     Phillip
> 
>      > Thanks,
>      > Domen
>      >
>      > Domen Kožar (3):
>      >    worktree: add post-worktree-add hook
>      >    worktree: add post-worktree-remove hook
>      >    worktree: run post-worktree-remove hook when pruning
>      >
>      >   Documentation/githooks.adoc |  41 +++++++++++++
>      >   builtin/worktree.c          |  73 ++++++++++++++++++-----
>      >   t/t2400-worktree-add.sh     | 113 +++++++++++++++++++++++++++++
>     +++++++
>      >   t/t2401-worktree-prune.sh   |  88 ++++++++++++++++++++++++++++
>      >   t/t2403-worktree-move.sh    |  44 ++++++++++++++
>      >   worktree.c                  |   1 -
>      >   worktree.h                  |   6 +-
>      >   7 files changed, 347 insertions(+), 19 deletions(-)
>      >
>      >
>      > base-commit: f85a7e662054a7b0d9070e432508831afa214b47
> 
> 


      parent reply	other threads:[~2026-07-13 13:19 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-09 23:36 [PATCH v1 0/3] worktree: add post-worktree-add and post-worktree-remove hooks Domen Kožar
2026-07-10  9:34 ` Phillip Wood
     [not found]   ` <CAMvcdZS=ZYbLmjKaGJvjQ_fWYhVbOzwMvYq+MMENWPYi_RiqvQ@mail.gmail.com>
2026-07-13 13:19     ` Phillip Wood [this message]

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

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

  git send-email \
    --in-reply-to=04051fe2-c54c-45e9-8773-92d40e84c765@gmail.com \
    --to=phillip.wood123@gmail.com \
    --cc=avarab@gmail.com \
    --cc=cdwhite3@pm.me \
    --cc=domen@cachix.org \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=phillip.wood@dunelm.org.uk \
    --cc=ps@pks.im \
    --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.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox