Re: [Quilt-dev] workaround "received broken pipe signal" bash bug (was [patch 2/8])

[Jean Delvare]

Hi Gary, John,

[Gary V. Vaughan]
> This does (I think there needs to be a separate long running non-shell
> process generating the output):
>
> $ cvs diff | (exit 1)

Unfortunately this is not a portable test (you need to be in a CVS
repository.)

This seems to work for me:
cat /dev/zero | true

Note that your "cvs diff" example above is interesting by itself. I
quite frequently run "cvs diff | less" and happened to get "Broken
pipe" error messages when quitting less before hitting the bottom of
the file. Obviously, now that I recompiled bash with the fix, it
doesn't show anymore. This clearly demonstrates that the problem is
with bash in the first place, and several other applications or use
cases are affected, not just quilt, so there's nothing to be fixed on
our side.

> There is a compile time option to bash (DONT_REPORT_SIGPIPE) which
> turns off the error message, but it is not part of the default build,
> so many vendor supplied and self built bash installations exhibit the
> problem.

At least Red Hat/Fedora and Debian enable that compile time option.
Gentoo did not the last time I checked but I would be very surprised if
they don't change their mind soon. Slackware doesn't have the option
and (unfortunately) I don't expect them to change. Can't remember
which of Mandriva and SuSE has it, I'll check again. The bottom line is
that 3 out of 6 major Linux distributions already use the compile time
option. I still need to check *BSD.

To me the solution is quite simple. Any distribution providing a bash
package and a quilt package have to enable the compile time option. If
they don't, this is a distribution bug and they have to fix it. For
other users, they have to install quilt by themselves already, so
installing bash shouldn't be a problem.

> It's not just Apple's bash :-(  Here is a different workaround that
> might be less contentious (my mailer will almost certainly wrap long
> lines, so you'll need to apply by hand):
>
> Index: quilt-HEAD/scripts/patchfns.in
> ===================================================================
> --- quilt-HEAD.orig/scripts/patchfns.in
> +++ quilt-HEAD/scripts/patchfns.in
> @@ -607,7 +607,8 @@ patch_header()
>   {
>          @AWK@ '
>          $1 == "***" || $1 == "---" \
> -               { exit }
> +               { stop=1 }
> +       stop    { next }

I don't buy this, sorry. This hurts readability and performance. And we
would need to do this in several places. And we would then have a hard
time remembering that some constructs should be avoided in later
additions to quilt. So let's just not do it.

If you really want things to improve, you should:
* Ask your distribution to enable the compile time option in their bash
package (unless they already do it.)
* Ask the bash people to change their mind and at least make it a runtime
option, if not the default.
If enough people ask for this, it might happen.

Thanks,
--
Jean Delvare