Re: preparing for 2.34.1

3 messages, 3 authors, 2021-11-22 · open the first message on its own page

Re: preparing for 2.34.1

From: Junio C Hamano <hidden>
Date: 2021-11-22 22:42:22

Johannes Schindelin [off-list ref] writes:
The quickest workaround for this is probably to special-case the editor
`echo`:
"GIT_EDITOR=: git cmd" would also be a common trick people would use
to bypass editor and take whatever is given as an initial template.
However, I could imagine that other scenarios call for an editor that
_also_ does not run in the terminal, and where also no real terminal is
available for saving and restoring.

I was tempted to suggest an `isatty(2)`, but that probably comes with its
own problems, too.
I think isatty(2) is pretty much our synonym to "are we talking to
an end-user sitting in front of the terminal".  Mostly we use it as
a way to control the progress bars, and use of editor on terminal
would be in line with these existing uses.

Re: preparing for 2.34.1

From: Johannes Schindelin <hidden>
Date: 2021-11-22 23:30:16

Hi Junio,

On Mon, 22 Nov 2021, Junio C Hamano wrote:
Johannes Schindelin [off-list ref] writes:
quoted
The quickest workaround for this is probably to special-case the editor
`echo`:
"GIT_EDITOR=: git cmd" would also be a common trick people would use
to bypass editor and take whatever is given as an initial template.
GIT_EDITOR=: is not a problem because of
https://github.com/git/git/blob/v2.34.0/editor.c#L59:

	if (strcmp(editor, ":")) {
		[...]
		term_fail = save_term(1);
		if (start_command(&p) < 0) {
			if (!term_fail)
				restore_term();
			[...]
		}

		[...]
		if (!term_fail)
			restore_term();
		[...]
	}
quoted
However, I could imagine that other scenarios call for an editor that
_also_ does not run in the terminal, and where also no real terminal is
available for saving and restoring.

I was tempted to suggest an `isatty(2)`, but that probably comes with its
own problems, too.
I think isatty(2) is pretty much our synonym to "are we talking to
an end-user sitting in front of the terminal".  Mostly we use it as
a way to control the progress bars, and use of editor on terminal
would be in line with these existing uses.
Indeed, I think that isatty(2) is a better indicator than isatty(1). We
sometimes _do_ redirect the output of, say, `git commit`, to capture the
commit hash that was generated. We typically do not redirect stderr,
though, unless calling from an application and capturing everything via
pipes. So isatty(2) strikes me as the best balance we can strike here.

Ciao,
Dscho

RE: preparing for 2.34.1

From: <hidden>
Date: 2021-11-22 23:51:46

On November 22, 2021 6:30 PM, Johannes Schindelin wrote:
On Mon, 22 Nov 2021, Junio C Hamano wrote:
quoted
Johannes Schindelin [off-list ref] writes:
quoted
The quickest workaround for this is probably to special-case the
editor
`echo`:
"GIT_EDITOR=: git cmd" would also be a common trick people would use
to bypass editor and take whatever is given as an initial template.
GIT_EDITOR=: is not a problem because of
https://github.com/git/git/blob/v2.34.0/editor.c#L59:

	if (strcmp(editor, ":")) {
		[...]
		term_fail = save_term(1);
		if (start_command(&p) < 0) {
			if (!term_fail)
				restore_term();
			[...]
		}

		[...]
		if (!term_fail)
			restore_term();
		[...]
	}
quoted
quoted
However, I could imagine that other scenarios call for an editor
that _also_ does not run in the terminal, and where also no real
terminal is available for saving and restoring.

I was tempted to suggest an `isatty(2)`, but that probably comes
with its own problems, too.
I think isatty(2) is pretty much our synonym to "are we talking to an
end-user sitting in front of the terminal".  Mostly we use it as a way
to control the progress bars, and use of editor on terminal would be
in line with these existing uses.
Indeed, I think that isatty(2) is a better indicator than isatty(1). We
sometimes _do_ redirect the output of, say, `git commit`, to capture the
commit hash that was generated. We typically do not redirect stderr,
though,
unless calling from an application and capturing everything via pipes. So
isatty(2) strikes me as the best balance we can strike here.
Please be careful of this one. isatty(2) may not be the issue, but the
filedes provided by Jenkins and other CI/CD systems over SSH often
mischaracterises the results from this call. Hacking the file descriptor
(2>&1 etc) prior to going to git is a common thing in scripts. Stdin is
frequently wrong when Jenkins sets up the environment to prompt for an SSH
passphrase - happens in non-Docker nohup situations - where git will end up
thinking it is interactive when it is not - not on NonStop fortunately, but
this happens with a Gentoo Hypervisor and Ubuntu VM. Also, stderr gets
redirected in scripts frequently in my experience - particularly on exotic
operating systems, crossing over from say, legacy NonStop or IBM TSO to
each's POSIX environment. I would expect breakages from this assumption, so
please be cautious.

-Randall
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help