[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] Re: "svn revert" performance

From: Philip Martin <philip_at_codematters.co.uk>
Date: 2003-11-01 02:42:25 CET

Gareth McCaughan <gareth.mccaughan@pobox.com> writes:

> +/** Restore the pristine version of each working copy named in @a paths,
> + * effectively undoing any local mods. This is equivalent to calling
> + * svn_client_revert once for each path, except that (1) it may be faster
> + * because it avoids redundant sleeping and (2) nonexistent paths
> + * will not cause an error to be returned -- instead a warning
> + * will be issued via @a svn_handle_warning if @a
> + * warn_on_nonexistence is true, or the problem will be completely
> + * ignored if not.
> + */
> +svn_error_t *
> +svn_client_revert_list (apr_array_header_t *paths,
> + svn_boolean_t recursive,
> + svn_client_ctx_t *ctx,
> + svn_boolean_t warn_on_nonexistence,

That parameter looks odd, I think the notify callback should make
that decision.

> + apr_pool_t *pool);
> +
> +
> /** Remove the 'conflicted' state on a working copy @a path. This will
> * not semantically resolve conflicts; it just allows @a path to be
> * committed in the future. The implementation details are opaque.
> Index: subversion/libsvn_client/revert.c
> ===================================================================
> --- subversion/libsvn_client/revert.c (revision 7591)
> +++ subversion/libsvn_client/revert.c (working copy)
> @@ -36,11 +36,11 @@
>
> /*** Code. ***/
>
> -svn_error_t *
> -svn_client_revert (const char *path,
> - svn_boolean_t recursive,
> - svn_client_ctx_t *ctx,
> - apr_pool_t *pool)
> +static svn_error_t *
> +svn_client__revert1 (const char *path,
> + svn_boolean_t recursive,
> + svn_client_ctx_t *ctx,
> + apr_pool_t *pool)
> {
> svn_wc_adm_access_t *adm_access;
> svn_boolean_t wc_root;
> @@ -107,9 +107,52 @@
> out:
>
> SVN_ERR (svn_wc_adm_close (adm_access));
> + return err;
> +}
>
> +svn_error_t *
> +svn_client_revert (const char *path,
> + svn_boolean_t recursive,
> + svn_client_ctx_t *ctx,
> + apr_pool_t *pool)
> +{
> + SVN_ERR (svn_client__revert1 (path, recursive, ctx, pool));

If an error is returned here it will bypass the sleep, but timestamps
may have been changed.

> +
> /* Sleep to ensure timestamp integrity. */
> svn_sleep_for_timestamps ();
> + return SVN_NO_ERROR;
> +}
>
> - return err;
> +svn_error_t *
> +svn_client_revert_list (apr_array_header_t *paths,
> + svn_boolean_t recursive,
> + svn_client_ctx_t *ctx,
> + svn_boolean_t warn_on_nonexistence,
> + apr_pool_t *pool)
> +{
> + int i;
> + apr_pool_t *subpool = svn_pool_create (pool);
> +
> + for (i = 0; i < paths->nelts; i++)
> + {
> + const char *path = ((const char **) (paths->elts))[i];
> + svn_error_t *err;
> +
> + err = svn_client__revert1 (path, recursive, ctx, subpool);
> + if (err)
> + {
> + if (err->apr_err != SVN_ERR_ENTRY_NOT_FOUND)
> + return err;
> + if (warn_on_nonexistence)
> + svn_handle_warning (stderr, err);

The client library should not call svn_handle_warning, it's not much
use to a GUI application. You should be calling the notify callback
instead.

Errors that are not returned need to cleared using svn_error_clear.

> + continue;
> + }
> + svn_pool_clear (subpool);

Clear pools at the start of the loop.

> + }
> +
> + svn_pool_destroy (subpool);
> +
> + /* Sleep to ensure timestamp integrity. */
> + svn_sleep_for_timestamps ();
> + return SVN_NO_ERROR;
> }
> Index: subversion/clients/cmdline/revert-cmd.c
> ===================================================================
> --- subversion/clients/cmdline/revert-cmd.c (revision 7591)
> +++ subversion/clients/cmdline/revert-cmd.c (working copy)
> @@ -60,31 +60,6 @@
> svn_cl__get_notifier (&ctx->notify_func, &ctx->notify_baton, FALSE, FALSE,
> FALSE, pool);
>
> - subpool = svn_pool_create (pool);

You don't appear to have deleted the subpool variable.

> - for (i = 0; i < targets->nelts; i++)
> - {
> - const char *target = ((const char **) (targets->elts))[i];
> - svn_error_t *err;
> -
> - err = svn_client_revert (target, recursive, ctx, subpool);
> - if (err)
> - {
> - if (err->apr_err == SVN_ERR_ENTRY_NOT_FOUND)
> - {
> - if (!opt_state->quiet)
> - {
> - svn_handle_warning (stderr, err);
> - }
> - continue;
> - }
> - else
> - return err;
> - }
> -
> - SVN_ERR (svn_cl__check_cancel (ctx->cancel_baton));
> - svn_pool_clear (subpool);
> - }
> -
> - svn_pool_destroy (subpool);
> - return SVN_NO_ERROR;
> + return svn_client_revert_list(targets, recursive, ctx,
> + !opt_state->quiet, pool);
> }
> Index: autogen.sh
> ===================================================================
> --- autogen.sh (revision 7591)
> +++ autogen.sh (working copy)

Don't include this file in the diff.

-- 
Philip Martin
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Nov 1 02:43:02 2003

This is an archived mail posted to the Subversion Dev mailing list.

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.