On Nov 24, 2009, at 8:05 AM, Bert Huijben wrote: > > >> -----Original Message----- >> From: phi...@apache.org [mailto:phi...@apache.org] >> Sent: dinsdag 24 november 2009 3:00 >> To: comm...@subversion.apache.org >> Subject: svn commit: r883696 - in /subversion/trunk/subversion: >> include/private/svn_wc_private.h libsvn_client/add.c >> libsvn_client/revert.c libsvn_wc/adm_files.c libsvn_wc/adm_files.h >> libsvn_wc/adm_ops.c libsvn_wc/lock.c >> >> Author: philip >> Date: Tue Nov 24 13:59:45 2009 >> New Revision: 883696 >> >> URL: http://svn.apache.org/viewvc?rev=883696&view=rev >> Log: >> A better fix for the SEGV fixed in r881265: replace another use >> of access batons. This no longer leaves .svn directories around. >> >> * subversion/libsvn_wc/adm_files.h >> (svn_wc__adm_destroy): Pass svn_wc__db_t and abspath rather than >> svn_wc_adm_access_t. >> >> * subversion/libsvn_wc/adm_files.c >> (svn_wc__adm_destroy): Use svn_wc__db_t directly. >> >> * subversion/libsvn_wc/adm_ops.c >> (svn_wc__internal_remove_from_revision_control): Revert r881265: no >> need to check for null access baton. >> >> * subversion/include/private/svn_wc_private.h >> (svn_wc_acquire_write_lock): Add anchor_abspath parameter. >> >> * subversion/libsvn_wc/lock.c >> (svn_wc_acquire_write_lock): Add anchor_abspath parameter and code. >> >> * subversion/libsvn_client/revert.c >> (revert): Acquire and release locks on the anchor. >> >> * subversion/libsvn_client/add.c >> (svn_client_add4): Update svn_wc_acquire_write_lock call. >> >> Modified: >> subversion/trunk/subversion/include/private/svn_wc_private.h >> subversion/trunk/subversion/libsvn_client/add.c >> subversion/trunk/subversion/libsvn_client/revert.c >> subversion/trunk/subversion/libsvn_wc/adm_files.c >> subversion/trunk/subversion/libsvn_wc/adm_files.h >> subversion/trunk/subversion/libsvn_wc/adm_ops.c >> subversion/trunk/subversion/libsvn_wc/lock.c >> >> Modified: subversion/trunk/subversion/include/private/svn_wc_private.h >> URL: >> http://svn.apache.org/viewvc/subversion/trunk/subversion/include/privat >> e/svn_wc_private.h?rev=883696&r1=883695&r2=883696&view=diff >> ======================================================================= >> ======= >> --- subversion/trunk/subversion/include/private/svn_wc_private.h >> (original) >> +++ subversion/trunk/subversion/include/private/svn_wc_private.h Tue >> Nov 24 13:59:45 2009 >> @@ -516,12 +516,17 @@ >> >> >> /** >> - * Recursively acquire write locks for @a local_abspath, using @a >> wc_ctx >> + * Recursively acquire write locks for @a local_abspath if >> + * @a anchor_abspath is NULL. If @a anchor_abspath is not NULL then >> + * recursively acquire write locks for the anchor of @a local_abspath >> + * and return the anchor path in @a *anchor_abspath. Use @a wc_ctx >> * for working copy access. >> */ >> svn_error_t * >> -svn_wc__acquire_write_lock(svn_wc_context_t *wc_ctx, >> +svn_wc__acquire_write_lock(const char **anchor_abspath, >> + svn_wc_context_t *wc_ctx, >> const char *local_abspath, >> + apr_pool_t *result_pool, >> apr_pool_t *scratch_pool); > > Please add a note that the anchor_abspath argument should be removed after we > move to a single database. In the single database there is no information on > a path stored in its parent, so you never have to lock an anchor for > updating/deleting/adding a child. > > I didn't review this issue, but maybe this code relying on a lock of the > parent should just lock the parent instead of requiring all the callers of > this new functions to think about an ancestor. > (Stop thinking about locking ancestors is most likely the reason for > introducing this function. Hyrum?)
I think this is a good intermediate step, but eventually we shouldn't need to bother with the anchor_abspath. The purpose for introducing these functions originally was that the access batons have now become just surrogates for write locks, and we should be able to replace them with calls to functions which do the locking for us. These are those functions. In any case, it's good to get some eyes on the locking code. (And it's also good to see that my Evil Plan to get more people involved in wc-ng is working.... Bwahahahaha!) I think the next step to take with this functions is to push the recursive nature of them into wc_db.c, so the wc_db interface looks and feels like it will when we are centralized. As Philip has discovered, we may need a bit of shim code in place until that time, but hopefully by implementing the wc_db API correctly, we can remove some of the work when we transition to the centralized metadata. -Hyrum
