Re: [PATCH] githooks: discuss Git operations in foreign repositories

[Eric Sunshine <sunshine@sunshineco.com>]

[administrivia: please reply inline rather than top-posting]

On Sun, Jan 8, 2023 at 2:45 PM Preston Tunnell Wilson
<prestontunnellwilson@gmail.com> wrote:
> Thank you for this wonderful remedy, Eric! I really appreciate the
> background context and how you framed the problem that I ran into.
>
> I have two questions:
> 1. Documentation is a great first step in addressing this, but I'm
> wondering if this should be automatic? If this is a best practice for
> hook authors, could `git` do this for them automatically when running
> hooks?

For the general case, probably not. "Best-practice" is context
sensitive. It may be best-practice when a hook needs to invoke Git
commands in some other repository (or worktree), but clearing those
variables automatically would, in some situations, break the much more
common case of the hook invoking Git commands in the local repository
(or worktree). The fact that those environment variables may have been
set manually by the user or automatically by Git further complicates
the situation.

> 2. Should we add something in the `git-worktree` documentation? In
> `Documentation/git-worktree.txt`, it mentions:
>
> > BUGS
> > ----
> > Multiple checkout in general is still experimental, and the support
> > for submodules is incomplete. ...
>
> Would it be helpful to plant a flag in the above documentation to
> point to this potential issue?

As noted above, we can't really call this a bug. Git is behaving as
intended. Whether the user set the variables manually or whether some
parent Git process set them automatically, the child Git respects the
variables as it should rather than second-guessing about the user's
intentions, and possibly guessing incorrectly.

So, no, I don't think this qualifies for the BUGS section of
git-wortkree, and mentioning this potential gotcha only in
git-worktree but not in any other hook-running command doesn't seem
ideal either. At present, the best place to discuss it seems to be
Documentation/githooks.txt, as this patch does. It may be possible to
argue that gitfaq.txt could talk about it, but considering that this
issue can manifest in many different ways (various error messages or
misbehaviors), it's difficult to come up with any text for the "Q"
which people would be likely to find when Googling. That's not to say
it shouldn't be mentioned elsewhere in the documentation, but rather
that I haven't come up with any better places than githooks.txt
itself.