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

Re: svn commit: r1099436 - in /subversion/trunk/subversion/libsvn_wc: adm_ops.c update_editor.c wc_db.c wc_db.h

From: Greg Stein <gstein_at_gmail.com>
Date: Wed, 4 May 2011 12:45:40 -0400

On Wed, May 4, 2011 at 12:34, Hyrum K Wright <hyrum_at_hyrumwright.org> wrote:
> On Wed, May 4, 2011 at 11:31 AM, Greg Stein <gstein_at_gmail.com> wrote:
>> On Wed, May 4, 2011 at 08:55,  <philip_at_apache.org> wrote:
>>> +++ subversion/trunk/subversion/libsvn_wc/wc_db.h Wed May  4 12:55:03 2011
>>> @@ -1215,7 +1215,10 @@ svn_wc__db_op_delete(svn_wc__db_t *db,
>>>  /* Make delete notifications for all paths in the delete list created
>>>  * by deleting LOCAL_ABSPATH.
>>>  *
>>> - * This function drops the delete list.
>>> + * This function drops the delete list.  NOTIFY_FUNC may be NULL in
>>> + * which case the table is dropped without any notification.
>>> + *
>>> + * ### Perhaps this should be part of svn_wc__db_op_delete?
>> I say "yes". If the actual deletion produces an error, then we still
>> want the table dropped before returning that error. Tho... it may be
>> that the sqlite transaction rollback takes the table with it(?). ...
>> in any case, these two functions need to be called as a pair, and that
>> is a bad pattern to enforce on the caller.
> I'll also note that we have a few other places where we do this, too.
> My spidey sense has been telling me we need to make the action and
> notification bits a single wc_db function call, but I haven't yet
> acted on it.

Likewise. These separate notification functions have been bugging me,
too. It introduces a coupling between wc_db and its callers -- those
callers need to do "something more" when they call into wc_db. If they
don't... then things start to go wrong.

I'll look into fixing these.

Received on 2011-05-04 18:46:08 CEST

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.