{"thread":{"id":"56105","subject":"progress test failure on fedora34","startedAt":"2021-07-14T12:39:51Z","lastAt":"2021-08-04T23:05:54Z","messageCount":26,"participants":["Fabian Stelzer","Ævar Arnfjörð Bjarmason","Alex Henrie","Jeff King","Felipe Contreras","ZheNing Hu","Junio C Hamano","SZEDER Gábor","Eric Sunshine"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"430036","messageId":"49498ed0-cfd5-2305-cee7-5c5939a19bcf@campoint.net","threadId":"56105","inReplyTo":null,"subject":"progress test failure on fedora34","fromName":"Fabian Stelzer","fromEmail":"fabian.stelzer@campoint.net","sentAt":"2021-07-14T12:39:46Z","receivedAt":"2021-07-14T12:39:51Z","isPatch":false,"sender":{"key":"fabian.stelzer@campoint.net","avatar":null},"body":"Hi,\nThe test t0500-progress-display.sh in current master fails on latest \nfedora34.\nThe break was introduced with:\n\n83ae1edff7ee0b7674bd556955d2cf1706bddb21\nab/fix-columns-to-80-during-tests (2021-06-29) 1 commit\n\nKind regards,\nFabian\n\n\nexpecting success of 0500.3 'progress display breaks long lines #1':\n     sed -e \"s/Z$//\" >expect <<\\EOF &&\nWorking hard.......2.........3.........4.........5.........6:   0% \n(100/100000)\nWorking hard.......2.........3.........4.........5.........6:   1% \n(1000/100000)\nWorking hard.......2.........3.........4.........5.........6: Z\n    10% (10000/100000)\n   100% (100000/100000)\n   100% (100000/100000), done.\nEOF\n\n     cat >in <<-\\EOF &&\n     progress 100\n     progress 1000\n     progress 10000\n     progress 100000\n     EOF\n     test-tool progress --total=100000 \\\n         \"Working hard.......2.........3.........4.........5.........6\" \\\n         stderr &&\n\n     show_cr out &&\n     test_cmp expect out\n\n++ sed -e 's/Z$//'\n++ cat\n++ test-tool progress --total=100000 'Working \nhard.......2.........3.........4.........5.........6'\n++ show_cr\n++ tr '\\015' Q\n++ sed -e 's/Q/\\\n/g'\n++ test_cmp expect out\n++ test 2 -ne 2\n++ eval 'diff -u' '\"$@\"'\n+++ diff -u expect out\n--- expect    2021-07-14 12:29:49.576970165 +0000\n+++ out    2021-07-14 12:29:49.580970145 +0000\n@@ -1,6 +1,5 @@\n  Working hard.......2.........3.........4.........5.........6:   0% \n(100/100000)\n  Working hard.......2.........3.........4.........5.........6:   1% \n(1000/100000)\n-Working hard.......2.........3.........4.........5.........6:\n-   10% (10000/100000)\n-  100% (100000/100000)\n-  100% (100000/100000), done.\n+Working hard.......2.........3.........4.........5.........6:  10% \n(10000/100000)\n+Working hard.......2.........3.........4.........5.........6: 100% \n(100000/100000)\n+Working hard.......2.........3.........4.........5.........6: 100% \n(100000/100000), done.\nerror: last command exited with $?=1\nnot ok 3 - progress display breaks long lines #1\n#\n#        sed -e \"s/Z$//\" >expect <<\\EOF &&\n#    Working hard.......2.........3.........4.........5.........6: 0% \n(100/100000)\n#    Working hard.......2.........3.........4.........5.........6: 1% \n(1000/100000)\n#    Working hard.......2.........3.........4.........5.........6: Z\n#       10% (10000/100000)\n#      100% (100000/100000)\n#      100% (100000/100000), done.\n#    EOF\n#\n#        cat >in <<-\\EOF &&\n#        progress 100\n#        progress 1000\n#        progress 10000\n#        progress 100000\n#        EOF\n#        test-tool progress --total=100000 \\\n#            \"Working \nhard.......2.........3.........4.........5.........6\" \\\n#            stderr &&\n#\n#        show_cr out &&\n#        test_cmp expect out\n#\n"},{"id":"430062","messageId":"87a6mo29dp.fsf@evledraar.gmail.com","threadId":"56105","inReplyTo":"49498ed0-cfd5-2305-cee7-5c5939a19bcf@campoint.net","subject":"Re: progress test failure on fedora34","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-07-14T15:35:25Z","receivedAt":"2021-07-14T15:38:41Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Jul 14 2021, Fabian Stelzer wrote:\n\n> Hi,\n> The test t0500-progress-display.sh in current master fails on latest\n> fedora34.\n> The break was introduced with:\n>\n> 83ae1edff7ee0b7674bd556955d2cf1706bddb21\n> ab/fix-columns-to-80-during-tests (2021-06-29) 1 commit\n>\n> Kind regards,\n> Fabian\n\nI have not been able to reproduce this, it seems the below E-Mail was\nword-wrapped by your mailer, which is especially bad here since getting\nto the bottom of this requires looking at the whitespace.\n\nIs there a way you could tar that up and send it (to me personally is\nfine, or some pastebin or whatever).\n\nI am able to reproduce something that looks like this if I\ns/COLUMNS=80/COLUMNS=79/g in the test-lib, but given that we set it to\n80, and that the progress.c code just ends up with an\natoi(getenv(\"COLUMNS\")), and we do our own wrapping (with no other fancy\nlogic) in progress.c, I'm not seeing right now how this could happen...\n"},{"id":"430072","messageId":"CAMMLpeQ5Lh8xfqTZoM74f616wE7ZhqWArL1WgGiMtiJSfrYcYg@mail.gmail.com","threadId":"56105","inReplyTo":"87a6mo29dp.fsf@evledraar.gmail.com","subject":"Re: progress test failure on fedora34","fromName":"Alex Henrie","fromEmail":"alexhenrie24@gmail.com","sentAt":"2021-07-14T16:35:30Z","receivedAt":"2021-07-14T16:35:44Z","isPatch":false,"sender":{"key":"alexhenrie24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5951993?v=4"},"body":"On Wed, Jul 14, 2021 at 9:39 AM Ævar Arnfjörð Bjarmason\n wrote:\n>\n>\n> On Wed, Jul 14 2021, Fabian Stelzer wrote:\n>\n> > Hi,\n> > The test t0500-progress-display.sh in current master fails on latest\n> > fedora34.\n> > The break was introduced with:\n> >\n> > 83ae1edff7ee0b7674bd556955d2cf1706bddb21\n> > ab/fix-columns-to-80-during-tests (2021-06-29) 1 commit\n> >\n> > Kind regards,\n> > Fabian\n>\n> I have not been able to reproduce this, it seems the below E-Mail was\n> word-wrapped by your mailer, which is especially bad here since getting\n> to the bottom of this requires looking at the whitespace.\n>\n> Is there a way you could tar that up and send it (to me personally is\n> fine, or some pastebin or whatever).\n>\n> I am able to reproduce something that looks like this if I\n> s/COLUMNS=80/COLUMNS=79/g in the test-lib, but given that we set it to\n> 80, and that the progress.c code just ends up with an\n> atoi(getenv(\"COLUMNS\")), and we do our own wrapping (with no other fancy\n> logic) in progress.c, I'm not seeing right now how this could happen...\n\nThis test also fails for me when using QTerminal or Konsole, but it\npasses on XTerm and LXTerminal.\n\n-Alex\n"},{"id":"430461","messageId":"87h7gsvybx.fsf@evledraar.gmail.com","threadId":"56105","inReplyTo":"CAMMLpeQ5Lh8xfqTZoM74f616wE7ZhqWArL1WgGiMtiJSfrYcYg@mail.gmail.com","subject":"Re: progress test failure on fedora34","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-07-18T08:05:44Z","receivedAt":"2021-07-18T08:08:28Z","isPatch":false,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"\nOn Wed, Jul 14 2021, Alex Henrie wrote:\n\n> On Wed, Jul 14, 2021 at 9:39 AM Ævar Arnfjörð Bjarmason\n> wrote:\n>>\n>>\n>> On Wed, Jul 14 2021, Fabian Stelzer wrote:\n>>\n>> > Hi,\n>> > The test t0500-progress-display.sh in current master fails on latest\n>> > fedora34.\n>> > The break was introduced with:\n>> >\n>> > 83ae1edff7ee0b7674bd556955d2cf1706bddb21\n>> > ab/fix-columns-to-80-during-tests (2021-06-29) 1 commit\n>> >\n>> > Kind regards,\n>> > Fabian\n>>\n>> I have not been able to reproduce this, it seems the below E-Mail was\n>> word-wrapped by your mailer, which is especially bad here since getting\n>> to the bottom of this requires looking at the whitespace.\n>>\n>> Is there a way you could tar that up and send it (to me personally is\n>> fine, or some pastebin or whatever).\n>>\n>> I am able to reproduce something that looks like this if I\n>> s/COLUMNS=80/COLUMNS=79/g in the test-lib, but given that we set it to\n>> 80, and that the progress.c code just ends up with an\n>> atoi(getenv(\"COLUMNS\")), and we do our own wrapping (with no other fancy\n>> logic) in progress.c, I'm not seeing right now how this could happen...\n>\n> This test also fails for me when using QTerminal or Konsole, but it\n> passes on XTerm and LXTerminal.\n\nI tried this on Debian 11 with QTerminal 0.16.1 and can't reproduce it,\nresized the window etc., always get COLUMNS=80 if I add some printf\ndebugging.\n\nDo you mind testing with an ad-hoc patch like this on top? It will fail\nright away, but should say COLUMNS = 80 in the output.\n\nThe only thing I can think of right now is that some terminals are doing\nsome evil trickery to LD_PRELOAD or whatever and intercept getenv() for\nCOLUMNS and the like, but that's an entirely unfounded hunch.\n\ndiff --git a/progress.c b/progress.c\nindex 680c6a8bf9..dca254b515 100644\n--- a/progress.c\n+++ b/progress.c\n@@ -144,6 +144,7 @@ static void display(struct progress *progress, uint64_t n, const char *done)\n size_t progress_line_len = progress->title_len +\n counters_sb->len + 2;\n int cols = term_columns();\n+ fprintf(stderr, \"cols = %d\\n\", cols);\n \n if (progress->split) {\n fprintf(stderr, \" %s%*s\", counters_sb->buf,\n"},{"id":"430480","messageId":"YPU/Enow8Mgn5f4j@coredump.intra.peff.net","threadId":"56105","inReplyTo":"87h7gsvybx.fsf@evledraar.gmail.com","subject":"Re: progress test failure on fedora34","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-07-19T09:00:02Z","receivedAt":"2021-07-19T09:00:05Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Jul 18, 2021 at 10:05:44AM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> > This test also fails for me when using QTerminal or Konsole, but it\n> > passes on XTerm and LXTerminal.\n> \n> I tried this on Debian 11 with QTerminal 0.16.1 and can't reproduce it,\n> resized the window etc., always get COLUMNS=80 if I add some printf\n> debugging.\n> \n> Do you mind testing with an ad-hoc patch like this on top? It will fail\n> right away, but should say COLUMNS = 80 in the output.\n> \n> The only thing I can think of right now is that some terminals are doing\n> some evil trickery to LD_PRELOAD or whatever and intercept getenv() for\n> COLUMNS and the like, but that's an entirely unfounded hunch.\n\nThat would be truly evil. :)\n\nAnother possible source of weirdness: some shells are picky about\nassigning to COLUMNS, and are eager to set it themselves. E.g.:\n\n $ echo $COLUMNS\n 119\n $ COLUMNS=80 bash -c 'echo $COLUMNS'\n 80\n $ COLUMNS=80 zsh -c 'echo $COLUMNS'\n 119\n\nSo zsh, even in a non-interactive shell, will set it. It does at least\naccept setting it, and will preserve it in sub-shells and forks. But it\nwill silently ignore invalid values, instead going back to the ioctl:\n\n $ zsh -c 'COLUMNS=80; echo $COLUMNS; COLUMNS=foo; echo $COLUMNS'\n 80\n 119\n\nmksh behaves the same way; I was tipped off to this by b082687cba\n(test-lib: skip test with COLUMNS=1 under mksh, 2012-04-27).\n\nI have trouble seeing how this could cause a problem since \"80\" seems\nlike a perfectly sensible value. And one would imagine that the same\nshell is being used in all cases above (but maybe not, depending on the\nconfiguration of the terminal programs?). But it's another possible\navenue of investigation.\n\n> diff --git a/progress.c b/progress.c\n> index 680c6a8bf9..dca254b515 100644\n> --- a/progress.c\n> +++ b/progress.c\n> @@ -144,6 +144,7 @@ static void display(struct progress *progress, uint64_t n, const char *done)\n> size_t progress_line_len = progress->title_len +\n> counters_sb->len + 2;\n> int cols = term_columns();\n> + fprintf(stderr, \"cols = %d\\n\", cols);\n> \n> if (progress->split) {\n> fprintf(stderr, \" %s%*s\", counters_sb->buf,\n\nYeah, this seems like a good start to see if the value we're getting is\nbogus. Likewise, it might be interesting for term_columns() to tell us\nif it's getting the value from $COLUMNS or from the ioctl (but it's hard\nto believe it wouldn't be from $COLUMNS, given the code).\n\n-Peff\n"},{"id":"430522","messageId":"CAMMLpeRhHn5ndPYzihm0rCidjNQM8c7b+Ta-JSZ=jidbEr=VRg@mail.gmail.com","threadId":"56105","inReplyTo":"87h7gsvybx.fsf@evledraar.gmail.com","subject":"Re: progress test failure on fedora34","fromName":"Alex Henrie","fromEmail":"alexhenrie24@gmail.com","sentAt":"2021-07-19T17:18:44Z","receivedAt":"2021-07-19T17:37:46Z","isPatch":false,"sender":{"key":"alexhenrie24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5951993?v=4"},"body":"On Sun, Jul 18, 2021 at 2:08 AM Ævar Arnfjörð Bjarmason\n wrote:\n>\n> On Wed, Jul 14 2021, Alex Henrie wrote:\n>\n> > On Wed, Jul 14, 2021 at 9:39 AM Ævar Arnfjörð Bjarmason\n> > wrote:\n> >>\n> >> On Wed, Jul 14 2021, Fabian Stelzer wrote:\n> >>\n> >> > Hi,\n> >> > The test t0500-progress-display.sh in current master fails on latest\n> >> > fedora34.\n> >> > The break was introduced with:\n> >> >\n> >> > 83ae1edff7ee0b7674bd556955d2cf1706bddb21\n> >> > ab/fix-columns-to-80-during-tests (2021-06-29) 1 commit\n> >> >\n> >> > Kind regards,\n> >> > Fabian\n> >>\n> >> I have not been able to reproduce this, it seems the below E-Mail was\n> >> word-wrapped by your mailer, which is especially bad here since getting\n> >> to the bottom of this requires looking at the whitespace.\n> >>\n> >> Is there a way you could tar that up and send it (to me personally is\n> >> fine, or some pastebin or whatever).\n> >>\n> >> I am able to reproduce something that looks like this if I\n> >> s/COLUMNS=80/COLUMNS=79/g in the test-lib, but given that we set it to\n> >> 80, and that the progress.c code just ends up with an\n> >> atoi(getenv(\"COLUMNS\")), and we do our own wrapping (with no other fancy\n> >> logic) in progress.c, I'm not seeing right now how this could happen...\n> >\n> > This test also fails for me when using QTerminal or Konsole, but it\n> > passes on XTerm and LXTerminal.\n>\n> I tried this on Debian 11 with QTerminal 0.16.1 and can't reproduce it,\n> resized the window etc., always get COLUMNS=80 if I add some printf\n> debugging.\n\nActually, it looks like the difference was that I didn't resize the\nXTerm or LXTerminal windows. The tests pass on all four if the\nterminal emulator window is exactly 80 columns wide, and they fail on\nall four if the window is any wider or narrower.\n\n-Alex\n"},{"id":"430537","messageId":"CAMMLpeSBCiMbr4QRckW4Df2bJq-7t_oj4s+6W3TpWZdYRMKVHg@mail.gmail.com","threadId":"56105","inReplyTo":"CAMMLpeRhHn5ndPYzihm0rCidjNQM8c7b+Ta-JSZ=jidbEr=VRg@mail.gmail.com","subject":"Re: progress test failure on fedora34","fromName":"Alex Henrie","fromEmail":"alexhenrie24@gmail.com","sentAt":"2021-07-19T18:21:13Z","receivedAt":"2021-07-19T19:15:03Z","isPatch":false,"sender":{"key":"alexhenrie24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5951993?v=4"},"body":"On Mon, Jul 19, 2021 at 11:18 AM Alex Henrie wrote:\n>\n> On Sun, Jul 18, 2021 at 2:08 AM Ævar Arnfjörð Bjarmason\n> wrote:\n> >\n> > On Wed, Jul 14 2021, Alex Henrie wrote:\n> >\n> > > On Wed, Jul 14, 2021 at 9:39 AM Ævar Arnfjörð Bjarmason\n> > > wrote:\n> > >>\n> > >> On Wed, Jul 14 2021, Fabian Stelzer wrote:\n> > >>\n> > >> > Hi,\n> > >> > The test t0500-progress-display.sh in current master fails on latest\n> > >> > fedora34.\n> > >> > The break was introduced with:\n> > >> >\n> > >> > 83ae1edff7ee0b7674bd556955d2cf1706bddb21\n> > >> > ab/fix-columns-to-80-during-tests (2021-06-29) 1 commit\n> > >> >\n> > >> > Kind regards,\n> > >> > Fabian\n> > >>\n> > >> I have not been able to reproduce this, it seems the below E-Mail was\n> > >> word-wrapped by your mailer, which is especially bad here since getting\n> > >> to the bottom of this requires looking at the whitespace.\n> > >>\n> > >> Is there a way you could tar that up and send it (to me personally is\n> > >> fine, or some pastebin or whatever).\n> > >>\n> > >> I am able to reproduce something that looks like this if I\n> > >> s/COLUMNS=80/COLUMNS=79/g in the test-lib, but given that we set it to\n> > >> 80, and that the progress.c code just ends up with an\n> > >> atoi(getenv(\"COLUMNS\")), and we do our own wrapping (with no other fancy\n> > >> logic) in progress.c, I'm not seeing right now how this could happen...\n> > >\n> > > This test also fails for me when using QTerminal or Konsole, but it\n> > > passes on XTerm and LXTerminal.\n> >\n> > I tried this on Debian 11 with QTerminal 0.16.1 and can't reproduce it,\n> > resized the window etc., always get COLUMNS=80 if I add some printf\n> > debugging.\n>\n> Actually, it looks like the difference was that I didn't resize the\n> XTerm or LXTerminal windows. The tests pass on all four if the\n> terminal emulator window is exactly 80 columns wide, and they fail on\n> all four if the window is any wider or narrower.\n\nI have narrowed the problem down to the `tput` command in test-lib.sh:\nWhen `tput` runs, $COLUMNS is reset to the width of the terminal\nemulator window.\n\n-Alex\n"},{"id":"430540","messageId":"60f5c7b867941_141e5c20886@natae.notmuch","threadId":"56105","inReplyTo":"87h7gsvybx.fsf@evledraar.gmail.com","subject":"Re: progress test failure on fedora34","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-07-19T18:43:04Z","receivedAt":"2021-07-19T19:15:06Z","isPatch":false,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Ævar Arnfjörð Bjarmason wrote:\n> \n> On Wed, Jul 14 2021, Alex Henrie wrote:\n> \n> > On Wed, Jul 14, 2021 at 9:39 AM Ævar Arnfjörð Bjarmason\n> > wrote:\n> >>\n> >>\n> >> On Wed, Jul 14 2021, Fabian Stelzer wrote:\n> >>\n> >> > Hi,\n> >> > The test t0500-progress-display.sh in current master fails on latest\n> >> > fedora34.\n> >> > The break was introduced with:\n> >> >\n> >> > 83ae1edff7ee0b7674bd556955d2cf1706bddb21\n> >> > ab/fix-columns-to-80-during-tests (2021-06-29) 1 commit\n> >> >\n> >> > Kind regards,\n> >> > Fabian\n> >>\n> >> I have not been able to reproduce this, it seems the below E-Mail was\n> >> word-wrapped by your mailer, which is especially bad here since getting\n> >> to the bottom of this requires looking at the whitespace.\n> >>\n> >> Is there a way you could tar that up and send it (to me personally is\n> >> fine, or some pastebin or whatever).\n> >>\n> >> I am able to reproduce something that looks like this if I\n> >> s/COLUMNS=80/COLUMNS=79/g in the test-lib, but given that we set it to\n> >> 80, and that the progress.c code just ends up with an\n> >> atoi(getenv(\"COLUMNS\")), and we do our own wrapping (with no other fancy\n> >> logic) in progress.c, I'm not seeing right now how this could happen...\n> >\n> > This test also fails for me when using QTerminal or Konsole, but it\n> > passes on XTerm and LXTerminal.\n> \n> I tried this on Debian 11 with QTerminal 0.16.1 and can't reproduce it,\n> resized the window etc., always get COLUMNS=80 if I add some printf\n> debugging.\n> \n> Do you mind testing with an ad-hoc patch like this on top? It will fail\n> right away, but should say COLUMNS = 80 in the output.\n> \n> The only thing I can think of right now is that some terminals are doing\n> some evil trickery to LD_PRELOAD or whatever and intercept getenv() for\n> COLUMNS and the like, but that's an entirely unfounded hunch.\n\nI'm able to reproduce this. The test fails when running directly with\nbash, but not with prove.\n\nAnd it seems to be a bug in bash:\n\n export COLUMNS=80\n\n echo \"COLUMNS: $COLUMNS\"\n cat > /tmp/expect < Ævar Arnfjörð Bjarmason wrote:\n> > \n> > On Wed, Jul 14 2021, Alex Henrie wrote:\n> > \n> > > On Wed, Jul 14, 2021 at 9:39 AM Ævar Arnfjörð Bjarmason\n> > > wrote:\n> > >>\n> > >>\n> > >> On Wed, Jul 14 2021, Fabian Stelzer wrote:\n> > >>\n> > >> > Hi,\n> > >> > The test t0500-progress-display.sh in current master fails on latest\n> > >> > fedora34.\n> > >> > The break was introduced with:\n> > >> >\n> > >> > 83ae1edff7ee0b7674bd556955d2cf1706bddb21\n> > >> > ab/fix-columns-to-80-during-tests (2021-06-29) 1 commit\n> > >> >\n> > >> > Kind regards,\n> > >> > Fabian\n> > >>\n> > >> I have not been able to reproduce this, it seems the below E-Mail was\n> > >> word-wrapped by your mailer, which is especially bad here since getting\n> > >> to the bottom of this requires looking at the whitespace.\n> > >>\n> > >> Is there a way you could tar that up and send it (to me personally is\n> > >> fine, or some pastebin or whatever).\n> > >>\n> > >> I am able to reproduce something that looks like this if I\n> > >> s/COLUMNS=80/COLUMNS=79/g in the test-lib, but given that we set it to\n> > >> 80, and that the progress.c code just ends up with an\n> > >> atoi(getenv(\"COLUMNS\")), and we do our own wrapping (with no other fancy\n> > >> logic) in progress.c, I'm not seeing right now how this could happen...\n> > >\n> > > This test also fails for me when using QTerminal or Konsole, but it\n> > > passes on XTerm and LXTerminal.\n> > \n> > I tried this on Debian 11 with QTerminal 0.16.1 and can't reproduce it,\n> > resized the window etc., always get COLUMNS=80 if I add some printf\n> > debugging.\n> > \n> > Do you mind testing with an ad-hoc patch like this on top? It will fail\n> > right away, but should say COLUMNS = 80 in the output.\n> > \n> > The only thing I can think of right now is that some terminals are doing\n> > some evil trickery to LD_PRELOAD or whatever and intercept getenv() for\n> > COLUMNS and the like, but that's an entirely unfounded hunch.\n> \n> I'm able to reproduce this. The test fails when running directly with\n> bash, but not with prove.\n> \n> And it seems to be a bug in bash:\n> \n> export COLUMNS=80\n> \n> echo \"COLUMNS: $COLUMNS\"\n> cat > /tmp/expect < foobar\n> EOF\n> echo \"COLUMNS: $COLUMNS\"\n> \n> I get:\n> \n> COLUMNS: 80\n> COLUMNS: 115\n> \n> Even directly in the console.\n\nHmm, from man bash:\n\n checkwinsize\n If set, bash checks the window size after each external (non‐builtin) com‐\n mand and, if necessary, updates the values of LINES and COLUMNS. This op‐\n tion is enabled by default.\n\nSeems like since bash 5.0 this is on by default.\n\n-- \nFelipe Contreras"},{"id":"430546","messageId":"CAMMLpeT3bJcr7mRDpxmk32VqpAbNpN=fgPjmkcY+0zOBYruybQ@mail.gmail.com","threadId":"56105","inReplyTo":"60f5d3ac6ce_14538820823@natae.notmuch","subject":"Re: progress test failure on fedora34","fromName":"Alex Henrie","fromEmail":"alexhenrie24@gmail.com","sentAt":"2021-07-19T20:42:24Z","receivedAt":"2021-07-19T23:11:22Z","isPatch":false,"sender":{"key":"alexhenrie24@gmail.com","avatar":"https://avatars.githubusercontent.com/u/5951993?v=4"},"body":"On Mon, Jul 19, 2021 at 1:34 PM Felipe Contreras\n wrote:\n>\n> Felipe Contreras wrote:\n> > Ævar Arnfjörð Bjarmason wrote:\n> > >\n> > > On Wed, Jul 14 2021, Alex Henrie wrote:\n> > >\n> > > > On Wed, Jul 14, 2021 at 9:39 AM Ævar Arnfjörð Bjarmason\n> > > > wrote:\n> > > >>\n> > > >>\n> > > >> On Wed, Jul 14 2021, Fabian Stelzer wrote:\n> > > >>\n> > > >> > Hi,\n> > > >> > The test t0500-progress-display.sh in current master fails on latest\n> > > >> > fedora34.\n> > > >> > The break was introduced with:\n> > > >> >\n> > > >> > 83ae1edff7ee0b7674bd556955d2cf1706bddb21\n> > > >> > ab/fix-columns-to-80-during-tests (2021-06-29) 1 commit\n> > > >> >\n> > > >> > Kind regards,\n> > > >> > Fabian\n> > > >>\n> > > >> I have not been able to reproduce this, it seems the below E-Mail was\n> > > >> word-wrapped by your mailer, which is especially bad here since getting\n> > > >> to the bottom of this requires looking at the whitespace.\n> > > >>\n> > > >> Is there a way you could tar that up and send it (to me personally is\n> > > >> fine, or some pastebin or whatever).\n> > > >>\n> > > >> I am able to reproduce something that looks like this if I\n> > > >> s/COLUMNS=80/COLUMNS=79/g in the test-lib, but given that we set it to\n> > > >> 80, and that the progress.c code just ends up with an\n> > > >> atoi(getenv(\"COLUMNS\")), and we do our own wrapping (with no other fancy\n> > > >> logic) in progress.c, I'm not seeing right now how this could happen...\n> > > >\n> > > > This test also fails for me when using QTerminal or Konsole, but it\n> > > > passes on XTerm and LXTerminal.\n> > >\n> > > I tried this on Debian 11 with QTerminal 0.16.1 and can't reproduce it,\n> > > resized the window etc., always get COLUMNS=80 if I add some printf\n> > > debugging.\n> > >\n> > > Do you mind testing with an ad-hoc patch like this on top? It will fail\n> > > right away, but should say COLUMNS = 80 in the output.\n> > >\n> > > The only thing I can think of right now is that some terminals are doing\n> > > some evil trickery to LD_PRELOAD or whatever and intercept getenv() for\n> > > COLUMNS and the like, but that's an entirely unfounded hunch.\n> >\n> > I'm able to reproduce this. The test fails when running directly with\n> > bash, but not with prove.\n> >\n> > And it seems to be a bug in bash:\n> >\n> > export COLUMNS=80\n> >\n> > echo \"COLUMNS: $COLUMNS\"\n> > cat > /tmp/expect < > foobar\n> > EOF\n> > echo \"COLUMNS: $COLUMNS\"\n> >\n> > I get:\n> >\n> > COLUMNS: 80\n> > COLUMNS: 115\n> >\n> > Even directly in the console.\n>\n> Hmm, from man bash:\n>\n> checkwinsize\n> If set, bash checks the window size after each external (non‐builtin) com‐\n> mand and, if necessary, updates the values of LINES and COLUMNS. This op‐\n> tion is enabled by default.\n>\n> Seems like since bash 5.0 this is on by default.\n\nIndeed, running `shopt -u checkwinsize` right after exporting COLUMNS\nin test-lib.sh fixes the tests. Great work!\n\n-Alex\n"},{"id":"430561","messageId":"60f61b81a3b74_14781320816@natae.notmuch","threadId":"56105","inReplyTo":"CAMMLpeT3bJcr7mRDpxmk32VqpAbNpN=fgPjmkcY+0zOBYruybQ@mail.gmail.com","subject":"Re: progress test failure on fedora34","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-07-20T00:40:33Z","receivedAt":"2021-07-20T01:02:47Z","isPatch":false,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"Alex Henrie wrote:\n> On Mon, Jul 19, 2021 at 1:34 PM Felipe Contreras\n> wrote:\n> >\n> > Felipe Contreras wrote:\n> > > Ævar Arnfjörð Bjarmason wrote:\n> > > >\n> > > > On Wed, Jul 14 2021, Alex Henrie wrote:\n> > > >\n> > > > > On Wed, Jul 14, 2021 at 9:39 AM Ævar Arnfjörð Bjarmason\n> > > > > wrote:\n> > > > >>\n> > > > >>\n> > > > >> On Wed, Jul 14 2021, Fabian Stelzer wrote:\n> > > > >>\n> > > > >> > Hi,\n> > > > >> > The test t0500-progress-display.sh in current master fails on latest\n> > > > >> > fedora34.\n> > > > >> > The break was introduced with:\n> > > > >> >\n> > > > >> > 83ae1edff7ee0b7674bd556955d2cf1706bddb21\n> > > > >> > ab/fix-columns-to-80-during-tests (2021-06-29) 1 commit\n> > > > >> >\n> > > > >> > Kind regards,\n> > > > >> > Fabian\n> > > > >>\n> > > > >> I have not been able to reproduce this, it seems the below E-Mail was\n> > > > >> word-wrapped by your mailer, which is especially bad here since getting\n> > > > >> to the bottom of this requires looking at the whitespace.\n> > > > >>\n> > > > >> Is there a way you could tar that up and send it (to me personally is\n> > > > >> fine, or some pastebin or whatever).\n> > > > >>\n> > > > >> I am able to reproduce something that looks like this if I\n> > > > >> s/COLUMNS=80/COLUMNS=79/g in the test-lib, but given that we set it to\n> > > > >> 80, and that the progress.c code just ends up with an\n> > > > >> atoi(getenv(\"COLUMNS\")), and we do our own wrapping (with no other fancy\n> > > > >> logic) in progress.c, I'm not seeing right now how this could happen...\n> > > > >\n> > > > > This test also fails for me when using QTerminal or Konsole, but it\n> > > > > passes on XTerm and LXTerminal.\n> > > >\n> > > > I tried this on Debian 11 with QTerminal 0.16.1 and can't reproduce it,\n> > > > resized the window etc., always get COLUMNS=80 if I add some printf\n> > > > debugging.\n> > > >\n> > > > Do you mind testing with an ad-hoc patch like this on top? It will fail\n> > > > right away, but should say COLUMNS = 80 in the output.\n> > > >\n> > > > The only thing I can think of right now is that some terminals are doing\n> > > > some evil trickery to LD_PRELOAD or whatever and intercept getenv() for\n> > > > COLUMNS and the like, but that's an entirely unfounded hunch.\n> > >\n> > > I'm able to reproduce this. The test fails when running directly with\n> > > bash, but not with prove.\n> > >\n> > > And it seems to be a bug in bash:\n> > >\n> > > export COLUMNS=80\n> > >\n> > > echo \"COLUMNS: $COLUMNS\"\n> > > cat > /tmp/expect < > > foobar\n> > > EOF\n> > > echo \"COLUMNS: $COLUMNS\"\n> > >\n> > > I get:\n> > >\n> > > COLUMNS: 80\n> > > COLUMNS: 115\n> > >\n> > > Even directly in the console.\n> >\n> > Hmm, from man bash:\n> >\n> > checkwinsize\n> > If set, bash checks the window size after each external (non‐builtin) com‐\n> > mand and, if necessary, updates the values of LINES and COLUMNS. This op‐\n> > tion is enabled by default.\n> >\n> > Seems like since bash 5.0 this is on by default.\n> \n> Indeed, running `shopt -u checkwinsize` right after exporting COLUMNS\n> in test-lib.sh fixes the tests. Great work!\n\nYeah, this fixes it, but it doesn't seem we are setting any\nbash-specific options right now, and additionally I don't think bash\nshould be doing that in the first place. If the shell is\nnon-interactive, why is checkwinsize being honored?\n\nMoreover, why does it work with prove? I'm investigating that right now,\nbut so far I haven't found any reason.\n\n-- \nFelipe Contreras"},{"id":"430718","messageId":"CAOLTT8T0oKQ2jCjcmjd6mfYeFFZESgCvV2iOLeUQ1b_byB2UZQ@mail.gmail.com","threadId":"56105","inReplyTo":"60f61b81a3b74_14781320816@natae.notmuch","subject":"Re: progress test failure on fedora34","fromName":"ZheNing Hu","fromEmail":"adlternative@gmail.com","sentAt":"2021-07-21T00:45:32Z","receivedAt":"2021-07-21T00:45:15Z","isPatch":false,"sender":{"key":"adlternative@gmail.com","avatar":"https://avatars.githubusercontent.com/u/58138461?v=4"},"body":"Hi, I met this bug too on my ArchLinux.\n\nFelipe Contreras 于2021年7月20日周二 上午9:04写道：\n\n>\n>\n> Yeah, this fixes it, but it doesn't seem we are setting any\n> bash-specific options right now, and additionally I don't think bash\n> should be doing that in the first place. If the shell is\n> non-interactive, why is checkwinsize being honored?\n>\n> Moreover, why does it work with prove? I'm investigating that right now,\n> but so far I haven't found any reason.\n>\n\nI ask this question on IRC #git, and ikke said that after bisecting,\nhe thought that\nthis bug was introduced in c49a177be.\n\n> --\n> Felipe Contreras\n\n--\nZheNing Hu\n"},{"id":"430732","messageId":"60f78b58dd8f7_14fda7208c5@natae.notmuch","threadId":"56105","inReplyTo":"CAOLTT8T0oKQ2jCjcmjd6mfYeFFZESgCvV2iOLeUQ1b_byB2UZQ@mail.gmail.com","subject":"Re: progress test failure on fedora34","fromName":"Felipe Contreras","fromEmail":"felipe.contreras@gmail.com","sentAt":"2021-07-21T02:50:00Z","receivedAt":"2021-07-21T02:50:09Z","isPatch":false,"sender":{"key":"felipe.contreras@gmail.com","avatar":"https://avatars.githubusercontent.com/u/8358?v=4"},"body":"ZheNing Hu wrote:\n> Hi, I met this bug too on my ArchLinux.\n> \n> Felipe Contreras 于2021年7月20日周二 上午9:04写道：\n> > Yeah, this fixes it, but it doesn't seem we are setting any\n> > bash-specific options right now, and additionally I don't think bash\n> > should be doing that in the first place. If the shell is\n> > non-interactive, why is checkwinsize being honored?\n> >\n> > Moreover, why does it work with prove? I'm investigating that right now,\n> > but so far I haven't found any reason.\n> \n> I ask this question on IRC #git, and ikke said that after bisecting,\n> he thought that\n> this bug was introduced in c49a177be.\n\nYes, this was already known. The root of this thread [1] mentions\n83ae1edff7, c49a177bec is the second parent of that comit.\n\nBut I don't think the bug is in git, it's in bash.\n\n[1] https://lore.kernel.org/git/49498ed0-cfd5-2305-cee7-5c5939a19bcf@campoint.net/\n\n-- \nFelipe Contreras"},{"id":"431262","messageId":"patch-1.1-f81f3911d5-20210726T235452Z-avarab@gmail.com","threadId":"56105","inReplyTo":"CAMMLpeT3bJcr7mRDpxmk32VqpAbNpN=fgPjmkcY+0zOBYruybQ@mail.gmail.com","subject":"[PATCH] test-lib.sh: use GIT_TEST_COLUMNS over COLUMNS","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-07-26T23:57:09Z","receivedAt":"2021-07-26T23:57:18Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"In my c49a177beca (test-lib.sh: set COLUMNS=80 for --verbose\nrepeatability, 2021-06-29) the test suite started breaking on recent\nversions of bash.\n\nThis is because it sets \"shopt -s checkwinsize\" starting with version\n5.0, furthermore it started setting COLUMNS under \"shopt -s\ncheckwinsize\" for non-interactive use around version 4.3.\n\nA narrow fix for that issue would be to add this just above our\nsetting of COLUMNS in test-lib.sh:\n\n\tshopt -u checkwinsize >/dev/null 2>&1\n\nBut we'd then be at the mercy of the next shell or terminal that wants\nto be clever about COLUMNS.\n\nLet's instead solve this more thoroughly. We'll now take\nGIT_TEST_COLUMNS over COLUMNS, and furthermore intentionally spoil the\nCOLUMNS variable to break any tests that rely on it being set to a\nsane value.\n\nIf something breaks because we have a codepath that's not\nterm_columns() checking COLUMNS we'd like to know about it, the narrow\n\"shopt -u checkwinsize\" fix won't give us that.\n\nThe \"shopt\" fix won't future-poof us against other 3rd party software\nchanges either. If that third-party software e.g. takes TIOCGWINSZ\nover columns on some platforms, our tests would be flaky and break\nthere even without this change.\n\nThis approach does mean that any tests of ours that expected to test\nterm_columns() behavior by setting COLUMNS will need to explicitly\nunset GIT_TEST_COLUMNS, or set it to the empty string. I'm doing the\nlatter in all the tests changed here.\n\nReported-by: Fabian Stelzer \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n\nI was able to reproduce the reported issue, turned out I just needed\nto run it with my /bin/bash on debian (it uses dash for /bin/sh by\ndefault).\n\nI wrote this on the 22nd, had a patch queued up, and thought I'd sent\nit, but evidently I did not. So here it is, finally.\n\n pager.c | 7 +++++++\n t/t3200-branch.sh | 8 ++++----\n t/t4052-stat-output.sh | 22 +++++++++++-----------\n t/t4205-log-pretty-formats.sh | 6 +++---\n t/t7004-tag.sh | 6 +++---\n t/t7006-pager.sh | 2 +-\n t/t7508-status.sh | 4 ++--\n t/t9002-column.sh | 23 ++++++++++-------------\n t/test-lib.sh | 13 +++++++++++--\n 9 files changed, 52 insertions(+), 39 deletions(-)\n\ndiff --git a/pager.c b/pager.c\nindex 52f27a6765..cfcc6dc04b 100644\n--- a/pager.c\n+++ b/pager.c\n@@ -165,6 +165,13 @@ int term_columns(void)\n \tterm_columns_at_startup = 80;\n \tterm_columns_guessed = 1;\n \n+\tcol_string = getenv(\"GIT_TEST_COLUMNS\");\n+\tif (col_string && (n_cols = atoi(col_string)) > 0) {\n+\t\tterm_columns_at_startup = n_cols;\n+\t\tterm_columns_guessed = 0;\n+\t\treturn term_columns_at_startup;\n+\t}\n+\n \tcol_string = getenv(\"COLUMNS\");\n \tif (col_string && (n_cols = atoi(col_string)) > 0) {\n \t\tterm_columns_at_startup = n_cols;\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex cc4b10236e..3e0b71a908 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -328,7 +328,7 @@ test_expect_success 'git branch --list -v with --abbrev' '\n '\n \n test_expect_success 'git branch --column' '\n-\tCOLUMNS=81 git branch --column=column >actual &&\n+\tGIT_TEST_COLUMNS= COLUMNS=81 git branch --column=column >actual &&\n \tcat >expect <<\\EOF &&\n a/b/c bam foo l * main n o/p r\n abc bar j/k m/m mb o/o q topic\n@@ -341,7 +341,7 @@ test_expect_success 'git branch --column with an extremely long branch name' '\n \tlong=z$long/$long/$long/$long &&\n \ttest_when_finished \"git branch -d $long\" &&\n \tgit branch $long &&\n-\tCOLUMNS=80 git branch --column=column >actual &&\n+\tGIT_TEST_COLUMNS= COLUMNS=80 git branch --column=column >actual &&\n \tcat >expect <actual &&\n+\tGIT_TEST_COLUMNS= COLUMNS=80 git branch >actual &&\n \tgit config --unset column.branch &&\n \tgit config --unset column.ui &&\n \tcat >expect <<\\EOF &&\n@@ -383,7 +383,7 @@ test_expect_success 'git branch --column -v should fail' '\n \n test_expect_success 'git branch -v with column.ui ignored' '\n \tgit config column.ui column &&\n-\tCOLUMNS=80 git branch -v | cut -c -8 | sed \"s/ *$//\" >actual &&\n+\tGIT_TEST_COLUMNS= COLUMNS=80 git branch -v | cut -c -8 | sed \"s/ *$//\" >actual &&\n \tgit config --unset column.ui &&\n \tcat >expect <<\\EOF &&\n a/b/c\ndiff --git a/t/t4052-stat-output.sh b/t/t4052-stat-output.sh\nindex 9eba436211..08f8615809 100755\n--- a/t/t4052-stat-output.sh\n+++ b/t/t4052-stat-output.sh\n@@ -111,7 +111,7 @@ cat >expect72 <<'EOF'\n abcd | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n EOF\n test_expect_success \"format-patch --cover-letter ignores COLUMNS (big change)\" '\n-\tCOLUMNS=200 git format-patch -1 --stdout --cover-letter >output &&\n+\tGIT_TEST_COLUMNS= COLUMNS=200 git format-patch -1 --stdout --cover-letter >output &&\n \tgrep \" | \" output >actual &&\n \ttest_cmp expect72 actual\n '\n@@ -131,7 +131,7 @@ EOF\n while read verb expect cmd args\n do\n \ttest_expect_success \"$cmd $verb COLUMNS (big change)\" '\n-\t\tCOLUMNS=200 git $cmd $args >output &&\n+\t\tGIT_TEST_COLUMNS= COLUMNS=200 git $cmd $args >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp \"$expect\" actual\n \t'\n@@ -139,7 +139,7 @@ do\n \tcase \"$cmd\" in diff|show) continue;; esac\n \n \ttest_expect_success \"$cmd --graph $verb COLUMNS (big change)\" '\n-\t\tCOLUMNS=200 git $cmd $args --graph >output &&\n+\t\tGIT_TEST_COLUMNS= COLUMNS=200 git $cmd $args --graph >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp \"$expect-graph\" actual\n \t'\n@@ -159,7 +159,7 @@ EOF\n while read verb expect cmd args\n do\n \ttest_expect_success \"$cmd $verb not enough COLUMNS (big change)\" '\n-\t\tCOLUMNS=40 git $cmd $args >output &&\n+\t\tGIT_TEST_COLUMNS= COLUMNS=40 git $cmd $args >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp \"$expect\" actual\n \t'\n@@ -167,7 +167,7 @@ do\n \tcase \"$cmd\" in diff|show) continue;; esac\n \n \ttest_expect_success \"$cmd --graph $verb not enough COLUMNS (big change)\" '\n-\t\tCOLUMNS=40 git $cmd $args --graph >output &&\n+\t\tGIT_TEST_COLUMNS= COLUMNS=40 git $cmd $args --graph >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp \"$expect-graph\" actual\n \t'\n@@ -302,7 +302,7 @@ EOF\n while read verb expect cmd args\n do\n \ttest_expect_success \"$cmd $verb COLUMNS (long filename)\" '\n-\t\tCOLUMNS=200 git $cmd $args >output &&\n+\t\tGIT_TEST_COLUMNS= COLUMNS=200 git $cmd $args >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp \"$expect\" actual\n \t'\n@@ -310,7 +310,7 @@ do\n \tcase \"$cmd\" in diff|show) continue;; esac\n \n \ttest_expect_success \"$cmd --graph $verb COLUMNS (long filename)\" '\n-\t\tCOLUMNS=200 git $cmd $args --graph >output &&\n+\t\tGIT_TEST_COLUMNS= COLUMNS=200 git $cmd $args --graph >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp \"$expect-graph\" actual\n \t'\n@@ -331,7 +331,7 @@ while read verb expect cmd args\n do\n \ttest_expect_success COLUMNS_CAN_BE_1 \\\n \t\t\"$cmd $verb prefix greater than COLUMNS (big change)\" '\n-\t\tCOLUMNS=1 git $cmd $args >output &&\n+\t\tGIT_TEST_COLUMNS= COLUMNS=1 git $cmd $args >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp \"$expect\" actual\n \t'\n@@ -340,7 +340,7 @@ do\n \n \ttest_expect_success COLUMNS_CAN_BE_1 \\\n \t\t\"$cmd --graph $verb prefix greater than COLUMNS (big change)\" '\n-\t\tCOLUMNS=1 git $cmd $args --graph >output &&\n+\t\tGIT_TEST_COLUMNS= COLUMNS=1 git $cmd $args --graph >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp \"$expect-graph\" actual\n \t'\n@@ -356,7 +356,7 @@ cat >expect <<'EOF'\n EOF\n test_expect_success 'merge --stat respects COLUMNS (big change)' '\n \tgit checkout -b branch HEAD^^ &&\n-\tCOLUMNS=100 git merge --stat --no-ff main^ >output &&\n+\tGIT_TEST_COLUMNS= COLUMNS=100 git merge --stat --no-ff main^ >output &&\n \tgrep \" | \" output >actual &&\n \ttest_cmp expect actual\n '\n@@ -365,7 +365,7 @@ cat >expect <<'EOF'\n aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 +++++++++++++++++++++++++++++++++++++++\n EOF\n test_expect_success 'merge --stat respects COLUMNS (long filename)' '\n-\tCOLUMNS=100 git merge --stat --no-ff main >output &&\n+\tGIT_TEST_COLUMNS= COLUMNS=100 git merge --stat --no-ff main >output &&\n \tgrep \" | \" output >actual &&\n \ttest_cmp expect actual\n '\ndiff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh\nindex 5865daa8f8..6c8e1b3f1a 100755\n--- a/t/t4205-log-pretty-formats.sh\n+++ b/t/t4205-log-pretty-formats.sh\n@@ -207,7 +207,7 @@ test_expect_success 'left alignment formatting at the nth column' '\n '\n \n test_expect_success 'left alignment formatting at the nth column' '\n-\tCOLUMNS=50 git log --pretty=\"tformat:%h %<|(-10)%s\" >actual &&\n+\tGIT_TEST_COLUMNS= COLUMNS=50 git log --pretty=\"tformat:%h %<|(-10)%s\" >actual &&\n \tqz_to_tab_space <<-EOF >expected &&\n \t$head1 message two Z\n \t$head2 message one Z\n@@ -350,7 +350,7 @@ test_expect_success 'right alignment formatting at the nth column' '\n '\n \n test_expect_success 'right alignment formatting at the nth column' '\n-\tCOLUMNS=50 git log --pretty=\"tformat:%h %>|(-10)%s\" >actual &&\n+\tGIT_TEST_COLUMNS= COLUMNS=50 git log --pretty=\"tformat:%h %>|(-10)%s\" >actual &&\n \tqz_to_tab_space <<-EOF >expected &&\n \t$head1 message two\n \t$head2 message one\n@@ -450,7 +450,7 @@ test_expect_success 'center alignment formatting at the nth column' '\n '\n \n test_expect_success 'center alignment formatting at the nth column' '\n-\tCOLUMNS=70 git log --pretty=\"tformat:%h %><|(-30)%s\" >actual &&\n+\tGIT_TEST_COLUMNS= COLUMNS=70 git log --pretty=\"tformat:%h %><|(-30)%s\" >actual &&\n \tqz_to_tab_space <<-EOF >expected &&\n \t$head1 message two Z\n \t$head2 message one Z\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex 2f72c5c688..1c3d8cfd16 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -372,7 +372,7 @@ test_expect_success 'tag -l -l works, as our buggy documenta\n '\n \n test_expect_success 'listing tags in column' '\n-\tCOLUMNS=41 git tag -l --column=row >actual &&\n+\tGIT_TEST_COLUMNS= COLUMNS=41 git tag -l --column=row >actual &&\n \tcat >expected <<\\EOF &&\n a1 aa1 cba t210 t211\n v0.2.1 v1.0 v1.0.1 v1.1.3\n@@ -383,7 +383,7 @@ EOF\n test_expect_success 'listing tags in column with column.*' '\n \ttest_config column.tag row &&\n \ttest_config column.ui dense &&\n-\tCOLUMNS=40 git tag -l >actual &&\n+\tGIT_TEST_COLUMNS= COLUMNS=40 git tag -l >actual &&\n \tcat >expected <<\\EOF &&\n a1 aa1 cba t210 t211\n v0.2.1 v1.0 v1.0.1 v1.1.3\n@@ -397,7 +397,7 @@ test_expect_success 'listing tag with -n --column should fail' '\n \n test_expect_success 'listing tags -n in column with column.ui ignored' '\n \ttest_config column.ui \"row dense\" &&\n-\tCOLUMNS=40 git tag -l -n >actual &&\n+\tGIT_TEST_COLUMNS= COLUMNS=40 git tag -l -n >actual &&\n \tcat >expected <<\\EOF &&\n a1 Foo\n aa1 Foo\ndiff --git a/t/t7006-pager.sh b/t/t7006-pager.sh\nindex 0e7cf75435..1b116366a3 100755\n--- a/t/t7006-pager.sh\n+++ b/t/t7006-pager.sh\n@@ -651,7 +651,7 @@ test_expect_success TTY 'git tag with auto-columns ' '\n \tcat >expect <<-\\EOF &&\n \tinitial one two three four five\n \tEOF\n-\ttest_terminal env PAGER=\"cat >actual\" COLUMNS=80 \\\n+\ttest_terminal env PAGER=\"cat >actual\" GIT_TEST_COLUMNS= COLUMNS=80 \\\n \t\tgit -c column.ui=auto tag --sort=authordate &&\n \ttest_cmp expect actual\n '\ndiff --git a/t/t7508-status.sh b/t/t7508-status.sh\nindex 2b72451ba3..669a3c7150 100755\n--- a/t/t7508-status.sh\n+++ b/t/t7508-status.sh\n@@ -108,13 +108,13 @@ test_expect_success 'status --column' '\n #\tdir2/modified untracked\n #\n EOF\n-\tCOLUMNS=50 git -c status.displayCommentPrefix=true status --column=\"column dense\" >output &&\n+\tGIT_TEST_COLUMNS= COLUMNS=50 git -c status.displayCommentPrefix=true status --column=\"column dense\" >output &&\n \ttest_cmp expect output\n '\n \n test_expect_success 'status --column status.displayCommentPrefix=false' '\n \tstrip_comments expect &&\n-\tCOLUMNS=49 git -c status.displayCommentPrefix=false status --column=\"column dense\" >output &&\n+\tGIT_TEST_COLUMNS= COLUMNS=49 git -c status.displayCommentPrefix=false status --column=\"column dense\" >output &&\n \ttest_cmp expect output\n '\n \ndiff --git a/t/t9002-column.sh b/t/t9002-column.sh\nindex 89983527b6..50cf3e7b42 100755\n--- a/t/t9002-column.sh\n+++ b/t/t9002-column.sh\n@@ -46,7 +46,7 @@ test_expect_success '80 columns' '\n \tcat >expected <<\\EOF &&\n one two three four five six seven eight nine ten eleven\n EOF\n-\tCOLUMNS=80 git column --mode=column actual &&\n+\tGIT_TEST_COLUMNS= COLUMNS=80 git column --mode=column actual &&\n \ttest_cmp expected actual\n '\n \n@@ -65,7 +65,7 @@ eleven\n EOF\n \n test_expect_success COLUMNS_CAN_BE_1 'COLUMNS = 1' '\n-\tCOLUMNS=1 git column --mode=column actual &&\n+\tGIT_TEST_COLUMNS= COLUMNS=1 git column --mode=column actual &&\n \ttest_cmp expected actual\n '\n \n@@ -74,9 +74,6 @@ test_expect_success 'width = 1' '\n \ttest_cmp expected actual\n '\n \n-COLUMNS=20\n-export COLUMNS\n-\n test_expect_success '20 columns' '\n \tcat >expected <<\\EOF &&\n one seven\n@@ -86,7 +83,7 @@ four ten\n five eleven\n six\n EOF\n-\tgit column --mode=column actual &&\n+\tGIT_TEST_COLUMNS= COLUMNS=20 git column --mode=column actual &&\n \ttest_cmp expected actual\n '\n \n@@ -99,7 +96,7 @@ four ten\n five eleven\n six\n EOF\n-\tgit column --mode=column,nodense < lista > actual &&\n+\tGIT_TEST_COLUMNS= COLUMNS=20 git column --mode=column,nodense < lista > actual &&\n \ttest_cmp expected actual\n '\n \n@@ -110,7 +107,7 @@ two six ten\n three seven eleven\n four eight\n EOF\n-\tgit column --mode=column,dense < lista > actual &&\n+\tGIT_TEST_COLUMNS= COLUMNS=20 git column --mode=column,dense < lista > actual &&\n \ttest_cmp expected actual\n '\n \n@@ -123,7 +120,7 @@ four ten\n five eleven\n six\n EOF\n-\tgit column --mode=column --padding 2 actual &&\n+\tGIT_TEST_COLUMNS= COLUMNS=20 git column --mode=column --padding 2 actual &&\n \ttest_cmp expected actual\n '\n \n@@ -136,7 +133,7 @@ test_expect_success '20 columns, indented' '\n five eleven\n six\n EOF\n-\tgit column --mode=column --indent=\" \" actual &&\n+\tGIT_TEST_COLUMNS= COLUMNS=20 git column --mode=column --indent=\" \" actual &&\n \ttest_cmp expected actual\n '\n \n@@ -149,7 +146,7 @@ seven eight\n nine ten\n eleven\n EOF\n-\tgit column --mode=row actual &&\n+\tGIT_TEST_COLUMNS= COLUMNS=20 git column --mode=row actual &&\n \ttest_cmp expected actual\n '\n \n@@ -162,7 +159,7 @@ seven eight\n nine ten\n eleven\n EOF\n-\tgit column --mode=row,nodense actual &&\n+\tGIT_TEST_COLUMNS= COLUMNS=20 git column --mode=row,nodense actual &&\n \ttest_cmp expected actual\n '\n \n@@ -173,7 +170,7 @@ four five six\n seven eight nine\n ten eleven\n EOF\n-\tgit column --mode=row,dense actual &&\n+\tGIT_TEST_COLUMNS= COLUMNS=20 git column --mode=row,dense actual &&\n \ttest_cmp expected actual\n '\n \ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 9e26860544..82771643ba 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -406,10 +406,19 @@ LANG=C\n LC_ALL=C\n PAGER=cat\n TZ=UTC\n-COLUMNS=80\n-export LANG LC_ALL PAGER TZ COLUMNS\n+export LANG LC_ALL PAGER TZ\n EDITOR=:\n \n+# For repeatability we need to set term_columns()'s idea of\n+# columns. We do this via GIT_TEST_COLUMNS and not COLUMNS because\n+# e.g. versions of bash >= 5.0 have \"shopt -s checkwinsize\" on by\n+# default. We could do \"shopt -u checkwinsize >/dev/null 2>&1\" here to\n+# fix that particular issue, but this is not shell specific, and\n+# future-proof the tests.\n+GIT_TEST_COLUMNS=80\n+COLUMNS=10\n+export GIT_TEST_COLUMNS COLUMNS\n+\n # A call to \"unset\" with no arguments causes at least Solaris 10\n # /usr/xpg4/bin/sh and /bin/ksh to bail out. So keep the unsets\n # deriving from the command substitution clustered with the other\n-- \n2.32.0.988.g1a6a4b2c5f\n\n"},{"id":"431324","messageId":"YQBEhj/Y6m0pqOth@coredump.intra.peff.net","threadId":"56105","inReplyTo":"patch-1.1-f81f3911d5-20210726T235452Z-avarab@gmail.com","subject":"Re: [PATCH] test-lib.sh: use GIT_TEST_COLUMNS over COLUMNS","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-07-27T17:38:14Z","receivedAt":"2021-07-27T17:38:20Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Jul 27, 2021 at 01:57:09AM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> In my c49a177beca (test-lib.sh: set COLUMNS=80 for --verbose\n> repeatability, 2021-06-29) the test suite started breaking on recent\n> versions of bash.\n> \n> This is because it sets \"shopt -s checkwinsize\" starting with version\n> 5.0, furthermore it started setting COLUMNS under \"shopt -s\n> checkwinsize\" for non-interactive use around version 4.3.\n> \n> A narrow fix for that issue would be to add this just above our\n> setting of COLUMNS in test-lib.sh:\n> \n> \tshopt -u checkwinsize >/dev/null 2>&1\n> \n> But we'd then be at the mercy of the next shell or terminal that wants\n> to be clever about COLUMNS.\n> \n> Let's instead solve this more thoroughly. We'll now take\n> GIT_TEST_COLUMNS over COLUMNS, and furthermore intentionally spoil the\n> COLUMNS variable to break any tests that rely on it being set to a\n> sane value.\n> \n> If something breaks because we have a codepath that's not\n> term_columns() checking COLUMNS we'd like to know about it, the narrow\n> \"shopt -u checkwinsize\" fix won't give us that.\n\nI guess people running with bash won't see the test breakage (because\nbash will quietly \"fix\" the COLUMNS setting). But enough people run with\n/bin/sh that we'll eventually notice.\n\n> This approach does mean that any tests of ours that expected to test\n> term_columns() behavior by setting COLUMNS will need to explicitly\n> unset GIT_TEST_COLUMNS, or set it to the empty string. I'm doing the\n> latter in all the tests changed here.\n\nThis is rather ugly, and I'm not in general a fan of adding more\ntest-only code into the bowels of Git itself. But it may be the least\nbad solution.\n\nThe only alternative I could come up with is _not_ to set COLUMNS\neverywhere, but instead insist on each individual test manually doing\n\"COLUMNS=80 git ...\". Setting it as a one-shot seems to be reliable\nenough. The downside is just that various tests will need to do so. We\nalready have quite a few that do, but I guess anything that uses the\nprogress meter is now subject to it.\n\n-Peff\n"},{"id":"431373","messageId":"xmqqzgu7b6of.fsf@gitster.g","threadId":"56105","inReplyTo":"YQBEhj/Y6m0pqOth@coredump.intra.peff.net","subject":"Re: [PATCH] test-lib.sh: use GIT_TEST_COLUMNS over COLUMNS","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-07-28T00:53:04Z","receivedAt":"2021-07-28T00:53:10Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n>> Let's instead solve this more thoroughly. We'll now take\n>> GIT_TEST_COLUMNS over COLUMNS, and furthermore intentionally spoil the\n>> COLUMNS variable to break any tests that rely on it being set to a\n>> sane value.\n>> \n>> If something breaks because we have a codepath that's not\n>> term_columns() checking COLUMNS we'd like to know about it, the narrow\n>> \"shopt -u checkwinsize\" fix won't give us that.\n>\n> I guess people running with bash won't see the test breakage (because\n> bash will quietly \"fix\" the COLUMNS setting). But enough people run with\n> /bin/sh that we'll eventually notice.\n>\n>> This approach does mean that any tests of ours that expected to test\n>> term_columns() behavior by setting COLUMNS will need to explicitly\n>> unset GIT_TEST_COLUMNS, or set it to the empty string. I'm doing the\n>> latter in all the tests changed here.\n>\n> This is rather ugly, and I'm not in general a fan of adding more\n> test-only code into the bowels of Git itself. But it may be the least\n> bad solution.\n\nYeah, this really look unsatisfactory, especially with this repeated\npattern that is hard to read:\n\n+\tGIT_TEST_COLUMNS= COLUMNS=81 git branch --column=column >actual &&\n+\tGIT_TEST_COLUMNS= COLUMNS=80 git branch --column=column >actual &&\n+\tGIT_TEST_COLUMNS= COLUMNS=80 git branch >actual &&\n+\tGIT_TEST_COLUMNS= COLUMNS=80 git branch -v | cut -c -8 | sed \"s/ *$//\" >actual &&\n\nPerhaps with something like\n\n\ttest_with_columns () {\n \tlocal columns=$1\n\t\tshift\n\t\tGIT_TEST_COLUMNS= COLUMNS=$columns \"$@\"\n\t}\n\nwe may be able to hide the ugly implementation detail like this:\n\n\ttest_with_columns 81 git branch --column=column >actual\n\nand may become a bit more palatable? A good thing is that this can\nbe done as two-step process, with its first step being \n\n s/^(\\s*)COLUMNS=(\\d+)/$1test_with_columns $2/;\n\nplus addition of the helper to test-lib, perhaps like so:\n\n\ttest_with_columns () {\n \tlocal columns=$1\n\t\tshift\n\t\tCOLUMNS=$columns \"$@\"\n\t}\n\nand the whole GIT_TEST_COLUMNS stuff being the second step.\n"},{"id":"431689","messageId":"patch-v2-1.3-739457b992f-20210802T134610Z-avarab@gmail.com","threadId":"56105","inReplyTo":"cover-v2-0.3-00000000000-20210802T134610Z-avarab@gmail.com","subject":"[PATCH v2 1/3] test-lib-functions.sh: rename test_must_fail_acceptable()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-08-02T13:46:26Z","receivedAt":"2021-08-02T13:46:42Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The test_must_fail_acceptable() is really a generic function that can\ncheck if something is a real \"git command\", e.g. \"git\", \"test-tool\"\netc. Let's rename it in preparation for using it in another test\nfunction.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/test-lib-functions.sh | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex e28411bb75a..37da7d9a99a 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -895,7 +895,7 @@ list_contains () {\n # accepted by test_must_fail(). If the command is run with env, the env\n # and its corresponding variable settings will be stripped before we\n # test the command being run.\n-test_must_fail_acceptable () {\n+is_git_command_name () {\n \tif test \"$1\" = \"env\"\n \tthen\n \t\tshift\n@@ -943,7 +943,7 @@ test_must_fail_acceptable () {\n # (Don't use 'success', use 'test_might_fail' instead.)\n #\n # Do not use this to run anything but \"git\" and other specific testable\n-# commands (see test_must_fail_acceptable()). We are not in the\n+# commands (see is_git_command_name()). We are not in the\n # business of vetting system supplied commands -- in other words, this\n # is wrong:\n #\n@@ -963,7 +963,7 @@ test_must_fail () {\n \t\t_test_ok=\n \t\t;;\n \tesac\n-\tif ! test_must_fail_acceptable \"$@\"\n+\tif ! is_git_command_name \"$@\"\n \tthen\n \t\techo >&7 \"test_must_fail: only 'git' is allowed: $*\"\n \t\treturn 1\n-- \n2.32.0.1070.gec115ccd780\n\n"},{"id":"431690","messageId":"patch-v2-3.3-6cbbb955e9a-20210802T134610Z-avarab@gmail.com","threadId":"56105","inReplyTo":"cover-v2-0.3-00000000000-20210802T134610Z-avarab@gmail.com","subject":"[PATCH v2 3/3] test-lib.sh: use GIT_TEST_COLUMNS over COLUMNS","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-08-02T13:46:28Z","receivedAt":"2021-08-02T13:46:46Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"In my c49a177beca (test-lib.sh: set COLUMNS=80 for --verbose\nrepeatability, 2021-06-29) the test suite started breaking on recent\nversions of bash.\n\nThis is because it sets \"shopt -s checkwinsize\" starting with version\n5.0, furthermore it started setting COLUMNS under \"shopt -s\ncheckwinsize\" for non-interactive use around version 4.3.\n\nA narrow fix for that issue would be to add this just above our\nsetting of COLUMNS in test-lib.sh:\n\n\tshopt -u checkwinsize >/dev/null 2>&1\n\nBut we'd then be at the mercy of the next shell or terminal that wants\nto be clever about COLUMNS.\n\nLet's instead solve this more thoroughly. We'll now take\nGIT_TEST_COLUMNS over COLUMNS, and furthermore intentionally spoil the\nCOLUMNS variable to break any tests that rely on it being set to a\nsane value.\n\nIf something breaks because we have a codepath that's not\nterm_columns() checking COLUMNS we'd like to know about it, the narrow\n\"shopt -u checkwinsize\" fix won't give us that.\n\nThe \"shopt\" fix won't future-poof us against other 3rd party software\nchanges either. If that third-party software e.g. takes TIOCGWINSZ\nover columns on some platforms, our tests would be flaky and break\nthere even without this change.\n\nThis approach does mean that any tests of ours that expected to test\nterm_columns() behavior by setting COLUMNS will need to explicitly\nunset GIT_TEST_COLUMNS, or set it to the empty string. Let's do that\nin the new test_with_columns() helper, which we previously changed all\nthe tests that set COLUMNS to use.\n\nReported-by: Fabian Stelzer \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n pager.c | 7 +++++++\n t/test-lib-functions.sh | 7 +++++++\n t/test-lib.sh | 13 +++++++++++--\n 3 files changed, 25 insertions(+), 2 deletions(-)\n\ndiff --git a/pager.c b/pager.c\nindex 52f27a6765c..cfcc6dc04bd 100644\n--- a/pager.c\n+++ b/pager.c\n@@ -165,6 +165,13 @@ int term_columns(void)\n \tterm_columns_at_startup = 80;\n \tterm_columns_guessed = 1;\n \n+\tcol_string = getenv(\"GIT_TEST_COLUMNS\");\n+\tif (col_string && (n_cols = atoi(col_string)) > 0) {\n+\t\tterm_columns_at_startup = n_cols;\n+\t\tterm_columns_guessed = 0;\n+\t\treturn term_columns_at_startup;\n+\t}\n+\n \tcol_string = getenv(\"COLUMNS\");\n \tif (col_string && (n_cols = atoi(col_string)) > 0) {\n \t\tterm_columns_at_startup = n_cols;\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 536339faaa2..959354e0c6e 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -1724,5 +1724,12 @@ test_with_columns () {\n \tlocal columns=$1\n \tshift\n \n+\tif ! is_git_command_name \"$@\"\n+\tthen\n+\t\techo >&7 \"test_with_columns: only 'git' is allowed: $*\"\n+\t\treturn 1\n+\tfi\n+\n+\tGIT_TEST_COLUMNS= \\\n \tCOLUMNS=$columns \"$@\"\n }\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex da13190970f..4bb231e5729 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -415,10 +415,19 @@ LANG=C\n LC_ALL=C\n PAGER=cat\n TZ=UTC\n-COLUMNS=80\n-export LANG LC_ALL PAGER TZ COLUMNS\n+export LANG LC_ALL PAGER TZ\n EDITOR=:\n \n+# For repeatability we need to set term_columns()'s idea of\n+# columns. We do this via GIT_TEST_COLUMNS and not COLUMNS because\n+# e.g. versions of bash >= 5.0 have \"shopt -s checkwinsize\" on by\n+# default. We could do \"shopt -u checkwinsize >/dev/null 2>&1\" here to\n+# fix that particular issue, but this is not shell specific, and\n+# future-proof the tests.\n+GIT_TEST_COLUMNS=80\n+COLUMNS=10\n+export GIT_TEST_COLUMNS COLUMNS\n+\n # A call to \"unset\" with no arguments causes at least Solaris 10\n # /usr/xpg4/bin/sh and /bin/ksh to bail out. So keep the unsets\n # deriving from the command substitution clustered with the other\n-- \n2.32.0.1070.gec115ccd780\n\n"},{"id":"431691","messageId":"cover-v2-0.3-00000000000-20210802T134610Z-avarab@gmail.com","threadId":"56105","inReplyTo":"patch-1.1-f81f3911d5-20210726T235452Z-avarab@gmail.com","subject":"[PATCH v2 0/3] test-lib.sh: use GIT_TEST_COLUMNS over COLUMNS","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-08-02T13:46:25Z","receivedAt":"2021-08-02T13:46:52Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"A v2 which should address the comments on v1, the large search-replace\nof existing COLUMN uses in the tests is now a separate step.\n\nThe below range-diff is rather confusing because the v1 1/1 is now\nmostly attributed to this new split-out commit, as a result it looks\nlike most of the commit message is being deleted, but it's in fact\nthere mostly in 3/3. I just re-worded the last paragraph a bit.\n\nÆvar Arnfjörð Bjarmason (3):\n test-lib-functions.sh: rename test_must_fail_acceptable()\n test-lib-functions.sh: add a test_with_columns function\n test-lib.sh: use GIT_TEST_COLUMNS over COLUMNS\n\n pager.c | 7 +++++++\n t/t3200-branch.sh | 8 ++++----\n t/t4052-stat-output.sh | 22 +++++++++++-----------\n t/t4205-log-pretty-formats.sh | 6 +++---\n t/t7004-tag.sh | 6 +++---\n t/t7006-pager.sh | 2 +-\n t/t7508-status.sh | 4 ++--\n t/t9002-column.sh | 23 ++++++++++-------------\n t/test-lib-functions.sh | 21 ++++++++++++++++++---\n t/test-lib.sh | 13 +++++++++++--\n 10 files changed, 70 insertions(+), 42 deletions(-)\n\nRange-diff against v1:\n-: ----------- > 1: 739457b992f test-lib-functions.sh: rename test_must_fail_acceptable()\n1: f81f3911d52 ! 2: 36c57178c55 test-lib.sh: use GIT_TEST_COLUMNS over COLUMNS\n @@ Metadata\n Author: Ævar Arnfjörð Bjarmason \n \n ## Commit message ##\n - test-lib.sh: use GIT_TEST_COLUMNS over COLUMNS\n + test-lib-functions.sh: add a test_with_columns function\n \n - In my c49a177beca (test-lib.sh: set COLUMNS=80 for --verbose\n - repeatability, 2021-06-29) the test suite started breaking on recent\n - versions of bash.\n + Add a helper function to wrap patterns of \"COLUMNS=N \" as\n + \"test_with_columns N \". This will be used and extended in the\n + next commit.\n \n - This is because it sets \"shopt -s checkwinsize\" starting with version\n - 5.0, furthermore it started setting COLUMNS under \"shopt -s\n - checkwinsize\" for non-interactive use around version 4.3.\n -\n - A narrow fix for that issue would be to add this just above our\n - setting of COLUMNS in test-lib.sh:\n -\n - shopt -u checkwinsize >/dev/null 2>&1\n -\n - But we'd then be at the mercy of the next shell or terminal that wants\n - to be clever about COLUMNS.\n -\n - Let's instead solve this more thoroughly. We'll now take\n - GIT_TEST_COLUMNS over COLUMNS, and furthermore intentionally spoil the\n - COLUMNS variable to break any tests that rely on it being set to a\n - sane value.\n -\n - If something breaks because we have a codepath that's not\n - term_columns() checking COLUMNS we'd like to know about it, the narrow\n - \"shopt -u checkwinsize\" fix won't give us that.\n -\n - The \"shopt\" fix won't future-poof us against other 3rd party software\n - changes either. If that third-party software e.g. takes TIOCGWINSZ\n - over columns on some platforms, our tests would be flaky and break\n - there even without this change.\n -\n - This approach does mean that any tests of ours that expected to test\n - term_columns() behavior by setting COLUMNS will need to explicitly\n - unset GIT_TEST_COLUMNS, or set it to the empty string. I'm doing the\n - latter in all the tests changed here.\n -\n - Reported-by: Fabian Stelzer \n Signed-off-by: Ævar Arnfjörð Bjarmason \n \n - ## pager.c ##\n -@@ pager.c: int term_columns(void)\n - \tterm_columns_at_startup = 80;\n - \tterm_columns_guessed = 1;\n - \n -+\tcol_string = getenv(\"GIT_TEST_COLUMNS\");\n -+\tif (col_string && (n_cols = atoi(col_string)) > 0) {\n -+\t\tterm_columns_at_startup = n_cols;\n -+\t\tterm_columns_guessed = 0;\n -+\t\treturn term_columns_at_startup;\n -+\t}\n -+\n - \tcol_string = getenv(\"COLUMNS\");\n - \tif (col_string && (n_cols = atoi(col_string)) > 0) {\n - \t\tterm_columns_at_startup = n_cols;\n -\n ## t/t3200-branch.sh ##\n @@ t/t3200-branch.sh: test_expect_success 'git branch --list -v with --abbrev' '\n '\n \n test_expect_success 'git branch --column' '\n -\tCOLUMNS=81 git branch --column=column >actual &&\n -+\tGIT_TEST_COLUMNS= COLUMNS=81 git branch --column=column >actual &&\n ++\ttest_with_columns 81 git branch --column=column >actual &&\n \tcat >expect <<\\EOF &&\n a/b/c bam foo l * main n o/p r\n abc bar j/k m/m mb o/o q topic\n @@ t/t3200-branch.sh: test_expect_success 'git branch --column with an extremely lo\n \ttest_when_finished \"git branch -d $long\" &&\n \tgit branch $long &&\n -\tCOLUMNS=80 git branch --column=column >actual &&\n -+\tGIT_TEST_COLUMNS= COLUMNS=80 git branch --column=column >actual &&\n ++\ttest_with_columns 80 git branch --column=column >actual &&\n \tcat >expect <actual &&\n -+\tGIT_TEST_COLUMNS= COLUMNS=80 git branch >actual &&\n ++\ttest_with_columns 80 git branch >actual &&\n \tgit config --unset column.branch &&\n \tgit config --unset column.ui &&\n \tcat >expect <<\\EOF &&\n @@ t/t3200-branch.sh: test_expect_success 'git branch --column -v should fail' '\n test_expect_success 'git branch -v with column.ui ignored' '\n \tgit config column.ui column &&\n -\tCOLUMNS=80 git branch -v | cut -c -8 | sed \"s/ *$//\" >actual &&\n -+\tGIT_TEST_COLUMNS= COLUMNS=80 git branch -v | cut -c -8 | sed \"s/ *$//\" >actual &&\n ++\ttest_with_columns 80 git branch -v | cut -c -8 | sed \"s/ *$//\" >actual &&\n \tgit config --unset column.ui &&\n \tcat >expect <<\\EOF &&\n a/b/c\n @@ t/t4052-stat-output.sh: cat >expect72 <<'EOF'\n EOF\n test_expect_success \"format-patch --cover-letter ignores COLUMNS (big change)\" '\n -\tCOLUMNS=200 git format-patch -1 --stdout --cover-letter >output &&\n -+\tGIT_TEST_COLUMNS= COLUMNS=200 git format-patch -1 --stdout --cover-letter >output &&\n ++\ttest_with_columns 200 git format-patch -1 --stdout --cover-letter >output &&\n \tgrep \" | \" output >actual &&\n \ttest_cmp expect72 actual\n '\n @@ t/t4052-stat-output.sh: EOF\n do\n \ttest_expect_success \"$cmd $verb COLUMNS (big change)\" '\n -\t\tCOLUMNS=200 git $cmd $args >output &&\n -+\t\tGIT_TEST_COLUMNS= COLUMNS=200 git $cmd $args >output &&\n ++\t\ttest_with_columns 200 git $cmd $args >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp \"$expect\" actual\n \t'\n @@ t/t4052-stat-output.sh: do\n \n \ttest_expect_success \"$cmd --graph $verb COLUMNS (big change)\" '\n -\t\tCOLUMNS=200 git $cmd $args --graph >output &&\n -+\t\tGIT_TEST_COLUMNS= COLUMNS=200 git $cmd $args --graph >output &&\n ++\t\ttest_with_columns 200 git $cmd $args --graph >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp \"$expect-graph\" actual\n \t'\n @@ t/t4052-stat-output.sh: EOF\n do\n \ttest_expect_success \"$cmd $verb not enough COLUMNS (big change)\" '\n -\t\tCOLUMNS=40 git $cmd $args >output &&\n -+\t\tGIT_TEST_COLUMNS= COLUMNS=40 git $cmd $args >output &&\n ++\t\ttest_with_columns 40 git $cmd $args >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp \"$expect\" actual\n \t'\n @@ t/t4052-stat-output.sh: do\n \n \ttest_expect_success \"$cmd --graph $verb not enough COLUMNS (big change)\" '\n -\t\tCOLUMNS=40 git $cmd $args --graph >output &&\n -+\t\tGIT_TEST_COLUMNS= COLUMNS=40 git $cmd $args --graph >output &&\n ++\t\ttest_with_columns 40 git $cmd $args --graph >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp \"$expect-graph\" actual\n \t'\n @@ t/t4052-stat-output.sh: EOF\n do\n \ttest_expect_success \"$cmd $verb COLUMNS (long filename)\" '\n -\t\tCOLUMNS=200 git $cmd $args >output &&\n -+\t\tGIT_TEST_COLUMNS= COLUMNS=200 git $cmd $args >output &&\n ++\t\ttest_with_columns 200 git $cmd $args >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp \"$expect\" actual\n \t'\n @@ t/t4052-stat-output.sh: do\n \n \ttest_expect_success \"$cmd --graph $verb COLUMNS (long filename)\" '\n -\t\tCOLUMNS=200 git $cmd $args --graph >output &&\n -+\t\tGIT_TEST_COLUMNS= COLUMNS=200 git $cmd $args --graph >output &&\n ++\t\ttest_with_columns 200 git $cmd $args --graph >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp \"$expect-graph\" actual\n \t'\n @@ t/t4052-stat-output.sh: while read verb expect cmd args\n \ttest_expect_success COLUMNS_CAN_BE_1 \\\n \t\t\"$cmd $verb prefix greater than COLUMNS (big change)\" '\n -\t\tCOLUMNS=1 git $cmd $args >output &&\n -+\t\tGIT_TEST_COLUMNS= COLUMNS=1 git $cmd $args >output &&\n ++\t\ttest_with_columns 1 git $cmd $args >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp \"$expect\" actual\n \t'\n @@ t/t4052-stat-output.sh: do\n \ttest_expect_success COLUMNS_CAN_BE_1 \\\n \t\t\"$cmd --graph $verb prefix greater than COLUMNS (big change)\" '\n -\t\tCOLUMNS=1 git $cmd $args --graph >output &&\n -+\t\tGIT_TEST_COLUMNS= COLUMNS=1 git $cmd $args --graph >output &&\n ++\t\ttest_with_columns 1 git $cmd $args --graph >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp \"$expect-graph\" actual\n \t'\n @@ t/t4052-stat-output.sh: cat >expect <<'EOF'\n test_expect_success 'merge --stat respects COLUMNS (big change)' '\n \tgit checkout -b branch HEAD^^ &&\n -\tCOLUMNS=100 git merge --stat --no-ff main^ >output &&\n -+\tGIT_TEST_COLUMNS= COLUMNS=100 git merge --stat --no-ff main^ >output &&\n ++\ttest_with_columns 100 git merge --stat --no-ff main^ >output &&\n \tgrep \" | \" output >actual &&\n \ttest_cmp expect actual\n '\n @@ t/t4052-stat-output.sh: cat >expect <<'EOF'\n EOF\n test_expect_success 'merge --stat respects COLUMNS (long filename)' '\n -\tCOLUMNS=100 git merge --stat --no-ff main >output &&\n -+\tGIT_TEST_COLUMNS= COLUMNS=100 git merge --stat --no-ff main >output &&\n ++\ttest_with_columns 100 git merge --stat --no-ff main >output &&\n \tgrep \" | \" output >actual &&\n \ttest_cmp expect actual\n '\n @@ t/t4205-log-pretty-formats.sh: test_expect_success 'left alignment formatting at\n \n test_expect_success 'left alignment formatting at the nth column' '\n -\tCOLUMNS=50 git log --pretty=\"tformat:%h %<|(-10)%s\" >actual &&\n -+\tGIT_TEST_COLUMNS= COLUMNS=50 git log --pretty=\"tformat:%h %<|(-10)%s\" >actual &&\n ++\ttest_with_columns 50 git log --pretty=\"tformat:%h %<|(-10)%s\" >actual &&\n \tqz_to_tab_space <<-EOF >expected &&\n \t$head1 message two Z\n \t$head2 message one Z\n @@ t/t4205-log-pretty-formats.sh: test_expect_success 'right alignment formatting a\n \n test_expect_success 'right alignment formatting at the nth column' '\n -\tCOLUMNS=50 git log --pretty=\"tformat:%h %>|(-10)%s\" >actual &&\n -+\tGIT_TEST_COLUMNS= COLUMNS=50 git log --pretty=\"tformat:%h %>|(-10)%s\" >actual &&\n ++\ttest_with_columns 50 git log --pretty=\"tformat:%h %>|(-10)%s\" >actual &&\n \tqz_to_tab_space <<-EOF >expected &&\n \t$head1 message two\n \t$head2 message one\n @@ t/t4205-log-pretty-formats.sh: test_expect_success 'center alignment formatting\n \n test_expect_success 'center alignment formatting at the nth column' '\n -\tCOLUMNS=70 git log --pretty=\"tformat:%h %><|(-30)%s\" >actual &&\n -+\tGIT_TEST_COLUMNS= COLUMNS=70 git log --pretty=\"tformat:%h %><|(-30)%s\" >actual &&\n ++\ttest_with_columns 70 git log --pretty=\"tformat:%h %><|(-30)%s\" >actual &&\n \tqz_to_tab_space <<-EOF >expected &&\n \t$head1 message two Z\n \t$head2 message one Z\n @@ t/t7004-tag.sh: test_expect_success 'tag -l -l works, as our\n \n test_expect_success 'listing tags in column' '\n -\tCOLUMNS=41 git tag -l --column=row >actual &&\n -+\tGIT_TEST_COLUMNS= COLUMNS=41 git tag -l --column=row >actual &&\n ++\ttest_with_columns 41 git tag -l --column=row >actual &&\n \tcat >expected <<\\EOF &&\n a1 aa1 cba t210 t211\n v0.2.1 v1.0 v1.0.1 v1.1.3\n @@ t/t7004-tag.sh: EOF\n \ttest_config column.tag row &&\n \ttest_config column.ui dense &&\n -\tCOLUMNS=40 git tag -l >actual &&\n -+\tGIT_TEST_COLUMNS= COLUMNS=40 git tag -l >actual &&\n ++\ttest_with_columns 40 git tag -l >actual &&\n \tcat >expected <<\\EOF &&\n a1 aa1 cba t210 t211\n v0.2.1 v1.0 v1.0.1 v1.1.3\n @@ t/t7004-tag.sh: test_expect_success 'listing tag with -n --column should fail' '\n test_expect_success 'listing tags -n in column with column.ui ignored' '\n \ttest_config column.ui \"row dense\" &&\n -\tCOLUMNS=40 git tag -l -n >actual &&\n -+\tGIT_TEST_COLUMNS= COLUMNS=40 git tag -l -n >actual &&\n ++\ttest_with_columns 40 git tag -l -n >actual &&\n \tcat >expected <<\\EOF &&\n a1 Foo\n aa1 Foo\n @@ t/t7006-pager.sh: test_expect_success TTY 'git tag with auto-columns ' '\n \tinitial one two three four five\n \tEOF\n -\ttest_terminal env PAGER=\"cat >actual\" COLUMNS=80 \\\n -+\ttest_terminal env PAGER=\"cat >actual\" GIT_TEST_COLUMNS= COLUMNS=80 \\\n ++\ttest_with_columns 80 test_terminal env PAGER=\"cat >actual\" \\\n \t\tgit -c column.ui=auto tag --sort=authordate &&\n \ttest_cmp expect actual\n '\n @@ t/t7508-status.sh: test_expect_success 'status --column' '\n #\n EOF\n -\tCOLUMNS=50 git -c status.displayCommentPrefix=true status --column=\"column dense\" >output &&\n -+\tGIT_TEST_COLUMNS= COLUMNS=50 git -c status.displayCommentPrefix=true status --column=\"column dense\" >output &&\n ++\ttest_with_columns 50 git -c status.displayCommentPrefix=true status --column=\"column dense\" >output &&\n \ttest_cmp expect output\n '\n \n test_expect_success 'status --column status.displayCommentPrefix=false' '\n \tstrip_comments expect &&\n -\tCOLUMNS=49 git -c status.displayCommentPrefix=false status --column=\"column dense\" >output &&\n -+\tGIT_TEST_COLUMNS= COLUMNS=49 git -c status.displayCommentPrefix=false status --column=\"column dense\" >output &&\n ++\ttest_with_columns 49 git -c status.displayCommentPrefix=false status --column=\"column dense\" >output &&\n \ttest_cmp expect output\n '\n \n @@ t/t9002-column.sh: test_expect_success '80 columns' '\n one two three four five six seven eight nine ten eleven\n EOF\n -\tCOLUMNS=80 git column --mode=column actual &&\n -+\tGIT_TEST_COLUMNS= COLUMNS=80 git column --mode=column actual &&\n ++\ttest_with_columns 80 git column --mode=column actual &&\n \ttest_cmp expected actual\n '\n \n @@ t/t9002-column.sh: eleven\n \n test_expect_success COLUMNS_CAN_BE_1 'COLUMNS = 1' '\n -\tCOLUMNS=1 git column --mode=column actual &&\n -+\tGIT_TEST_COLUMNS= COLUMNS=1 git column --mode=column actual &&\n ++\ttest_with_columns 1 git column --mode=column actual &&\n \ttest_cmp expected actual\n '\n \n @@ t/t9002-column.sh: four ten\n six\n EOF\n -\tgit column --mode=column actual &&\n -+\tGIT_TEST_COLUMNS= COLUMNS=20 git column --mode=column actual &&\n ++\ttest_with_columns 20 git column --mode=column actual &&\n \ttest_cmp expected actual\n '\n \n @@ t/t9002-column.sh: four ten\n six\n EOF\n -\tgit column --mode=column,nodense < lista > actual &&\n -+\tGIT_TEST_COLUMNS= COLUMNS=20 git column --mode=column,nodense < lista > actual &&\n ++\ttest_with_columns 20 git column --mode=column,nodense < lista > actual &&\n \ttest_cmp expected actual\n '\n \n @@ t/t9002-column.sh: two six ten\n four eight\n EOF\n -\tgit column --mode=column,dense < lista > actual &&\n -+\tGIT_TEST_COLUMNS= COLUMNS=20 git column --mode=column,dense < lista > actual &&\n ++\ttest_with_columns 20 git column --mode=column,dense < lista > actual &&\n \ttest_cmp expected actual\n '\n \n @@ t/t9002-column.sh: four ten\n six\n EOF\n -\tgit column --mode=column --padding 2 actual &&\n -+\tGIT_TEST_COLUMNS= COLUMNS=20 git column --mode=column --padding 2 actual &&\n ++\ttest_with_columns 20 git column --mode=column --padding 2 actual &&\n \ttest_cmp expected actual\n '\n \n @@ t/t9002-column.sh: test_expect_success '20 columns, indented' '\n six\n EOF\n -\tgit column --mode=column --indent=\" \" actual &&\n -+\tGIT_TEST_COLUMNS= COLUMNS=20 git column --mode=column --indent=\" \" actual &&\n ++\ttest_with_columns 20 git column --mode=column --indent=\" \" actual &&\n \ttest_cmp expected actual\n '\n \n @@ t/t9002-column.sh: seven eight\n eleven\n EOF\n -\tgit column --mode=row actual &&\n -+\tGIT_TEST_COLUMNS= COLUMNS=20 git column --mode=row actual &&\n ++\ttest_with_columns 20 git column --mode=row actual &&\n \ttest_cmp expected actual\n '\n \n @@ t/t9002-column.sh: seven eight\n eleven\n EOF\n -\tgit column --mode=row,nodense actual &&\n -+\tGIT_TEST_COLUMNS= COLUMNS=20 git column --mode=row,nodense actual &&\n ++\ttest_with_columns 20 git column --mode=row,nodense actual &&\n \ttest_cmp expected actual\n '\n \n @@ t/t9002-column.sh: four five six\n ten eleven\n EOF\n -\tgit column --mode=row,dense actual &&\n -+\tGIT_TEST_COLUMNS= COLUMNS=20 git column --mode=row,dense actual &&\n ++\ttest_with_columns 20 git column --mode=row,dense actual &&\n \ttest_cmp expected actual\n '\n \n \n - ## t/test-lib.sh ##\n -@@ t/test-lib.sh: LANG=C\n - LC_ALL=C\n - PAGER=cat\n - TZ=UTC\n --COLUMNS=80\n --export LANG LC_ALL PAGER TZ COLUMNS\n -+export LANG LC_ALL PAGER TZ\n - EDITOR=:\n - \n -+# For repeatability we need to set term_columns()'s idea of\n -+# columns. We do this via GIT_TEST_COLUMNS and not COLUMNS because\n -+# e.g. versions of bash >= 5.0 have \"shopt -s checkwinsize\" on by\n -+# default. We could do \"shopt -u checkwinsize >/dev/null 2>&1\" here to\n -+# fix that particular issue, but this is not shell specific, and\n -+# future-proof the tests.\n -+GIT_TEST_COLUMNS=80\n -+COLUMNS=10\n -+export GIT_TEST_COLUMNS COLUMNS\n + ## t/test-lib-functions.sh ##\n +@@ t/test-lib-functions.sh: test_region () {\n + test_readlink () {\n + \tperl -le 'print readlink($_) for @ARGV' \"$@\"\n + }\n ++\n ++# Test a with a given number of COLUMNS in the environment.\n ++test_with_columns () {\n ++\tlocal columns=$1\n ++\tshift\n +\n - # A call to \"unset\" with no arguments causes at least Solaris 10\n - # /usr/xpg4/bin/sh and /bin/ksh to bail out. So keep the unsets\n - # deriving from the command substitution clustered with the other\n ++\tCOLUMNS=$columns \"$@\"\n ++}\n-: ----------- > 3: 6cbbb955e9a test-lib.sh: use GIT_TEST_COLUMNS over COLUMNS\n-- \n2.32.0.1070.gec115ccd780\n\n"},{"id":"431692","messageId":"patch-v2-2.3-36c57178c55-20210802T134610Z-avarab@gmail.com","threadId":"56105","inReplyTo":"cover-v2-0.3-00000000000-20210802T134610Z-avarab@gmail.com","subject":"[PATCH v2 2/3] test-lib-functions.sh: add a test_with_columns function","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-08-02T13:46:27Z","receivedAt":"2021-08-02T13:46:59Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add a helper function to wrap patterns of \"COLUMNS=N \" as\n\"test_with_columns N \". This will be used and extended in the\nnext commit.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t3200-branch.sh | 8 ++++----\n t/t4052-stat-output.sh | 22 +++++++++++-----------\n t/t4205-log-pretty-formats.sh | 6 +++---\n t/t7004-tag.sh | 6 +++---\n t/t7006-pager.sh | 2 +-\n t/t7508-status.sh | 4 ++--\n t/t9002-column.sh | 23 ++++++++++-------------\n t/test-lib-functions.sh | 8 ++++++++\n 8 files changed, 42 insertions(+), 37 deletions(-)\n\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex cc4b10236e2..e568156ca83 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -328,7 +328,7 @@ test_expect_success 'git branch --list -v with --abbrev' '\n '\n \n test_expect_success 'git branch --column' '\n-\tCOLUMNS=81 git branch --column=column >actual &&\n+\ttest_with_columns 81 git branch --column=column >actual &&\n \tcat >expect <<\\EOF &&\n a/b/c bam foo l * main n o/p r\n abc bar j/k m/m mb o/o q topic\n@@ -341,7 +341,7 @@ test_expect_success 'git branch --column with an extremely long branch name' '\n \tlong=z$long/$long/$long/$long &&\n \ttest_when_finished \"git branch -d $long\" &&\n \tgit branch $long &&\n-\tCOLUMNS=80 git branch --column=column >actual &&\n+\ttest_with_columns 80 git branch --column=column >actual &&\n \tcat >expect <actual &&\n+\ttest_with_columns 80 git branch >actual &&\n \tgit config --unset column.branch &&\n \tgit config --unset column.ui &&\n \tcat >expect <<\\EOF &&\n@@ -383,7 +383,7 @@ test_expect_success 'git branch --column -v should fail' '\n \n test_expect_success 'git branch -v with column.ui ignored' '\n \tgit config column.ui column &&\n-\tCOLUMNS=80 git branch -v | cut -c -8 | sed \"s/ *$//\" >actual &&\n+\ttest_with_columns 80 git branch -v | cut -c -8 | sed \"s/ *$//\" >actual &&\n \tgit config --unset column.ui &&\n \tcat >expect <<\\EOF &&\n a/b/c\ndiff --git a/t/t4052-stat-output.sh b/t/t4052-stat-output.sh\nindex 9eba436211f..3a91df50dd8 100755\n--- a/t/t4052-stat-output.sh\n+++ b/t/t4052-stat-output.sh\n@@ -111,7 +111,7 @@ cat >expect72 <<'EOF'\n abcd | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n EOF\n test_expect_success \"format-patch --cover-letter ignores COLUMNS (big change)\" '\n-\tCOLUMNS=200 git format-patch -1 --stdout --cover-letter >output &&\n+\ttest_with_columns 200 git format-patch -1 --stdout --cover-letter >output &&\n \tgrep \" | \" output >actual &&\n \ttest_cmp expect72 actual\n '\n@@ -131,7 +131,7 @@ EOF\n while read verb expect cmd args\n do\n \ttest_expect_success \"$cmd $verb COLUMNS (big change)\" '\n-\t\tCOLUMNS=200 git $cmd $args >output &&\n+\t\ttest_with_columns 200 git $cmd $args >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp \"$expect\" actual\n \t'\n@@ -139,7 +139,7 @@ do\n \tcase \"$cmd\" in diff|show) continue;; esac\n \n \ttest_expect_success \"$cmd --graph $verb COLUMNS (big change)\" '\n-\t\tCOLUMNS=200 git $cmd $args --graph >output &&\n+\t\ttest_with_columns 200 git $cmd $args --graph >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp \"$expect-graph\" actual\n \t'\n@@ -159,7 +159,7 @@ EOF\n while read verb expect cmd args\n do\n \ttest_expect_success \"$cmd $verb not enough COLUMNS (big change)\" '\n-\t\tCOLUMNS=40 git $cmd $args >output &&\n+\t\ttest_with_columns 40 git $cmd $args >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp \"$expect\" actual\n \t'\n@@ -167,7 +167,7 @@ do\n \tcase \"$cmd\" in diff|show) continue;; esac\n \n \ttest_expect_success \"$cmd --graph $verb not enough COLUMNS (big change)\" '\n-\t\tCOLUMNS=40 git $cmd $args --graph >output &&\n+\t\ttest_with_columns 40 git $cmd $args --graph >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp \"$expect-graph\" actual\n \t'\n@@ -302,7 +302,7 @@ EOF\n while read verb expect cmd args\n do\n \ttest_expect_success \"$cmd $verb COLUMNS (long filename)\" '\n-\t\tCOLUMNS=200 git $cmd $args >output &&\n+\t\ttest_with_columns 200 git $cmd $args >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp \"$expect\" actual\n \t'\n@@ -310,7 +310,7 @@ do\n \tcase \"$cmd\" in diff|show) continue;; esac\n \n \ttest_expect_success \"$cmd --graph $verb COLUMNS (long filename)\" '\n-\t\tCOLUMNS=200 git $cmd $args --graph >output &&\n+\t\ttest_with_columns 200 git $cmd $args --graph >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp \"$expect-graph\" actual\n \t'\n@@ -331,7 +331,7 @@ while read verb expect cmd args\n do\n \ttest_expect_success COLUMNS_CAN_BE_1 \\\n \t\t\"$cmd $verb prefix greater than COLUMNS (big change)\" '\n-\t\tCOLUMNS=1 git $cmd $args >output &&\n+\t\ttest_with_columns 1 git $cmd $args >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp \"$expect\" actual\n \t'\n@@ -340,7 +340,7 @@ do\n \n \ttest_expect_success COLUMNS_CAN_BE_1 \\\n \t\t\"$cmd --graph $verb prefix greater than COLUMNS (big change)\" '\n-\t\tCOLUMNS=1 git $cmd $args --graph >output &&\n+\t\ttest_with_columns 1 git $cmd $args --graph >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp \"$expect-graph\" actual\n \t'\n@@ -356,7 +356,7 @@ cat >expect <<'EOF'\n EOF\n test_expect_success 'merge --stat respects COLUMNS (big change)' '\n \tgit checkout -b branch HEAD^^ &&\n-\tCOLUMNS=100 git merge --stat --no-ff main^ >output &&\n+\ttest_with_columns 100 git merge --stat --no-ff main^ >output &&\n \tgrep \" | \" output >actual &&\n \ttest_cmp expect actual\n '\n@@ -365,7 +365,7 @@ cat >expect <<'EOF'\n aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 +++++++++++++++++++++++++++++++++++++++\n EOF\n test_expect_success 'merge --stat respects COLUMNS (long filename)' '\n-\tCOLUMNS=100 git merge --stat --no-ff main >output &&\n+\ttest_with_columns 100 git merge --stat --no-ff main >output &&\n \tgrep \" | \" output >actual &&\n \ttest_cmp expect actual\n '\ndiff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh\nindex 5865daa8f8d..a035f749537 100755\n--- a/t/t4205-log-pretty-formats.sh\n+++ b/t/t4205-log-pretty-formats.sh\n@@ -207,7 +207,7 @@ test_expect_success 'left alignment formatting at the nth column' '\n '\n \n test_expect_success 'left alignment formatting at the nth column' '\n-\tCOLUMNS=50 git log --pretty=\"tformat:%h %<|(-10)%s\" >actual &&\n+\ttest_with_columns 50 git log --pretty=\"tformat:%h %<|(-10)%s\" >actual &&\n \tqz_to_tab_space <<-EOF >expected &&\n \t$head1 message two Z\n \t$head2 message one Z\n@@ -350,7 +350,7 @@ test_expect_success 'right alignment formatting at the nth column' '\n '\n \n test_expect_success 'right alignment formatting at the nth column' '\n-\tCOLUMNS=50 git log --pretty=\"tformat:%h %>|(-10)%s\" >actual &&\n+\ttest_with_columns 50 git log --pretty=\"tformat:%h %>|(-10)%s\" >actual &&\n \tqz_to_tab_space <<-EOF >expected &&\n \t$head1 message two\n \t$head2 message one\n@@ -450,7 +450,7 @@ test_expect_success 'center alignment formatting at the nth column' '\n '\n \n test_expect_success 'center alignment formatting at the nth column' '\n-\tCOLUMNS=70 git log --pretty=\"tformat:%h %><|(-30)%s\" >actual &&\n+\ttest_with_columns 70 git log --pretty=\"tformat:%h %><|(-30)%s\" >actual &&\n \tqz_to_tab_space <<-EOF >expected &&\n \t$head1 message two Z\n \t$head2 message one Z\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex 2f72c5c6883..e893c43d705 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -372,7 +372,7 @@ test_expect_success 'tag -l -l works, as our buggy documenta\n '\n \n test_expect_success 'listing tags in column' '\n-\tCOLUMNS=41 git tag -l --column=row >actual &&\n+\ttest_with_columns 41 git tag -l --column=row >actual &&\n \tcat >expected <<\\EOF &&\n a1 aa1 cba t210 t211\n v0.2.1 v1.0 v1.0.1 v1.1.3\n@@ -383,7 +383,7 @@ EOF\n test_expect_success 'listing tags in column with column.*' '\n \ttest_config column.tag row &&\n \ttest_config column.ui dense &&\n-\tCOLUMNS=40 git tag -l >actual &&\n+\ttest_with_columns 40 git tag -l >actual &&\n \tcat >expected <<\\EOF &&\n a1 aa1 cba t210 t211\n v0.2.1 v1.0 v1.0.1 v1.1.3\n@@ -397,7 +397,7 @@ test_expect_success 'listing tag with -n --column should fail' '\n \n test_expect_success 'listing tags -n in column with column.ui ignored' '\n \ttest_config column.ui \"row dense\" &&\n-\tCOLUMNS=40 git tag -l -n >actual &&\n+\ttest_with_columns 40 git tag -l -n >actual &&\n \tcat >expected <<\\EOF &&\n a1 Foo\n aa1 Foo\ndiff --git a/t/t7006-pager.sh b/t/t7006-pager.sh\nindex 0e7cf75435e..5189699debf 100755\n--- a/t/t7006-pager.sh\n+++ b/t/t7006-pager.sh\n@@ -651,7 +651,7 @@ test_expect_success TTY 'git tag with auto-columns ' '\n \tcat >expect <<-\\EOF &&\n \tinitial one two three four five\n \tEOF\n-\ttest_terminal env PAGER=\"cat >actual\" COLUMNS=80 \\\n+\ttest_with_columns 80 test_terminal env PAGER=\"cat >actual\" \\\n \t\tgit -c column.ui=auto tag --sort=authordate &&\n \ttest_cmp expect actual\n '\ndiff --git a/t/t7508-status.sh b/t/t7508-status.sh\nindex 2b72451ba3e..808b8f50576 100755\n--- a/t/t7508-status.sh\n+++ b/t/t7508-status.sh\n@@ -108,13 +108,13 @@ test_expect_success 'status --column' '\n #\tdir2/modified untracked\n #\n EOF\n-\tCOLUMNS=50 git -c status.displayCommentPrefix=true status --column=\"column dense\" >output &&\n+\ttest_with_columns 50 git -c status.displayCommentPrefix=true status --column=\"column dense\" >output &&\n \ttest_cmp expect output\n '\n \n test_expect_success 'status --column status.displayCommentPrefix=false' '\n \tstrip_comments expect &&\n-\tCOLUMNS=49 git -c status.displayCommentPrefix=false status --column=\"column dense\" >output &&\n+\ttest_with_columns 49 git -c status.displayCommentPrefix=false status --column=\"column dense\" >output &&\n \ttest_cmp expect output\n '\n \ndiff --git a/t/t9002-column.sh b/t/t9002-column.sh\nindex 89983527b62..9151d69bcf1 100755\n--- a/t/t9002-column.sh\n+++ b/t/t9002-column.sh\n@@ -46,7 +46,7 @@ test_expect_success '80 columns' '\n \tcat >expected <<\\EOF &&\n one two three four five six seven eight nine ten eleven\n EOF\n-\tCOLUMNS=80 git column --mode=column actual &&\n+\ttest_with_columns 80 git column --mode=column actual &&\n \ttest_cmp expected actual\n '\n \n@@ -65,7 +65,7 @@ eleven\n EOF\n \n test_expect_success COLUMNS_CAN_BE_1 'COLUMNS = 1' '\n-\tCOLUMNS=1 git column --mode=column actual &&\n+\ttest_with_columns 1 git column --mode=column actual &&\n \ttest_cmp expected actual\n '\n \n@@ -74,9 +74,6 @@ test_expect_success 'width = 1' '\n \ttest_cmp expected actual\n '\n \n-COLUMNS=20\n-export COLUMNS\n-\n test_expect_success '20 columns' '\n \tcat >expected <<\\EOF &&\n one seven\n@@ -86,7 +83,7 @@ four ten\n five eleven\n six\n EOF\n-\tgit column --mode=column actual &&\n+\ttest_with_columns 20 git column --mode=column actual &&\n \ttest_cmp expected actual\n '\n \n@@ -99,7 +96,7 @@ four ten\n five eleven\n six\n EOF\n-\tgit column --mode=column,nodense < lista > actual &&\n+\ttest_with_columns 20 git column --mode=column,nodense < lista > actual &&\n \ttest_cmp expected actual\n '\n \n@@ -110,7 +107,7 @@ two six ten\n three seven eleven\n four eight\n EOF\n-\tgit column --mode=column,dense < lista > actual &&\n+\ttest_with_columns 20 git column --mode=column,dense < lista > actual &&\n \ttest_cmp expected actual\n '\n \n@@ -123,7 +120,7 @@ four ten\n five eleven\n six\n EOF\n-\tgit column --mode=column --padding 2 actual &&\n+\ttest_with_columns 20 git column --mode=column --padding 2 actual &&\n \ttest_cmp expected actual\n '\n \n@@ -136,7 +133,7 @@ test_expect_success '20 columns, indented' '\n five eleven\n six\n EOF\n-\tgit column --mode=column --indent=\" \" actual &&\n+\ttest_with_columns 20 git column --mode=column --indent=\" \" actual &&\n \ttest_cmp expected actual\n '\n \n@@ -149,7 +146,7 @@ seven eight\n nine ten\n eleven\n EOF\n-\tgit column --mode=row actual &&\n+\ttest_with_columns 20 git column --mode=row actual &&\n \ttest_cmp expected actual\n '\n \n@@ -162,7 +159,7 @@ seven eight\n nine ten\n eleven\n EOF\n-\tgit column --mode=row,nodense actual &&\n+\ttest_with_columns 20 git column --mode=row,nodense actual &&\n \ttest_cmp expected actual\n '\n \n@@ -173,7 +170,7 @@ four five six\n seven eight nine\n ten eleven\n EOF\n-\tgit column --mode=row,dense actual &&\n+\ttest_with_columns 20 git column --mode=row,dense actual &&\n \ttest_cmp expected actual\n '\n \ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 37da7d9a99a..536339faaa2 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -1718,3 +1718,11 @@ test_region () {\n test_readlink () {\n \tperl -le 'print readlink($_) for @ARGV' \"$@\"\n }\n+\n+# Test a with a given number of COLUMNS in the environment.\n+test_with_columns () {\n+\tlocal columns=$1\n+\tshift\n+\n+\tCOLUMNS=$columns \"$@\"\n+}\n-- \n2.32.0.1070.gec115ccd780\n\n"},{"id":"431729","messageId":"20210802171429.GC23408@szeder.dev","threadId":"56105","inReplyTo":"patch-v2-2.3-36c57178c55-20210802T134610Z-avarab@gmail.com","subject":"Re: [PATCH v2 2/3] test-lib-functions.sh: add a test_with_columns function","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2021-08-02T17:14:29Z","receivedAt":"2021-08-02T17:14:35Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Mon, Aug 02, 2021 at 03:46:27PM +0200, Ævar Arnfjörð Bjarmason wrote:\n> Add a helper function to wrap patterns of \"COLUMNS=N \" as\n> \"test_with_columns N \". This will be used and extended in the\n> next commit.\n\n\n> diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\n> index 37da7d9a99a..536339faaa2 100644\n> --- a/t/test-lib-functions.sh\n> +++ b/t/test-lib-functions.sh\n> @@ -1718,3 +1718,11 @@ test_region () {\n> test_readlink () {\n> \tperl -le 'print readlink($_) for @ARGV' \"$@\"\n> }\n> +\n> +# Test a with a given number of COLUMNS in the environment.\n> +test_with_columns () {\n> +\tlocal columns=$1\n> +\tshift\n> +\n> +\tCOLUMNS=$columns \"$@\"\n> +}\n\nThis function needs some redirections to separate the tested command's\nstandard error from the function's '-x' trace, see a5bf824f3b (t:\nprevent '-x' tracing from interfering with test helpers' stderr,\n2018-02-25).\n\n"},{"id":"431731","messageId":"CAPig+cS5MyZ9iFC3kaipMmPSq93DoiQzoFffYds9rASOkJXWFg@mail.gmail.com","threadId":"56105","inReplyTo":"20210802171429.GC23408@szeder.dev","subject":"Re: [PATCH v2 2/3] test-lib-functions.sh: add a test_with_columns function","fromName":"Eric Sunshine","fromEmail":"sunshine@sunshineco.com","sentAt":"2021-08-02T17:24:07Z","receivedAt":"2021-08-02T17:24:25Z","isPatch":true,"sender":{"key":"sunshine@sunshineco.com","avatar":"https://avatars.githubusercontent.com/u/163641?v=4"},"body":"On Mon, Aug 2, 2021 at 1:14 PM SZEDER Gábor wrote:\n> On Mon, Aug 02, 2021 at 03:46:27PM +0200, Ævar Arnfjörð Bjarmason wrote:\n> > +# Test a with a given number of COLUMNS in the environment.\n> > +test_with_columns () {\n> > + local columns=$1\n> > + shift\n> > +\n> > + COLUMNS=$columns \"$@\"\n> > +}\n>\n> This function needs some redirections to separate the tested command's\n> standard error from the function's '-x' trace, see a5bf824f3b (t:\n> prevent '-x' tracing from interfering with test helpers' stderr,\n> 2018-02-25).\n\nThis is also potentially problematic when a test is expected to fail,\nin which case we avoid the:\n\n FOO=bar command\n\nidiom and instead use:\n\n test_must_fail env FOO=bar command\n"},{"id":"432008","messageId":"patch-v3-1.3-f45590a76d5-20210804T230335Z-avarab@gmail.com","threadId":"56105","inReplyTo":"cover-v3-0.3-00000000000-20210804T230335Z-avarab@gmail.com","subject":"[PATCH v3 1/3] test-lib-functions.sh: rename test_must_fail_acceptable()","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-08-04T23:05:44Z","receivedAt":"2021-08-04T23:05:52Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The test_must_fail_acceptable() is really a generic function that can\ncheck if something is a real \"git command\", e.g. \"git\", \"test-tool\"\netc. Let's rename it in preparation for using it in another test\nfunction.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/test-lib-functions.sh | 6 +++---\n 1 file changed, 3 insertions(+), 3 deletions(-)\n\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex e28411bb75a..37da7d9a99a 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -895,7 +895,7 @@ list_contains () {\n # accepted by test_must_fail(). If the command is run with env, the env\n # and its corresponding variable settings will be stripped before we\n # test the command being run.\n-test_must_fail_acceptable () {\n+is_git_command_name () {\n \tif test \"$1\" = \"env\"\n \tthen\n \t\tshift\n@@ -943,7 +943,7 @@ test_must_fail_acceptable () {\n # (Don't use 'success', use 'test_might_fail' instead.)\n #\n # Do not use this to run anything but \"git\" and other specific testable\n-# commands (see test_must_fail_acceptable()). We are not in the\n+# commands (see is_git_command_name()). We are not in the\n # business of vetting system supplied commands -- in other words, this\n # is wrong:\n #\n@@ -963,7 +963,7 @@ test_must_fail () {\n \t\t_test_ok=\n \t\t;;\n \tesac\n-\tif ! test_must_fail_acceptable \"$@\"\n+\tif ! is_git_command_name \"$@\"\n \tthen\n \t\techo >&7 \"test_must_fail: only 'git' is allowed: $*\"\n \t\treturn 1\n-- \n2.33.0.rc0.597.gc569a812f0a\n\n"},{"id":"432009","messageId":"cover-v3-0.3-00000000000-20210804T230335Z-avarab@gmail.com","threadId":"56105","inReplyTo":"cover-v2-0.3-00000000000-20210802T134610Z-avarab@gmail.com","subject":"[PATCH v3 0/3] test-lib.sh: use GIT_TEST_COLUMNS over COLUMNS","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-08-04T23:05:43Z","receivedAt":"2021-08-04T23:05:53Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"This v3 addresses SZEDER's note in <20210802171429.GC23408@szeder.dev>\nthat we were missing redirection. The \"is_git_command_name\" check has\nalso been moved one commit earlier, as it should.\n\nÆvar Arnfjörð Bjarmason (3):\n test-lib-functions.sh: rename test_must_fail_acceptable()\n test-lib-functions.sh: add a test_with_columns function\n test-lib.sh: use GIT_TEST_COLUMNS over COLUMNS\n\n pager.c | 7 +++++++\n t/t3200-branch.sh | 8 ++++----\n t/t4052-stat-output.sh | 22 +++++++++++-----------\n t/t4205-log-pretty-formats.sh | 6 +++---\n t/t7004-tag.sh | 6 +++---\n t/t7006-pager.sh | 2 +-\n t/t7508-status.sh | 4 ++--\n t/t9002-column.sh | 23 ++++++++++-------------\n t/test-lib-functions.sh | 21 ++++++++++++++++++---\n t/test-lib.sh | 13 +++++++++++--\n 10 files changed, 70 insertions(+), 42 deletions(-)\n\nRange-diff against v2:\n1: 739457b992f = 1: f45590a76d5 test-lib-functions.sh: rename test_must_fail_acceptable()\n2: 36c57178c55 ! 2: 53e6e25ece6 test-lib-functions.sh: add a test_with_columns function\n @@ t/test-lib-functions.sh: test_region () {\n +\tlocal columns=$1\n +\tshift\n +\n -+\tCOLUMNS=$columns \"$@\"\n -+}\n ++\tif ! is_git_command_name \"$@\"\n ++\tthen\n ++\t\techo >&7 \"test_with_columns: only 'git' is allowed: $*\"\n ++\t\treturn 1\n ++\tfi\n ++\n ++\tCOLUMNS=$columns \"$@\" 2>&7\n ++} 7>&2 2>&4\n3: 6cbbb955e9a ! 3: 74acba0f9ca test-lib.sh: use GIT_TEST_COLUMNS over COLUMNS\n @@ pager.c: int term_columns(void)\n \n ## t/test-lib-functions.sh ##\n @@ t/test-lib-functions.sh: test_with_columns () {\n - \tlocal columns=$1\n - \tshift\n + \t\treturn 1\n + \tfi\n \n -+\tif ! is_git_command_name \"$@\"\n -+\tthen\n -+\t\techo >&7 \"test_with_columns: only 'git' is allowed: $*\"\n -+\t\treturn 1\n -+\tfi\n -+\n +\tGIT_TEST_COLUMNS= \\\n - \tCOLUMNS=$columns \"$@\"\n - }\n + \tCOLUMNS=$columns \"$@\" 2>&7\n + } 7>&2 2>&4\n \n ## t/test-lib.sh ##\n @@ t/test-lib.sh: LANG=C\n-- \n2.33.0.rc0.597.gc569a812f0a\n\n"},{"id":"432010","messageId":"patch-v3-3.3-74acba0f9ca-20210804T230335Z-avarab@gmail.com","threadId":"56105","inReplyTo":"cover-v3-0.3-00000000000-20210804T230335Z-avarab@gmail.com","subject":"[PATCH v3 3/3] test-lib.sh: use GIT_TEST_COLUMNS over COLUMNS","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-08-04T23:05:46Z","receivedAt":"2021-08-04T23:05:54Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"In my c49a177beca (test-lib.sh: set COLUMNS=80 for --verbose\nrepeatability, 2021-06-29) the test suite started breaking on recent\nversions of bash.\n\nThis is because it sets \"shopt -s checkwinsize\" starting with version\n5.0, furthermore it started setting COLUMNS under \"shopt -s\ncheckwinsize\" for non-interactive use around version 4.3.\n\nA narrow fix for that issue would be to add this just above our\nsetting of COLUMNS in test-lib.sh:\n\n\tshopt -u checkwinsize >/dev/null 2>&1\n\nBut we'd then be at the mercy of the next shell or terminal that wants\nto be clever about COLUMNS.\n\nLet's instead solve this more thoroughly. We'll now take\nGIT_TEST_COLUMNS over COLUMNS, and furthermore intentionally spoil the\nCOLUMNS variable to break any tests that rely on it being set to a\nsane value.\n\nIf something breaks because we have a codepath that's not\nterm_columns() checking COLUMNS we'd like to know about it, the narrow\n\"shopt -u checkwinsize\" fix won't give us that.\n\nThe \"shopt\" fix won't future-poof us against other 3rd party software\nchanges either. If that third-party software e.g. takes TIOCGWINSZ\nover columns on some platforms, our tests would be flaky and break\nthere even without this change.\n\nThis approach does mean that any tests of ours that expected to test\nterm_columns() behavior by setting COLUMNS will need to explicitly\nunset GIT_TEST_COLUMNS, or set it to the empty string. Let's do that\nin the new test_with_columns() helper, which we previously changed all\nthe tests that set COLUMNS to use.\n\nReported-by: Fabian Stelzer \nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n pager.c | 7 +++++++\n t/test-lib-functions.sh | 1 +\n t/test-lib.sh | 13 +++++++++++--\n 3 files changed, 19 insertions(+), 2 deletions(-)\n\ndiff --git a/pager.c b/pager.c\nindex 52f27a6765c..cfcc6dc04bd 100644\n--- a/pager.c\n+++ b/pager.c\n@@ -165,6 +165,13 @@ int term_columns(void)\n \tterm_columns_at_startup = 80;\n \tterm_columns_guessed = 1;\n \n+\tcol_string = getenv(\"GIT_TEST_COLUMNS\");\n+\tif (col_string && (n_cols = atoi(col_string)) > 0) {\n+\t\tterm_columns_at_startup = n_cols;\n+\t\tterm_columns_guessed = 0;\n+\t\treturn term_columns_at_startup;\n+\t}\n+\n \tcol_string = getenv(\"COLUMNS\");\n \tif (col_string && (n_cols = atoi(col_string)) > 0) {\n \t\tterm_columns_at_startup = n_cols;\ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex adc853109e4..0970c1293a8 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -1730,5 +1730,6 @@ test_with_columns () {\n \t\treturn 1\n \tfi\n \n+\tGIT_TEST_COLUMNS= \\\n \tCOLUMNS=$columns \"$@\" 2>&7\n } 7>&2 2>&4\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex db61081d6b8..fc589226189 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -415,10 +415,19 @@ LANG=C\n LC_ALL=C\n PAGER=cat\n TZ=UTC\n-COLUMNS=80\n-export LANG LC_ALL PAGER TZ COLUMNS\n+export LANG LC_ALL PAGER TZ\n EDITOR=:\n \n+# For repeatability we need to set term_columns()'s idea of\n+# columns. We do this via GIT_TEST_COLUMNS and not COLUMNS because\n+# e.g. versions of bash >= 5.0 have \"shopt -s checkwinsize\" on by\n+# default. We could do \"shopt -u checkwinsize >/dev/null 2>&1\" here to\n+# fix that particular issue, but this is not shell specific, and\n+# future-proof the tests.\n+GIT_TEST_COLUMNS=80\n+COLUMNS=10\n+export GIT_TEST_COLUMNS COLUMNS\n+\n # A call to \"unset\" with no arguments causes at least Solaris 10\n # /usr/xpg4/bin/sh and /bin/ksh to bail out. So keep the unsets\n # deriving from the command substitution clustered with the other\n-- \n2.33.0.rc0.597.gc569a812f0a\n\n"},{"id":"432011","messageId":"patch-v3-2.3-53e6e25ece6-20210804T230335Z-avarab@gmail.com","threadId":"56105","inReplyTo":"cover-v3-0.3-00000000000-20210804T230335Z-avarab@gmail.com","subject":"[PATCH v3 2/3] test-lib-functions.sh: add a test_with_columns function","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-08-04T23:05:45Z","receivedAt":"2021-08-04T23:05:54Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Add a helper function to wrap patterns of \"COLUMNS=N \" as\n\"test_with_columns N \". This will be used and extended in the\nnext commit.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason \n---\n t/t3200-branch.sh | 8 ++++----\n t/t4052-stat-output.sh | 22 +++++++++++-----------\n t/t4205-log-pretty-formats.sh | 6 +++---\n t/t7004-tag.sh | 6 +++---\n t/t7006-pager.sh | 2 +-\n t/t7508-status.sh | 4 ++--\n t/t9002-column.sh | 23 ++++++++++-------------\n t/test-lib-functions.sh | 14 ++++++++++++++\n 8 files changed, 48 insertions(+), 37 deletions(-)\n\ndiff --git a/t/t3200-branch.sh b/t/t3200-branch.sh\nindex cc4b10236e2..e568156ca83 100755\n--- a/t/t3200-branch.sh\n+++ b/t/t3200-branch.sh\n@@ -328,7 +328,7 @@ test_expect_success 'git branch --list -v with --abbrev' '\n '\n \n test_expect_success 'git branch --column' '\n-\tCOLUMNS=81 git branch --column=column >actual &&\n+\ttest_with_columns 81 git branch --column=column >actual &&\n \tcat >expect <<\\EOF &&\n a/b/c bam foo l * main n o/p r\n abc bar j/k m/m mb o/o q topic\n@@ -341,7 +341,7 @@ test_expect_success 'git branch --column with an extremely long branch name' '\n \tlong=z$long/$long/$long/$long &&\n \ttest_when_finished \"git branch -d $long\" &&\n \tgit branch $long &&\n-\tCOLUMNS=80 git branch --column=column >actual &&\n+\ttest_with_columns 80 git branch --column=column >actual &&\n \tcat >expect <actual &&\n+\ttest_with_columns 80 git branch >actual &&\n \tgit config --unset column.branch &&\n \tgit config --unset column.ui &&\n \tcat >expect <<\\EOF &&\n@@ -383,7 +383,7 @@ test_expect_success 'git branch --column -v should fail' '\n \n test_expect_success 'git branch -v with column.ui ignored' '\n \tgit config column.ui column &&\n-\tCOLUMNS=80 git branch -v | cut -c -8 | sed \"s/ *$//\" >actual &&\n+\ttest_with_columns 80 git branch -v | cut -c -8 | sed \"s/ *$//\" >actual &&\n \tgit config --unset column.ui &&\n \tcat >expect <<\\EOF &&\n a/b/c\ndiff --git a/t/t4052-stat-output.sh b/t/t4052-stat-output.sh\nindex 9eba436211f..3a91df50dd8 100755\n--- a/t/t4052-stat-output.sh\n+++ b/t/t4052-stat-output.sh\n@@ -111,7 +111,7 @@ cat >expect72 <<'EOF'\n abcd | 1000 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n EOF\n test_expect_success \"format-patch --cover-letter ignores COLUMNS (big change)\" '\n-\tCOLUMNS=200 git format-patch -1 --stdout --cover-letter >output &&\n+\ttest_with_columns 200 git format-patch -1 --stdout --cover-letter >output &&\n \tgrep \" | \" output >actual &&\n \ttest_cmp expect72 actual\n '\n@@ -131,7 +131,7 @@ EOF\n while read verb expect cmd args\n do\n \ttest_expect_success \"$cmd $verb COLUMNS (big change)\" '\n-\t\tCOLUMNS=200 git $cmd $args >output &&\n+\t\ttest_with_columns 200 git $cmd $args >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp \"$expect\" actual\n \t'\n@@ -139,7 +139,7 @@ do\n \tcase \"$cmd\" in diff|show) continue;; esac\n \n \ttest_expect_success \"$cmd --graph $verb COLUMNS (big change)\" '\n-\t\tCOLUMNS=200 git $cmd $args --graph >output &&\n+\t\ttest_with_columns 200 git $cmd $args --graph >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp \"$expect-graph\" actual\n \t'\n@@ -159,7 +159,7 @@ EOF\n while read verb expect cmd args\n do\n \ttest_expect_success \"$cmd $verb not enough COLUMNS (big change)\" '\n-\t\tCOLUMNS=40 git $cmd $args >output &&\n+\t\ttest_with_columns 40 git $cmd $args >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp \"$expect\" actual\n \t'\n@@ -167,7 +167,7 @@ do\n \tcase \"$cmd\" in diff|show) continue;; esac\n \n \ttest_expect_success \"$cmd --graph $verb not enough COLUMNS (big change)\" '\n-\t\tCOLUMNS=40 git $cmd $args --graph >output &&\n+\t\ttest_with_columns 40 git $cmd $args --graph >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp \"$expect-graph\" actual\n \t'\n@@ -302,7 +302,7 @@ EOF\n while read verb expect cmd args\n do\n \ttest_expect_success \"$cmd $verb COLUMNS (long filename)\" '\n-\t\tCOLUMNS=200 git $cmd $args >output &&\n+\t\ttest_with_columns 200 git $cmd $args >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp \"$expect\" actual\n \t'\n@@ -310,7 +310,7 @@ do\n \tcase \"$cmd\" in diff|show) continue;; esac\n \n \ttest_expect_success \"$cmd --graph $verb COLUMNS (long filename)\" '\n-\t\tCOLUMNS=200 git $cmd $args --graph >output &&\n+\t\ttest_with_columns 200 git $cmd $args --graph >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp \"$expect-graph\" actual\n \t'\n@@ -331,7 +331,7 @@ while read verb expect cmd args\n do\n \ttest_expect_success COLUMNS_CAN_BE_1 \\\n \t\t\"$cmd $verb prefix greater than COLUMNS (big change)\" '\n-\t\tCOLUMNS=1 git $cmd $args >output &&\n+\t\ttest_with_columns 1 git $cmd $args >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp \"$expect\" actual\n \t'\n@@ -340,7 +340,7 @@ do\n \n \ttest_expect_success COLUMNS_CAN_BE_1 \\\n \t\t\"$cmd --graph $verb prefix greater than COLUMNS (big change)\" '\n-\t\tCOLUMNS=1 git $cmd $args --graph >output &&\n+\t\ttest_with_columns 1 git $cmd $args --graph >output &&\n \t\tgrep \" | \" output >actual &&\n \t\ttest_cmp \"$expect-graph\" actual\n \t'\n@@ -356,7 +356,7 @@ cat >expect <<'EOF'\n EOF\n test_expect_success 'merge --stat respects COLUMNS (big change)' '\n \tgit checkout -b branch HEAD^^ &&\n-\tCOLUMNS=100 git merge --stat --no-ff main^ >output &&\n+\ttest_with_columns 100 git merge --stat --no-ff main^ >output &&\n \tgrep \" | \" output >actual &&\n \ttest_cmp expect actual\n '\n@@ -365,7 +365,7 @@ cat >expect <<'EOF'\n aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1000 +++++++++++++++++++++++++++++++++++++++\n EOF\n test_expect_success 'merge --stat respects COLUMNS (long filename)' '\n-\tCOLUMNS=100 git merge --stat --no-ff main >output &&\n+\ttest_with_columns 100 git merge --stat --no-ff main >output &&\n \tgrep \" | \" output >actual &&\n \ttest_cmp expect actual\n '\ndiff --git a/t/t4205-log-pretty-formats.sh b/t/t4205-log-pretty-formats.sh\nindex 5865daa8f8d..a035f749537 100755\n--- a/t/t4205-log-pretty-formats.sh\n+++ b/t/t4205-log-pretty-formats.sh\n@@ -207,7 +207,7 @@ test_expect_success 'left alignment formatting at the nth column' '\n '\n \n test_expect_success 'left alignment formatting at the nth column' '\n-\tCOLUMNS=50 git log --pretty=\"tformat:%h %<|(-10)%s\" >actual &&\n+\ttest_with_columns 50 git log --pretty=\"tformat:%h %<|(-10)%s\" >actual &&\n \tqz_to_tab_space <<-EOF >expected &&\n \t$head1 message two Z\n \t$head2 message one Z\n@@ -350,7 +350,7 @@ test_expect_success 'right alignment formatting at the nth column' '\n '\n \n test_expect_success 'right alignment formatting at the nth column' '\n-\tCOLUMNS=50 git log --pretty=\"tformat:%h %>|(-10)%s\" >actual &&\n+\ttest_with_columns 50 git log --pretty=\"tformat:%h %>|(-10)%s\" >actual &&\n \tqz_to_tab_space <<-EOF >expected &&\n \t$head1 message two\n \t$head2 message one\n@@ -450,7 +450,7 @@ test_expect_success 'center alignment formatting at the nth column' '\n '\n \n test_expect_success 'center alignment formatting at the nth column' '\n-\tCOLUMNS=70 git log --pretty=\"tformat:%h %><|(-30)%s\" >actual &&\n+\ttest_with_columns 70 git log --pretty=\"tformat:%h %><|(-30)%s\" >actual &&\n \tqz_to_tab_space <<-EOF >expected &&\n \t$head1 message two Z\n \t$head2 message one Z\ndiff --git a/t/t7004-tag.sh b/t/t7004-tag.sh\nindex 2f72c5c6883..e893c43d705 100755\n--- a/t/t7004-tag.sh\n+++ b/t/t7004-tag.sh\n@@ -372,7 +372,7 @@ test_expect_success 'tag -l -l works, as our buggy documenta\n '\n \n test_expect_success 'listing tags in column' '\n-\tCOLUMNS=41 git tag -l --column=row >actual &&\n+\ttest_with_columns 41 git tag -l --column=row >actual &&\n \tcat >expected <<\\EOF &&\n a1 aa1 cba t210 t211\n v0.2.1 v1.0 v1.0.1 v1.1.3\n@@ -383,7 +383,7 @@ EOF\n test_expect_success 'listing tags in column with column.*' '\n \ttest_config column.tag row &&\n \ttest_config column.ui dense &&\n-\tCOLUMNS=40 git tag -l >actual &&\n+\ttest_with_columns 40 git tag -l >actual &&\n \tcat >expected <<\\EOF &&\n a1 aa1 cba t210 t211\n v0.2.1 v1.0 v1.0.1 v1.1.3\n@@ -397,7 +397,7 @@ test_expect_success 'listing tag with -n --column should fail' '\n \n test_expect_success 'listing tags -n in column with column.ui ignored' '\n \ttest_config column.ui \"row dense\" &&\n-\tCOLUMNS=40 git tag -l -n >actual &&\n+\ttest_with_columns 40 git tag -l -n >actual &&\n \tcat >expected <<\\EOF &&\n a1 Foo\n aa1 Foo\ndiff --git a/t/t7006-pager.sh b/t/t7006-pager.sh\nindex 0e7cf75435e..5189699debf 100755\n--- a/t/t7006-pager.sh\n+++ b/t/t7006-pager.sh\n@@ -651,7 +651,7 @@ test_expect_success TTY 'git tag with auto-columns ' '\n \tcat >expect <<-\\EOF &&\n \tinitial one two three four five\n \tEOF\n-\ttest_terminal env PAGER=\"cat >actual\" COLUMNS=80 \\\n+\ttest_with_columns 80 test_terminal env PAGER=\"cat >actual\" \\\n \t\tgit -c column.ui=auto tag --sort=authordate &&\n \ttest_cmp expect actual\n '\ndiff --git a/t/t7508-status.sh b/t/t7508-status.sh\nindex 2b72451ba3e..808b8f50576 100755\n--- a/t/t7508-status.sh\n+++ b/t/t7508-status.sh\n@@ -108,13 +108,13 @@ test_expect_success 'status --column' '\n #\tdir2/modified untracked\n #\n EOF\n-\tCOLUMNS=50 git -c status.displayCommentPrefix=true status --column=\"column dense\" >output &&\n+\ttest_with_columns 50 git -c status.displayCommentPrefix=true status --column=\"column dense\" >output &&\n \ttest_cmp expect output\n '\n \n test_expect_success 'status --column status.displayCommentPrefix=false' '\n \tstrip_comments expect &&\n-\tCOLUMNS=49 git -c status.displayCommentPrefix=false status --column=\"column dense\" >output &&\n+\ttest_with_columns 49 git -c status.displayCommentPrefix=false status --column=\"column dense\" >output &&\n \ttest_cmp expect output\n '\n \ndiff --git a/t/t9002-column.sh b/t/t9002-column.sh\nindex 89983527b62..9151d69bcf1 100755\n--- a/t/t9002-column.sh\n+++ b/t/t9002-column.sh\n@@ -46,7 +46,7 @@ test_expect_success '80 columns' '\n \tcat >expected <<\\EOF &&\n one two three four five six seven eight nine ten eleven\n EOF\n-\tCOLUMNS=80 git column --mode=column actual &&\n+\ttest_with_columns 80 git column --mode=column actual &&\n \ttest_cmp expected actual\n '\n \n@@ -65,7 +65,7 @@ eleven\n EOF\n \n test_expect_success COLUMNS_CAN_BE_1 'COLUMNS = 1' '\n-\tCOLUMNS=1 git column --mode=column actual &&\n+\ttest_with_columns 1 git column --mode=column actual &&\n \ttest_cmp expected actual\n '\n \n@@ -74,9 +74,6 @@ test_expect_success 'width = 1' '\n \ttest_cmp expected actual\n '\n \n-COLUMNS=20\n-export COLUMNS\n-\n test_expect_success '20 columns' '\n \tcat >expected <<\\EOF &&\n one seven\n@@ -86,7 +83,7 @@ four ten\n five eleven\n six\n EOF\n-\tgit column --mode=column actual &&\n+\ttest_with_columns 20 git column --mode=column actual &&\n \ttest_cmp expected actual\n '\n \n@@ -99,7 +96,7 @@ four ten\n five eleven\n six\n EOF\n-\tgit column --mode=column,nodense < lista > actual &&\n+\ttest_with_columns 20 git column --mode=column,nodense < lista > actual &&\n \ttest_cmp expected actual\n '\n \n@@ -110,7 +107,7 @@ two six ten\n three seven eleven\n four eight\n EOF\n-\tgit column --mode=column,dense < lista > actual &&\n+\ttest_with_columns 20 git column --mode=column,dense < lista > actual &&\n \ttest_cmp expected actual\n '\n \n@@ -123,7 +120,7 @@ four ten\n five eleven\n six\n EOF\n-\tgit column --mode=column --padding 2 actual &&\n+\ttest_with_columns 20 git column --mode=column --padding 2 actual &&\n \ttest_cmp expected actual\n '\n \n@@ -136,7 +133,7 @@ test_expect_success '20 columns, indented' '\n five eleven\n six\n EOF\n-\tgit column --mode=column --indent=\" \" actual &&\n+\ttest_with_columns 20 git column --mode=column --indent=\" \" actual &&\n \ttest_cmp expected actual\n '\n \n@@ -149,7 +146,7 @@ seven eight\n nine ten\n eleven\n EOF\n-\tgit column --mode=row actual &&\n+\ttest_with_columns 20 git column --mode=row actual &&\n \ttest_cmp expected actual\n '\n \n@@ -162,7 +159,7 @@ seven eight\n nine ten\n eleven\n EOF\n-\tgit column --mode=row,nodense actual &&\n+\ttest_with_columns 20 git column --mode=row,nodense actual &&\n \ttest_cmp expected actual\n '\n \n@@ -173,7 +170,7 @@ four five six\n seven eight nine\n ten eleven\n EOF\n-\tgit column --mode=row,dense actual &&\n+\ttest_with_columns 20 git column --mode=row,dense actual &&\n \ttest_cmp expected actual\n '\n \ndiff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh\nindex 37da7d9a99a..adc853109e4 100644\n--- a/t/test-lib-functions.sh\n+++ b/t/test-lib-functions.sh\n@@ -1718,3 +1718,17 @@ test_region () {\n test_readlink () {\n \tperl -le 'print readlink($_) for @ARGV' \"$@\"\n }\n+\n+# Test a with a given number of COLUMNS in the environment.\n+test_with_columns () {\n+\tlocal columns=$1\n+\tshift\n+\n+\tif ! is_git_command_name \"$@\"\n+\tthen\n+\t\techo >&7 \"test_with_columns: only 'git' is allowed: $*\"\n+\t\treturn 1\n+\tfi\n+\n+\tCOLUMNS=$columns \"$@\" 2>&7\n+} 7>&2 2>&4\n-- \n2.33.0.rc0.597.gc569a812f0a\n\n"}]}