Re: progress test failure on fedora34

[Jeff King <peff@peff.net>]

On Sun, Jul 18, 2021 at 10:05:44AM +0200, Ævar Arnfjörð Bjarmason wrote:

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

That would be truly evil. :)

Another possible source of weirdness: some shells are picky about
assigning to COLUMNS, and are eager to set it themselves. E.g.:

  $ echo $COLUMNS
  119
  $ COLUMNS=80 bash -c 'echo $COLUMNS'
  80
  $ COLUMNS=80 zsh -c 'echo $COLUMNS'
  119

So zsh, even in a non-interactive shell, will set it. It does at least
accept setting it, and will preserve it in sub-shells and forks. But it
will silently ignore invalid values, instead going back to the ioctl:

  $ zsh -c 'COLUMNS=80; echo $COLUMNS; COLUMNS=foo; echo $COLUMNS'
  80
  119

mksh behaves the same way; I was tipped off to this by b082687cba
(test-lib: skip test with COLUMNS=1 under mksh, 2012-04-27).

I have trouble seeing how this could cause a problem since "80" seems
like a perfectly sensible value. And one would imagine that the same
shell is being used in all cases above (but maybe not, depending on the
configuration of the terminal programs?). But it's another possible
avenue of investigation.

> diff --git a/progress.c b/progress.c
> index 680c6a8bf9..dca254b515 100644
> --- a/progress.c
> +++ b/progress.c
> @@ -144,6 +144,7 @@ static void display(struct progress *progress, uint64_t n, const char *done)
>                         size_t progress_line_len = progress->title_len +
>                                                 counters_sb->len + 2;
>                         int cols = term_columns();
> +                       fprintf(stderr, "cols = %d\n", cols);
>  
>                         if (progress->split) {
>                                 fprintf(stderr, "  %s%*s", counters_sb->buf,

Yeah, this seems like a good start to see if the value we're getting is
bogus. Likewise, it might be interesting for term_columns() to tell us
if it's getting the value from $COLUMNS or from the ioctl (but it's hard
to believe it wouldn't be from $COLUMNS, given the code).

-Peff