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

From: Eric Sunshine <sunshine@sunshineco.com>
Date: Jan 8, 2023, 23:25 UTC
Message-ID: <CAPig+cS_dXL-Q6NZtUJxDOL4-Q=MJv8fPEPAnEPuONaNF8-sCA@mail.gmail.com>
In-Reply-To: <CAC-j02O6z4sG85LpRNzEZ52Y-McurYDa_VnVXtqFVPBFu9kbug@mail.gmail.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:

Show 8 quoted lines

> 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.

Show 10 quoted lines

> 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.