git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
* preparing for 2.34.1
@ 2021-11-22 17:37 Junio C Hamano
  2021-11-22 20:51 ` Carlo Arenas
                   ` (2 more replies)
  0 siblings, 3 replies; 10+ messages in thread
From: Junio C Hamano @ 2021-11-22 17:37 UTC (permalink / raw)
  To: git

There are a few topics [*] to fix regressions introduced in the
previous cycle, which should be in 2.34.1 and I've merged them to
'master'. I hope we can merge them to 'maint' (which now point at
2.34) and tag 2.34.1 in a few days.

After that, in yet another few days, we will see most of the stalled
topics ejected from the tree, the tip of 'next' rewound, and we will
start the cycle toward 2.35 by starting to take new topics, by the
beginning of the next week at the latest.

Thanks.



* ab/update-submitting-patches (2021-11-13) 1 commit
  (merged to 'next' on 2021-11-19 at b44f4d0eb0)
 + SubmittingPatches: fix Asciidoc syntax in "GitHub CI" section

 Doc fix.


* ev/pull-already-up-to-date-is-noop (2021-11-18) 1 commit
  (merged to 'next' on 2021-11-19 at 2d8f0cd000)
 + pull: should be noop when already-up-to-date

 "git pull" with any strategy when the other side is behind us
 should succeed as it is a no-op, but doesn't.


* hm/paint-hits-in-log-grep (2021-11-19) 1 commit
  (merged to 'next' on 2021-11-19 at e146d25c7c)
 + Revert "grep/pcre2: fix an edge case concerning ascii patterns and UTF-8 data"

 "git grep" looking in a blob that has non-UTF8 payload was
 completely broken when linked with certain versions of PCREv2
 library in the latest release.



^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: preparing for 2.34.1
  2021-11-22 17:37 preparing for 2.34.1 Junio C Hamano
@ 2021-11-22 20:51 ` Carlo Arenas
  2021-11-22 22:28   ` Johannes Schindelin
  2021-11-22 21:50 ` Derrick Stolee
  2021-11-23 17:18 ` preparing for 2.34.1 (2nd round) Junio C Hamano
  2 siblings, 1 reply; 10+ messages in thread
From: Carlo Arenas @ 2021-11-22 20:51 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Mon, Nov 22, 2021 at 11:54 AM Junio C Hamano <gitster@pobox.com> wrote:
>
> There are a few topics [*] to fix regressions introduced in the
> previous cycle

There was another regression report[1] that I just fished out of my
spam folder and that I introduced in 3d411afabc (editor: save and
reset terminal after calling EDITOR, 2021-10-05), but that I have not
yet produced a fix for or fully understood.

The gist is that some people seem to be using a hack[2] dscho posted a
few years ago to get info from git programmatically and I just didn't
expect someone would try to invoke the EDITOR unless they had a
terminal, so the fix might be to just add an isatty(0) call, but
reverting that commit would be also an option.

Carlo

[1] https://lore.kernel.org/git/ee302c98-da27-da43-e684-c7ec8b225360@gmx.net/
[2] https://yhbt.net/lore/all/nycvar.QRO.7.76.6.1903221436590.41@tvgsbejvaqbjf.bet/T/

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: preparing for 2.34.1
  2021-11-22 17:37 preparing for 2.34.1 Junio C Hamano
  2021-11-22 20:51 ` Carlo Arenas
@ 2021-11-22 21:50 ` Derrick Stolee
  2021-11-22 22:45   ` Junio C Hamano
  2021-11-23 17:18 ` preparing for 2.34.1 (2nd round) Junio C Hamano
  2 siblings, 1 reply; 10+ messages in thread
From: Derrick Stolee @ 2021-11-22 21:50 UTC (permalink / raw)
  To: Junio C Hamano, git

On 11/22/21 12:37 PM, Junio C Hamano wrote:
> There are a few topics [*] to fix regressions introduced in the
> previous cycle, which should be in 2.34.1 and I've merged them to
> 'master'. I hope we can merge them to 'maint' (which now point at
> 2.34) and tag 2.34.1 in a few days.
> 
> After that, in yet another few days, we will see most of the stalled
> topics ejected from the tree, the tip of 'next' rewound, and we will
> start the cycle toward 2.35 by starting to take new topics, by the
> beginning of the next week at the latest.

There is a patch deep in a thread [1] that is a regression
in 2.34.0 that would be good to include in this release. It
fixes a sparse-checkout bug via a simple revert (plus a new
test).

Sorry that the patch was buried deep and was easy to miss.

[1] https://lore.kernel.org/git/72fffbff-16f7-fa17-b212-67aae9e1b034@gmail.com/

Thanks,
-Stolee

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: preparing for 2.34.1
  2021-11-22 20:51 ` Carlo Arenas
@ 2021-11-22 22:28   ` Johannes Schindelin
  2021-11-22 22:42     ` Junio C Hamano
  2021-11-22 22:50     ` Carlo Arenas
  0 siblings, 2 replies; 10+ messages in thread
From: Johannes Schindelin @ 2021-11-22 22:28 UTC (permalink / raw)
  To: Carlo Arenas; +Cc: Junio C Hamano, git

Hi Carlo,

On Mon, 22 Nov 2021, Carlo Arenas wrote:

> On Mon, Nov 22, 2021 at 11:54 AM Junio C Hamano <gitster@pobox.com> wrote:
> >
> > There are a few topics [*] to fix regressions introduced in the
> > previous cycle
>
> There was another regression report[1] that I just fished out of my
> spam folder and that I introduced in 3d411afabc (editor: save and
> reset terminal after calling EDITOR, 2021-10-05), but that I have not
> yet produced a fix for or fully understood.
>
> The gist is that some people seem to be using a hack[2] dscho posted a

You mean

https://lore.kernel.org/git/nycvar.QRO.7.76.6.1903221436590.41@tvgsbejvaqbjf.bet/

right? I.e. figuring out what Git considers its system config location via

	GIT_EDITOR=echo git config --system -e 2>/dev/null

> few years ago to get info from git programmatically and I just didn't
> expect someone would try to invoke the EDITOR unless they had a
> terminal, so the fix might be to just add an isatty(0) call, but
> reverting that commit would be also an option.

The quickest workaround for this is probably to special-case the editor
`echo`:

-- snip --
diff --git a/editor.c b/editor.c
index 674309eed8bd..1b97f7da9920 100644
--- a/editor.c
+++ b/editor.c
@@ -51,12 +51,11 @@ const char *git_sequence_editor(void)
 static int launch_specified_editor(const char *editor, const char *path,
 				   struct strbuf *buffer, const char *const *env)
 {
-	int term_fail;
-
 	if (!editor)
 		return error("Terminal is dumb, but EDITOR unset");

 	if (strcmp(editor, ":")) {
+		int save_and_restore_term = strcmp(editor, "echo");
 		struct strbuf realpath = STRBUF_INIT;
 		const char *args[] = { editor, NULL, NULL };
 		struct child_process p = CHILD_PROCESS_INIT;
@@ -86,9 +85,10 @@ static int launch_specified_editor(const char *editor, const char *path,
 		p.env = env;
 		p.use_shell = 1;
 		p.trace2_child_class = "editor";
-		term_fail = save_term(1);
+		if (save_and_restore_term)
+			save_and_restore_term = !save_term(1);
 		if (start_command(&p) < 0) {
-			if (!term_fail)
+			if (save_and_restore_term)
 				restore_term();
 			strbuf_release(&realpath);
 			return error("unable to start editor '%s'", editor);
@@ -97,7 +97,7 @@ static int launch_specified_editor(const char *editor, const char *path,
 		sigchain_push(SIGINT, SIG_IGN);
 		sigchain_push(SIGQUIT, SIG_IGN);
 		ret = finish_command(&p);
-		if (!term_fail)
+		if (!save_and_restore_term)
 			restore_term();
 		strbuf_release(&realpath);
 		sig = ret - 128;
-- snap --

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.

Ciao,
Dscho

>
> Carlo
>
> [1] https://lore.kernel.org/git/ee302c98-da27-da43-e684-c7ec8b225360@gmx.net/
> [2] https://yhbt.net/lore/all/nycvar.QRO.7.76.6.1903221436590.41@tvgsbejvaqbjf.bet/T/
>
>

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: preparing for 2.34.1
  2021-11-22 22:28   ` Johannes Schindelin
@ 2021-11-22 22:42     ` Junio C Hamano
  2021-11-22 23:30       ` Johannes Schindelin
  2021-11-22 22:50     ` Carlo Arenas
  1 sibling, 1 reply; 10+ messages in thread
From: Junio C Hamano @ 2021-11-22 22:42 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Carlo Arenas, git

Johannes Schindelin <Johannes.Schindelin@gmx.de> 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.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: preparing for 2.34.1
  2021-11-22 21:50 ` Derrick Stolee
@ 2021-11-22 22:45   ` Junio C Hamano
  0 siblings, 0 replies; 10+ messages in thread
From: Junio C Hamano @ 2021-11-22 22:45 UTC (permalink / raw)
  To: Derrick Stolee; +Cc: git

Derrick Stolee <stolee@gmail.com> writes:

> On 11/22/21 12:37 PM, Junio C Hamano wrote:
>> There are a few topics [*] to fix regressions introduced in the
>> previous cycle, which should be in 2.34.1 and I've merged them to
>> 'master'. I hope we can merge them to 'maint' (which now point at
>> 2.34) and tag 2.34.1 in a few days.
>> 
>> After that, in yet another few days, we will see most of the stalled
>> topics ejected from the tree, the tip of 'next' rewound, and we will
>> start the cycle toward 2.35 by starting to take new topics, by the
>> beginning of the next week at the latest.
>
> There is a patch deep in a thread [1] that is a regression
> in 2.34.0 that would be good to include in this release. It
> fixes a sparse-checkout bug via a simple revert (plus a new
> test).
>
> Sorry that the patch was buried deep and was easy to miss.
>
> [1] https://lore.kernel.org/git/72fffbff-16f7-fa17-b212-67aae9e1b034@gmail.com/

Instead of these notes, it would be much easier for people to
realize that this (and previous) week is mostly about regression
fixing and it is not welcome to drown the list with new patches, if
you just resent the patch with 

    Subject: [PATCH/regression] fix blah

I would say.

Having said that, thanks for a reminder.  I did read it, but
apparently forgot to include it in the list of things needed for
'master' when I prepared it last night.


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: preparing for 2.34.1
  2021-11-22 22:28   ` Johannes Schindelin
  2021-11-22 22:42     ` Junio C Hamano
@ 2021-11-22 22:50     ` Carlo Arenas
  1 sibling, 0 replies; 10+ messages in thread
From: Carlo Arenas @ 2021-11-22 22:50 UTC (permalink / raw)
  To: Johannes Schindelin; +Cc: Junio C Hamano, git, phillip.wood123

On Mon, Nov 22, 2021 at 2:28 PM Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> I was tempted to suggest an `isatty(2)`, but that probably comes with its
> own problems, too.

I went with isatty(1) in the suggested minimal bugfix[1] since I think
it is safe to assume that no one that doesn't have stdout connected to
a terminal should even care if the invoking editor dies and messes
with their terminal, and hopefully whoever is hacking git commands
that invoke an editor to read its output, is actually not doing it
through a tty but a pipe.

What problems do you foresee with isatty?, the current code already
checks for errors on accessing the console, but as Phillip
suggested[2] in that thread a signal might be generated for some of
those errors when the terminal is shared with other processes.

Carlo

CC: +Phillip

[1] https://lore.kernel.org/git/20211122222850.674-1-carenas@gmail.com/
[2] https://lore.kernel.org/git/04ab7301-ea34-476c-eae4-4044fef74b91@gmail.com/T/#u

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: preparing for 2.34.1
  2021-11-22 22:42     ` Junio C Hamano
@ 2021-11-22 23:30       ` Johannes Schindelin
  2021-11-22 23:51         ` rsbecker
  0 siblings, 1 reply; 10+ messages in thread
From: Johannes Schindelin @ 2021-11-22 23:30 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Carlo Arenas, git

Hi Junio,

On Mon, 22 Nov 2021, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> 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.

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();
		[...]
	}

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

^ permalink raw reply	[flat|nested] 10+ messages in thread

* RE: preparing for 2.34.1
  2021-11-22 23:30       ` Johannes Schindelin
@ 2021-11-22 23:51         ` rsbecker
  0 siblings, 0 replies; 10+ messages in thread
From: rsbecker @ 2021-11-22 23:51 UTC (permalink / raw)
  To: 'Junio C Hamano'; +Cc: 'Carlo Arenas', git

On November 22, 2021 6:30 PM, Johannes Schindelin wrote:
> On Mon, 22 Nov 2021, Junio C Hamano wrote:
> 
> > Johannes Schindelin <Johannes.Schindelin@gmx.de> 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.
> 
> 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();
> 		[...]
> 	}
> 
> > > 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


^ permalink raw reply	[flat|nested] 10+ messages in thread

* preparing for 2.34.1 (2nd round)
  2021-11-22 17:37 preparing for 2.34.1 Junio C Hamano
  2021-11-22 20:51 ` Carlo Arenas
  2021-11-22 21:50 ` Derrick Stolee
@ 2021-11-23 17:18 ` Junio C Hamano
  2 siblings, 0 replies; 10+ messages in thread
From: Junio C Hamano @ 2021-11-23 17:18 UTC (permalink / raw)
  To: git

Junio C Hamano <gitster@pobox.com> writes:

There are a few topics [*] to fix regressions introduced in the
previous cycle, which should be in 2.34.1 and I've merged them to
'master'. I hope we can merge them to 'maint' (which now point at
2.34) and tag 2.34.1 in a day or two.

After that, in yet another few days, we will see most of the stalled
topics ejected from the tree, the tip of 'next' rewound, and we will
start the cycle toward 2.35 by starting to take new topics,
hopefully by the beginning of the next week.


Thanks.


Fixes since v2.34
-----------------

 * "git grep" looking in a blob that has non-UTF8 payload was
   completely broken when linked with certain versions of PCREv2
   library in the latest release.

 * "git pull" with any strategy when the other side is behind us
   should succeed as it is a no-op, but doesn't.
   (merge ea1954af ev/pull-already-up-to-date-is-noop later to maint).

 * An earlier change in 2.34.0 caused JGit application (that abused
   GIT_EDITOR mechanism when invoking "git config") to get stuck with
   a SIGTTOU signal; it has been reverted.
   (merge e3f7e01b50 jc/save-restore-terminal-revert later to maint).

 * An earlier change that broke .gitignore matching has been reverted.
   (merge 33c5d6c845 ds/add-rm-with-sparse-index later to maint).

 * SubmittingPatches document gained a syntactically incorrect mark-up,
   which has been corrected.
   (merge edbd9f37 ab/update-submitting-patches later to maint).

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-11-23 17:18 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-11-22 17:37 preparing for 2.34.1 Junio C Hamano
2021-11-22 20:51 ` Carlo Arenas
2021-11-22 22:28   ` Johannes Schindelin
2021-11-22 22:42     ` Junio C Hamano
2021-11-22 23:30       ` Johannes Schindelin
2021-11-22 23:51         ` rsbecker
2021-11-22 22:50     ` Carlo Arenas
2021-11-22 21:50 ` Derrick Stolee
2021-11-22 22:45   ` Junio C Hamano
2021-11-23 17:18 ` preparing for 2.34.1 (2nd round) Junio C Hamano

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).