[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: C. Michael Pilato <cmpilato_at_collab.net>
Date: 2003-11-01 02:24:47 CET

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

> A first pass at a patch is attached (rather than included,
> because I have reason to suspect that my MUA sometimes
> wraps lines: sorry). Things I'm not entirely happy about:

I'm reviewing your comments, not your patch. Just so you know.

> 1. The new svn_client_revert_list function never returns
> an error when an entry doesn't exist. That's the behaviour
> svn_cl__revert (currently its only caller) wants, but it's
> not necessarily what every imaginable caller would want.
> Should there be another flag passed in to determine
> whether nonexistent entries should be treated just like
> other errors?

Hm... I really don't think we need both svn_client_revert() and
svn_client_revert_list(). Just make svn_client_revert() *not* take a
const char * path, and start taking an apr_array_header_t * targets
list.

> 2. The old svn_cl__revert used to call svn_cl__check_cancel
> after each individual reversion. It's not so easy to do that
> down in the depths, and in any case there's less need now
> that we aren't sleeping for 1s per reverted path. But, still,
> it makes me uneasy not to have the check there at all for
> an operation that can in principle take an unbounded amount
> of time. Should there be a callback or something?

svn_client_revert() should be handed a 'ctx' client callback back,
from which it should be able to call ctx->cancel_func(ctx->cancel_baton).

> 3. Is "svn_client_revert_list" the best name for the new function?
> Is it right for both this and the old function that reverts a single
> path to continue to exist? (I think the answer to the latter question
> is "yes", but am open to persuasion.)

I answered this already. Also, I'd prefer not to see a function named
svn_client__revert1() -- first of all, the "1" is just ... ugly. But
also, if this function is never called from outside of revert.c, we
need to drop the "svn_client__" prefix (which implies intra-module
globalness).

---------------------------------------------------------------------
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:25:51 2003

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