On Wed, May 4, 2011 at 22:49, Hyrum K Wright <hy...@hyrumwright.org> wrote: > On Wed, May 4, 2011 at 8:46 PM, <gst...@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
