Daniel Näslund <dan...@longitudo.com> writes: > On Thu, Dec 17, 2009 at 06:10:18PM +0000, Philip Martin wrote: >> Daniel Näslund <dan...@longitudo.com> writes: >> > >> > + SVN_ERR(svn_wc__release_write_lock(ib->ctx->wc_ctx, >> > + remove_target_abspath, >> > + ib->iter_pool)); >> > + >> >> Why the change in behaviour? The old code would leave the locks if >> the remove call failed. Is this because you are no longer locking the >> external? > > A comment in svn_wc_acquire_write_locks(): > > [[[ > /* The current lock paradigm is that each directory holds a lock for itself, > and there are no inherited locks. In the eventual wc-ng paradigm, a > lock on a directory, would imply a infinite-depth lock on the children. > But since we aren't quite there yet, we do the infinite locking > manually (and be sure to release them in svn_wc__release_write_lock(). */ > ]]] > > Why leave locks we have created if the operation we attempted to do > failed?
That's how locks work. - take the lock - make changes possibly leaving the wc inconsistent - complete changes making the wc consistent - remove lock If an error occurs that aborts the operation while the wc is inconsistent then the lock should not be removed. The original code was ignoring LEFT_LOCAL_MOD errors, treating them as not-an-error, so it removed locks in that case. For other errors it left the locks. The other problem with your new code is that if the svn_wc__release_write_lock call returns an error any error from the svn_wc_remove_from_revision_control2 call will leak. -- Philip
