On Wed, May 4, 2011 at 12:34, Hyrum K Wright <hy...@hyrumwright.org> wrote: > On Wed, May 4, 2011 at 11:31 AM, Greg Stein <gst...@gmail.com> wrote: >> On Wed, May 4, 2011 at 08:55, <phi...@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. Cheers, -g
