You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Vincent Lefevre <vi...@vinc17.net> on 2021/08/24 15:13:57 UTC
Broken pipe on the diff command with --diff-cmd
Hi,
I've found the following issue. When I pipe the output of
"svn diff --diff-cmd diff" and a broken pipe occurs on the
diff command, I get a "Broken pipe" error:
diff: standard output: Broken pipe
svn: E200012: 'diff' returned 2
Tested with svn, version 1.14.1 (r1886195) under Debian 11 (bullseye).
The cause is that svn runs the diff command with SIGPIPE ignored,
so that diff gets an EPIPE write error instead of being killed by
the signal. I think that a fix should be to reset SIGPIPE to the
default action just before executing the external diff command.
To reproduce the bug:
----------------------------------------
#!/bin/sh
set -e
export LC_ALL=C
mkdir my-test-svn
cd my-test-svn
svnadmin create svn
svn co file://`pwd`/svn wc
cd wc
seq 10000 > file
svn add file
svn diff | head
svn diff --diff-cmd diff | head
cd ../..
rm -rf my-test-svn
----------------------------------------
This gives:
Checked out revision 0.
A file
Index: file
===================================================================
--- file (nonexistent)
+++ file (working copy)
@@ -0,0 +1,10000 @@
+1
+2
+3
+4
+5
Index: file
===================================================================
--- file (nonexistent)
+++ file (working copy)
@@ -0,0 +1,10000 @@
+1
+2
+3
+4
+5
diff: standard output: Broken pipe
svn: E200012: 'diff' returned 2
--
Vincent Lefèvre <vi...@vinc17.net> - Web: <https://www.vinc17.net/>
100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/>
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)
Re: Broken pipe on the diff command with --diff-cmd
Posted by Vincent Lefevre <vi...@vinc17.net>.
On 2021-08-25 19:18:49 -0400, Nathan Hartman wrote:
> I don't see mention of it in the issue tracker. Please could you file
> it there?
Done: https://issues.apache.org/jira/browse/SVN-4879
> I started looking into it but ran out of time for now. Anyway, SIGPIPE
> is ignored throughout the client, for good reasons, since r848296:
>
> (relevant bits of log message):
>
> - Ignore SIGPIPE, to prevent wedging the repos on network errors,
> or when piping the output to a command that quits before all data
> is read, e.g. head, more or less.
>
> SIGPIPE is disabled in the client (and 2 other programs) by:
>
> +#ifdef SIGPIPE
> + /* Disable SIGPIPE generation for the platforms that have it. */
> + apr_signal(SIGPIPE, SIG_IGN);
> +#endif
>
> It would be easy to temporarily re-enable it before executing diff-cmd
> but first it's important to make sure that won't break another use
> case which the current disabling is handling. (An example of one
> question that comes to mind: is it possible to use --diff-cmd with
> 'svn log -r#### --diff' and if so are there any ramifications to
> re-enabling SIGPIPE in that scenario? I haven't checked yet.)
I added a simpler suggestion in the report:
A possible fix (to be tested): instead of ignoring SIGPIPE (using
SIG_IGN), use a handler that does nothing. That way, when a command is
executed, SIGPIPE will be reset to the default action in the command
(i.e. killing the command, unless the command itself changes how it
handles SIGPIPE).
--
Vincent Lefèvre <vi...@vinc17.net> - Web: <https://www.vinc17.net/>
100% accessible validated (X)HTML - Blog: <https://www.vinc17.net/blog/>
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)
Re: Broken pipe on the diff command with --diff-cmd
Posted by Nathan Hartman <ha...@gmail.com>.
On Tue, Aug 24, 2021 at 11:14 AM Vincent Lefevre <vi...@vinc17.net>
wrote:
>
> Hi,
>
> I've found the following issue. When I pipe the output of
> "svn diff --diff-cmd diff" and a broken pipe occurs on the
> diff command, I get a "Broken pipe" error:
>
> diff: standard output: Broken pipe
> svn: E200012: 'diff' returned 2
>
> Tested with svn, version 1.14.1 (r1886195) under Debian 11 (bullseye).
>
> The cause is that svn runs the diff command with SIGPIPE ignored,
> so that diff gets an EPIPE write error instead of being killed by
> the signal. I think that a fix should be to reset SIGPIPE to the
> default action just before executing the external diff command.
Confirmed.
I don't see mention of it in the issue tracker. Please could you file
it there?
I started looking into it but ran out of time for now. Anyway, SIGPIPE
is ignored throughout the client, for good reasons, since r848296:
(relevant bits of log message):
- Ignore SIGPIPE, to prevent wedging the repos on network errors,
or when piping the output to a command that quits before all data
is read, e.g. head, more or less.
SIGPIPE is disabled in the client (and 2 other programs) by:
+#ifdef SIGPIPE
+ /* Disable SIGPIPE generation for the platforms that have it. */
+ apr_signal(SIGPIPE, SIG_IGN);
+#endif
It would be easy to temporarily re-enable it before executing diff-cmd
but first it's important to make sure that won't break another use
case which the current disabling is handling. (An example of one
question that comes to mind: is it possible to use --diff-cmd with
'svn log -r#### --diff' and if so are there any ramifications to
re-enabling SIGPIPE in that scenario? I haven't checked yet.)
Thanks,
Nathan