Subject: Re: [PATCH] test: fix for COLUMNS and bash 5

From: Ævar Arnfjörð Bjarmason <[email-masked]>
Date: Fri, 06 Aug 2021 01:35:42 +0200

On Thu, Aug 05 2021, Felipe Contreras wrote:

> Since c49a177bec (test-lib.sh: set COLUMNS=80 for --verbose
> repeatability, 2021-06-29) multiple tests have been failing when using
> bash 5 because checkwinsize is enabled by default, therefore COLUMNS is
> reset using TIOCGWINSZ even for non-interactive shells.
>
> It's debatable whether or not bash should even be doing that, but for
> now we can avoid this undesirable behavior by disabling this option.
>
> Reported-by: Fabian Stelzer <[email-masked]>
> Signed-off-by: Felipe Contreras <[email-masked]>

I've got an alternative way of solving the same immeditate issue in[1],
there's discussion on that approach in the latest What's Cooking[2]. I
belive this approach would make at least SZEDER happier than with my
series :)

As noted in that discussion I'd prefer going for mine, but would also be
fine with this if it's what Junio decides to pick up. We should have one
or the other before 2.33 is out.

My preference for mine is in no small part that I'd like to not be
responsible for into-the-past test suite breakage the next time a
popular shell decides to be clever about COLUMNS.

But this way we'll solve the immediate problem with bash, and I can say
I told you so if that submarine breakage occurs with this approach :)

1. https://lore.kernel.org/git/[email-masked]/
2. https://lore.kernel.org/git/[email-masked]/

> ---
>  t/test-lib.sh | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/t/test-lib.sh b/t/test-lib.sh
> index db61081d6b..a2b7dfecee 100644
> --- a/t/test-lib.sh
> +++ b/t/test-lib.sh
> @@ -419,6 +419,12 @@ COLUMNS=80
>  export LANG LC_ALL PAGER TZ COLUMNS
>  EDITOR=:
>  
> +# Since bash 5.0, checkwinsize is enabled by default which does update the
> +# COLUMNS variable every time a command completes, even for non-interactive
> +# shells.

I don't think this needs updating, but FWIW I think that around bash 4.X
(I think 4.1, but that's from memory) is where it started doing this for
non-interactive shells, so we could have had breakage going back that
far.

But we're seeing this in practice now because with 5.0 this was turned
on by default.

I.e. I think if you turn on checkwinsize it'll also break with the tests
on bash 4.1 (or whatever it was), but not on 3.x.

None of that's something I've tested, just from memory of skimming the
bash commit logs for changes related to checkwinsize a some days ago
when I came up with my fix for this.