Re: [PATCH] Re: ra_local doesn't report post-commit errors

Daniel Shahaf wrote on Sat, Sep 17, 2016 at 06:52:44 +0000:
> What concerns me at the moment is that the patch makes deltify_etc()
> return an error in situations where previously it did not. I think
> there are three possible solutions to this: revv the API deltify_etc()
> implements; make the change in 1.10 as an API erratum; or move the new
> logic from deltify_etc() to the libsvn_client consumers (svn and
> svnmucc). Haven't yet chosen among them.

Looking into this, there are two warning comments in the cmdline
client's commit callback, svn_cl__print_commit_info().

One of them is from r857282 (r17208), almost 11 years old now, saying
that scripts may interpret having stderr as implying "commit failed".
The other, 18 months young, is similar:

  /* Be very careful with returning errors from this callback as those
     will be returned as errors from editor->close_edit(...), which may
     cause callers to assume that the commit itself failed.
     ⋮
   */

It seems to me that:

- If there is a post-commit error, its error chain should be printed to
  stderr. We can signal to scripts that the commit succeeded by setting
  the exit code to EXIT_SUCCESS or by using "W160013" rather than
  "E160013" for the error messages ("W" for "warning").

- svn_cl__print_commit_info() should just return an error if there was
  a post-commit error. If that confuses some libsvn_client caller into
  wrongly thinking the commit failed, then that caller has a bug. (The
  commit callback can already return an error even if the commit
  succeeded, for example, whenever writing to stdout fails.)

Cheers,

Daniel
Received on 2016-09-19 09:01:28 CEST