> -----Original Message-----
> From: Ivan Zhakov [mailto:ivan_at_visualsvn.com]
> Sent: maandag 26 oktober 2015 09:05
> To: dev_at_subversion.apache.org; Bert Huijben <bert_at_qqmail.nl>
> Subject: Re: svn commit: r1705843 - in /subversion/trunk/subversion:
> libsvn_client/externals.c tests/cmdline/externals_tests.py
>
> On 29 September 2015 at 15:16, <rhuijben_at_apache.org> wrote:
> > Author: rhuijben
> > Date: Tue Sep 29 12:16:04 2015
> > New Revision: 1705843
> >
> > URL: http://svn.apache.org/viewvc?rev=1705843&view=rev
> > Log:
> > Remove registration of external in the EXTERNALS table when we find a
> > versioned node that takes its place during update.
> >
>
> >
> ==========================================================
> ====================
> > --- subversion/trunk/subversion/libsvn_client/externals.c (original)
> > +++ subversion/trunk/subversion/libsvn_client/externals.c Tue Sep 29
> 12:16:04 2015
> > @@ -231,6 +231,40 @@ switch_dir_external(const char *local_ab
> >
> > if (node_url)
> > {
> > + svn_boolean_t is_wcroot;
> > +
> > + SVN_ERR(svn_wc__is_wcroot(&is_wcroot, ctx->wc_ctx,
> local_abspath,
> > + pool));
> > +
> > + if (! is_wcroot)
> > + {
> > + /* This can't be a directory external! */
> > +
> > + err = svn_wc__external_remove(ctx->wc_ctx, defining_abspath,
> > + local_abspath,
> > + TRUE /* declaration_only */,
> > + ctx->cancel_func, ctx->cancel_baton,
> > + pool);
> > +
> > + if (err && err->apr_err == SVN_ERR_WC_PATH_NOT_FOUND)
> > + {
> > + /* New external... No problem that we can't remove it */
> > + svn_error_clear(err);
> > + err = NULL;
> > + }
> > +
> > + return
> svn_error_createf(SVN_ERR_WC_PATH_UNEXPECTED_STATUS, err,
> Hi Bert,
>
> I'm not sure that passing error from svn_wc__external_remove() to
> error chain is the good idea. Why do not return it in the same way
> like we handle error from svn_wc__is_wcroot()?
That first call can typically never error (as the location is at least below the primary working copy and the result already in the hashtable in wc_db), which makes the error handling mostly a no-op.
In the second case the current code guarantees that the actual result is always the same error... a pattern we use in many cases. The secondary error is not that interesting, while the unexpected status us.
The external_remove code only tries to remove the bad information to help you not see this error again, as you would have before this patch. If it fails nothing is lost. (But I don't like to just clear error chains).
Most likely users will never see the internal error anyway, as some outside code filters it to the top level error code only in the notification handling. (Processing errors in externals are usually not fatal)
Bert
Received on 2015-10-26 10:36:49 CET