GNU bug report logs - #72536
[PATCH] tap: new `--merge-prefix' option to prefix stderr before merging

Previous Next

To reply to this bug, email your comments to 72536 AT debbugs.gnu.org.
There is no need to reopen the bug first.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Richard Hansen <rhansen <at> rhansen.org>
To: automake-patches <at> gnu.org
Subject: [PATCH] tap: new `--merge-prefix' option to prefix stderr before
 merging
Date: Fri, 9 Aug 2024 04:38:48 -0400

[Message part 1 (text/plain, inline)]

Attached are a couple of patches to improve the TAP driver.  I don't know if `--merge-prefix' is the best name for the new option; suggestions for alternatives would be appreciated.

Thanks,
Richard


[PATCH 1/2] tap: new `--merge-prefix' option to prefix stderr before merging

* lib/tap-driver.sh: Add a new `--merge-prefix' option to direct the TAP
driver to prefix each line of the test script's standard error with the given
string before merging it with the test script's standard output.  This is
useful when standard error lines might be confused with test results.
* doc/automake.texi: Document the new `--merge-prefix' option.
* t/tap-merge-prefix.sh: New test.
* t/list-of-tests.mk (handwritten_TESTS): Add it.

[PATCH 2/2] tap: close fd 3 when invoking the test script

* lib/tap-driver.sh: Close file descriptor 3 when invoking the test script to
avoid potential conflicts with the test script.

[0001-tap-new-merge-prefix-option-to-prefix-stderr-before-.patch (text/x-patch, attachment)]

[0002-tap-close-fd-3-when-invoking-the-test-script.patch (text/x-patch, attachment)]

[OpenPGP_signature.asc (application/pgp-signature, attachment)]

Message #8 received at 72536 <at> debbugs.gnu.org (full text, mbox):

From: Karl Berry <karl <at> freefriends.org>
To: rhansen <at> rhansen.org
Cc: 72536 <at> debbugs.gnu.org
Subject: Re: [bug#72536] [PATCH] tap: new `--merge-prefix' option to prefix
 stderr before merging
Date: Fri, 9 Aug 2024 19:20:40 -0600

Hi Richard - thanks much for the patch.  It looks quite reasonable.
I'll check and install it as soon as I have a chance. I think we also
need to ask you to sign a copyright disclaimer for this, since it's not
exactly trivial. I'll write you separately about that.

Thanks again,
Karl

Message #11 received at 72536 <at> debbugs.gnu.org (full text, mbox):

From: Jacob Bachmeyer <jcb62281 <at> gmail.com>
To: Richard Hansen <rhansen <at> rhansen.org>
Cc: 72536 <at> debbugs.gnu.org
Subject: Re: [bug#72536] [PATCH] tap: new `--merge-prefix' option to prefix
 stderr before merging
Date: Fri, 09 Aug 2024 21:47:32 -0500

Richard Hansen wrote:
> Attached are a couple of patches to improve the TAP driver.  I don't 
> know if `--merge-prefix' is the best name for the new option; 
> suggestions for alternatives would be appreciated.

I would suggest `--label-stderr' or `--stderr-message-prefix' as better 
option names to open a discussion.  The main problem I see with 
`--merge-prefix' is that it is using "merge" in a sense that can only be 
understood with the detailed context of what the TAP driver does.


-- Jacob

Message #14 received at 72536 <at> debbugs.gnu.org (full text, mbox):

From: Richard Hansen <rhansen <at> rhansen.org>
To: jcb62281 <at> gmail.com
Cc: 72536 <at> debbugs.gnu.org
Subject: Re: [bug#72536] [PATCH] tap: new `--merge-prefix' option to prefix
 stderr before merging
Date: Sat, 10 Aug 2024 02:55:00 -0400

[Message part 1 (text/plain, inline)]

On 2024-08-09 22:47:32-04:00, Jacob Bachmeyer wrote:
> I would suggest `--label-stderr' or `--stderr-message-prefix' as better 
> option names to open a discussion.  The main problem I see with 
> `--merge-prefix' is that it is using "merge" in a sense that can only be 
> understood with the detailed context of what the TAP driver does.

Good point.  How about `--stderr-prefix`?  The downside to not including 
"merge" in the option name is that it isn't obviously tied to the 
`--merge` option.

I could make this option independent of `--merge` and always prefix each 
line of the test script's stderr.  That would make it easier for users 
to understand the option's behavior, even if it would rarely be used 
without `--merge`.

Or I could change it to something like `--merge-with-prefix` and have it 
automatically turn on `--merge`.


On a separate note, it's difficult to portably add a `#` character to a 
make variable, as you can see in the test I added.  Yet that is the one 
character that users will almost certainly want to include in the 
prefix.  I wonder if this should have a default prefix so that users 
don't have to figure out how to pass `#`.  Maybe something like:

  * `--stderr-prefix` by itself to prefix with the default of `# `
  * `--stderr-prefix custom-prefix` to prefix with `custom-prefix`

This would be a bit more complicated to implement because the script 
would need a heuristic to determine whether the user passed a custom 
prefix or not (such as, "there is an argument after `--stderr-prefix` 
and that argument does not begin with `-`").

-Richard


p.s. I requested a copyright assignment form.

[OpenPGP_signature.asc (application/pgp-signature, attachment)]

Message #17 received at 72536 <at> debbugs.gnu.org (full text, mbox):

From: Karl Berry <karl <at> freefriends.org>
To: rhansen <at> rhansen.org
Cc: 72536 <at> debbugs.gnu.org, jcb62281 <at> gmail.com
Subject: Re: [bug#72536] [PATCH] tap: new `--merge-prefix' option to prefix
 stderr before merging
Date: Sat, 10 Aug 2024 15:52:55 -0600

    How about `--stderr-prefix`?  
    ...
    I could make this option independent of `--merge` and always prefix each 
    line of the test script's stderr.

Sounds sensible to me. Clear behavior is good. I like that better than
implicitly turning on --merge.

    I wonder if this should have a default prefix so that users 
    don't have to figure out how to pass `#`. 

Hmm.

       * `--stderr-prefix` by itself to prefix with the default of `# `

I haven't studied this, but I wonder about "#stderr# " as the default.
Maybe that's too verbose in practice.

       * `--stderr-prefix custom-prefix` to prefix with `custom-prefix`
    This would be a bit more complicated to implement because the script 
    would need a heuristic to determine whether the user passed a custom 
    prefix or not (such as, "there is an argument after `--stderr-prefix` 
    and that argument does not begin with `-`").

Optional argument heuristics don't sound ideal to me. tap-driver.sh
takes its final non-option argument, so if the new option ended up last,
there would be:
 tap-driver.sh ... --stderr-prefix TEST-COMMAND
and nothing good would result. There's also the question of what to do
if the possibly-optional argument was "--". Let's just not go there.

Instead, as a nod to practicality aka kludge, I wonder about replacing
the string "sharp" (or "octothorpe", or some such) in the argument with a #.

    difficult to portably add a `#` character to a 
    make variable, as you can see in the test I added.  

Perusing your patch again, I see you used
    printf %03o
to get around that. I hope that's portable in practice.

I wonder if automake actually works under EBCDIC. I wouldn't be
surprised if ASCII is assumed more or less throughout. Anyway, nice
defensive thinking there. :)

    p.s. I requested a copyright assignment form.

Thanks much for all.

Message #20 received at 72536 <at> debbugs.gnu.org (full text, mbox):

From: Jacob Bachmeyer <jcb62281 <at> gmail.com>
To: Karl Berry <karl <at> freefriends.org>
Cc: 72536 <at> debbugs.gnu.org, rhansen <at> rhansen.org
Subject: Re: [bug#72536] [PATCH] tap: new `--merge-prefix' option to prefix
 stderr before merging
Date: Sat, 10 Aug 2024 17:45:53 -0500

Karl Berry wrote:
>     How about `--stderr-prefix`?  
>     ...
>     I could make this option independent of `--merge` and always prefix each 
>     line of the test script's stderr.
>
> Sounds sensible to me. Clear behavior is good. I like that better than
> implicitly turning on --merge.
>   

I like this idea, too.

> [...]
>
> Instead, as a nod to practicality aka kludge, I wonder about replacing
> the string "sharp" (or "octothorpe", or some such) in the argument with a #.
>   

How about recognizing the Autoconf @%:@ quadrigraph for #?  If need be, 
it could be escaped as @%@&t@:@ to get past the regular quadrigraph 
processing.


-- Jacob

Message #23 received at 72536 <at> debbugs.gnu.org (full text, mbox):

From: Richard Hansen <rhansen <at> rhansen.org>
To: 72536 <at> debbugs.gnu.org
Cc: Jacob Bachmeyer <jcb62281 <at> gmail.com>, Karl Berry <karl <at> freefriends.org>
Subject: [PATCH v2] tap: new `--stderr-prefix' option to prefix test script's, 
 stderr
Date: Mon, 12 Aug 2024 03:58:48 -0400

[Message part 1 (text/plain, inline)]

Attached is v2 of the patch series.  Changes from v1:

  * Renamed the option from `--merge-prefix' to `--stderr-prefix'
  * stderr is now always prefixed (even with `--no-merge')
  * Added Autoconf-style quadrigraphs @%:@ (for #) and @&t@ (for empty 
string)
  * Bumped `scriptversion'
  * Reordered the patches (close fd 3 is the 1st patch now)
  * Added examples to the documentation
  * Reworked the tests:
     - `#' is now passed via command-line variable assignment instead 
of in the Makefile, avoiding the octal printf mess
     - quadrigraph replacement is tested
     - tested with and without --merge

I expect to hear back from assign <at> gnu.org today (Monday) with the 
copyright assignment agreement; I'll get that signed and sent ASAP.

Thanks,
Richard

[0001-tap-close-fd-3-before-invoking-the-test-script.patch (text/x-patch, attachment)]

[0002-tap-new-stderr-prefix-option-to-prefix-test-script-s.patch (text/x-patch, attachment)]

[OpenPGP_signature.asc (application/pgp-signature, attachment)]

Message #26 received at 72536 <at> debbugs.gnu.org (full text, mbox):

From: Richard Hansen <rhansen <at> rhansen.org>
To: 72536 <at> debbugs.gnu.org
Cc: Jacob Bachmeyer <jcb62281 <at> gmail.com>, Karl Berry <karl <at> freefriends.org>
Subject: [PATCH v3] tap: new `--stderr-prefix' option to prefix test script's
 stderr
Date: Mon, 3 Feb 2025 19:00:57 -0500

[Message part 1 (text/plain, inline)]

It took a couple of months, but FSF accepted the copyright assignment. 
And then I promptly forgot to follow up on this because I was in the 
middle of something else.  Sorry about that!

Supposedly I have not appeared in copyright.list for Automake yet.  (FSF 
signed on 2024-09-24, so I would expect to be in by now.)  Please let me 
know if I need to do anything, such as forward the mutually signed 
agreement PDF to someone.

Attached is v3 of the patch series.  Changes from v2:
  * Fixed a race condition
  * Fixed accidental discard of test script's final line if the output 
didn't end with a terminating newline
  * Moved the line prefixing logic to a separate function for 
readability and to make it easier to add comments for subtle points
  * The driver's own stderr is no longer mixed with the test script's 
stderr
  * Rewrote the new tests as *.tap
  * Docs now suggest `[stderr]' prefix instead of `<stderr>'
  * Minor improvements to Automake's own test infrastructure

Thanks,
Richard

[0001-tests-turn-off-set-x-in-diag_.patch (text/x-patch, attachment)]

[0002-tests-prefix-every-line-of-diag_-text-not-just-the-f.patch (text/x-patch, attachment)]

[0003-tests-use-printf-instead-of-echo.patch (text/x-patch, attachment)]

[0004-tests-support-getting-diag_-message-from-stdin.patch (text/x-patch, attachment)]

[0005-tests-move-space-from-diag_-to-diag_string_.patch (text/x-patch, attachment)]

[0006-tap-close-fd-3-before-invoking-the-test-script.patch (text/x-patch, attachment)]

[0007-tap-minimize-stderr-redirection-to-just-the-test-scr.patch (text/x-patch, attachment)]

[0008-tap-new-stderr-prefix-option-to-prefix-test-script-s.patch (text/x-patch, attachment)]

[0009-tap-include-malformed-output-in-error-message-to-aid.patch (text/x-patch, attachment)]

[0010-tests-use-stderr-prefix-for-TAP-tests.patch (text/x-patch, attachment)]

[OpenPGP_signature.asc (application/pgp-signature, attachment)]

Message #29 received at 72536 <at> debbugs.gnu.org (full text, mbox):

From: Karl Berry <karl <at> freefriends.org>
To: rhansen <at> rhansen.org
Cc: 72536 <at> debbugs.gnu.org
Subject: Re: [PATCH v3] tap: new `--stderr-prefix' option to prefix test
 script's stderr
Date: Wed, 5 Feb 2025 15:18:33 -0700

Hi Richard,

    Supposedly I have not appeared in copyright.list for Automake yet.

Ah, now I find you listed there. Yay! No need to do anything more on the
copyright front.

    Attached is v3 of the patch series.  Changes from v2:

Thanks much. Will review asap. --best, karl.

Message #32 received at 72536 <at> debbugs.gnu.org (full text, mbox):

From: Karl Berry <karl <at> freefriends.org>
To: rhansen <at> rhansen.org
Cc: 72536 <at> debbugs.gnu.org
Subject: Re: [PATCH v3] tap: new `--stderr-prefix' option to prefix test
 script's stderr
Date: Wed, 19 Feb 2025 10:47:33 -0700

Hi Richard and all,

    Attached is v3 of the patch series.  Changes from v2:

Thanks so much for your careful work. I read through the changes and
nothing problematic jumped out at me.

(Trivial: I think it's conventional for us to use test instead of [,
even when we're assuming a modern shell. I'll tweak that after.)

However ... when I ran make check with the changes installed, the
tap-merge-stdout-stderr test failed. Did it succeed for you? Before I
delved into it, thought I'd check back. Below is the log I got. (I'm
running on Rocky Linux 9, x86_64-linux, though I doubt that matters.)

Thanks again,
Karl

am-test-lib.sh: Running from installcheck: no
am-test-lib.sh: Test Protocol: none
am-test-lib.sh: PATH = /u/karl/gnu/src/akarl/bin:/u/karl/gnu/src/akarl/t/ax:.:/u/karl/bin:/l/bin:/u/texlive/karl/Master/bin/x86_64-linux:/usr/local/gnu/emacs-21.4r9/bin:/usr/local/gnu/bin:/usr/local/bin:/usr/bin:/usr/sbin:/r/tug/home/tubprod/svn/misc:/u/texlive/karl/Master/tlpkg/bin:/r/tug/home/karl/bin
++ pwd
/u/karl/gnu/src/akarl/t/tap-merge-stdout-stderr.dir
+ fetch_tap_driver
+ AM_TAP_AWK=gawk
+ export AM_TAP_AWK
+ get_shell_script tap-driver.sh tap-driver
+ am_source=tap-driver.sh
+ am_target=tap-driver
+ test '!' -f tap-driver
+ test x = xyes
+ cp -f /u/karl/gnu/src/akarl/lib/tap-driver.sh tap-driver
+ sed 10q tap-driver
#! /bin/sh
# Copyright (C) 2011-2025 Free Software Foundation, Inc.
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation; either version 2, or (at your option)
# any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
+ unset am_target am_source
+ cat
+ . tap-setup.sh
++ test -f ../../bin/automake
++ test -f ../../runtest
++ test -d ../../t
++ test '!' -f Makefile.am
++ mv Makefile.am Makefile.am~
++ test -d ../tap-common-setup.dir
++ cp -fpR ../tap-common-setup.dir/Makefile ../tap-common-setup.dir/Makefile.am ../tap-common-setup.dir/Makefile.in ../tap-common-setup.dir/aclocal.m4 ../tap-common-setup.dir/compile ../tap-common-setup.dir/config.log ../tap-common-setup.dir/config.status ../tap-common-setup.dir/configure ../tap-common-setup.dir/configure.ac ../tap-common-setup.dir/depcomp ../tap-common-setup.dir/install-sh ../tap-common-setup.dir/missing .
++ fetch_tap_driver
++ AM_TAP_AWK=gawk
++ export AM_TAP_AWK
++ get_shell_script tap-driver.sh tap-driver
++ am_source=tap-driver.sh
++ am_target=tap-driver
++ test '!' -f tap-driver
++ rm -f tap-driver
++ test x = xyes
++ cp -f /u/karl/gnu/src/akarl/lib/tap-driver.sh tap-driver
++ sed 10q tap-driver
#! /bin/sh
# Copyright (C) 2011-2025 Free Software Foundation, Inc.
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
# the Free Software Foundation; either version 2, or (at your option)
# any later version.
#
# This program is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
++ unset am_target am_source
++ test -f Makefile.am~
++ mv -f Makefile.am~ Makefile.am
++ echo 'TEST_LOG_DRIVER = $(srcdir)/tap-driver'
++ automake-1.17 --foreign -Werror -Wall Makefile
++ ./config.status Makefile
config.status: creating Makefile
++ :
+ cat
+ chmod a+x all.test
+ run_make -O check
+ am__make_redirect_stdout=no
+ am__make_redirect_stderr=no
+ am__make_redirect_stdall=no
+ am__make_flags=
+ am__make_rc_exp=0
+ am_make_rc=0
+ test 2 -gt 0
+ case $1 in
+ am__make_redirect_stdout=yes
+ shift
+ test 1 -gt 0
+ case $1 in
+ break
+ using_gmake
+ case $am__using_gmake in
+ make --version -v
+ grep GNU
GNU Make 4.4.1
License GPLv3+: GNU GPL version 3 or later <https://gnu.org/licenses/gpl.html>
+ am__using_gmake=yes
+ return 0
+ :
+ test x '!=' x
+ : exec make check
+ set +x
make[2]: *** [Makefile:492: test-suite.log] Error 1
make[1]: *** [Makefile:627: check-TESTS] Error 2
make: *** [Makefile:823: check-am] Error 2
+ am_make_rc=2
+ test 2 -eq 253
+ test no = yes
+ test yes = yes
+ cat stdout
make  check-TESTS
make[1]: Entering directory '/u/karl/gnu/src/akarl/t/tap-merge-stdout-stderr.dir'
make[2]: Entering directory '/u/karl/gnu/src/akarl/t/tap-merge-stdout-stderr.dir'
ERROR: all.test 2 # OUT-OF-ORDER (expecting 1)
ERROR: all.test 4 # OUT-OF-ORDER (expecting 2)
ERROR: all.test 1 # OUT-OF-ORDER (expecting 3)
ERROR: all.test 3 # OUT-OF-ORDER (expecting 4)
# all.test: foo foo foo
============================================================================
Testsuite summary for tap-common-setup 1.0
============================================================================
# TOTAL: 4
# PASS:  0
# SKIP:  0
# XFAIL: 0
# FAIL:  0
# XPASS: 0
# ERROR: 4
============================================================================
See ./test-suite.log for debugging.
============================================================================
make[2]: Leaving directory '/u/karl/gnu/src/akarl/t/tap-merge-stdout-stderr.dir'
make[1]: Leaving directory '/u/karl/gnu/src/akarl/t/tap-merge-stdout-stderr.dir'
+ test no = yes
+ case $am__make_rc_exp in
+ test 0 -ge 0
+ test 0 -le 255
+ test 2 -eq 0
+ return 1
+ am_exit_trap 1
+ exit_status=1
+ set +e
+ cd /u/karl/gnu/src/akarl
+ test none = tap
+ case $am_explicit_skips in
+ test 1 -eq 0
+ keep_testdirs=yes
+ am_keeping_testdirs
+ case $keep_testdirs in
+ return 0
+ set +x
FAIL t/tap-merge-stdout-stderr.sh (exit status: 1)

Message #35 received at 72536 <at> debbugs.gnu.org (full text, mbox):

From: Karl Berry <karl <at> freefriends.org>
To: rhansen <at> rhansen.org
Cc: 72536 <at> debbugs.gnu.org
Subject: Re: [PATCH v3] tap: new `--stderr-prefix' option to prefix test
 script's stderr
Date: Wed, 19 Feb 2025 10:48:02 -0700

A separate question (for anyone), when we get that far ... what's the
best way to apply/commit a patch series like this?  Sorry for my lack of
git-fu. When I run "git am mboxfile" after saving your msg in mboxfile,
it applies fine, but then git log -n 1 -p shows that only the first
Subject: is used, apparently losing the information you nicely put into
all the other Subject:s. Or will all the different subjects show up
again when it's pushed, somehow?

I did a bunch of searching without great results. Maybe I have to save
each msg in a separate file and then apply the directory? Seems like
that should not be necessary. --thanks, karl.

Message #38 received at 72536 <at> debbugs.gnu.org (full text, mbox):

From: Jacob Bachmeyer <jcb62281 <at> gmail.com>
To: Karl Berry <karl <at> freefriends.org>, rhansen <at> rhansen.org
Cc: 72536 <at> debbugs.gnu.org
Subject: Re: [bug#72536] [PATCH v3] tap: new `--stderr-prefix' option to
 prefix test script's stderr
Date: Wed, 19 Feb 2025 20:59:00 -0600

[Message part 1 (text/plain, inline)]

On 2/19/25 11:48, Karl Berry wrote:
> A separate question (for anyone), when we get that far ... what's the
> best way to apply/commit a patch series like this?  Sorry for my lack of
> git-fu. When I run "git am mboxfile" after saving your msg in mboxfile,
> it applies fine, but then git log -n 1 -p shows that only the first
> Subject: is used, apparently losing the information you nicely put into
> all the other Subject:s.

A quick check of `git help log` suggests that you were asking `git log` 
to only output one record.  That it outputs the first Subject and not 
the last suggests that git-am may have only applied one patch instead of 
the whole series.

>   Or will all the different subjects show up
> again when it's pushed, somehow?

No, a push exactly transfers your local history and branch tip.

> I did a bunch of searching without great results. Maybe I have to save
> each msg in a separate file and then apply the directory?

I seem to recall that that is what I have always had to do:  one message 
per file.

>   Seems like that should not be necessary.

I suspect that you are having problems with git-mailsplit but I do not 
know how to fix the issue.


-- Jacob

[Message part 2 (text/html, inline)]

Message #41 received at 72536 <at> debbugs.gnu.org (full text, mbox):

From: Richard Hansen <rhansen <at> rhansen.org>
To: Karl Berry <karl <at> freefriends.org>
Cc: 72536 <at> debbugs.gnu.org
Subject: Re: [PATCH v3] tap: new `--stderr-prefix' option to prefix test
 script's stderr
Date: Thu, 20 Feb 2025 01:15:49 -0500

[Message part 1 (text/plain, inline)]

On 2/19/25 12:47, Karl Berry wrote:
> However ... when I ran make check with the changes installed, the
> tap-merge-stdout-stderr test failed. Did it succeed for you? Before I
> delved into it, thought I'd check back.

Doh!  There's a race condition and no portable (POSIX) way to fix it, I 
think.  The proposed change spawns an asynchronous subshell (a pipeline 
stage) to read stderr, prefix each line, and write the results to 
stdout.  That subshell is a separate process/thread that races with the 
direct writes to stdout.

The only race-free* fix is to use a select or poll loop to read from 
both stderr and stdout simultaneously from a single thread. 
Unfortunately, there is no portable way (that I know of) to use those 
system interfaces from a shell script.  Perl, Python, etc. can do it, 
but those add a new dependency.  It's not practical to ship a C file and 
compile that when the user runs the tests (right?).

I can reduce the severity of the out-of-order writes by (a) not piping 
stderr through the prefixing function when the prefix is the empty 
string, and (b) avoiding stdout buffering when stderr is prefixed. 
These changes might provide good enough results for most users.  Users 
that require stronger guarantees can wrap their test program in a custom 
helper utility rather than rely on Automake's TAP driver to do the 
prefixing and merging.

Thoughts and suggestions would be appreciated.

-Richard


*Even a select/poll loop wouldn't be completely race-free: if the test 
process writes to both stdout and stderr while a previous write is still 
being processed then the next loop iteration won't know whether the 
stdout or the stderr write happened first.

[OpenPGP_signature.asc (application/pgp-signature, attachment)]

Message #44 received at 72536 <at> debbugs.gnu.org (full text, mbox):

From: Richard Hansen <rhansen <at> rhansen.org>
To: Karl Berry <karl <at> freefriends.org>
Cc: 72536 <at> debbugs.gnu.org
Subject: Re: [PATCH v3] tap: new `--stderr-prefix' option to prefix test
 script's stderr
Date: Thu, 20 Feb 2025 01:22:58 -0500

[Message part 1 (text/plain, inline)]

On 2/19/25 12:48, Karl Berry wrote:
> A separate question (for anyone), when we get that far ... what's the
> best way to apply/commit a patch series like this?

  1. Save all of the *.patch attachments to disk.
  2. Run `git am /path/to/*.patch`.

Usually I use `git send-email` to email commits; that creates a separate 
email per commit.  I'm not sure why I attached them instead; maybe I was 
mimicking what others have historically done on this mailing list?  I 
can switch to a separate email per commit if preferred.

-Richard

[OpenPGP_signature.asc (application/pgp-signature, attachment)]

Message #47 received at 72536 <at> debbugs.gnu.org (full text, mbox):

From: Karl Berry <karl <at> freefriends.org>
To: rhansen <at> rhansen.org
Cc: 72536 <at> debbugs.gnu.org
Subject: Re: [PATCH v3] tap: new `--stderr-prefix' option to prefix test
 script's stderr
Date: Thu, 20 Feb 2025 16:23:01 -0700

Hi Richard and Jacob,

    rh> can switch to a separate email per commit if preferred.

No need. For most purposes, it's much more convenient to have one email
with the whole patch series than many individual messages. I'll save the
patches to separate files.

    jb> I suspect that you are having problems with git-mailsplit 

Never heard of it before, didn't come up in my searches. Makes sense
now.  Thanks much. -k

Message #50 received at 72536 <at> debbugs.gnu.org (full text, mbox):

From: Karl Berry <karl <at> freefriends.org>
To: rhansen <at> rhansen.org
Cc: 72536 <at> debbugs.gnu.org
Subject: Re: [PATCH v3] tap: new `--stderr-prefix' option to prefix test
 script's stderr
Date: Thu, 20 Feb 2025 16:23:01 -0700

    It's not practical to ship a C file and 
    compile that when the user runs the tests (right?).

Right. Also right that we don't want to induce a dependency on
perl/python/whatever for running tests.

    (a) not piping stderr through the prefixing function when the prefix
    is the empty string, and (b) avoiding stdout buffering when stderr
    is prefixed.

Those sound desirable in any case. Especially the first.
It's highly desirable for existing behavior to remain the same, as much
as possible.

    These changes might provide good enough results for most users. 

I suppose so, since "most users" won't be using the stderr prefixing
since it's never been available before :).

I guess that would fix the problem with the existing
tap-merge-stdout-stderr test, since then there should be no change from
how it currently works.

But what about the new test you wrote?  Isn't any test that a) outputs
to both stdout and stderr and b) uses the new merge prefix going to be
subject to the race condition?

    Users that require stronger guarantees can wrap their test program
    in a custom helper utility rather than rely on Automake's TAP driver
    to do the prefixing and merging.

That feels like asking an awful lot of the users. If we have to resort
to that, it sounds like something we should show an example of. And
maybe use for the new test, if that's possible.

Overall, a change that creates a known race condition with no fix feels
bad to me. We can be sure that the people who run automake tests with -j
cranked up will be reporting it, and "there's no fix, just ignore the
failure" isn't an acceptable answer to automated build systems.

    Thoughts and suggestions would be appreciated.

Maybe the answer is for the new test to use --no-merge, or otherwise
avoid the race condition by outputting all of stdout and then all of
stderr (with the prefix). Which admittedly somewhat, though not
entirely, defeats the purpose of the new change, but we just can't have
a test that will randomly fail.

In practice, outside the Automake test suite, I would guess that your
alleviations above would often suffice, as you suggest. On the other
hand, inducing race conditions in a package's test just because it
happens to use the new merge prefixing doesn't seem desirable, even if
it would rarely be noticed.

Or am I misunderstanding? Wdyt? --thanks, karl.

Message #53 received at 72536 <at> debbugs.gnu.org (full text, mbox):

From: Jacob Bachmeyer <jcb62281 <at> gmail.com>
To: Karl Berry <karl <at> freefriends.org>, rhansen <at> rhansen.org
Cc: 72536 <at> debbugs.gnu.org
Subject: Re: [bug#72536] [PATCH v3] tap: new `--stderr-prefix' option to
 prefix test script's stderr
Date: Thu, 20 Feb 2025 18:24:15 -0600

On 2/20/25 17:23, Karl Berry wrote:
> Hi Richard and Jacob,
>
>      rh> can switch to a separate email per commit if preferred.
>
> No need. For most purposes, it's much more convenient to have one email
> with the whole patch series than many individual messages. I'll save the
> patches to separate files.
>
>      jb> I suspect that you are having problems with git-mailsplit
>
> Never heard of it before, didn't come up in my searches. Makes sense
> now.  Thanks much.

As far as I can tell, Git expects the "one patch per message" model, 
since it extracts the subject header and body of the message to form the 
commit message.

To use one email with the whole patch series, you will have to split out 
the patches manually.


-- Jacob

Message #56 received at 72536 <at> debbugs.gnu.org (full text, mbox):

From: Jacob Bachmeyer <jcb62281 <at> gmail.com>
To: Karl Berry <karl <at> freefriends.org>, rhansen <at> rhansen.org
Cc: 72536 <at> debbugs.gnu.org
Subject: Re: [bug#72536] [PATCH v3] tap: new `--stderr-prefix' option to
 prefix test script's stderr
Date: Thu, 20 Feb 2025 18:29:30 -0600

On 2/20/25 17:23, Karl Berry wrote:
>      It's not practical to ship a C file and
>      compile that when the user runs the tests (right?).
>
> Right. Also right that we don't want to induce a dependency on
> perl/python/whatever for running tests.
>
>      (a) not piping stderr through the prefixing function when the prefix
>      is the empty string, and (b) avoiding stdout buffering when stderr
>      is prefixed.
>
> Those sound desirable in any case. Especially the first.
> It's highly desirable for existing behavior to remain the same, as much
> as possible.

Could we filter both stdout and stderr through functions that timestamp 
each line, then merge the two and sort on the timestamps?

Something like (while read line; do echo `date` "$line"; done) for the 
timestamping function?  (OK, so we would need to get high-precision 
timestamps from date(1) for that to work...)


-- Jacob

Message #59 received at 72536 <at> debbugs.gnu.org (full text, mbox):

From: Karl Berry <karl <at> freefriends.org>
To: jcb62281 <at> gmail.com
Cc: 72536 <at> debbugs.gnu.org, rhansen <at> rhansen.org
Subject: Re: [bug#72536] [PATCH v3] tap: new `--stderr-prefix' option to
 prefix test script's stderr
Date: Fri, 21 Feb 2025 16:11:42 -0700

    we would need to get high-precision timestamps from date(1) 

Sounds like a nightmare :(. -k

Message #62 received at 72536 <at> debbugs.gnu.org (full text, mbox):

From: Richard Hansen <rhansen <at> rhansen.org>
To: Karl Berry <karl <at> freefriends.org>, Jacob Bachmeyer <jcb62281 <at> gmail.com>
Cc: 72536 <at> debbugs.gnu.org
Subject: Re: [PATCH v3] tap: new `--stderr-prefix' option to prefix test
 script's stderr
Date: Fri, 21 Feb 2025 19:20:40 -0500

[Message part 1 (text/plain, inline)]

On 2/20/25 18:23, Karl Berry wrote:
>> (a) not piping stderr through the prefixing function when the prefix 
>> is the empty string, and (b) avoiding stdout buffering when stderr 
>> is prefixed.
> 
> Those sound desirable in any case. Especially the first. 
> It's highly desirable for existing behavior to remain the same, as much 
> as possible.

OK, will do.

> But what about the new test you wrote?  Isn't any test that a) outputs 
> to both stdout and stderr and b) uses the new merge prefix going to be 
> subject to the race condition?

To be clear, the only negative effect of the race condition is that 
stderr lines might appear out of order relative to stdout lines, even 
when --merge is passed.  (All stderr lines are still processed in order 
relative to other stderr lines, and the same goes for the stdout lines 
relative to other stdout lines.)  That reordering shouldn't break any 
user's TAP tests because they shouldn't be outputting TAP commands to 
stderr anyway (and certainly not if stderr lines are prefixed).

The reordering might break some of the tests in Automake that test the 
TAP driver itself, but only if the tests care about line ordering 
between stderr and stdout.  The new test I added doesn't care, so it's 
fine.  The t/tap-merge-stdout-stderr.sh test does care about order, 
which is why it broke.

I think the only major downside to the reordering is that it is harder 
for users to understand logs when trying to figure out a bug in their 
own code (like it is with --no-merge).  Users can remove --stderr-prefix 
(once I make the changes proposed in my previous email) or they can run 
the test program directly (not via the TAP driver) to get a more 
accurate understanding of output timing.

The order of processing (and logging) should not differ from the order 
of the test program's write(2) calls by much.  Out-of-order processing 
only happens if the test program writes to stdout then stderr but the 
kernel wakes up the TAP driver's thread/process that is reading stderr 
before it wakes up the TAP driver's thread/process that is reading 
stdout.  (Or vice-versa.)

As I mentioned before, the out-of-order processing can also happen with 
select/poll (the kernel might not wake up the reading thread/process 
until after the test program writes to both stdout and stderr), but 
select/poll is considerably less expensive than context switches (IIUC) 
so the odds of in-order processing are better.

>> Users that require stronger guarantees can wrap their test program 
>> in a custom helper utility rather than rely on Automake's TAP driver 
>> to do the prefixing and merging.
> 
> That feels like asking an awful lot of the users.

I'm hoping most users won't care much about minor reordering; that 
having --stderr-prefix is better than not having it even if it has some 
reordering issues.  But maybe not---maybe it would be better to omit the 
feature if it doesn't quite meet users' expectations.

I can make it an error to specify both --stderr-prefix and --merge so 
that users don't expect in-order processing when they pass 
--stderr-prefix.  Later, if further testing shows that --stderr-prefix 
and --merge together isn't too bad, we can remove the restriction.

> If we have to resort 
> to that, it sounds like something we should show an example of. And 
> maybe use for the new test, if that's possible.

I've been frustrated enough by shell's lack of select/poll that I'm 
tempted to create a new utility to help with cases like this.  For example:

    fdmux --mode=line "1:[stdout] %l" "2:[stderr] %l" -- foo arg

would run 'foo arg1', select/poll its FDs 1 and 2, prefix stdout lines 
with "[stdout] " and stderr lines with "[stderr] ", and write the 
prefixed lines to stdout.

If a utility like that was widely available then devs could wrap their 
test programs in it and not bother with the TAP driver's --stderr-prefix 
and --merge options.  The problem is that there's a near 0% chance such 
a utility would be added to POSIX, which means it wouldn't be widely 
available.  A pure shell+awk script like Automake's TAP driver can run 
just about everywhere.

-Richard

[OpenPGP_signature.asc (application/pgp-signature, attachment)]

Message #65 received at 72536 <at> debbugs.gnu.org (full text, mbox):

From: Richard Hansen <rhansen <at> rhansen.org>
To: jcb62281 <at> gmail.com, Karl Berry <karl <at> freefriends.org>
Cc: 72536 <at> debbugs.gnu.org
Subject: Re: [bug#72536] [PATCH v3] tap: new `--stderr-prefix' option to
 prefix test script's stderr
Date: Fri, 21 Feb 2025 19:21:29 -0500

[Message part 1 (text/plain, inline)]

On 2/20/25 19:29, Jacob Bachmeyer wrote:
> Could we filter both stdout and stderr through functions that timestamp 
> each line, then merge the two and sort on the timestamps?

Unfortunately this wouldn't help---it would be subject to the same 
reordering race condition.  The timestamp value that is added to a line 
would depend on when the kernel woke up the reading thread/process, so 
the timestamps wouldn't always accurately represent the time the test 
program wrote the line.  If the kernel woke up the stderr and stdout 
threads in the opposite order of writing, then the timestamps would be 
in the wrong order.

-Richard

[OpenPGP_signature.asc (application/pgp-signature, attachment)]

Message #68 received at 72536 <at> debbugs.gnu.org (full text, mbox):

From: Richard Hansen <rhansen <at> rhansen.org>
To: Karl Berry <karl <at> freefriends.org>, Jacob Bachmeyer <jcb62281 <at> gmail.com>
Cc: 72536 <at> debbugs.gnu.org
Subject: [PATCH v4] tap: new `--stderr-prefix' option to prefix test script's
 stderr
Date: Fri, 21 Feb 2025 19:39:52 -0500

[Message part 1 (text/plain, inline)]

Attached is v4 of the patch series.  Changes from v3:

  * Don't process stderr if not prefixing stderr (stderr and stdout 
will refer to the same underlying file if --merge is passed, preserving 
the previous behavior)
  * Flush each line of stdout when prefixing to hopefully reduce 
out-of-order processing of stderr vs. stdout.
  * Document that --stderr-prefix can cause out-of-order processing, 
even with --merge.
  * Re-wrap comments at column 80.
  * Use `test ...` instead of `[ ... ]`.
  * Add tests for added terminating newline on final stderr line.
  * Add some additional comments to the new tests to make them easier 
to read.

[0001-tests-turn-off-set-x-in-diag_.patch (text/x-patch, attachment)]

[0002-tests-prefix-every-line-of-diag_-text-not-just-the-f.patch (text/x-patch, attachment)]

[0003-tests-use-printf-instead-of-echo.patch (text/x-patch, attachment)]

[0004-tests-support-getting-diag_-message-from-stdin.patch (text/x-patch, attachment)]

[0005-tests-move-space-from-diag_-to-diag_string_.patch (text/x-patch, attachment)]

[0006-tap-close-fd-3-before-invoking-the-test-script.patch (text/x-patch, attachment)]

[0007-tap-minimize-stderr-redirection-to-just-the-test-scr.patch (text/x-patch, attachment)]

[0008-tap-new-stderr-prefix-option-to-prefix-test-script-s.patch (text/x-patch, attachment)]

[0009-tap-include-malformed-output-in-error-message-to-aid.patch (text/x-patch, attachment)]

[0010-tests-use-stderr-prefix-for-TAP-tests.patch (text/x-patch, attachment)]

[OpenPGP_signature.asc (application/pgp-signature, attachment)]

Message #71 received at 72536 <at> debbugs.gnu.org (full text, mbox):

From: Jacob Bachmeyer <jcb62281 <at> gmail.com>
To: Richard Hansen <rhansen <at> rhansen.org>, Karl Berry <karl <at> freefriends.org>
Cc: 72536 <at> debbugs.gnu.org
Subject: Re: [bug#72536] [PATCH v3] tap: new `--stderr-prefix' option to
 prefix test script's stderr
Date: Fri, 21 Feb 2025 18:40:27 -0600

On 2/21/25 18:21, Richard Hansen wrote:
> On 2/20/25 19:29, Jacob Bachmeyer wrote:
>> Could we filter both stdout and stderr through functions that 
>> timestamp each line, then merge the two and sort on the timestamps?
>
> Unfortunately this wouldn't help---it would be subject to the same 
> reordering race condition.  The timestamp value that is added to a 
> line would depend on when the kernel woke up the reading 
> thread/process, so the timestamps wouldn't always accurately represent 
> the time the test program wrote the line.  If the kernel woke up the 
> stderr and stdout threads in the opposite order of writing, then the 
> timestamps would be in the wrong order.

Now we are getting into the "crazy hack" category, but how about an 
LD_PRELOAD module that gets put in the test process and wraps write(3) 
to add timestamps?

On systems with GNU libc, that module could replace stdout and stderr 
with custom streams that prepend timestamps to the start of each line 
before calling write(2).

For an even crazier (but even less portable) hack, I seem to recall that 
there is a way to attach a BPF program to a process as a syscall filter 
in current Linux kernels.  Could we use *that* to wrap write(2) and add 
timestamps?  :-)


-- Jacob

Message #74 received at 72536 <at> debbugs.gnu.org (full text, mbox):

From: Karl Berry <karl <at> freefriends.org>
To: rhansen <at> rhansen.org, 72536 <at> debbugs.gnu.org
Subject: Re: [PATCH v3] tap: new `--stderr-prefix' option to prefix test
 script's stderr
Date: Sun, 23 Feb 2025 10:53:01 -0700

Hi Richard,

    Attached is v4 of the patch series.  Changes from v3:

I pushed it. Thanks!!

    the only negative effect of the race condition 

I adapted some of this text for the manual, since it was helpful at
least for me to gain some sort of understanding of what might happen and
when. Thanks for writing everything, both the patches and the email :),
in such detail.

    I'm hoping most users won't care much about minor reordering; 

I expect you are right.

    I can make it an error to specify both --stderr-prefix and --merge 

Let's release it as you wrote it and see what develops.

     fdmux --mode=line "1:[stdout] %l" "2:[stderr] %l" -- foo arg

You could propose it to coreutils ...  --thanks again, karl.

P.S. Jacob: thanks for the crazy hacks :). I'm content to leave them as
thought experiments :).

Message #77 received at 72536 <at> debbugs.gnu.org (full text, mbox):

From: Karl Berry <karl <at> freefriends.org>
To: rhansen <at> rhansen.org, 72536 <at> debbugs.gnu.org
Subject: Re: [PATCH v3] tap: new `--stderr-prefix' option to prefix test
 script's stderr
Date: Sun, 23 Feb 2025 10:59:32 -0700

Hi Richard - here's my tinkering with the description in the manual
(pushed).  Let me know if you see any problems.

Meanwhile, I guess we can close this. If anything more is pending from
your side, let me know or feel free to re-open. Thanks again. -k

-----------------------------------------------------------------------------
doc: move most --stderr-prefix doc to a new subsubsection.

Still in https://bugs.gnu.org/72536.

* doc/automake.texi (TAP setup and examples):
(TAP stderr examples),
TAP stderr prefixing): split TAP examples
into new subsubsections. Add discussion of race condition and
other reordering from the open bug.
* NEWS: mention --stderr-prefix.
diff --git a/NEWS b/NEWS
index 34876f3db..26b433314 100644
--- a/NEWS
+++ b/NEWS
@@ -6,6 +6,9 @@ New in 1.x:

 * New features added

+  - New option --stderr-prefix for tap-driver.sh, to prefix each line of
+    stderr from a test script with a given string. (bug#72536)
+
   - Support for Algol 68 added, based on the GNU Algol 68 compiler. (bug#75807)

   - New option dist-bzip3 for bzip3 compression of distributions. (bug#73795)
@@ -18,6 +21,9 @@ New in 1.x:
   - Avoid Perl 5.41.8+ precedence warning for use of !!.
     (https://lists.gnu.org/archive/html/automake/2025-01/msg00000.html)

+  - a Perl path containing whitespace now emits a warning instead of
+    an error, so ./configure PERL='/usr/bin/env perl' can work. (bug#74453)
+
   - The py-compile script once again does nothing (successfully) if the
     PYTHON environment variable is set to ":", or anything that isn't a
     Python interpreter (according to $PYTHON -V). Exception: if PYTHON
diff --git a/doc/automake.texi b/doc/automake.texi
index 9fa94689f..bb66aea5c 100644
--- a/doc/automake.texi
+++ b/doc/automake.texi
@@ -10516,6 +10516,7 @@ with the @samp{#} character) in the testsuite progress output too; by
 default, TAP diagnostics are only copied to the @file{.log} file.
 @item --no-comments
 Revert the effects of @option{--comments}.
+
 @item --merge
 Instruct the test driver to merge the test scripts' standard error into
 their standard output.  This is necessary if you want to ensure that
@@ -10526,8 +10527,10 @@ tracing active).  As a downside, this option might cause the test
 harness to get confused if anything that appears on standard error
 looks like a test result.  Use @option{--stderr-prefix} to prefix each
 line of standard error to avoid this problem.
+
 @item --no-merge
 Revert the effects of @option{--merge}.
+
 @item --stderr-prefix @var{STRING}
 Prefix each line of the test script's standard error with
 @code{@var{STRING}}.  Defaults to the empty string.  This option makes
@@ -10549,39 +10552,13 @@ literal characters @samp{@@%:@@} in the prefix (@samp{@@%@@&t@@:@@}, for
 example).
 @end table

-For example, in shell, @samp{--stderr-prefix "@@%:@@[stderr] "} will
-prefix each line of the test script's standard error with
-@samp{#[stderr] }.
-
-When prefixing standard error lines, the test program's standard error
-lines can be processed out of order relative to the test program's
-standard output lines, even when @option{--merge} is used.  This can be
-due to many factors, including:
-
-@itemize @bullet
-@item
-Output buffering in the test program: The test program might be
-accumulating output data and writing it to the operating system in
-batches, not as soon as it is available.  You can try
-@uref{man:setvbuf(3),,setvbuf(3)} to change the buffering mode in the
-test program, or you can add regular flushes.  OS-specific utilities to
-control buffering may be available, such as
-@uref{man:stdbuf(1),,stdbuf(1)} on GNU/Linux systems.
+For example, the option @samp{--stderr-prefix "@@%:@@[stderr] "}
+passed on the command line will prefix each line of the test script's
+standard error with @samp{#[stderr] }.

-@item
-Concurrent processing of standard output and standard error: Due to a
-limitation of the shell command language, the TAP driver processes
-standard output in a different thread or process than standard error.
-Thus, processing order is influenced by how the operating system
-schedules the two threads or processes for execution.
-
-@item
-Line-based processing: When prefixing standard error, the TAP driver
-waits until a complete line is read from the test program before
-processing the line.  This reduces the chances that characters from a
-standard output line are mixed with characters from a standard error
-line.
-@end itemize
+For more details on @option{--stderr-prefix} processing, especially
+the occasional output reordering due to a race condition, @pxref{TAP
+prefixing stderr}.

 @item --diagnostic-string @var{STRING}
 Change the string that introduces TAP diagnostics from the default value
@@ -10596,7 +10573,18 @@ the ``official'' TAP protocol does not allow for such a customization; so
 don't use it if you can avoid it.
 @end table

-@noindent
+@menu
+* TAP setup and examples:: Setup and examples of TAP usage.
+* TAP stderr examples::    Examples of stderr issues and processing.
+* TAP prefixing stderr::   Details about @option{--stderr-prefix} processing.
+@end menu
+
+@node TAP setup and examples
+@subsubsection TAP setup and examples
+
+@cindex TAP setup, example of
+@cindex setting up TAP, example
+
 Here is an example of how the TAP driver can be set up and used.

 @c Keep in sync with tap-doc2.sh
@@ -10670,7 +10658,19 @@ PASS: baz.test 1
 ...
 % @kbd{echo exit status: $?}
 exit status: 0
+@end example

+@node TAP stderr examples
+@subsubsection TAP stderr examples
+
+@cindex TAP stderr example
+@cindex stderr example in TAP
+
+Here is a sequence of examples showing the issue with merging stdout
+and stderr, and the resolution with @option{--stderr-prefix}.  Read
+the comments for specifics.
+
+@example
 % @kbd{cat stderr-demo.test}
 #!/bin/sh
 echo 'ok 1'
@@ -10721,9 +10721,69 @@ PASS: stderr-demo.test 1
 ok 2
 PASS: stderr-demo.test 2
 1..2
-
 @end example

+@node TAP prefixing stderr
+@subsubsection TAP prefixing stderr
+
+@cindex TAP prefixing stderr
+@cindex prefixing stderr in TAP
+@cindex stderr prefixing in TAP
+@cindex race condition in stderr prefixing
+
+@c see https://bugs.gnu.org/72536 for this feature and discussion.
+
+When using the above @option{--stderr-prefix} TAP option, the test
+program's standard error lines might occasionally be processed out of
+order relative to the test program's standard output lines,
+particularly when @option{--merge} is also used.  This can be due to
+many factors, including:
+
+@itemize @bullet
+@item
+Output buffering in the test program: The test program might be
+accumulating output data and writing it to the operating system in
+batches, not as soon as it is available.  You can try setvbuf(3) to
+change the buffering mode in the test program, or you can add regular
+flushes.  OS-specific utilities to control buffering may be available,
+such as stdbuf(1) on GNU/Linux systems.
+
+@item
+Concurrent processing of standard output and standard error: Due to a
+limitation of the shell command language, the TAP driver processes
+standard output in a different thread or process than standard error,
+causing a race condition.  Thus, processing order is influenced by how
+the operating system schedules the two threads or processes for
+execution.
+
+@item
+Line-based processing: When prefixing standard error, the TAP driver
+waits until a complete line is read from the test program before
+processing the line.  This reduces the chances that characters from a
+standard output line are mixed with characters from a standard error
+line.
+@end itemize
+
+Thus, stderr lines might appear out of order relative to stdout lines,
+even when @option{--merge} is passed.  All stderr lines are still
+processed in order relative to other stderr lines, and the same goes
+for the stdout lines relative to other stdout lines.  So the
+reordering shouldn't break a package's TAP tests, since there's no
+reason to output TAP commands to stderr; even more so when stderr
+lines are prefixed.
+
+When working on a particular test, if you find the reordering is
+causing trouble, you can omit @option{--stderr-prefix}, or run the
+test directly (not via TAP) to get a more accurate understanding of
+output timing.
+
+Overall, though, the order of processing (and logging) should not
+differ from the order of the test program's write(2) calls by much.
+Out-of-order processing only happens if the test program writes to
+stdout then stderr but the kernel wakes up the TAP driver's
+thread/process that is reading stderr before it wakes up the TAP
+driver's thread/process that is reading stdout, or vice versa.
+
 @node Incompatibility with other TAP parsers and drivers
 @subsection Incompatibility with other TAP parsers and drivers


compile finished at Sun Feb 23 09:55:43 2025

Message #85 received at 72536 <at> debbugs.gnu.org (full text, mbox):

From: Richard Hansen <rhansen <at> rhansen.org>
To: Karl Berry <karl <at> freefriends.org>, 72536 <at> debbugs.gnu.org
Subject: [PATCH] tap: followups for the new `--stderr-prefix' option
Date: Sun, 23 Feb 2025 20:23:15 -0500

[Message part 1 (text/plain, inline)]

On 2/23/25 12:53, Karl Berry wrote:
> I pushed it. Thanks!!

Thank you!  And thank you for the documentation improvements.

I have some minor follow-up commits, attached:

  1. Minor documentation revisions.
  2. Change command_ok_ to support shell functions.
  3. New test to check stdout/stderr processing order.

-Richard

[0001-doc-Revise-stderr-prefix-documentation.patch (text/x-patch, attachment)]

[0002-tests-add-shell-function-support-to-command_ok_.patch (text/x-patch, attachment)]

[0003-tests-test-TAP-stdout-stderr-order-with-merge-stderr.patch (text/x-patch, attachment)]

[OpenPGP_signature.asc (application/pgp-signature, attachment)]

Message #88 received at 72536 <at> debbugs.gnu.org (full text, mbox):

From: Karl Berry <karl <at> freefriends.org>
To: rhansen <at> rhansen.org
Cc: 72536 <at> debbugs.gnu.org
Subject: Re: [PATCH] tap: followups for the new `--stderr-prefix' option
Date: Mon, 24 Feb 2025 15:41:54 -0700

    I have some minor follow-up commits, attached:

Looks good to me. Pushed. Thanks again, Richard. -k

Message #91 received at 72536 <at> debbugs.gnu.org (full text, mbox):

From: Karl Berry <karl <at> freefriends.org>
To: rhansen <at> rhansen.org
Cc: 72536 <at> debbugs.gnu.org
Subject: Re: [PATCH] tap: followups for the new `--stderr-prefix' option
Date: Thu, 27 Feb 2025 14:01:00 -0700

Hi Richard - a friend (Nelson Beebe) ran the automake test suite on
numerous systems on the 1.17.90 pretest.  There was a plethora of
failures, and they seem to related to the new stderr-prefix failure and
shell quoting. Maybe.

It seems that the prefixing happened in general? That tap output doesn't
make much sense to me at first blush, I'm afraid. Anyway, I'll append
one particular failure (on Debian12/x86 system), and send you
separately a tarball with full logs. (I can't attach it here because my
friend doesn't want all his system info floating around in public. It's
fine to show non-identified data.)

Or, briefly looking at the logs, I wonder if his incredibly complex
$HOME/config.site file could be interfering with the tests -- though it
never did before. Maybe we somehow have to disable that for automake
test. Hmm.

(Aside: looks like our "compatible" change to py-compile is broken
too. Everything's broken. Sigh.)

Help? I'll be mostly offline the next couple of days, but hope to look
at it all in more detail this weekend. Thanks,
Karl

Here is the list of all failures on his Debian 12/x86 system:
34 lines matching "^FAIL:"
  11535:FAIL: t/instspc
  14168:FAIL: t/instspc.tap 16 - ampersand in destdir
  14541:FAIL: t/instspc.tap 19 - leftpar in builddir
  14579:FAIL: t/instspc.tap 20 - leftpar in destdir
  14628:FAIL: t/instspc.tap 21 - rightpar in builddir
  14676:FAIL: t/instspc.tap 22 - rightpar in destdir
  14801:FAIL: t/instspc.tap 23 - pipe in builddir
  14902:FAIL: t/instspc.tap 24 - pipe in destdir
  16959:FAIL: t/instspc.tap 37 - comma in builddir
  17040:FAIL: t/instspc.tap 38 - comma in destdir
  17472:FAIL: t/instspc.tap 41 - semicol in builddir
  17573:FAIL: t/instspc.tap 42 - semicol in destdir
  18001:FAIL: t/instspc.tap 45 - less in builddir
  18122:FAIL: t/instspc.tap 46 - less in destdir
  18227:FAIL: t/instspc.tap 47 - more in builddir
  18300:FAIL: t/instspc.tap 48 - more in destdir
  20029:FAIL: t/instspc.tap 59 - space in builddir
  20128:FAIL: t/instspc.tap 60 - space in destdir
  20232:FAIL: t/instspc.tap 61 - tab in builddir
  20331:FAIL: t/instspc.tap 62 - tab in destdir
  21630:FAIL: t/instspc.tap 72 - quadrigraph0 in destdir
  21733:FAIL: t/instspc.tap 73 - quadrigraph1 in builddir
  21831:FAIL: t/instspc.tap 74 - quadrigraph1 in destdir
  21935:FAIL: t/instspc.tap 75 - quadrigraph2 in builddir
  22033:FAIL: t/instspc.tap 76 - quadrigraph2 in destdir
  22153:FAIL: t/instspc.tap 77 - quadrigraph3 in builddir
  22261:FAIL: t/instspc.tap 78 - quadrigraph3 in destdir
  22672:FAIL: t/instspc.tap 81 - a_b in builddir
  22773:FAIL: t/instspc.tap 82 - a_b in destdir
  22890:FAIL: t/instspc.tap 83 - a__b in builddir
  23008:FAIL: t/instspc.tap 84 - a__b in destdir
  26584:FAIL: t/nodef
  26730:FAIL: t/nodef2
  27924:FAIL: t/py-compile-usage

And here is what I see for one particular failure:
FAIL: t/instspc.tap 16 - ampersand in destdir
#[stderr]+ result_ 'not ok' -D '' -r '' -- 'ampersand in destdir'
#[stderr]+ set +x
#[stderr]+ cd /local/build/cc/automake-1.17.90/t/instspc.dir
#[stderr]+ not am_keeping_testdirs
#[stderr]+ am_keeping_testdirs
#[stderr]+ case $keep_testdirs in
#[stderr]+ return 1
#[stderr]+ test 'not ok' = ok
#[stderr]+ : For shells with busted 'set -e.'
#[stderr]+ for test_name in $test_names_list
#[stderr]+ eval 'test_string=${instspc__percent}'
#[stderr]++ test_string=%
#[stderr]+ test x% = x
#[stderr]+ mkdir ./%
#[stderr]+ case $test_string in
#[stderr]+ for where in build dest
#[stderr]+ case $where in
#[stderr]+ build=./%
#[stderr]+ dest=/local/build/cc/automake-1.17.90/t/instspc.dir/dest-percent
#[stderr]+ cd ./%
#[stderr]+ r=ok
#[stderr]+ ../configure --prefix /%-prefix

Message #96 received at 72536 <at> debbugs.gnu.org (full text, mbox):

From: Richard Hansen <rhansen <at> rhansen.org>
To: Karl Berry <karl <at> freefriends.org>
Cc: 72536 <at> debbugs.gnu.org
Subject: Re: [PATCH] tap: followups for the new `--stderr-prefix' option
Date: Thu, 27 Feb 2025 18:11:27 -0500

[Message part 1 (text/plain, inline)]

On 2/27/25 16:01, Karl Berry wrote:
> Hi Richard - a friend (Nelson Beebe) ran the automake test suite on
> numerous systems on the 1.17.90 pretest.  There was a plethora of
> failures, and they seem to related to the new stderr-prefix failure and
> shell quoting. Maybe.

Looking.  Feel free to revert all of my recent changes until this is 
figured out -- we can always replay them.

[OpenPGP_signature.asc (application/pgp-signature, attachment)]

Message #99 received at 72536 <at> debbugs.gnu.org (full text, mbox):

From: Richard Hansen <rhansen <at> rhansen.org>
To: Karl Berry <karl <at> freefriends.org>
Cc: 72536 <at> debbugs.gnu.org
Subject: Re: [PATCH] tap: followups for the new `--stderr-prefix' option
Date: Thu, 27 Feb 2025 18:36:06 -0500

[Message part 1 (text/plain, inline)]

On 2/27/25 16:01, Karl Berry wrote:
> Or, briefly looking at the logs, I wonder if his incredibly complex
> $HOME/config.site file could be interfering with the tests -- though it
> never did before. Maybe we somehow have to disable that for automake
> test. Hmm.

On first glance it appears to be a quoting issue with $HOME/.config.site:

    #[stderr]+ ../configure --prefix '/&-prefix'
    configure: loading site script <redacted>/.config.site
    [...]
    #[stderr]<redacted>/.config.site: line 57: -prefix/lib64: No such 
file or directory
    [...]
    #[stderr]+ make all
    gcc -DPACKAGE_NAME=\"instspc\" -DPACKAGE_TARNAME=\"instspc\" 
-DPACKAGE_VERSION=\"1.0\" -DPACKAGE_STRING=\"instspc\ 1.0\" 
-DPACKAGE_BUGREPORT=\"\" -DPACKAGE_URL=\"\" -DPACKAGE=\"instspc\" 
-DVERSION=\"1.0\" -I. -I..   -I/&-prefix/include  -I/&-prefix/include 
-MT source.o -MD -MP -MF .deps/source.Tpo -c -o source.o ../source.c
    #[stderr]/bin/bash: line 1: -prefix/include: No such file or directory
    #[stderr]/bin/bash: line 1: -prefix/include: No such file or directory
    #[stderr]make: *** [Makefile:663: source.o] Error 127

On my system I see:

    [...]
    #[stderr]+ make all
    gcc -DPACKAGE_NAME=\"instspc\" -DPACKAGE_TARNAME=\"instspc\" 
-DPACKAGE_VERSION=\"1.0\" -DPACKAGE_STRING=\"instspc\ 1.0\" 
-DPACKAGE_BUGREPORT=\"\" -DPACKAGE_URL=\"\" -DPACKAGE=\"instspc\" 
-DVERSION=\"1.0\" -I. -I..     -g -O2 -MT source.o -MD -MP -MF 
.deps/source.Tpo -c -o source.o ../source.c

Note that on the passing system the `-I/&-prefix/include 
-I/&-prefix/include` arguments aren't passed.  The unquoted `&` in those 
arguments is definitely causing problems on the failing system, but I 
don't yet know if the bug is in $HOME/.config.site or if the 
$HOME/.config.site is exposing a bug that isn't normally exposed.

[OpenPGP_signature.asc (application/pgp-signature, attachment)]

Message #102 received at 72536 <at> debbugs.gnu.org (full text, mbox):

From: Richard Hansen <rhansen <at> rhansen.org>
To: Karl Berry <karl <at> freefriends.org>
Cc: 72536 <at> debbugs.gnu.org
Subject: Re: [PATCH] tap: followups for the new `--stderr-prefix' option
Date: Thu, 27 Feb 2025 20:14:01 -0500

[Message part 1 (text/plain, inline)]

On 2/27/25 18:36, Richard Hansen wrote:
> On first glance it appears to be a quoting issue with $HOME/.config.site:

I got a copy of the config.site and confirmed that there are some 
under-quoted variable expansions in it.  I'm not sure why it didn't fail 
before.

Rather than rely on the system's config.site to be correct, I think it's 
best to force CONFIG_SITE to /dev/null to disable any custom site 
defaults while running the tests.  I sent a patch to a separate bug:

  https://debbugs.gnu.org/76622

[OpenPGP_signature.asc (application/pgp-signature, attachment)]

Message #105 received at 72536 <at> debbugs.gnu.org (full text, mbox):

From: Karl Berry <karl <at> freefriends.org>
To: rhansen <at> rhansen.org
Cc: 72536 <at> debbugs.gnu.org
Subject: Re: [PATCH] tap: followups for the new `--stderr-prefix' option
Date: Fri, 28 Feb 2025 10:05:54 -0700

Hi Richard - thanks so much for looking into this. I hope Nelson's
config.site is all it is.  That would explain why the tests worked for
everyone else. I agree with your patch, in any case, will install soonly.
I'm surprised it has never come up before.

Now if only we could figure out the AM_TRY_RUN thing. If you have any
ideas, they would be welcome :).
https://lists.gnu.org/archive/html/automake/2025-02/msg00032.html
and following ...

Thanks,
Karl

Message #108 received at 72536 <at> debbugs.gnu.org (full text, mbox):

From: Karl Berry <karl <at> freefriends.org>
To: 72536 <at> debbugs.gnu.org
Subject: Re: bug#72536: closed ([PATCH] tap: new `--merge-prefix' option to
 prefix stderr before merging)
Date: Fri, 11 Apr 2025 11:08:25 -0600

With the disabling of config.site for running automake's tests (thanks
Richard), hopefully this is done. Closing.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.