{"thread":{"id":"55975","subject":"[PATCH] progress.c tests: fix breakage with COLUMNS != 80","startedAt":"2021-06-21T07:01:32Z","lastAt":"2021-06-29T11:29:43Z","messageCount":10,"participants":["Ævar Arnfjörð Bjarmason","Jeff King","SZEDER Gábor","Philip Oakley","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"428020","messageId":"patch-1.1-cba5d88ca35-20210621T070114Z-avarab@gmail.com","threadId":"55975","inReplyTo":null,"subject":"[PATCH] progress.c tests: fix breakage with COLUMNS != 80","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-06-21T07:01:23Z","receivedAt":"2021-06-21T07:01:32Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"The tests added in 2bb74b53a49 (Test the progress display, 2019-09-16)\nbroke under anything except COLUMNS=80, i.e. when running them under\nthe \"-v\" mode under a differently sized terminal.\n\nLet's set the expected number of COLUMNS at the start of the test to\nfix that bug. It's handy not do do this in test-progress.c itself, in\ncase we'd like to test for a different number of COLUMNS, either\nmanually or in a future test.\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n t/t0500-progress-display.sh | 5 +++++\n 1 file changed, 5 insertions(+)\n\ndiff --git a/t/t0500-progress-display.sh b/t/t0500-progress-display.sh\nindex 22058b503ac..66c092a0fe3 100755\n--- a/t/t0500-progress-display.sh\n+++ b/t/t0500-progress-display.sh\n@@ -8,6 +8,11 @@ show_cr () {\n \ttr '\\015' Q | sed -e \"s/Q/<CR>\\\\$LF/g\"\n }\n \n+test_expect_success 'setup COLUMNS' '\n+\tCOLUMNS=80 &&\n+\texport COLUMNS\n+'\n+\n test_expect_success 'simple progress display' '\n \tcat >expect <<-\\EOF &&\n \tWorking hard: 1<CR>\n-- \n2.32.0.595.g55105f1e212\n\n"},{"id":"428345","messageId":"YNPISWEBxISC30DW@coredump.intra.peff.net","threadId":"55975","inReplyTo":"patch-1.1-cba5d88ca35-20210621T070114Z-avarab@gmail.com","subject":"Re: [PATCH] progress.c tests: fix breakage with COLUMNS != 80","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-06-23T23:48:25Z","receivedAt":"2021-06-23T23:48:31Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Jun 21, 2021 at 09:01:23AM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> The tests added in 2bb74b53a49 (Test the progress display, 2019-09-16)\n> broke under anything except COLUMNS=80, i.e. when running them under\n> the \"-v\" mode under a differently sized terminal.\n> \n> Let's set the expected number of COLUMNS at the start of the test to\n> fix that bug. It's handy not do do this in test-progress.c itself, in\n> case we'd like to test for a different number of COLUMNS, either\n> manually or in a future test.\n\nHmm. So I can easily reproduce the problem here, and your patch fixes\nit. But my first thought was: shouldn't test-lib.sh be handling this to\ngive all of the scripts a uniform environment?\n\nAnd indeed, we _do_ unset COLUMNS there. So I think the problem isn't\na bad setting of $COLUMNS, but rather that in \"-v\" mode, the\nsub-command's stderr is hooked to our tty, and term_columns() is smart\nenough to use TIOCGWINSZ to get the value (at least on some platforms).\n\nSetting $COLUMNS again in the environment fixes it, because we prefer\nthat value to trying the ioctl.\n\nSo I don't think what you have here is wrong (though the commit message\nis a little misleading). But it seems like the original intent of our\n\"unset COLUMNS\" in test-lib.sh would best be fulfilled by setting it to\na known value there (like 80), rather than unsetting it.\n\nI admit this a _bit_ of a nitpick (since as far as we know none of the\nother scripts care about the terminal width), so I'm OK with this as-is\nif you feel strongly the other way.\n\n-Peff\n"},{"id":"428360","messageId":"20210624051253.GG6312@szeder.dev","threadId":"55975","inReplyTo":"YNPISWEBxISC30DW@coredump.intra.peff.net","subject":"Re: [PATCH] progress.c tests: fix breakage with COLUMNS != 80","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2021-06-24T05:12:53Z","receivedAt":"2021-06-24T05:13:02Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Wed, Jun 23, 2021 at 07:48:25PM -0400, Jeff King wrote:\n> On Mon, Jun 21, 2021 at 09:01:23AM +0200, Ævar Arnfjörð Bjarmason wrote:\n> \n> > The tests added in 2bb74b53a49 (Test the progress display, 2019-09-16)\n> > broke under anything except COLUMNS=80, i.e. when running them under\n> > the \"-v\" mode under a differently sized terminal.\n> > \n> > Let's set the expected number of COLUMNS at the start of the test to\n> > fix that bug. It's handy not do do this in test-progress.c itself, in\n> > case we'd like to test for a different number of COLUMNS, either\n> > manually or in a future test.\n> \n> Hmm. So I can easily reproduce the problem here, and your patch fixes\n> it. But my first thought was: shouldn't test-lib.sh be handling this to\n> give all of the scripts a uniform environment?\n> \n> And indeed, we _do_ unset COLUMNS there. So I think the problem isn't\n> a bad setting of $COLUMNS, but rather that in \"-v\" mode, the\n> sub-command's stderr is hooked to our tty, and term_columns() is smart\n> enough to use TIOCGWINSZ to get the value (at least on some platforms).\n> \n> Setting $COLUMNS again in the environment fixes it, because we prefer\n> that value to trying the ioctl.\n> \n> So I don't think what you have here is wrong (though the commit message\n> is a little misleading).\n\nIt is misleading indeed and needs to be updated.  I did my own\nanalysis and arrived to the same conclusions wrt COLUMNS being unset\nvs. the ioctl() and stderr being a tty.\n\n> But it seems like the original intent of our\n> \"unset COLUMNS\" in test-lib.sh would best be fulfilled by setting it to\n> a known value there (like 80), rather than unsetting it.\n> \n> I admit this a _bit_ of a nitpick (since as far as we know none of the\n> other scripts care about the terminal width), so I'm OK with this as-is\n> if you feel strongly the other way.\n\nI remember one commit-graph test that does check the number of lines\nin the progress output, assuming one progress line per commit-graph\nlayer, which breaks when we break the progress line in a too narrow\nterminal.  Running './t5324-split-commit-graph.sh -v -i' in a 46\ncolumn wide terminal fails for me, but succeeds with 47 columns.\n\nI do suggest setting COLUMNS=80 in 'test-lib.sh'.\n\n"},{"id":"428362","messageId":"YNQawgw0EzG86k8S@coredump.intra.peff.net","threadId":"55975","inReplyTo":"20210624051253.GG6312@szeder.dev","subject":"Re: [PATCH] progress.c tests: fix breakage with COLUMNS != 80","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-06-24T05:40:18Z","receivedAt":"2021-06-24T05:40:20Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 24, 2021 at 07:12:53AM +0200, SZEDER Gábor wrote:\n\n> > I admit this a _bit_ of a nitpick (since as far as we know none of the\n> > other scripts care about the terminal width), so I'm OK with this as-is\n> > if you feel strongly the other way.\n> \n> I remember one commit-graph test that does check the number of lines\n> in the progress output, assuming one progress line per commit-graph\n> layer, which breaks when we break the progress line in a too narrow\n> terminal.  Running './t5324-split-commit-graph.sh -v -i' in a 46\n> column wide terminal fails for me, but succeeds with 47 columns.\n> \n> I do suggest setting COLUMNS=80 in 'test-lib.sh'.\n\nThanks for providing a concrete example. I agree we should just set it\nfor all scripts via test-lib.sh, then.\n\n-Peff\n"},{"id":"428366","messageId":"patch-1.1-765c2793122-20210624T101839Z-avarab@gmail.com","threadId":"55975","inReplyTo":"patch-1.1-cba5d88ca35-20210621T070114Z-avarab@gmail.com","subject":"[PATCH] test-lib.sh: set COLUMNS=80 for --verbose repeatability","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-06-24T10:19:31Z","receivedAt":"2021-06-24T10:19:42Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Some tests will fail under --verbose because while we've unset COLUMNS\nsince b1d645b58ac (tests: unset COLUMNS inherited from environment,\n2012-03-27), we also look for the columns with an ioctl(..,\nTIOCGWINSZ, ...) on some platforms. By setting COLUMNS again we\npreempt the TIOCGWINSZ lookup in pager.c's term_columns(), it'll take\nCOLUMNS over TIOCGWINSZ,\n\nThis fixes the t0500-progress-display.sh test when run as:\n\n    ./t0500-progress-display.sh --verbose\n\nIt broke because of a combination of the this issue and the progress\noutput reacting to the column width since 545dc345ebd (progress: break\ntoo long progress bar lines, 2019-04-12). The\nt5324-split-commit-graph.sh fails in a similar manner due to progress\noutput, see [1] for details.\n\nA more narrow fix here would be to only do this in the --verbose mode,\nbut there's no harm in setting this for everything. If we're not\nconnected to a TTY the COLUMNS setting won't matter.\n\nSee ea77e675e56 (Make \"git help\" react to window size correctly,\n2005-12-18) and ad6c3739a33 (pager: find out the terminal width before\nspawning the pager, 2012-02-12) for how the TIOCGWINSZ code ended up\nin pager.c\n\n1. http://lore.kernel.org/git/20210624051253.GG6312@szeder.dev\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n\nThis replaces a more narrow fix in\nhttps://lore.kernel.org/git/patch-1.1-cba5d88ca35-20210621T070114Z-avarab@gmail.com/,\nwhich as SZEDER points out was also needed by another test using the\nprogress.c code.\n\n t/test-lib.sh | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 54938c64279..1a6ca772d6c 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -406,14 +406,15 @@ LANG=C\n LC_ALL=C\n PAGER=cat\n TZ=UTC\n-export LANG LC_ALL PAGER TZ\n+COLUMNS=80\n+export LANG LC_ALL PAGER TZ COLUMNS\n EDITOR=:\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 # ones.\n-unset VISUAL EMAIL LANGUAGE COLUMNS $(\"$PERL_PATH\" -e '\n+unset VISUAL EMAIL LANGUAGE $(\"$PERL_PATH\" -e '\n \tmy @env = keys %ENV;\n \tmy $ok = join(\"|\", qw(\n \t\tTRACE\n-- \n2.32.0.605.g06ef811e153\n\n"},{"id":"428378","messageId":"YNSbzYvwSC0ZOo+Y@coredump.intra.peff.net","threadId":"55975","inReplyTo":"patch-1.1-765c2793122-20210624T101839Z-avarab@gmail.com","subject":"Re: [PATCH] test-lib.sh: set COLUMNS=80 for --verbose repeatability","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2021-06-24T14:50:53Z","receivedAt":"2021-06-24T14:50:56Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jun 24, 2021 at 12:19:31PM +0200, Ævar Arnfjörð Bjarmason wrote:\n\n> Some tests will fail under --verbose because while we've unset COLUMNS\n> since b1d645b58ac (tests: unset COLUMNS inherited from environment,\n> 2012-03-27), we also look for the columns with an ioctl(..,\n> TIOCGWINSZ, ...) on some platforms. By setting COLUMNS again we\n> preempt the TIOCGWINSZ lookup in pager.c's term_columns(), it'll take\n> COLUMNS over TIOCGWINSZ,\n> \n> This fixes the t0500-progress-display.sh test when run as:\n> \n>     ./t0500-progress-display.sh --verbose\n> \n> It broke because of a combination of the this issue and the progress\n> output reacting to the column width since 545dc345ebd (progress: break\n> too long progress bar lines, 2019-04-12). The\n> t5324-split-commit-graph.sh fails in a similar manner due to progress\n> output, see [1] for details.\n> \n> A more narrow fix here would be to only do this in the --verbose mode,\n> but there's no harm in setting this for everything. If we're not\n> connected to a TTY the COLUMNS setting won't matter.\n\nThanks, this explanation and location make sense to me.\n\n(minor s/the this/this/ in the third paragraph above).\n\n-Peff\n"},{"id":"428380","messageId":"63a4aa3c-590d-f00f-2550-2564964f054a@iee.email","threadId":"55975","inReplyTo":"YNPISWEBxISC30DW@coredump.intra.peff.net","subject":"Re: [PATCH] progress.c tests: fix breakage with COLUMNS != 80","fromName":"Philip Oakley","fromEmail":"philipoakley@iee.email","sentAt":"2021-06-24T15:05:33Z","receivedAt":"2021-06-24T15:05:37Z","isPatch":true,"sender":{"key":"philipoakley@iee.email","avatar":"https://avatars.githubusercontent.com/u/914343?v=4"},"body":"On 24/06/2021 00:48, Jeff King wrote:\n> On Mon, Jun 21, 2021 at 09:01:23AM +0200, Ævar Arnfjörð Bjarmason wrote:\n>\n>> The tests added in 2bb74b53a49 (Test the progress display, 2019-09-16)\n>> broke under anything except COLUMNS=80, i.e. when running them under\n>> the \"-v\" mode under a differently sized terminal.\n>>\n>> Let's set the expected number of COLUMNS at the start of the test to\n>> fix that bug. It's handy not do do this in test-progress.c itself, in\n>> case we'd like to test for a different number of COLUMNS, either\n>> manually or in a future test.\n> Hmm. So I can easily reproduce the problem here, and your patch fixes\n> it. But my first thought was: shouldn't test-lib.sh be handling this to\n> give all of the scripts a uniform environment?\n>\n> And indeed, we _do_ unset COLUMNS there. So I think the problem isn't\n> a bad setting of $COLUMNS, but rather that in \"-v\" mode, the\n> sub-command's stderr is hooked to our tty, and term_columns() is smart\n> enough to use TIOCGWINSZ to get the value (at least on some platforms).\n>\n> Setting $COLUMNS again in the environment fixes it, because we prefer\n> that value to trying the ioctl.\n>\n> So I don't think what you have here is wrong (though the commit message\n> is a little misleading). But it seems like the original intent of our\n> \"unset COLUMNS\" in test-lib.sh would best be fulfilled by setting it to\n> a known value there (like 80), rather than unsetting it.\n>\n> I admit this a _bit_ of a nitpick (since as far as we know none of the\n> other scripts care about the terminal width), so I'm OK with this as-is\n> if you feel strongly the other way.\n>\n> -Peff\nThere has been a similar recent issue [1] on Git for Windows regarding\nthese settings where different terminals had different views  about the\nsize of the display window which resulted in incorrect out puts.\n\nPhilip\n\n[1] https://github.com/git-for-windows/git/issues/3235\n"},{"id":"428546","messageId":"20210627074419.GH6312@szeder.dev","threadId":"55975","inReplyTo":"patch-1.1-765c2793122-20210624T101839Z-avarab@gmail.com","subject":"Re: [PATCH] test-lib.sh: set COLUMNS=80 for --verbose repeatability","fromName":"SZEDER Gábor","fromEmail":"szeder.dev@gmail.com","sentAt":"2021-06-27T07:44:19Z","receivedAt":"2021-06-27T07:44:27Z","isPatch":true,"sender":{"key":"szeder.dev@gmail.com","avatar":"https://avatars.githubusercontent.com/u/116324?v=4"},"body":"On Thu, Jun 24, 2021 at 12:19:31PM +0200, Ævar Arnfjörð Bjarmason wrote:\n> Some tests will fail under --verbose because while we've unset COLUMNS\n> since b1d645b58ac (tests: unset COLUMNS inherited from environment,\n> 2012-03-27), we also look for the columns with an ioctl(..,\n> TIOCGWINSZ, ...) on some platforms. By setting COLUMNS again we\n> preempt the TIOCGWINSZ lookup in pager.c's term_columns(), it'll take\n> COLUMNS over TIOCGWINSZ,\n> \n> This fixes the t0500-progress-display.sh test when run as:\n> \n>     ./t0500-progress-display.sh --verbose\n> \n> It broke because of a combination of the this issue and the progress\n> output reacting to the column width since 545dc345ebd (progress: break\n> too long progress bar lines, 2019-04-12). The\n> t5324-split-commit-graph.sh fails in a similar manner due to progress\n> output, see [1] for details.\n\nThis issue is not progress-specific.\n\nI run the test suite with 'export COLUMNS=10' in 'test-lib.sh' and\ngot:\n\n  t0500-progress-display.sh                (Wstat: 256 Tests: 12 Failed: 10)\n    Failed tests:  1-5, 7-11\n  t4012-diff-binary.sh                     (Wstat: 256 Tests: 13 Failed: 1)\n    Failed test:  13\n  t4052-stat-output.sh                     (Wstat: 256 Tests: 78 Failed: 14)\n    Failed tests:  3-5, 13, 17, 21, 38-41, 49, 52, 55, 57\n  t5510-fetch.sh                           (Wstat: 256 Tests: 181 Failed: 1)\n    Failed test:  175\n  t5324-split-commit-graph.sh              (Wstat: 256 Tests: 36 Failed: 1)\n    Failed test:  22\n  t5150-request-pull.sh                    (Wstat: 256 Tests: 10 Failed: 1)\n    Failed test:  6\n  t4016-diff-quote.sh                      (Wstat: 256 Tests: 5 Failed: 1)\n    Failed test:  5\n\nThen with COLUMNS=238:\n\n  t0500-progress-display.sh                (Wstat: 256 Tests: 12 Failed: 4)\n    Failed tests:  3-6\n  t4012-diff-binary.sh                     (Wstat: 256 Tests: 13 Failed: 1)\n    Failed test:  13\n  t4052-stat-output.sh                     (Wstat: 256 Tests: 78 Failed: 3)\n    Failed tests:  3-5\n\nFrom these only the failures in t0500 and t5324 are caused by the\nprogress display, the rest fail with things like:\n\n  --- expect      2021-06-26 17:07:58.489958439 +0000\n  +++ actual      2021-06-26 17:07:58.957964694 +0000\n  @@ -1,2 +1,2 @@\n    binfile  |   Bin 0 -> 1026 bytes\n  - textfile | 10000 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n  + textfile | 10000 ++++++++\n  \n  --- expect80    2021-06-26 17:07:58.321956193 +0000\n  +++ actual      2021-06-26 17:07:58.333956354 +0000\n  @@ -1 +1 @@\n  - ...aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1 +\n  + ...aaaaaaaaaaaa | 1 +\n  \n  --- expect      2021-06-26 17:09:05.198849586 +0000\n  +++ actual      2021-06-26 17:09:05.194849532 +0000\n  @@ -1,2 +1,2 @@\n  -main                 -> origin/main\n  +main       -> origin/main\n   looooooooooooong-tag -> looooooooooooong-tag\n  \n  --- expect      2021-06-26 17:14:31.819199273 +0000\n  +++ request.fuzzy       2021-06-26 17:14:31.951201026 +0000\n  @@ -16,4 +16,5 @@\n   ----------------------------------------------------------------\n   SHORTLOG\n  \n  -DIFFSTAT\n  + ...nic.txt | 5 +++++\n  + 1 file changed, 5 insertions(+)\n  \n  \n  --- expect      2021-06-26 16:55:41.632510754 +0000\n  +++ actual      2021-06-26 16:55:42.052516149 +0000\n  @@ -1,2 +1,2 @@\n    binfile  |   Bin 0 -> 1026 bytes\n  - textfile | 10000 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n  + textfile | 10000 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++\n  \n  --- expect80    2021-06-26 16:55:41.416507979 +0000\n  +++ actual      2021-06-26 16:55:41.436508236 +0000\n  @@ -1 +1 @@\n  - ...aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1 +\n  + aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa | 1 +\n\nSo there are more diffstat-related failures than progress-related.\n\n> A more narrow fix here would be to only do this in the --verbose mode,\n> but there's no harm in setting this for everything. If we're not\n> connected to a TTY the COLUMNS setting won't matter.\n\nThis is wrong, we check the terminal width even when not connected to\na TTY, therefore COLUMNS definitely matters; all the failures reported\nabove were with '--verbose-log'.\n\n> See ea77e675e56 (Make \"git help\" react to window size correctly,\n> 2005-12-18) and ad6c3739a33 (pager: find out the terminal width before\n> spawning the pager, 2012-02-12) for how the TIOCGWINSZ code ended up\n> in pager.c\n> \n> 1. http://lore.kernel.org/git/20210624051253.GG6312@szeder.dev\n> \n> Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n> ---\n> \n> This replaces a more narrow fix in\n> https://lore.kernel.org/git/patch-1.1-cba5d88ca35-20210621T070114Z-avarab@gmail.com/,\n> which as SZEDER points out was also needed by another test using the\n> progress.c code.\n> \n>  t/test-lib.sh | 5 +++--\n>  1 file changed, 3 insertions(+), 2 deletions(-)\n> \n> diff --git a/t/test-lib.sh b/t/test-lib.sh\n> index 54938c64279..1a6ca772d6c 100644\n> --- a/t/test-lib.sh\n> +++ b/t/test-lib.sh\n> @@ -406,14 +406,15 @@ LANG=C\n>  LC_ALL=C\n>  PAGER=cat\n>  TZ=UTC\n> -export LANG LC_ALL PAGER TZ\n> +COLUMNS=80\n> +export LANG LC_ALL PAGER TZ COLUMNS\n>  EDITOR=:\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>  # ones.\n> -unset VISUAL EMAIL LANGUAGE COLUMNS $(\"$PERL_PATH\" -e '\n> +unset VISUAL EMAIL LANGUAGE $(\"$PERL_PATH\" -e '\n>  \tmy @env = keys %ENV;\n>  \tmy $ok = join(\"|\", qw(\n>  \t\tTRACE\n> -- \n> 2.32.0.605.g06ef811e153\n> \n"},{"id":"428661","messageId":"xmqqczs530qa.fsf@gitster.g","threadId":"55975","inReplyTo":"20210627074419.GH6312@szeder.dev","subject":"Re: [PATCH] test-lib.sh: set COLUMNS=80 for --verbose repeatability","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2021-06-29T01:42:37Z","receivedAt":"2021-06-29T01:42:43Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"SZEDER Gábor <szeder.dev@gmail.com> writes:\n\n> This issue is not progress-specific.\n\nAs discussed elsewhere with Dscho on a Windows specific workaround,\nprogress is not the only consumer of term_columns() and the code\nbeing \"fixed\" here would affect other things like \"diff --stat\" and\nhunk header title (aka funcname).\n\nTogether with a typo Peff pointed out, the patch deserves an updated\nlog message before it becomes ready.  The patch text looks good.\n\nThanks.\n\n\n"},{"id":"428739","messageId":"patch-1.1-bd2bd89a2b-20210629T112910Z-avarab@gmail.com","threadId":"55975","inReplyTo":"patch-1.1-765c2793122-20210624T101839Z-avarab@gmail.com","subject":"[PATCH v2] test-lib.sh: set COLUMNS=80 for --verbose repeatability","fromName":"Ævar Arnfjörð Bjarmason","fromEmail":"avarab@gmail.com","sentAt":"2021-06-29T11:29:36Z","receivedAt":"2021-06-29T11:29:43Z","isPatch":true,"sender":{"key":"avarab@gmail.com","avatar":"https://avatars.githubusercontent.com/u/45301?v=4"},"body":"Some tests will fail under --verbose because while we've unset COLUMNS\nsince b1d645b58ac (tests: unset COLUMNS inherited from environment,\n2012-03-27), we also look for the columns with an ioctl(..,\nTIOCGWINSZ, ...) on some platforms. By setting COLUMNS again we\npreempt the TIOCGWINSZ lookup in pager.c's term_columns(), it'll take\nCOLUMNS over TIOCGWINSZ,\n\nThis fixes t0500-progress-display.sh., which broke because of a\ncombination of the this issue and the progress output reacting to the\ncolumn width since 545dc345ebd (progress: break too long progress bar\nlines, 2019-04-12). The t5324-split-commit-graph.sh fails in a similar\nmanner due to progress output, see [1] for details.\n\nThe issue is not specific to progress.c, the diff code also checks\nCOLUMNS and some of its tests can be made to fail in a similar\nmanner[2], anything that invokes a pager is potentially affected.\n\nSee ea77e675e56 (Make \"git help\" react to window size correctly,\n2005-12-18) and ad6c3739a33 (pager: find out the terminal width before\nspawning the pager, 2012-02-12) for how the TIOCGWINSZ code ended up\nin pager.c\n\n1. http://lore.kernel.org/git/20210624051253.GG6312@szeder.dev\n2. https://lore.kernel.org/git/20210627074419.GH6312@szeder.dev/\n\nSigned-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n---\n\nNo code changes since v1, just the suggested commit message change.\n\nRange-diff against v1:\n1:  765c279312 ! 1:  bd2bd89a2b test-lib.sh: set COLUMNS=80 for --verbose repeatability\n    @@ Commit message\n         preempt the TIOCGWINSZ lookup in pager.c's term_columns(), it'll take\n         COLUMNS over TIOCGWINSZ,\n     \n    -    This fixes the t0500-progress-display.sh test when run as:\n    +    This fixes t0500-progress-display.sh., which broke because of a\n    +    combination of the this issue and the progress output reacting to the\n    +    column width since 545dc345ebd (progress: break too long progress bar\n    +    lines, 2019-04-12). The t5324-split-commit-graph.sh fails in a similar\n    +    manner due to progress output, see [1] for details.\n     \n    -        ./t0500-progress-display.sh --verbose\n    -\n    -    It broke because of a combination of the this issue and the progress\n    -    output reacting to the column width since 545dc345ebd (progress: break\n    -    too long progress bar lines, 2019-04-12). The\n    -    t5324-split-commit-graph.sh fails in a similar manner due to progress\n    -    output, see [1] for details.\n    -\n    -    A more narrow fix here would be to only do this in the --verbose mode,\n    -    but there's no harm in setting this for everything. If we're not\n    -    connected to a TTY the COLUMNS setting won't matter.\n    +    The issue is not specific to progress.c, the diff code also checks\n    +    COLUMNS and some of its tests can be made to fail in a similar\n    +    manner[2], anything that invokes a pager is potentially affected.\n     \n         See ea77e675e56 (Make \"git help\" react to window size correctly,\n         2005-12-18) and ad6c3739a33 (pager: find out the terminal width before\n    @@ Commit message\n         in pager.c\n     \n         1. http://lore.kernel.org/git/20210624051253.GG6312@szeder.dev\n    +    2. https://lore.kernel.org/git/20210627074419.GH6312@szeder.dev/\n     \n         Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>\n     \n\n t/test-lib.sh | 5 +++--\n 1 file changed, 3 insertions(+), 2 deletions(-)\n\ndiff --git a/t/test-lib.sh b/t/test-lib.sh\nindex 54938c6427..1a6ca772d6 100644\n--- a/t/test-lib.sh\n+++ b/t/test-lib.sh\n@@ -406,14 +406,15 @@ LANG=C\n LC_ALL=C\n PAGER=cat\n TZ=UTC\n-export LANG LC_ALL PAGER TZ\n+COLUMNS=80\n+export LANG LC_ALL PAGER TZ COLUMNS\n EDITOR=:\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 # ones.\n-unset VISUAL EMAIL LANGUAGE COLUMNS $(\"$PERL_PATH\" -e '\n+unset VISUAL EMAIL LANGUAGE $(\"$PERL_PATH\" -e '\n \tmy @env = keys %ENV;\n \tmy $ok = join(\"|\", qw(\n \t\tTRACE\n-- \n2.32.0.615.g90fb4d7369\n\n"}]}