Re: [BUG REPORT] git-gui invokes prepare-commit-msg hook incorrectly

From: Sean Allred <allred.sean@gmail.com>
Date: Jul 5, 2024, 20:56 UTC
Message-ID: <m034onpng4.fsf@epic96565.epic.com>
In-Reply-To: <CAPig+cRQPrtGBTxM49nUeHvsVr0qEOnKZ5W_4by=A9mXEsR3DA@mail.gmail.com>

Eric Sunshine <sunshine@sunshineco.com> writes:

Show 13 quoted lines

> On Fri, Jul 5, 2024 at 3:23 PM brianmlyles <brianmlyles@gmail.com> wrote:
>> I noticed that commits from certain users were ending up in our
>> repository with comment-like lines in the commit message. [...]
>> [...]
>> This seems like a bug in git-gui. I see two fixes, but I'm not sure
>> which is more correct:
>> [...]
>> - Have git-gui create the commit in a way that causes the message to be
>>   washed
>>
>> The latter seems like it would be more consistent with other workflows
>> where the user is seeing the message in an editor, so my instinct is
>> that it would be the better fix.

There is a third option -- new plumbing in git (a la git-interpret-trailers) to expose the logic of `cleanup_message`. This comes with some nice flexibility, but introduces complexity around transferring state (e.g. passed options to git-commit) that would probably be best to avoid.

The second option above does seem simpler.

Show 18 quoted lines

> A patch to make git-gui strip comment lines had been previously
> applied[1,2], however, it badly broke git-gui when running with old
> Tcl versions, such as on macOS[3,4]. The breakage was not
> insurmountable, and a patch[5,6] was submitted to resolve it.
> Unfortunately, the then-maintainer of git-gui lost interest in the
> project about that point, thus left the issue hanging. Thus, to this
> day, git-gui still doesn't strip comment lines.
>
> Resurrecting these patches would be one way forward, assuming the new
> git-gui maintainer[7] (who is Cc:'d) would be interested.
>
> [1]: v2: https://lore.kernel.org/git/20210218181937.83419-1-me@yadavpratyush.com/
> [2]: v1: https://lore.kernel.org/git/20210202200301.44282-1-me@yadavpratyush.com/
> [3]: https://lore.kernel.org/git/CAPig+cT-sfgMDi9-6AEKF85NtOiXeqddJjk-pYuhDtTVAE-UEw@mail.gmail.com/
> [4]: https://lore.kernel.org/git/CAPig+cSC8uNfoAjDKdBNheod9_0-pCD-K_2kwt+J8USnoyQ7Aw@mail.gmail.com/
> [5]: https://lore.kernel.org/git/20210228231110.24076-1-sunshine@sunshineco.com/
> [6]: https://lore.kernel.org/git/CAPig+cRQN4PjfxEOZ8ZBA_uttsRPS8DPDgToM_JFvichDDh_HQ@mail.gmail.com/
> [7]: https://lore.kernel.org/git/0241021e-0b17-4031-ad9f-8abe8e0c0097@kdbg.org/

I haven't looked super closely at the patches you've linked, Eric, but it seems like those are specific to stripping comment characters. As I've noted elsewhere[1], there's potentially more to strip than just comments (like patch scissors). I suspect the only paths forward to guarantee that message-washing happens would either be an option to git-commit to explicitly enable it OR (probably preferred) have git-gui invoke git-commit with an appropriate editor instead of using -F.

[1]: https://lore.kernel.org/git/m0h6d3pphu.fsf@epic96565.epic.com/T/#u

-Sean

-- 
Sean Allred