On Wed, May 4, 2011 at 22:49, Hyrum K Wright <hyrum_at_hyrumwright.org> wrote:
> On Wed, May 4, 2011 at 8:46 PM, <gstein_at_apache.org> wrote:
>> Author: gstein
>> Date: Thu May 5 01:46:31 2011
>> New Revision: 1099657
>>
>> URL: http://svn.apache.org/viewvc?rev=1099657&view=rev
>> Log:
>> Combine the changelist modification notification into the operation
>> itself, so that (in the future) we can make guarantees about dropping the
>> temporary table. Add cancellation support, too.
>>
>> Add a missing clear of the iterpool in db_op_delete.
>>
>> Leave markers for future unification.
>>
>> * subversion/libsvn_wc/wc_db.h:
>> (svn_wc__db_op_set_chnagelist): rename a couple parameters (that
>> differed by a single character) for clarity. add notification and
>> cancellation parameters.
>> (svn_wc__db_changelist_list_notify): remove
>>
>> * subversion/libsvn_wc/wc_db.c:
>> (svn_wc__db_op_set_changelist): combine with ...
>> (svn_wc__db_changelist_list_notify): ... this. leave some comments.
>> adjust a bit of pool usage since we have an iterpool that can be used
>> as a better scratch_pool in the early part of the function. early-exit
>> if there is no NOTIFY_FUNC. fix an implicit 64-bit to 32-bit
>> conversion for the ACTION localvar. add cancellation.
>> (svn_wc__db_op_delete): clear the iterpool, and adjust some localvar
>> initialization to after that call.
>>
>> * subversion/libsvn_wc/adm_ops.c:
>> (add_from_disk, changelist_walker): shift the notification directly into
>> the call to db_op_set_changelist.
>>
>> Modified:
>> subversion/trunk/subversion/libsvn_wc/adm_ops.c
>> subversion/trunk/subversion/libsvn_wc/wc_db.c
>> subversion/trunk/subversion/libsvn_wc/wc_db.h
>>
>>...
>>
>> Modified: subversion/trunk/subversion/libsvn_wc/wc_db.c
>> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_wc/wc_db.c?rev=1099657&r1=1099656&r2=1099657&view=diff
>> ==============================================================================
>> --- subversion/trunk/subversion/libsvn_wc/wc_db.c (original)
>> +++ subversion/trunk/subversion/libsvn_wc/wc_db.c Thu May 5 01:46:31 2011
>> @@ -3567,6 +3567,10 @@ svn_wc__db_op_set_changelist(svn_wc__db_
>>...
>> @@ -3594,32 +3601,31 @@ svn_wc__db_op_set_changelist(svn_wc__db_
>> NOT_IMPLEMENTED();
>> }
>>
>> - SVN_ERR(svn_wc__db_with_txn(wcroot, local_relpath, with_triggers, &wtb,
>> - scratch_pool));
>> + wtb.cb_baton = &scb;
>>
>> - SVN_ERR(flush_entries(wcroot, local_abspath, scratch_pool));
>> + /* ### fix up the code below: if the callback is invokved, then the
>> + ### 'changelist_list' table may exist. We should ensure it gets dropped
>> + ### before we exit this function. */
>>
>> - return SVN_NO_ERROR;
>> -}
>> + SVN_ERR(svn_wc__db_with_txn(wcroot, local_relpath, with_triggers, &wtb,
>> + iterpool));
>> + SVN_ERR(flush_entries(wcroot, local_abspath, iterpool));
>>
>> + /* ### can we unify this notification logic, in some way, with the
>> + ### similar logic in op_delete? ... I think we probably want a
>> + ### notify_callback that represents the inner loop. the statement
>> + ### selection and binding is probably similar (especially if we
>> + ### remove like_arg, as questioned below). the unification could
>> + ### look similar to db_with_txn or the with_triggers stuff. */
>
> I agree that it would be nice to unify whatever delayed notification
> stuffs we have in wc_db. I think that ideally, we would shove the
> equivalent of svn_wc_notify_t into the database, and then use that to
> populate the svn_wc_notify_t when doing the actual notifications. (I
> know that svn_wc_notify_t is a beast, though, so maybe this is a
> chance to think designing something a bit more intelligent.)
>
> There would be some complexity, though, in serializing svn_wc_notify_t
> to the DB. We could either make the temp table mirror the actual
> struct, which leaves lots of NULL values, or serialize the required
> values with skels. In the latter case, we'd probably need to write
> custom sqlite functions to do the serialization, since it all happens
> within the various triggers.
I was thinking of the three cases. Not a generalized serialization of
notify_t :-)
* do some work
* sent notifications
* cleanup
[ do all the above "safely" ]
Thoughts?
Cheers,
-g
Received on 2011-05-05 07:36:29 CEST