* 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 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 related [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 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
* 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 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 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
* 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 public 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).