{"thread":{"id":"56959","subject":"preparing for 2.34.1","startedAt":"2021-11-22T17:37:19Z","lastAt":"2021-11-23T17:18:52Z","messageCount":10,"participants":["Junio C Hamano","Carlo Arenas","Derrick Stolee","Johannes Schindelin","rsbecker@nexbridge.com"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"441980","messageId":"xmqqr1b8gkhg.fsf@gitster.g","threadId":"56959","inReplyTo":null,"subject":"preparing for 2.34.1","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-11-22T17:37:15Z","receivedAt":"2021-11-22T17:37:19Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"There are a few topics [*] to fix regressions introduced in the\nprevious cycle, which should be in 2.34.1 and I've merged them to\n'master'. I hope we can merge them to 'maint' (which now point at\n2.34) and tag 2.34.1 in a few days.\n\nAfter that, in yet another few days, we will see most of the stalled\ntopics ejected from the tree, the tip of 'next' rewound, and we will\nstart the cycle toward 2.35 by starting to take new topics, by the\nbeginning of the next week at the latest.\n\nThanks.\n\n\n\n* ab/update-submitting-patches (2021-11-13) 1 commit\n (merged to 'next' on 2021-11-19 at b44f4d0eb0)\n + SubmittingPatches: fix Asciidoc syntax in \"GitHub CI\" section\n\n Doc fix.\n\n\n* ev/pull-already-up-to-date-is-noop (2021-11-18) 1 commit\n (merged to 'next' on 2021-11-19 at 2d8f0cd000)\n + pull: should be noop when already-up-to-date\n\n \"git pull\" with any strategy when the other side is behind us\n should succeed as it is a no-op, but doesn't.\n\n\n* hm/paint-hits-in-log-grep (2021-11-19) 1 commit\n (merged to 'next' on 2021-11-19 at e146d25c7c)\n + Revert \"grep/pcre2: fix an edge case concerning ascii patterns and UTF-8 data\"\n\n \"git grep\" looking in a blob that has non-UTF8 payload was\n completely broken when linked with certain versions of PCREv2\n library in the latest release.\n\n\n"},{"id":"442018","messageId":"CAPUEsphNH9pfQoHqVgJfkQCU-Li45dz4QtGtDjWu5bDV9A3PEg@mail.gmail.com","threadId":"56959","inReplyTo":"xmqqr1b8gkhg.fsf@gitster.g","subject":"Re: preparing for 2.34.1","fromName":"Carlo Arenas","fromEmail":"carenas@gmail.com","sentAt":"2021-11-22T20:51:55Z","receivedAt":"2021-11-22T20:52:12Z","isPatch":false,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Mon, Nov 22, 2021 at 11:54 AM Junio C Hamano wrote:\n>\n> There are a few topics [*] to fix regressions introduced in the\n> previous cycle\n\nThere was another regression report[1] that I just fished out of my\nspam folder and that I introduced in 3d411afabc (editor: save and\nreset terminal after calling EDITOR, 2021-10-05), but that I have not\nyet produced a fix for or fully understood.\n\nThe gist is that some people seem to be using a hack[2] dscho posted a\nfew years ago to get info from git programmatically and I just didn't\nexpect someone would try to invoke the EDITOR unless they had a\nterminal, so the fix might be to just add an isatty(0) call, but\nreverting that commit would be also an option.\n\nCarlo\n\n[1] https://lore.kernel.org/git/ee302c98-da27-da43-e684-c7ec8b225360@gmx.net/\n[2] https://yhbt.net/lore/all/nycvar.QRO.7.76.6.1903221436590.41@tvgsbejvaqbjf.bet/T/\n"},{"id":"442027","messageId":"6cc0bc51-cc28-989c-d666-75b707f2c080@gmail.com","threadId":"56959","inReplyTo":"xmqqr1b8gkhg.fsf@gitster.g","subject":"Re: preparing for 2.34.1","fromName":"Derrick Stolee","fromEmail":"stolee@gmail.com","sentAt":"2021-11-22T21:50:32Z","receivedAt":"2021-11-22T21:50:36Z","isPatch":false,"sender":{"key":"stolee@gmail.com","avatar":"https://avatars.githubusercontent.com/u/570044?v=4"},"body":"On 11/22/21 12:37 PM, Junio C Hamano wrote:\n> There are a few topics [*] to fix regressions introduced in the\n> previous cycle, which should be in 2.34.1 and I've merged them to\n> 'master'. I hope we can merge them to 'maint' (which now point at\n> 2.34) and tag 2.34.1 in a few days.\n> \n> After that, in yet another few days, we will see most of the stalled\n> topics ejected from the tree, the tip of 'next' rewound, and we will\n> start the cycle toward 2.35 by starting to take new topics, by the\n> beginning of the next week at the latest.\n\nThere is a patch deep in a thread [1] that is a regression\nin 2.34.0 that would be good to include in this release. It\nfixes a sparse-checkout bug via a simple revert (plus a new\ntest).\n\nSorry that the patch was buried deep and was easy to miss.\n\n[1] https://lore.kernel.org/git/72fffbff-16f7-fa17-b212-67aae9e1b034@gmail.com/\n\nThanks,\n-Stolee\n"},{"id":"442038","messageId":"nycvar.QRO.7.76.6.2111222319100.63@tvgsbejvaqbjf.bet","threadId":"56959","inReplyTo":"CAPUEsphNH9pfQoHqVgJfkQCU-Li45dz4QtGtDjWu5bDV9A3PEg@mail.gmail.com","subject":"Re: preparing for 2.34.1","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-11-22T22:28:22Z","receivedAt":"2021-11-22T22:28:33Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Carlo,\n\nOn Mon, 22 Nov 2021, Carlo Arenas wrote:\n\n> On Mon, Nov 22, 2021 at 11:54 AM Junio C Hamano wrote:\n> >\n> > There are a few topics [*] to fix regressions introduced in the\n> > previous cycle\n>\n> There was another regression report[1] that I just fished out of my\n> spam folder and that I introduced in 3d411afabc (editor: save and\n> reset terminal after calling EDITOR, 2021-10-05), but that I have not\n> yet produced a fix for or fully understood.\n>\n> The gist is that some people seem to be using a hack[2] dscho posted a\n\nYou mean\n\nhttps://lore.kernel.org/git/nycvar.QRO.7.76.6.1903221436590.41@tvgsbejvaqbjf.bet/\n\nright? I.e. figuring out what Git considers its system config location via\n\n\tGIT_EDITOR=echo git config --system -e 2>/dev/null\n\n> few years ago to get info from git programmatically and I just didn't\n> expect someone would try to invoke the EDITOR unless they had a\n> terminal, so the fix might be to just add an isatty(0) call, but\n> reverting that commit would be also an option.\n\nThe quickest workaround for this is probably to special-case the editor\n`echo`:\n\n-- snip --\ndiff --git a/editor.c b/editor.c\nindex 674309eed8bd..1b97f7da9920 100644\n--- a/editor.c\n+++ b/editor.c\n@@ -51,12 +51,11 @@ const char *git_sequence_editor(void)\n static int launch_specified_editor(const char *editor, const char *path,\n \t\t\t\t struct strbuf *buffer, const char *const *env)\n {\n-\tint term_fail;\n-\n \tif (!editor)\n \t\treturn error(\"Terminal is dumb, but EDITOR unset\");\n\n \tif (strcmp(editor, \":\")) {\n+\t\tint save_and_restore_term = strcmp(editor, \"echo\");\n \t\tstruct strbuf realpath = STRBUF_INIT;\n \t\tconst char *args[] = { editor, NULL, NULL };\n \t\tstruct child_process p = CHILD_PROCESS_INIT;\n@@ -86,9 +85,10 @@ static int launch_specified_editor(const char *editor, const char *path,\n \t\tp.env = env;\n \t\tp.use_shell = 1;\n \t\tp.trace2_child_class = \"editor\";\n-\t\tterm_fail = save_term(1);\n+\t\tif (save_and_restore_term)\n+\t\t\tsave_and_restore_term = !save_term(1);\n \t\tif (start_command(&p) < 0) {\n-\t\t\tif (!term_fail)\n+\t\t\tif (save_and_restore_term)\n \t\t\t\trestore_term();\n \t\t\tstrbuf_release(&realpath);\n \t\t\treturn error(\"unable to start editor '%s'\", editor);\n@@ -97,7 +97,7 @@ static int launch_specified_editor(const char *editor, const char *path,\n \t\tsigchain_push(SIGINT, SIG_IGN);\n \t\tsigchain_push(SIGQUIT, SIG_IGN);\n \t\tret = finish_command(&p);\n-\t\tif (!term_fail)\n+\t\tif (!save_and_restore_term)\n \t\t\trestore_term();\n \t\tstrbuf_release(&realpath);\n \t\tsig = ret - 128;\n-- snap --\n\nHowever, I could imagine that other scenarios call for an editor that\n_also_ does not run in the terminal, and where also no real terminal is\navailable for saving and restoring.\n\nI was tempted to suggest an `isatty(2)`, but that probably comes with its\nown problems, too.\n\nCiao,\nDscho\n\n>\n> Carlo\n>\n> [1] https://lore.kernel.org/git/ee302c98-da27-da43-e684-c7ec8b225360@gmx.net/\n> [2] https://yhbt.net/lore/all/nycvar.QRO.7.76.6.1903221436590.41@tvgsbejvaqbjf.bet/T/\n>\n>\n"},{"id":"442050","messageId":"xmqqmtlvbynr.fsf@gitster.g","threadId":"56959","inReplyTo":"nycvar.QRO.7.76.6.2111222319100.63@tvgsbejvaqbjf.bet","subject":"Re: preparing for 2.34.1","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-11-22T22:42:16Z","receivedAt":"2021-11-22T22:42:22Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> The quickest workaround for this is probably to special-case the editor\n> `echo`:\n\n\"GIT_EDITOR=: git cmd\" would also be a common trick people would use\nto bypass editor and take whatever is given as an initial template.\n\n> However, I could imagine that other scenarios call for an editor that\n> _also_ does not run in the terminal, and where also no real terminal is\n> available for saving and restoring.\n>\n> I was tempted to suggest an `isatty(2)`, but that probably comes with its\n> own problems, too.\n\nI think isatty(2) is pretty much our synonym to \"are we talking to\nan end-user sitting in front of the terminal\". Mostly we use it as\na way to control the progress bars, and use of editor on terminal\nwould be in line with these existing uses.\n"},{"id":"442056","messageId":"xmqqilwjbyj4.fsf@gitster.g","threadId":"56959","inReplyTo":"6cc0bc51-cc28-989c-d666-75b707f2c080@gmail.com","subject":"Re: preparing for 2.34.1","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-11-22T22:45:03Z","receivedAt":"2021-11-22T22:45:29Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Derrick Stolee writes:\n\n> On 11/22/21 12:37 PM, Junio C Hamano wrote:\n>> There are a few topics [*] to fix regressions introduced in the\n>> previous cycle, which should be in 2.34.1 and I've merged them to\n>> 'master'. I hope we can merge them to 'maint' (which now point at\n>> 2.34) and tag 2.34.1 in a few days.\n>> \n>> After that, in yet another few days, we will see most of the stalled\n>> topics ejected from the tree, the tip of 'next' rewound, and we will\n>> start the cycle toward 2.35 by starting to take new topics, by the\n>> beginning of the next week at the latest.\n>\n> There is a patch deep in a thread [1] that is a regression\n> in 2.34.0 that would be good to include in this release. It\n> fixes a sparse-checkout bug via a simple revert (plus a new\n> test).\n>\n> Sorry that the patch was buried deep and was easy to miss.\n>\n> [1] https://lore.kernel.org/git/72fffbff-16f7-fa17-b212-67aae9e1b034@gmail.com/\n\nInstead of these notes, it would be much easier for people to\nrealize that this (and previous) week is mostly about regression\nfixing and it is not welcome to drown the list with new patches, if\nyou just resent the patch with \n\n Subject: [PATCH/regression] fix blah\n\nI would say.\n\nHaving said that, thanks for a reminder. I did read it, but\napparently forgot to include it in the list of things needed for\n'master' when I prepared it last night.\n\n"},{"id":"442058","messageId":"CAPUEsph+F5inaga266xdFoc6KibjFo7hYh0ycdAkM9Q8yk5Ryg@mail.gmail.com","threadId":"56959","inReplyTo":"nycvar.QRO.7.76.6.2111222319100.63@tvgsbejvaqbjf.bet","subject":"Re: preparing for 2.34.1","fromName":"Carlo Arenas","fromEmail":"carenas@gmail.com","sentAt":"2021-11-22T22:50:08Z","receivedAt":"2021-11-22T22:50:24Z","isPatch":false,"sender":{"key":"carenas@gmail.com","avatar":"https://avatars.githubusercontent.com/u/76036?v=4"},"body":"On Mon, Nov 22, 2021 at 2:28 PM Johannes Schindelin\n wrote:\n> I was tempted to suggest an `isatty(2)`, but that probably comes with its\n> own problems, too.\n\nI went with isatty(1) in the suggested minimal bugfix[1] since I think\nit is safe to assume that no one that doesn't have stdout connected to\na terminal should even care if the invoking editor dies and messes\nwith their terminal, and hopefully whoever is hacking git commands\nthat invoke an editor to read its output, is actually not doing it\nthrough a tty but a pipe.\n\nWhat problems do you foresee with isatty?, the current code already\nchecks for errors on accessing the console, but as Phillip\nsuggested[2] in that thread a signal might be generated for some of\nthose errors when the terminal is shared with other processes.\n\nCarlo\n\nCC: +Phillip\n\n[1] https://lore.kernel.org/git/20211122222850.674-1-carenas@gmail.com/\n[2] https://lore.kernel.org/git/04ab7301-ea34-476c-eae4-4044fef74b91@gmail.com/T/#u\n"},{"id":"442064","messageId":"nycvar.QRO.7.76.6.2111230025440.63@tvgsbejvaqbjf.bet","threadId":"56959","inReplyTo":"xmqqmtlvbynr.fsf@gitster.g","subject":"Re: preparing for 2.34.1","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2021-11-22T23:30:07Z","receivedAt":"2021-11-22T23:30:16Z","isPatch":false,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Junio,\n\nOn Mon, 22 Nov 2021, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n>\n> > The quickest workaround for this is probably to special-case the editor\n> > `echo`:\n>\n> \"GIT_EDITOR=: git cmd\" would also be a common trick people would use\n> to bypass editor and take whatever is given as an initial template.\n\nGIT_EDITOR=: is not a problem because of\nhttps://github.com/git/git/blob/v2.34.0/editor.c#L59:\n\n\tif (strcmp(editor, \":\")) {\n\t\t[...]\n\t\tterm_fail = save_term(1);\n\t\tif (start_command(&p) < 0) {\n\t\t\tif (!term_fail)\n\t\t\t\trestore_term();\n\t\t\t[...]\n\t\t}\n\n\t\t[...]\n\t\tif (!term_fail)\n\t\t\trestore_term();\n\t\t[...]\n\t}\n\n> > However, I could imagine that other scenarios call for an editor that\n> > _also_ does not run in the terminal, and where also no real terminal is\n> > available for saving and restoring.\n> >\n> > I was tempted to suggest an `isatty(2)`, but that probably comes with its\n> > own problems, too.\n>\n> I think isatty(2) is pretty much our synonym to \"are we talking to\n> an end-user sitting in front of the terminal\". Mostly we use it as\n> a way to control the progress bars, and use of editor on terminal\n> would be in line with these existing uses.\n\nIndeed, I think that isatty(2) is a better indicator than isatty(1). We\nsometimes _do_ redirect the output of, say, `git commit`, to capture the\ncommit hash that was generated. We typically do not redirect stderr,\nthough, unless calling from an application and capturing everything via\npipes. So isatty(2) strikes me as the best balance we can strike here.\n\nCiao,\nDscho\n"},{"id":"442066","messageId":"039401d7dffb$e5198800$af4c9800$@nexbridge.com","threadId":"56959","inReplyTo":"nycvar.QRO.7.76.6.2111230025440.63@tvgsbejvaqbjf.bet","subject":"RE: preparing for 2.34.1","fromName":"","fromEmail":"rsbecker@nexbridge.com","sentAt":"2021-11-22T23:51:34Z","receivedAt":"2021-11-22T23:51:46Z","isPatch":false,"sender":{"key":"randall.becker@nexbridge.ca","avatar":"https://avatars.githubusercontent.com/u/28956764?v=4"},"body":"On November 22, 2021 6:30 PM, Johannes Schindelin wrote:\n> On Mon, 22 Nov 2021, Junio C Hamano wrote:\n> \n> > Johannes Schindelin writes:\n> >\n> > > The quickest workaround for this is probably to special-case the\n> > > editor\n> > > `echo`:\n> >\n> > \"GIT_EDITOR=: git cmd\" would also be a common trick people would use\n> > to bypass editor and take whatever is given as an initial template.\n> \n> GIT_EDITOR=: is not a problem because of\n> https://github.com/git/git/blob/v2.34.0/editor.c#L59:\n> \n> \tif (strcmp(editor, \":\")) {\n> \t\t[...]\n> \t\tterm_fail = save_term(1);\n> \t\tif (start_command(&p) < 0) {\n> \t\t\tif (!term_fail)\n> \t\t\t\trestore_term();\n> \t\t\t[...]\n> \t\t}\n> \n> \t\t[...]\n> \t\tif (!term_fail)\n> \t\t\trestore_term();\n> \t\t[...]\n> \t}\n> \n> > > However, I could imagine that other scenarios call for an editor\n> > > that _also_ does not run in the terminal, and where also no real\n> > > terminal is available for saving and restoring.\n> > >\n> > > I was tempted to suggest an `isatty(2)`, but that probably comes\n> > > with its own problems, too.\n> >\n> > I think isatty(2) is pretty much our synonym to \"are we talking to an\n> > end-user sitting in front of the terminal\". Mostly we use it as a way\n> > to control the progress bars, and use of editor on terminal would be\n> > in line with these existing uses.\n> \n> Indeed, I think that isatty(2) is a better indicator than isatty(1). We\n> sometimes _do_ redirect the output of, say, `git commit`, to capture the\n> commit hash that was generated. We typically do not redirect stderr,\nthough,\n> unless calling from an application and capturing everything via pipes. So\n> isatty(2) strikes me as the best balance we can strike here.\n\nPlease be careful of this one. isatty(2) may not be the issue, but the\nfiledes provided by Jenkins and other CI/CD systems over SSH often\nmischaracterises the results from this call. Hacking the file descriptor\n(2>&1 etc) prior to going to git is a common thing in scripts. Stdin is\nfrequently wrong when Jenkins sets up the environment to prompt for an SSH\npassphrase - happens in non-Docker nohup situations - where git will end up\nthinking it is interactive when it is not - not on NonStop fortunately, but\nthis happens with a Gentoo Hypervisor and Ubuntu VM. Also, stderr gets\nredirected in scripts frequently in my experience - particularly on exotic\noperating systems, crossing over from say, legacy NonStop or IBM TSO to\neach's POSIX environment. I would expect breakages from this assumption, so\nplease be cautious.\n\n-Randall\n\n"},{"id":"442175","messageId":"xmqq35nmaiyx.fsf@gitster.g","threadId":"56959","inReplyTo":"xmqqr1b8gkhg.fsf@gitster.g","subject":"preparing for 2.34.1 (2nd round)","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-11-23T17:18:46Z","receivedAt":"2021-11-23T17:18:52Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano writes:\n\nThere are a few topics [*] to fix regressions introduced in the\nprevious cycle, which should be in 2.34.1 and I've merged them to\n'master'. I hope we can merge them to 'maint' (which now point at\n2.34) and tag 2.34.1 in a day or two.\n\nAfter that, in yet another few days, we will see most of the stalled\ntopics ejected from the tree, the tip of 'next' rewound, and we will\nstart the cycle toward 2.35 by starting to take new topics,\nhopefully by the beginning of the next week.\n\n\nThanks.\n\n\nFixes since v2.34\n-----------------\n\n * \"git grep\" looking in a blob that has non-UTF8 payload was\n completely broken when linked with certain versions of PCREv2\n library in the latest release.\n\n * \"git pull\" with any strategy when the other side is behind us\n should succeed as it is a no-op, but doesn't.\n (merge ea1954af ev/pull-already-up-to-date-is-noop later to maint).\n\n * An earlier change in 2.34.0 caused JGit application (that abused\n GIT_EDITOR mechanism when invoking \"git config\") to get stuck with\n a SIGTTOU signal; it has been reverted.\n (merge e3f7e01b50 jc/save-restore-terminal-revert later to maint).\n\n * An earlier change that broke .gitignore matching has been reverted.\n (merge 33c5d6c845 ds/add-rm-with-sparse-index later to maint).\n\n * SubmittingPatches document gained a syntactically incorrect mark-up,\n which has been corrected.\n (merge edbd9f37 ab/update-submitting-patches later to maint).\n"}]}