git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Felipe Contreras <felipe.contreras@gmail.com>
Cc: git@vger.kernel.org, "ZheNing Hu" <adlternative@gmail.com>,
	"Fabian Stelzer" <fabian.stelzer@campoint.net>,
	"SZEDER Gábor" <szeder.dev@gmail.com>,
	"Junio C Hamano" <gitster@pobox.com>
Subject: Re: [PATCH] test: fix for COLUMNS and bash 5
Date: Fri, 06 Aug 2021 01:35:42 +0200	[thread overview]
Message-ID: <87k0kzzc91.fsf@evledraar.gmail.com> (raw)
In-Reply-To: <20210805194825.1796765-1-felipe.contreras@gmail.com>


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

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

  reply	other threads:[~2021-08-05 23:46 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-05 19:48 [PATCH] test: fix for COLUMNS and bash 5 Felipe Contreras
2021-08-05 23:35 ` Ævar Arnfjörð Bjarmason [this message]
2021-08-06 16:15   ` Felipe Contreras
2021-08-06 14:49 ` SZEDER Gábor
2021-08-06 16:59   ` Junio C Hamano
2021-08-23 12:59     ` Ævar Arnfjörð Bjarmason
2021-08-23 20:24       ` Junio C Hamano
2021-08-06 16:44 ` [PATCH v2] " Felipe Contreras
2021-08-07  1:37   ` ZheNing Hu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87k0kzzc91.fsf@evledraar.gmail.com \
    --to=avarab@gmail.com \
    --cc=adlternative@gmail.com \
    --cc=fabian.stelzer@campoint.net \
    --cc=felipe.contreras@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=szeder.dev@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).