You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by st...@apache.org on 2011/10/25 18:39:38 UTC

svn commit: r1188774 - /subversion/trunk/subversion/libsvn_client/externals.c

Author: stsp
Date: Tue Oct 25 16:39:37 2011
New Revision: 1188774

URL: http://svn.apache.org/viewvc?rev=1188774&view=rev
Log:
Fix issue #4044, "empty parents of externals not removed along with externals
on update".

* subversion/libsvn_client/externals.c
  (svn_client__handle_externals): When removing externals, remove the
    defining directory if it ends up being unversioned and empty.
    This is a workaround for our inability to remove the defining
    dir when delete_entry() in the update editor is called for it.
    At that point, external children have not been removed yet, so
    the update editor leaves the directory behind on disk, unversioned,
    because it isn't empty.

Modified:
    subversion/trunk/subversion/libsvn_client/externals.c

Modified: subversion/trunk/subversion/libsvn_client/externals.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/externals.c?rev=1188774&r1=1188773&r2=1188774&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_client/externals.c (original)
+++ subversion/trunk/subversion/libsvn_client/externals.c Tue Oct 25 16:39:37 2011
@@ -943,6 +943,7 @@ svn_client__handle_externals(apr_hash_t 
     {
       const char *item_abspath = svn__apr_hash_index_key(hi);
       const char *defining_abspath = svn__apr_hash_index_val(hi);
+      svn_wc_status3_t *defining_status;
 
       svn_pool_clear(iterpool);
 
@@ -951,6 +952,20 @@ svn_client__handle_externals(apr_hash_t 
                           handle_external_item_removal(&eb, defining_abspath,
                                                        item_abspath, iterpool),
                           iterpool));
+
+      /* Is DEFINING_ABSPATH now an unversioned directory we can remove? */
+      SVN_ERR(svn_wc_status3(&defining_status, ctx->wc_ctx, defining_abspath,
+                             iterpool, iterpool));
+      if (defining_status->node_status == svn_wc_status_unversioned)
+        {
+          svn_error_t *err;
+
+          err = svn_io_dir_remove_nonrecursive(defining_abspath, iterpool);
+          if (err && APR_STATUS_IS_ENOTEMPTY(err->apr_err))
+            svn_error_clear(err);
+          else
+            SVN_ERR(err);
+        }
     }
 
 



Re: svn commit: r1188774 - /subversion/trunk/subversion/libsvn_client/externals.c

Posted by Stefan Sperling <st...@elego.de>.
On Wed, Oct 26, 2011 at 05:17:36PM +0200, Bert Huijben wrote:
> > +      /* Is DEFINING_ABSPATH now an unversioned directory we can
> > remove? */
> > +      SVN_ERR(svn_wc_status3(&defining_status, ctx->wc_ctx,
> > defining_abspath,
> > +                             iterpool, iterpool));
> 
> Defining abspath is the directory that contain(s/ed) the svn:externals property, not the parent of the externals directory. There may be several levels inbetween as you can define externals on any subdirectory level.
> 
> The check should be on the parent of the external and probably recurse upwards.
> 

I agree, good point.

> For the backport: I'm not sure if this is really a problem that we must fix. The same behavior applies to any unversioned directory.
> 

It is pointless to leave the directory there. It will just cause tree
conflicts later when the user updates up or down to a revision where the
directory re-appears.

It's an edge-case, but it's worth doing IMO.

RE: svn commit: r1188774 - /subversion/trunk/subversion/libsvn_client/externals.c

Posted by Bert Huijben <be...@qqmail.nl>.

> -----Original Message-----
> From: stsp@apache.org [mailto:stsp@apache.org]
> Sent: dinsdag 25 oktober 2011 18:40
> To: commits@subversion.apache.org
> Subject: svn commit: r1188774 -
> /subversion/trunk/subversion/libsvn_client/externals.c
> 
> Author: stsp
> Date: Tue Oct 25 16:39:37 2011
> New Revision: 1188774
> 
> URL: http://svn.apache.org/viewvc?rev=1188774&view=rev
> Log:
> Fix issue #4044, "empty parents of externals not removed along with
> externals
> on update".
> 
> * subversion/libsvn_client/externals.c
>   (svn_client__handle_externals): When removing externals, remove the
>     defining directory if it ends up being unversioned and empty.
>     This is a workaround for our inability to remove the defining
>     dir when delete_entry() in the update editor is called for it.
>     At that point, external children have not been removed yet, so
>     the update editor leaves the directory behind on disk, unversioned,
>     because it isn't empty.
> 
> Modified:
>     subversion/trunk/subversion/libsvn_client/externals.c
> 
> Modified: subversion/trunk/subversion/libsvn_client/externals.c
> URL:
> http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_client/e
> xternals.c?rev=1188774&r1=1188773&r2=1188774&view=diff
> ==========================================================
> ====================
> --- subversion/trunk/subversion/libsvn_client/externals.c (original)
> +++ subversion/trunk/subversion/libsvn_client/externals.c Tue Oct 25
> 16:39:37 2011
> @@ -943,6 +943,7 @@ svn_client__handle_externals(apr_hash_t
>      {
>        const char *item_abspath = svn__apr_hash_index_key(hi);
>        const char *defining_abspath = svn__apr_hash_index_val(hi);
> +      svn_wc_status3_t *defining_status;
> 
>        svn_pool_clear(iterpool);
> 
> @@ -951,6 +952,20 @@ svn_client__handle_externals(apr_hash_t
>                            handle_external_item_removal(&eb, defining_abspath,
>                                                         item_abspath, iterpool),
>                            iterpool));
> +
> +      /* Is DEFINING_ABSPATH now an unversioned directory we can
> remove? */
> +      SVN_ERR(svn_wc_status3(&defining_status, ctx->wc_ctx,
> defining_abspath,
> +                             iterpool, iterpool));

Defining abspath is the directory that contain(s/ed) the svn:externals property, not the parent of the externals directory. There may be several levels inbetween as you can define externals on any subdirectory level.

The check should be on the parent of the external and probably recurse upwards.


For the backport: I'm not sure if this is really a problem that we must fix. The same behavior applies to any unversioned directory.

	Bert