Re: git: 14a5c1068d37 - main - man: do not ignore the exit status of roff tools

Hi, Wolfram,

On 2024-04-20 01:30, Wolfram Schneider wrote:
> The branch main has been updated by wosch:
> 
> URL: https://cgit.FreeBSD.org/src/commit/?id=14a5c1068d3751173dc41f3097b12e95791b2160
> 
> commit 14a5c1068d3751173dc41f3097b12e95791b2160
> Author:     Wolfram Schneider <wosch@FreeBSD.org>
> AuthorDate: 2024-04-20 08:24:58 +0000
> Commit:     Wolfram Schneider <wosch@FreeBSD.org>
> CommitDate: 2024-04-20 08:30:33 +0000
> 
>      man: do not ignore the exit status of roff tools
>      
>      PR:             223516
>      Approved by:    emaste, bapt
>      Differential Revision:  https://reviews.freebsd.org/D44798
> ---
>   usr.bin/man/man.sh | 8 +++++++-
>   1 file changed, 7 insertions(+), 1 deletion(-)
> 
> diff --git a/usr.bin/man/man.sh b/usr.bin/man/man.sh
> index fd51648757a9..03803b777463 100755
> --- a/usr.bin/man/man.sh
> +++ b/usr.bin/man/man.sh
> @@ -33,6 +33,12 @@
>   # it is better to terminate it.
>   ulimit -t 20
>   
> +# do not ignore the exit status of roff tools
> +set -o pipefail
> +
> +# ignore SIGPIPE exits because pagers may exit before reading all their input.
> +trap '' SIGPIPE
> +
>   # Usage: add_to_manpath path
>   # Adds a variable to manpath while ensuring we don't have duplicates.
>   # Returns true if we were able to add something. False otherwise.
> @@ -312,7 +318,7 @@ man_check_for_so() {
>   	# We need to loop to accommodate multiple .so directives.
>   	while true
>   	do
> -		line=$($cattool "$manpage" | head -n1)
> +		line=$($cattool "$manpage" 2>/dev/null | head -n1)
>   		case "$line" in
>   		.so*)	trim "${line#.so}"
>   			decho "$manpage includes $tstr"

I noticed that after this change, man(1)'ing sh(1) would give me:

mandoc: <stdin>: SYSERR: write: Broken pipe
mandoc: see above the output for SYSERR messages

if I don't finish scrolling through the whole manpage first, which I 
think is not desirable (we should be able to quit $MANPAGER early).

If I do this then the error message would be gone:

diff --git a/usr.bin/man/man.sh b/usr.bin/man/man.sh
index 24a0464689cc..42ae86cfec5b 100755
--- a/usr.bin/man/man.sh
+++ b/usr.bin/man/man.sh
@@ -381,7 +381,7 @@ man_display_page() {
         if [ -n "$tflag" ]; then
                 pipeline="mandoc -Tps $mandoc_args"
         else
-               pipeline="mandoc $mandoc_args | $MANPAGER"
+               pipeline="mandoc $mandoc_args 2>/dev/null | $MANPAGER"
         fi

         if ! $cattool "$manpage" | eval "$testline"; then

but I'm not sure if that would defeat the purpose of the original 
change.  Could you please take a look?

Cheers,