Re: [patch] Issue #3039

[David Glasser <gl...@davidglasser.net>]

On Dec 10, 2007 6:04 PM, Daniel Rall <dl...@collab.net> wrote:
> Quickly sending a patch which appears to fix issue #3039 on my way out the
> door...


The guts of this (svn_wc__adm_extend_lock_to_tree) looks good.  A few
comments:

> Index: subversion/libsvn_wc/lock.c
> ===================================================================

> +/* WC entry walker callbacks for svn_wc__adm_extend_lock_to_tree(). */
> +static svn_wc_entry_callbacks2_t extend_lock_walker =
> +{
> +  extend_lock_found_entry,
> +  extend_lock_handle_error

You can just use svn_wc__walker_default_error_handler here (it's the
same).

> Index: subversion/libsvn_wc/update_editor.c
> ===================================================================
> --- subversion/libsvn_wc/update_editor.c	(revision 28380)
> +++ subversion/libsvn_wc/update_editor.c	(working copy)
> @@ -1110,6 +1110,9 @@
>
>    SVN_ERR(svn_wc_adm_retrieve(&adm_access, eb->adm_access,
>                                parent_path, pool));
> +  /* Deleting PATH requires that any children it has are also locked
> +     (issue #3039). */
> +  SVN_ERR(svn_wc__adm_extend_lock_to_tree(eb->adm_access, pool));

This should be done inside the code that actually *runs* the deletion,
not the code that builds the log, so that this bug fix also fixes the
fact that "svn cleanup" doesn't work *at all* for running delete-entry
loggy items that delete directories which contain directories.

That probably means log.c(log_do_delete_entry), (there might be an
argument for svn_wc_remove_from_revision_control, but I think that's
wrong).

--dave


-- 
David Glasser | glasser@davidglasser.net | http://www.davidglasser.net/

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: [patch] Issue #3039

[Daniel Rall <dl...@collab.net>]

On Mon, 10 Dec 2007, David Glasser wrote:

> On Dec 10, 2007 6:04 PM, Daniel Rall <dl...@collab.net> wrote:
...
> > Index: subversion/libsvn_wc/lock.c
> > ===================================================================
> 
> > +/* WC entry walker callbacks for svn_wc__adm_extend_lock_to_tree(). */
> > +static svn_wc_entry_callbacks2_t extend_lock_walker =
> > +{
> > +  extend_lock_found_entry,
> > +  extend_lock_handle_error
> 
> You can just use svn_wc__walker_default_error_handler here (it's the
> same).
 
Duh, I only wrote that...  :P

> > Index: subversion/libsvn_wc/update_editor.c
> > ===================================================================
> > --- subversion/libsvn_wc/update_editor.c	(revision 28380)
> > +++ subversion/libsvn_wc/update_editor.c	(working copy)
> > @@ -1110,6 +1110,9 @@
> >
> >    SVN_ERR(svn_wc_adm_retrieve(&adm_access, eb->adm_access,
> >                                parent_path, pool));
> > +  /* Deleting PATH requires that any children it has are also locked
> > +     (issue #3039). */
> > +  SVN_ERR(svn_wc__adm_extend_lock_to_tree(eb->adm_access, pool));
> 
> This should be done inside the code that actually *runs* the deletion,
> not the code that builds the log, so that this bug fix also fixes the
> fact that "svn cleanup" doesn't work *at all* for running delete-entry
> loggy items that delete directories which contain directories.
> 
> That probably means log.c(log_do_delete_entry), (there might be an
> argument for svn_wc_remove_from_revision_control, but I think that's
> wrong).

Good suggestions, Dave.  Patch updated, and committed to trunk in r28389.