Alex Henrie wrote:
> On Mon, Jul 19, 2021 at 1:34 PM Felipe Contreras
> <felipe.contreras@xxxxxxxxx> wrote:
> >
> > Felipe Contreras wrote:
> > > Ævar Arnfjörð Bjarmason wrote:
> > > >
> > > > On Wed, Jul 14 2021, Alex Henrie wrote:
> > > >
> > > > > On Wed, Jul 14, 2021 at 9:39 AM Ævar Arnfjörð Bjarmason
> > > > > <avarab@xxxxxxxxx> wrote:
> > > > >>
> > > > >>
> > > > >> On Wed, Jul 14 2021, Fabian Stelzer wrote:
> > > > >>
> > > > >> > Hi,
> > > > >> > The test t0500-progress-display.sh in current master fails on latest
> > > > >> > fedora34.
> > > > >> > The break was introduced with:
> > > > >> >
> > > > >> > 83ae1edff7ee0b7674bd556955d2cf1706bddb21
> > > > >> > ab/fix-columns-to-80-during-tests (2021-06-29) 1 commit
> > > > >> >
> > > > >> > Kind regards,
> > > > >> > Fabian
> > > > >>
> > > > >> I have not been able to reproduce this, it seems the below E-Mail was
> > > > >> word-wrapped by your mailer, which is especially bad here since getting
> > > > >> to the bottom of this requires looking at the whitespace.
> > > > >>
> > > > >> Is there a way you could tar that up and send it (to me personally is
> > > > >> fine, or some pastebin or whatever).
> > > > >>
> > > > >> I am able to reproduce something that looks like this if I
> > > > >> s/COLUMNS=80/COLUMNS=79/g in the test-lib, but given that we set it to
> > > > >> 80, and that the progress.c code just ends up with an
> > > > >> atoi(getenv("COLUMNS")), and we do our own wrapping (with no other fancy
> > > > >> logic) in progress.c, I'm not seeing right now how this could happen...
> > > > >
> > > > > This test also fails for me when using QTerminal or Konsole, but it
> > > > > passes on XTerm and LXTerminal.
> > > >
> > > > I tried this on Debian 11 with QTerminal 0.16.1 and can't reproduce it,
> > > > resized the window etc., always get COLUMNS=80 if I add some printf
> > > > debugging.
> > > >
> > > > Do you mind testing with an ad-hoc patch like this on top? It will fail
> > > > right away, but should say COLUMNS = 80 in the output.
> > > >
> > > > The only thing I can think of right now is that some terminals are doing
> > > > some evil trickery to LD_PRELOAD or whatever and intercept getenv() for
> > > > COLUMNS and the like, but that's an entirely unfounded hunch.
> > >
> > > I'm able to reproduce this. The test fails when running directly with
> > > bash, but not with prove.
> > >
> > > And it seems to be a bug in bash:
> > >
> > > export COLUMNS=80
> > >
> > > echo "COLUMNS: $COLUMNS"
> > > cat > /tmp/expect <<EOF
> > > foobar
> > > EOF
> > > echo "COLUMNS: $COLUMNS"
> > >
> > > I get:
> > >
> > > COLUMNS: 80
> > > COLUMNS: 115
> > >
> > > Even directly in the console.
> >
> > Hmm, from man bash:
> >
> > checkwinsize
> > If set, bash checks the window size after each external (non‐builtin) com‐
> > mand and, if necessary, updates the values of LINES and COLUMNS. This op‐
> > tion is enabled by default.
> >
> > Seems like since bash 5.0 this is on by default.
>
> Indeed, running `shopt -u checkwinsize` right after exporting COLUMNS
> in test-lib.sh fixes the tests. Great work!
Yeah, this fixes it, but it doesn't seem we are setting any
bash-specific options right now, and additionally I don't think bash
should be doing that in the first place. If the shell is
non-interactive, why is checkwinsize being honored?
Moreover, why does it work with prove? I'm investigating that right now,
but so far I haven't found any reason.
--
Felipe Contreras
