On 29 September 2015 at 15:16, <rhuij...@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()? I mean code like this: [[ 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); } else if (err) return svn_error_trace(err); return svn_error_createf(SVN_ERR_WC_PATH_UNEXPECTED_STATUS, NULL, ... ]]] See attached patch. Otherwise error message could be confusing, in some cases. For example if svn_wc__external_remove() return SVN_ERR_CANCELED error message will be something like: [[[ The external '%s' defined in %s at '%s' cannot be checked out because '%s' is already a versioned path. The operation was interrupted ]]] -- Ivan Zhakov
Index: subversion/libsvn_client/externals.c =================================================================== --- subversion/libsvn_client/externals.c (revision 1710064) +++ subversion/libsvn_client/externals.c (working copy) @@ -250,10 +250,11 @@ { /* New external... No problem that we can't remove it */ svn_error_clear(err); - err = NULL; } + else if (err) + return svn_error_trace(err); - return svn_error_createf(SVN_ERR_WC_PATH_UNEXPECTED_STATUS, err, + return svn_error_createf(SVN_ERR_WC_PATH_UNEXPECTED_STATUS, NULL, _("The external '%s' defined in %s at '%s' " "cannot be checked out because '%s' is " "already a versioned path."),