Ross Mark <rossm@controllingedge.com.au> writes:
I like the basic idea. Have you run the regression tests with this
patch applied?
> --- subversion/libsvn_client/externals.c (revision 12580)
> +++ subversion/libsvn_client/externals.c (working copy)
> @@ -285,15 +285,7 @@
> }
> else if (! compare_external_items (new_item, old_item))
> {
> - /* ### Better yet, compare_external_items should report the
> - nature of the difference. That way, when it's just a change
> - in the "-r REV" portion, for example, we could do an update
> - here instead of a relegation followed by full checkout. */
> -
> - SVN_ERR (relegate_external (path,
> - ib->ctx->cancel_func,
> - ib->ctx->cancel_baton,
> - ib->pool));
> + svn_error_t *err;
>
> /* First notify that we're about to handle an external. */
> if (ib->ctx->notify_func)
> @@ -306,12 +298,41 @@
> svn_wc_notify_state_unknown,
> SVN_INVALID_REVNUM);
>
> - SVN_ERR (svn_client__checkout_internal (NULL, new_item->url, path,
> - &(new_item->revision),
> - &(new_item->revision),
> - TRUE, /* recurse */
> - ib->timestamp_sleep,
> - ib->ctx, ib->pool));
> + /* Try doing a switch first */
> + err = svn_client_switch (NULL,
> + path,
> + new_item->url,
> + &(new_item->revision),
> + TRUE,
> + ib->ctx,
> + ib->pool);
> + if (err)
> + {
> + /* The switch failed so do a cleanup and delete the old*/
> + svn_error_clear (err);
> + err = svn_client_cleanup(path, ib->ctx, ib->pool);
What's the reason for calling svn_client_cleanup? Given that cleanup
will steal WC locks it's probably not the right thing to do.
> + if (err)
> + svn_error_clear(err);
Hmm, that ignores all errors, is that the right thing to do or should
it only be certain specific errors? I suppose one could argue that
the original code would always do the relegate/checkout stuff and so
all errors is OK. I'm not sure what's best here.
> +
> + /* ### Better yet, compare_external_items should report the
> + nature of the difference. That way, when it's just a change
> + in the "-r REV" portion, for example, we could do an update
> + here instead of a relegation followed by full checkout. */
> +
> + SVN_ERR (relegate_external (path,
> + ib->ctx->cancel_func,
> + ib->ctx->cancel_baton,
> + ib->pool));
> +
> +
> +
> + SVN_ERR (svn_client__checkout_internal (NULL, new_item->url, path,
> + &(new_item->revision),
> + &(new_item->revision),
> + TRUE, /* recurse */
> + ib->timestamp_sleep,
> + ib->ctx, ib->pool));
> + }
> }
--
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Jan 4 21:38:52 2005