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.
Advertising
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
