[PATCH] Fix 'svn rm wc/some/dir/'

[Philip Martin <ph...@codematters.co.uk>]

Hello

As described in issue 611

$ svn rm wc/some/dir/
$ svn ci wc
$ svn up wc
$ svn st wc
?   wc/some/dir

Deleting and commiting a working copy directory leaves behind an
unversioned directory.

Well it looks quite simple to fix, but I hesitate to apply this patch
because a) I don't understand the "Don't remove" comment, b) we're
near alpha.  Does this look correct?  Can anyone explain the "Don't
remove" comment?

Part of issue #611.  Fix 'svn rm wc/some/dir/' bug.

* subversion/libsvn_wc/log.c (svn_wc__run_log): Pysically delete
  directories marked for deletion.
  
* subversion/tests/clients/cmdline/copy_tests.py
  (resurrect_deleted_dir): Remove non-deletion workaround.

* subversion/tests/clients/cmdline/update_tests.py
  (update_delete_modified_files):  Remove non-deletion workaround.

* subversion/tests/clients/cmdline/commit_tests.py
  (hudson_part_1_variation_1): Don't workaround non-deletion.


Index: ./subversion/libsvn_wc/log.c
===================================================================
--- ./subversion/libsvn_wc/log.c
+++ ./subversion/libsvn_wc/log.c	Wed Jul 17 00:39:29 2002
@@ -1168,10 +1168,10 @@
       SVN_ERR (svn_wc_entry (&thisdir_entry, path, FALSE, pool));
 
       /* Blow away the entire administrative dir, and all those below
-         it too.  Don't remove any working files, though. */
+         it too. */
       SVN_ERR (svn_wc_remove_from_revision_control (path,
                                                     SVN_WC_ENTRY_THIS_DIR,
-                                                    FALSE, pool));
+                                                    TRUE, pool));
 
       /* If revnum of this dir is greater than parent's revnum, then
          recreate 'deleted' entry in parent. */
Index: ./subversion/tests/clients/cmdline/copy_tests.py
===================================================================
--- ./subversion/tests/clients/cmdline/copy_tests.py
+++ ./subversion/tests/clients/cmdline/copy_tests.py	Wed Jul 17 00:57:03 2002
@@ -389,9 +389,6 @@
                                             wc_dir):
     return 1
 
-  shutil.rmtree(wc_dir + '/A/D/G') # 'svn rm --force' won't remove the dir,
-                                   #  either before or after commit!
-
   # Use 'svn cp -r1 URL URL' to resurrect the deleted directory, where
   # the two URLs are identical.  This used to trigger a failure.  
   url = svntest.main.test_area_url + '/' \
Index: ./subversion/tests/clients/cmdline/update_tests.py
===================================================================
--- ./subversion/tests/clients/cmdline/update_tests.py
+++ ./subversion/tests/clients/cmdline/update_tests.py	Wed Jul 17 00:50:12 2002
@@ -692,8 +692,6 @@
   if len(stderr_lines) != 0:
     print "updating after commit failed"
     return 1
-  # ### Bug! Directory is not deleted (issue #611). Use brute force
-  shutil.rmtree(G_path)
 
   # Backdate to restore deleted items
   stdout_lines, stderr_lines = svntest.main.run_svn(None, 'up', '-r1', wc_dir)
Index: ./subversion/tests/clients/cmdline/commit_tests.py
===================================================================
--- ./subversion/tests/clients/cmdline/commit_tests.py
+++ ./subversion/tests/clients/cmdline/commit_tests.py	Wed Jul 17 01:05:33 2002
@@ -640,7 +640,7 @@
 
   # Expected disk tree:  everything except files in H
   expected_disk = svntest.main.greek_state.copy()
-  expected_disk.remove('A/D/H/chi', 'A/D/H/omega', 'A/D/H/psi')
+  expected_disk.remove('A/D/H', 'A/D/H/chi', 'A/D/H/omega', 'A/D/H/psi')
 
   # Expected status after update:  totally clean revision 2, minus H.
   expected_status = svntest.actions.get_virginal_state(wc_dir, 2)


-- 
Philip Martin

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

Re: [PATCH] Fix 'svn rm wc/some/dir/'

[Ben Collins-Sussman <su...@collab.net>]

Philip Martin <ph...@codematters.co.uk> writes:

> Well it looks quite simple to fix, but I hesitate to apply this patch
> because a) I don't understand the "Don't remove" comment, b) we're
> near alpha.  Does this look correct?  Can anyone explain the "Don't
> remove" comment?

Philip, this patch looks correct to me.  I think I wrote that comment
long ago, and that it's now obsolete...(/me hears gstein groaning...)

I think the comment refers to a time when the DESTROY_WF flag to
svn_wc_remove_from_revision_control() did something different.  (Like,
that flag used to indicate whether or not to leave locally-modified
files within the directory.)  But now that routine *always* preserves
locally modified files, regardless of the flag's value.  The flag only
toggles between two behaviors:

   * recursively remove .svn areas only
   * recursivel remove both .svn areas and files too.

So your simple change looks safe to me, and looks like the Right
answer.  Go ahead and apply it.  I'm very happy to see issue #611
fixed for Alpha... it would have been very obvious to newbies.

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