Re: [PATCH] fix for issue #2867: svn cleanup dies on obstructed subdirs

[Karl Fogel <kf...@red-bean.com>]

"Miller, Eric" <Er...@amd.com> writes:
>  [[[
> Fix issue #2867: svn cleanup dies on obstructed subdirs
>
> * subversion/libsvn_wc/log.c
>   (svn_wc_cleanup2): Display a warning for obstructed subdirs and clear
> the error
> ]]]
>
> I'm sure there is a better way to handle the info message (no message?)
> but I left it in anyways.
>
> Could someone review this?

Sure...

> Index: subversion/libsvn_wc/log.c
> ===================================================================
> --- subversion/libsvn_wc/log.c	(revision 28235)
> +++ subversion/libsvn_wc/log.c	(working copy)
> @@ -2475,6 +2475,7 @@
>    int wc_format_version;
>    apr_pool_t *subpool;
>    svn_boolean_t killme, kill_adm_only;
> +  svn_error_t *err;
>  
>    /* Check cancellation; note that this catches recursive calls too. */
>    if (cancel_func)
> @@ -2513,8 +2514,24 @@
>            /* Sub-directories */
>            SVN_ERR(svn_io_check_path(entry_path, &kind, subpool));
>            if (kind == svn_node_dir)
> -            SVN_ERR(svn_wc_cleanup2(entry_path, diff3_cmd,
> -                                    cancel_func, cancel_baton, subpool));
> +	    {
> +	      err = svn_wc_cleanup2(entry_path, diff3_cmd,
> +				    cancel_func, cancel_baton, subpool);
> +
> +	      /* Make an exception for obstructed subdirs */
> +	      if(err)
> +		{
> +		  if(err->apr_err == SVN_ERR_WC_NOT_DIRECTORY)
> +		    {
> +		      printf("Ignoring obstruction on %s\n", entry_path);
> +		      svn_error_clear(err);
> +		    }
> +		  else
> +		    {
> +		      SVN_ERR(err);
> +		    }
> +		}
> +	    }
>          }
>        else
>          {

One rule is that we can't printf() from within a library.  We can only
return errors (we can chain errors together, delay their return till
after the work is done, whatever, but simply printing is a no-no in a
library).

The other thing that worries me is, are you sure that this is the only
circumstance in which SVN_ERR_WC_NOT_DIRECTORY can be returned?

Third: a reproduction case would greatly help.  (If there was a thread
preceding your message, my apologies, I don't have it at hand.)

Best,
-Karl

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

RE: [PATCH] fix for issue #2867: svn cleanup dies on obstructed subdirs

["Miller, Eric" <Er...@amd.com>]

> One rule is that we can't printf() from within a library.  We can only
> return errors (we can chain errors together, delay their return till
> after the work is done, whatever, but simply printing is a no-no in a
> library).

Definitely (I made a note to this effect).  This was a debug thing and
shouldn't have been in the patch.
 
> The other thing that worries me is, are you sure that this is the only
> circumstance in which SVN_ERR_WC_NOT_DIRECTORY can be returned?

Good question.  I suppose I'd have to dive into svn_wc_cleanup2() to
find out?

> Third: a reproduction case would greatly help.  (If there was a thread
> preceding your message, my apologies, I don't have it at hand.)

http://subversion.tigris.org/issues/show_bug.cgi?id=2867

I guess the fundamental question is - should an obstruction be
considered a fatal error during svn cleanup?  It isn't for status (or
status -u finally) it is of course for update and commit.

If it is, then I believe a -force option for cleanup is in order.

Eric



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