Subject: Re: [PATCH] editor: only save (and restore) the terminal if using a tty

From: Junio C Hamano <[email-masked]>
Date: Tue, 23 Nov 2021 09:27:31 -0800

Phillip Wood <[email-masked]> writes:

> Checking if stdout is a terminal fixes the Eclipse case where stdout
> is a pipe or /dev/null but if git is started in the background from a 
> terminal then calling isatty() will not prevent git from receiving
> SIGTTOU. For example if the user is using a gui editor then the 
> following used to work
>
> GIT_EDITOR=gedit git commit&

It is a good one ;-)

> Now git receives SIGTTOU when the editor exits because we call
> tcsetattr() from a background process group. One can argue it does not 
> make much sense to be starting git in the background but it did work
> before these changes. I think a combination of isatty() and
> tcgetpgrp() is probably the best solution.

If we are not foreground, we can and we should just skip doing this
whole save/restore thing, no?  The "editor might screw up the stty
setting so we restore to help the buggy editor" is releavant only
when the said editor actually uses the terminal, and it would get
TTOU if it tries to do so from the background.

So, I agree it would be a good thing to do, if we still want to do
it.  In the meantime, I've reverted the offending change from 'master'
and am planning to merge it down for 2.34.1 but I do not mind taking
a corrected change (not incremental on top of the broken 2.34.0) for
future releases.

jgit stopped doing 'GIT_EDITOR=echo git config --system --edit' thing,
but not everybody will upgrade immediately.