[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

[PATCH] issue #3014 "svn log | head" should not print "Write error: Broken pipe"

From: Stefan Sperling <stsp_at_elego.de>
Date: Mon, 28 Mar 2011 16:03:10 +0200

Here is a patch that attempts to fix issue #3014.

In the issue, Julian suggests that there should be a policy as to when
svn should and should not report errors on broken pipes.
I think that would complicate things too much.
So this patch takes a much simpler approach (see log message).

Does anyone see problems with this approach?
It's a bit hackish, but works.

This patch touches everything expect svnrdump, for now.

[[[
Fix issue #3014 "svn log | head" should not print "Write error: Broken pipe".

Make svn and other commands to exit silently if a pipe write error occured.
The rationale being that the process closing the pipe is also responsible
for printing a related error message, if necessary.

* subversion/libsvn_subr/cmdline.c
  (svn_cmdline_fputs, svn_cmdline_fflush): Return
   SVN_ERR_IO_PIPE_WRITE_ERROR when writes fail due to EPIPE, so that
   callers can choose to ignore this kind of error.
  (svn_cmdline_handle_exit_error): Don't print message for pipe write errors.

* subversion/libsvn_subr/opt.c
  (svn_opt_print_generic_help2): Don't print message for pipe write errors.

* subversion/libsvn_subr/io.c
  (do_io_file_wrapper_cleanup): Same.

* subversion/svn/notify.c
  (notify): Same.

* subversion/svn/main.c
  (main): Same.

* subversion/svn/obliterate-cmd.c
  (notify): Same.

* subversion/svnadmin/main.c
  (main): Same.

* subversion/include/svn_error_codes.h
  (SVN_ERR_IO_PIPE_WRITE_ERROR): New error code.
]]]

Index: subversion/libsvn_subr/cmdline.c
===================================================================
--- subversion/libsvn_subr/cmdline.c (revision 1086209)
+++ subversion/libsvn_subr/cmdline.c (working copy)
@@ -338,7 +338,19 @@ svn_cmdline_fputs(const char *string, FILE* stream
   if (fputs(out, stream) == EOF)
     {
       if (errno)
- return svn_error_wrap_apr(errno, _("Write error"));
+ {
+ err = svn_error_wrap_apr(errno, _("Write error"));
+
+ /* ### Issue #3014: Return a specific error for broken pipes,
+ * ### with a single element in the error chain. */
+ if (APR_STATUS_IS_EPIPE(err->apr_err))
+ {
+ svn_error_clear(err);
+ return svn_error_create(SVN_ERR_IO_PIPE_WRITE_ERROR, NULL, NULL);
+ }
+ else
+ return svn_error_return(err);
+ }
       else
         return svn_error_create
           (SVN_ERR_IO_WRITE_ERROR, NULL, NULL);
@@ -355,7 +367,19 @@ svn_cmdline_fflush(FILE *stream)
   if (fflush(stream) == EOF)
     {
       if (errno)
- return svn_error_wrap_apr(errno, _("Write error"));
+ {
+ svn_error_t *err = svn_error_wrap_apr(errno, _("Write error"));
+
+ /* ### Issue #3014: Return a specific error for broken pipes,
+ * ### with a single element in the error chain. */
+ if (APR_STATUS_IS_EPIPE(err->apr_err))
+ {
+ svn_error_clear(err);
+ return svn_error_create(SVN_ERR_IO_PIPE_WRITE_ERROR, NULL, NULL);
+ }
+ else
+ return svn_error_return(err);
+ }
       else
         return svn_error_create(SVN_ERR_IO_WRITE_ERROR, NULL, NULL);
     }
@@ -376,7 +400,15 @@ svn_cmdline_handle_exit_error(svn_error_t *err,
                               apr_pool_t *pool,
                               const char *prefix)
 {
- svn_handle_error2(err, stderr, FALSE, prefix);
+ /* Issue #3014:
+ * Don't print anything on broken pipes. The pipe was likely
+ * closed by the process at the other end. We expect that
+ * process to perform error reporting as necessary.
+ *
+ * ### This assumes that there is only one error in a chain for
+ * ### SVN_ERR_IO_PIPE_WRITE_ERROR. See svn_cmdline_fputs(). */
+ if (err->apr_err != SVN_ERR_IO_PIPE_WRITE_ERROR)
+ svn_handle_error2(err, stderr, FALSE, prefix);
   svn_error_clear(err);
   if (pool)
     svn_pool_destroy(pool);
Index: subversion/libsvn_subr/opt.c
===================================================================
--- subversion/libsvn_subr/opt.c (revision 1086209)
+++ subversion/libsvn_subr/opt.c (working copy)
@@ -383,7 +383,15 @@ svn_opt_print_generic_help2(const char *header,
   return;
 
  print_error:
- svn_handle_error2(err, stderr, FALSE, "svn: ");
+ /* Issue #3014:
+ * Don't print anything on broken pipes. The pipe was likely
+ * closed by the process at the other end. We expect that
+ * process to perform error reporting as necessary.
+ *
+ * ### This assumes that there is only one error in a chain for
+ * ### SVN_ERR_IO_PIPE_WRITE_ERROR. See svn_cmdline_fputs(). */
+ if (err->apr_err != SVN_ERR_IO_PIPE_WRITE_ERROR)
+ svn_handle_error2(err, stderr, FALSE, "svn: ");
   svn_error_clear(err);
 }
 
Index: subversion/libsvn_subr/io.c
===================================================================
--- subversion/libsvn_subr/io.c (revision 1086209)
+++ subversion/libsvn_subr/io.c (working copy)
@@ -3034,6 +3034,11 @@ do_io_file_wrapper_cleanup(apr_file_t *file, apr_s
     name = NULL;
   svn_error_clear(err);
 
+ /* ### Issue #3014: Return a specific error for broken pipes,
+ * ### with a single element in the error chain. */
+ if (APR_STATUS_IS_EPIPE(status))
+ return svn_error_create(SVN_ERR_IO_PIPE_WRITE_ERROR, NULL, NULL);
+
   if (name)
     return svn_error_wrap_apr(status, _(msg),
                               svn_dirent_local_style(name, pool));
Index: subversion/svn/notify.c
===================================================================
--- subversion/svn/notify.c (revision 1086209)
+++ subversion/svn/notify.c (working copy)
@@ -918,7 +918,15 @@ notify(void *baton, const svn_wc_notify_t *n, apr_
   if (!nb->had_print_error)
     {
       nb->had_print_error = TRUE;
- svn_handle_error2(err, stderr, FALSE, "svn: ");
+ /* Issue #3014:
+ * Don't print anything on broken pipes. The pipe was likely
+ * closed by the process at the other end. We expect that
+ * process to perform error reporting as necessary.
+ *
+ * ### This assumes that there is only one error in a chain for
+ * ### SVN_ERR_IO_PIPE_WRITE_ERROR. See svn_cmdline_fputs(). */
+ if (err->apr_err != SVN_ERR_IO_PIPE_WRITE_ERROR)
+ svn_handle_error2(err, stderr, FALSE, "svn: ");
     }
   svn_error_clear(err);
 }
Index: subversion/svn/main.c
===================================================================
--- subversion/svn/main.c (revision 1086209)
+++ subversion/svn/main.c (working copy)
@@ -2594,8 +2594,17 @@ main(int argc, const char *argv[])
           err = svn_error_quick_wrap(err,
                                      _("Please see the 'svn upgrade' command"));
         }
- svn_handle_error2(err, stderr, FALSE, "svn: ");
 
+ /* Issue #3014:
+ * Don't print anything on broken pipes. The pipe was likely
+ * closed by the process at the other end. We expect that
+ * process to perform error reporting as necessary.
+ *
+ * ### This assumes that there is only one error in a chain for
+ * ### SVN_ERR_IO_PIPE_WRITE_ERROR. See svn_cmdline_fputs(). */
+ if (err->apr_err != SVN_ERR_IO_PIPE_WRITE_ERROR)
+ svn_handle_error2(err, stderr, FALSE, "svn: ");
+
       /* Tell the user about 'svn cleanup' if any error on the stack
          was about locked working copies. */
       for (tmp_err = err; tmp_err; tmp_err = tmp_err->child)
Index: subversion/svn/obliterate-cmd.c
===================================================================
--- subversion/svn/obliterate-cmd.c (revision 1086209)
+++ subversion/svn/obliterate-cmd.c (working copy)
@@ -78,7 +78,16 @@ notify(void *baton, const svn_wc_notify_t *n, apr_
   if (!nb->had_print_error)
     {
       nb->had_print_error = TRUE;
- svn_handle_error2(err, stderr, FALSE, "svn: ");
+
+ /* Issue #3014:
+ * Don't print anything on broken pipes. The pipe was likely
+ * closed by the process at the other end. We expect that
+ * process to perform error reporting as necessary.
+ *
+ * ### This assumes that there is only one error in a chain for
+ * ### SVN_ERR_IO_PIPE_WRITE_ERROR. See svn_cmdline_fputs(). */
+ if (err->apr_err != SVN_ERR_IO_PIPE_WRITE_ERROR)
+ svn_handle_error2(err, stderr, FALSE, "svn: ");
     }
   svn_error_clear(err);
 }
Index: subversion/svnadmin/main.c
===================================================================
--- subversion/svnadmin/main.c (revision 1086209)
+++ subversion/svnadmin/main.c (working copy)
@@ -1975,7 +1975,15 @@ main(int argc, const char *argv[])
       err = svn_cmdline_fflush(stdout);
       if (err)
         {
- svn_handle_error2(err, stderr, FALSE, "svnadmin: ");
+ /* Issue #3014:
+ * Don't print anything on broken pipes. The pipe was likely
+ * closed by the process at the other end. We expect that
+ * process to perform error reporting as necessary.
+ *
+ * ### This assumes that there is only one error in a chain for
+ * ### SVN_ERR_IO_PIPE_WRITE_ERROR. See svn_cmdline_fputs(). */
+ if (err->apr_err != SVN_ERR_IO_PIPE_WRITE_ERROR)
+ svn_handle_error2(err, stderr, FALSE, "svnadmin: ");
           svn_error_clear(err);
           return EXIT_FAILURE;
         }
Index: subversion/include/svn_error_codes.h
===================================================================
--- subversion/include/svn_error_codes.h (revision 1086209)
+++ subversion/include/svn_error_codes.h (working copy)
@@ -279,6 +279,11 @@ SVN_ERROR_START
              SVN_ERR_IO_CATEGORY_START + 6,
              "Write error")
 
+ /** @since New in 1.7. */
+ SVN_ERRDEF(SVN_ERR_IO_PIPE_WRITE_ERROR,
+ SVN_ERR_IO_CATEGORY_START + 7,
+ "Write error in pipe")
+
   /* stream errors */
 
   SVN_ERRDEF(SVN_ERR_STREAM_UNEXPECTED_EOF,
Received on 2011-03-28 16:03:46 CEST

This is an archived mail posted to the Subversion Dev mailing list.