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

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Aug 5, 2021, 23:35 UTC
Message-ID: <87k0kzzc91.fsf@evledraar.gmail.com>
In-Reply-To: <20210805194825.1796765-1-felipe.contreras@gmail.com>

On Thu, Aug 05 2021, Felipe Contreras wrote:

Show 10 quoted lines

> 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 <fabian.stelzer@campoint.net>
> Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>

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/cover-v3-0.3-00000000000-20210804T230335Z-avarab@gmail.com/
2. https://lore.kernel.org/git/cover-v3-0.3-00000000000-20210804T230335Z-avarab@gmail.com/

Show 15 quoted lines

> ---
>  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.