Re: progress test failure on fedora34

From: Jeff King <peff@peff.net>
Date: Jul 19, 2021, 09:00 UTC
Message-ID: <YPU/Enow8Mgn5f4j@coredump.intra.peff.net>
In-Reply-To: <87h7gsvybx.fsf@evledraar.gmail.com>

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

Show 13 quoted lines

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

Show 12 quoted lines

> 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