Rev 2782: (Marius Cruger) bzr rm should remove clean subtrees (#111665, r=bialix) in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

Canonical.com Patch Queue Manager pqm at pqm.ubuntu.com
Mon Sep 3 08:02:17 BST 2007


At file:///home/pqm/archives/thelove/bzr/%2Btrunk/

------------------------------------------------------------
revno: 2782
revision-id: pqm at pqm.ubuntu.com-20070903070212-g8basoqekm8c489k
parent: pqm at pqm.ubuntu.com-20070903062644-0rt7302gl1to109v
parent: bialix at ukr.net-20070903052947-keh3u7dl382c0slg
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Mon 2007-09-03 08:02:12 +0100
message:
  (Marius Cruger) bzr rm should remove clean subtrees (#111665, r=bialix)
modified:
  NEWS                           NEWS-20050323055033-4e00b5db738777ff
  bzrlib/errors.py               errors.py-20050309040759-20512168c4e14fbd
  bzrlib/tests/__init__.py       selftest.py-20050531073622-8d0e3c8845c97a64
  bzrlib/tests/blackbox/test_remove.py test_remove.py-20060530011439-fika5rm84lon0goe-1
  bzrlib/tests/workingtree_implementations/test_remove.py test_remove.py-20070413183901-rvnp85rtc0q0sclp-1
  bzrlib/workingtree.py          workingtree.py-20050511021032-29b6ec0a681e02e3
    ------------------------------------------------------------
    revno: 2778.1.1
    merged: bialix at ukr.net-20070903052947-keh3u7dl382c0slg
    parent: pqm at pqm.ubuntu.com-20070903031921-8msn0bmzubicv5b1
    parent: amanic at gmail.com-20070813204022-s6g7t8e660x9zq9y
    committer: Alexander Belchenko <bialix at ukr.net>
    branch nick: rm.marius
    timestamp: Mon 2007-09-03 08:29:47 +0300
    message:
      merged with bzr.dev
    ------------------------------------------------------------
    revno: 2655.2.17
    merged: amanic at gmail.com-20070813204022-s6g7t8e660x9zq9y
    parent: amanic at gmail.com-20070813202904-g5gxzrf6sto99vgd
    committer: Marius Kruger <amanic at gmail.com>
    branch nick: bzr.rm_delete_working_file
    timestamp: Mon 2007-08-13 22:40:22 +0200
    message:
      Minor doc and spacing updates.
    ------------------------------------------------------------
    revno: 2655.2.16
    merged: amanic at gmail.com-20070813202904-g5gxzrf6sto99vgd
    parent: amanic at gmail.com-20070813200003-llmiom64a9sqj4gl
    committer: Marius Kruger <amanic at gmail.com>
    branch nick: bzr.rm_delete_working_file
    timestamp: Mon 2007-08-13 22:29:04 +0200
    message:
      * Rename `unknown_files_in_directory` to `unknown_nested_files`, which 
        is hopefully a little better.
      * Also consider unknown non-files as precious, so don't just delete them.
    ------------------------------------------------------------
    revno: 2655.2.15
    merged: amanic at gmail.com-20070813200003-llmiom64a9sqj4gl
    parent: amanic at gmail.com-20070811134033-xvpmchsolj3k3ijd
    committer: Marius Kruger <amanic at gmail.com>
    branch nick: bzr.rm_delete_working_file
    timestamp: Mon 2007-08-13 22:00:03 +0200
    message:
      Apply Alexander's comments:
      * Rename getTree => get_tree.
      * Rename getCommittedTree => get_committed_tree.
      * Extracted helper asserts: assertRemovedAndDeleted and
        assertRemovedAndNotDeleted
      * Fixed some docs.
      * Fixed my set to list conversion.
    ------------------------------------------------------------
    revno: 2655.2.14
    merged: amanic at gmail.com-20070811134033-xvpmchsolj3k3ijd
    parent: amanic at gmail.com-20070811133042-4t8tbcd0iuf316kg
    parent: pqm at pqm.ubuntu.com-20070810230629-bcp0rgmbhp0z35e1
    committer: Marius Kruger <amanic at gmail.com>
    branch nick: bzr.rm_delete_working_file
    timestamp: Sat 2007-08-11 15:40:33 +0200
    message:
      merge with mainline
    ------------------------------------------------------------
    revno: 2655.2.13
    merged: amanic at gmail.com-20070811133042-4t8tbcd0iuf316kg
    parent: amanic at gmail.com-20070811112905-tmibrlb8h5ezu5am
    committer: Marius Kruger <amanic at gmail.com>
    branch nick: bzr.rm_delete_working_file
    timestamp: Sat 2007-08-11 15:30:42 +0200
    message:
      Updated NEWS
    ------------------------------------------------------------
    revno: 2655.2.12
    merged: amanic at gmail.com-20070811112905-tmibrlb8h5ezu5am
    parent: amanic at gmail.com-20070809215100-th4poo3976eztba6
    committer: Marius Kruger <amanic at gmail.com>
    branch nick: bzr.rm_delete_working_file
    timestamp: Sat 2007-08-11 13:29:05 +0200
    message:
      Remove unknown ignored files wihtout needing --force'
    ------------------------------------------------------------
    revno: 2655.2.11
    merged: amanic at gmail.com-20070809215100-th4poo3976eztba6
    parent: amanic at gmail.com-20070809212245-67p3cutlpdl9tzfj
    committer: Marius Kruger <amanic at gmail.com>
    branch nick: bzr.rm_delete_working_file
    timestamp: Thu 2007-08-09 23:51:00 +0200
    message:
      * Update NEWS
      * Test removing directories with renames in (without --force)
      * update comment in workingtree
    ------------------------------------------------------------
    revno: 2655.2.10
    merged: amanic at gmail.com-20070809212245-67p3cutlpdl9tzfj
    parent: amanic at gmail.com-20070809204546-mru5ohlb5tlmoxsz
    committer: Marius Kruger <amanic at gmail.com>
    branch nick: bzr.rm_delete_working_file
    timestamp: Thu 2007-08-09 23:22:45 +0200
    message:
      add test for removing a direcory with something that has been moved out
    ------------------------------------------------------------
    revno: 2655.2.9
    merged: amanic at gmail.com-20070809204546-mru5ohlb5tlmoxsz
    parent: amanic at gmail.com-20070809165051-0kyj9w1hox2ng2n1
    committer: Marius Kruger <amanic at gmail.com>
    branch nick: bzr.rm_delete_working_file
    timestamp: Thu 2007-08-09 22:45:46 +0200
    message:
      * workingtree_implementations/test_remove 
        - Simplify tests by using the tree creating functions more and 
          remove the silly variables.
    ------------------------------------------------------------
    revno: 2655.2.8
    merged: amanic at gmail.com-20070809165051-0kyj9w1hox2ng2n1
    parent: amanic at gmail.com-20070809064355-r3v8ncgw9wumk5jp
    committer: Marius Kruger <amanic at gmail.com>
    branch nick: bzr.rm_delete_working_file
    timestamp: Thu 2007-08-09 18:50:51 +0200
    message:
      * workingtree_implementations/test_remove
        - Let getTree check if the files were really created.
        - Add utility method getCommittedTree.
        - Fix test_remove_nonempty_directory to not check for the removal of empty 
          directories.
        - Fix test_dont_remove_directory_with_unknowns to check if we refuse to 
          delete directories containing unknown files or directories.
        - Fix some method names and docstrings as per review
      * workingtree.remove
        - Don't automatically remove empty directories.
    ------------------------------------------------------------
    revno: 2655.2.7
    merged: amanic at gmail.com-20070809064355-r3v8ncgw9wumk5jp
    parent: amanic at gmail.com-20070805013923-bp4zn0iama2rcfb6
    parent: pqm at pqm.ubuntu.com-20070808113110-acathx9d9mpfo80z
    committer: Marius Kruger <amanic at gmail.com>
    branch nick: bzr.rm_delete_working_file
    timestamp: Thu 2007-08-09 08:43:55 +0200
    message:
      merge with bzr.dev
    ------------------------------------------------------------
    revno: 2655.2.6
    merged: amanic at gmail.com-20070805013923-bp4zn0iama2rcfb6
    parent: amanic at gmail.com-20070804203629-asas198adwjjfexw
    committer: Marius Kruger <amanic at gmail.com>
    branch nick: bzr.rm_delete_working_file
    timestamp: Sun 2007-08-05 03:39:23 +0200
    message:
      * workingtree.remove
        - Make sure we delete subtrees.
        - Fix minor bug where unknowns were shown multiple times.
      * workingtree_implementations.test_remove
        - Make test_remove_nonempty_directory test that unknown empty directories
          are removed.
        - Make test_force_remove_nonempty_directory test that unknown subtrees
          are removed regardless of content.
    ------------------------------------------------------------
    revno: 2655.2.5
    merged: amanic at gmail.com-20070804203629-asas198adwjjfexw
    parent: amanic at gmail.com-20070804201202-ek3hht4o71cjpl44
    committer: Marius Kruger <amanic at gmail.com>
    branch nick: bzr.rm_delete_working_file
    timestamp: Sat 2007-08-04 22:36:29 +0200
    message:
      * Improve BzrRemoveChangedFilesError message.
      * Add some spaces in tests/__init__.assertInWorkingTree parameter list.
      * Remove some unneeded imports.
    ------------------------------------------------------------
    revno: 2655.2.4
    merged: amanic at gmail.com-20070804201202-ek3hht4o71cjpl44
    parent: amanic at gmail.com-20070804150516-eekxw9tu6y9aoga7
    committer: Marius Kruger <amanic at gmail.com>
    branch nick: bzr.rm_delete_working_file
    timestamp: Sat 2007-08-04 22:12:02 +0200
    message:
      * workingtree.remove
        - Recursively remove directories when force is given.
        - Fix some doc strings and code formatting.
        - Remove TODO which I believe has been done.
      * workingtree_implementations/test_remove
        - Test recursive removing of directories when forced.
        - Fix some doc strings.
        - Rename test_remove_subtree to test_remove_keep_subtree.
          move and update it. (although it looks redundant to me)
        - Let test_non_cwd check if it was properly unversioned too.
    ------------------------------------------------------------
    revno: 2655.2.3
    merged: amanic at gmail.com-20070804150516-eekxw9tu6y9aoga7
    parent: amanic at gmail.com-20070731194215-4d9r1l48kr4or3wj
    parent: pqm at pqm.ubuntu.com-20070801171451-en3tds1hzlru2j83
    committer: Marius Kruger <amanic at gmail.com>
    branch nick: bzr.intertree_compare_ignores_param
    timestamp: Sat 2007-08-04 17:05:16 +0200
    message:
      merge with bzr.dev
=== modified file 'NEWS'
--- a/NEWS	2007-09-03 04:41:51 +0000
+++ b/NEWS	2007-09-03 07:02:12 +0000
@@ -76,6 +76,16 @@
    * Avoid trouble when Windows ssh calls itself 'plink' but no plink
      binary is present.  (Martin Albisetti, #107155)
 
+    * ``bzr remove`` should remove clean subtrees.
+      Now it will remove (without needing ``--force``) subtrees that contain no
+      files with text changes or modified files.
+      With ``--force`` it removes the subtree regardless of text changes or
+      unknown files.
+      Directories with renames in or out (but not changed otherwise)
+      will now be removed without needing ``--force``.
+      Unknown ignored files will be deleted without needing ``--force``.
+      (Marius Kruger, #111665)
+
   IMPROVEMENTS:
 
    * Add the option "--show-diff" to the commit command in order to display

=== modified file 'bzrlib/errors.py'
--- a/bzrlib/errors.py	2007-09-03 04:35:49 +0000
+++ b/bzrlib/errors.py	2007-09-03 07:02:12 +0000
@@ -1769,7 +1769,8 @@
 class BzrRemoveChangedFilesError(BzrError):
     """Used when user is trying to remove changed files."""
 
-    _fmt = ("Can't remove changed or unknown files:\n%(changes_as_text)s"
+    _fmt = ("Can't safely remove modified or unknown files:\n"
+        "%(changes_as_text)s"
         "Use --keep to not delete them, or --force to delete them regardless.")
 
     def __init__(self, tree_delta):

=== modified file 'bzrlib/tests/__init__.py'
--- a/bzrlib/tests/__init__.py	2007-09-03 04:35:49 +0000
+++ b/bzrlib/tests/__init__.py	2007-09-03 07:02:12 +0000
@@ -2078,7 +2078,7 @@
         else:
             self.failIf(osutils.lexists(path),path+" exists")
 
-    def assertInWorkingTree(self,path,root_path='.',tree=None):
+    def assertInWorkingTree(self, path, root_path='.', tree=None):
         """Assert whether path or paths are in the WorkingTree"""
         if tree is None:
             tree = workingtree.WorkingTree.open(root_path)
@@ -2089,7 +2089,7 @@
             self.assertIsNot(tree.path2id(path), None,
                 path+' not in working tree.')
 
-    def assertNotInWorkingTree(self,path,root_path='.',tree=None):
+    def assertNotInWorkingTree(self, path, root_path='.', tree=None):
         """Assert whether path or paths are not in the WorkingTree"""
         if tree is None:
             tree = workingtree.WorkingTree.open(root_path)

=== modified file 'bzrlib/tests/blackbox/test_remove.py'
--- a/bzrlib/tests/blackbox/test_remove.py	2007-07-12 10:14:25 +0000
+++ b/bzrlib/tests/blackbox/test_remove.py	2007-08-04 20:36:29 +0000
@@ -15,7 +15,7 @@
 # Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
 
 
-import os, re, shlex
+import os
 
 from bzrlib.tests.blackbox import ExternalBase
 from bzrlib.workingtree import WorkingTree
@@ -59,7 +59,7 @@
         f.close()
 
     def run_bzr_remove_changed_files(self, error_regexes, files_to_remove):
-        error_regexes.extend(["Can't remove changed or unknown files:",
+        error_regexes.extend(["Can't safely remove modified or unknown files:",
             'Use --keep to not delete them,'
             ' or --force to delete them regardless.'
             ])

=== modified file 'bzrlib/tests/workingtree_implementations/test_remove.py'
--- a/bzrlib/tests/workingtree_implementations/test_remove.py	2007-06-28 06:11:06 +0000
+++ b/bzrlib/tests/workingtree_implementations/test_remove.py	2007-08-13 20:00:03 +0000
@@ -17,47 +17,63 @@
 """Tests for interface conformance of 'WorkingTree.remove'"""
 
 from bzrlib.tests.workingtree_implementations import TestCaseWithWorkingTree
-from bzrlib import errors, osutils
+from bzrlib import errors, ignores, osutils
 
 class TestRemove(TestCaseWithWorkingTree):
     """Tests WorkingTree.remove"""
 
     files = ['a', 'b/', 'b/c', 'd/']
     rfiles = ['b/c', 'b', 'a', 'd']
-    a = ['a']
-    b = ['b']
-    b_c = ['b', 'b/c']
 
-    def getTree(self):
+    def get_tree(self, files):
         tree = self.make_branch_and_tree('.')
-        self.build_tree(TestRemove.files)
-        return tree
+        self.build_tree(files)
+        self.failUnlessExists(files)
+        return tree
+
+    def get_committed_tree(self, files, message="Committing"):
+        tree = self.get_tree(files)
+        tree.add(files)
+        tree.commit(message)
+        self.assertInWorkingTree(files)
+        return tree
+
+    def assertRemovedAndDeleted(self, files):
+        self.assertNotInWorkingTree(files)
+        self.failIfExists(files)
+
+    def assertRemovedAndNotDeleted(self, files):
+        self.assertNotInWorkingTree(files)
+        self.failUnlessExists(files)
 
     def test_remove_keep(self):
-        """Check that files are unversioned but not deleted."""
-        tree = self.getTree()
+        """Check that files and directories are unversioned but not deleted."""
+        tree = self.get_tree(TestRemove.files)
         tree.add(TestRemove.files)
         self.assertInWorkingTree(TestRemove.files)
 
         tree.remove(TestRemove.files)
-        self.assertNotInWorkingTree(TestRemove.files)
-        self.failUnlessExists(TestRemove.files)
+        self.assertRemovedAndNotDeleted(TestRemove.files)
+
+    def test_remove_keep_subtree(self):
+        """Check that a directory is unversioned but not deleted."""
+        tree = self.make_branch_and_tree('.')
+        subtree = self.make_branch_and_tree('subtree')
+        tree.add('subtree', 'subtree-id')
+
+        tree.remove('subtree')
+        self.assertRemovedAndNotDeleted('subtree')
 
     def test_remove_unchanged_files(self):
         """Check that unchanged files are removed and deleted."""
-        tree = self.getTree()
-        tree.add(TestRemove.files)
-        tree.commit("files must not have changes")
-        self.assertInWorkingTree(TestRemove.files)
+        tree = self.get_committed_tree(TestRemove.files)
 
         tree.remove(TestRemove.files, keep_files=False)
-
-        self.assertNotInWorkingTree(TestRemove.files)
-        self.failIfExists(TestRemove.files)
+        self.assertRemovedAndDeleted(TestRemove.files)
 
     def test_remove_added_files(self):
         """Removal of newly added files must fail."""
-        tree = self.getTree()
+        tree = self.get_tree(TestRemove.files)
         tree.add(TestRemove.files)
         self.assertInWorkingTree(TestRemove.files)
         err = self.assertRaises(errors.BzrRemoveChangedFilesError, tree.remove,
@@ -69,37 +85,29 @@
 
     def test_remove_changed_file(self):
         """Removal of a changed files must fail."""
-        tree = self.getTree()
-        tree.add(TestRemove.a)
-        tree.commit("make sure a is versioned")
+        tree = self.get_committed_tree('a')
         self.build_tree_contents([('a', "some other new content!")])
-        self.assertInWorkingTree(TestRemove.a)
+        self.assertInWorkingTree('a')
         err = self.assertRaises(errors.BzrRemoveChangedFilesError, tree.remove,
-            TestRemove.a, keep_files=False)
+            'a', keep_files=False)
         self.assertContainsRe(err.changes_as_text, '(?s)modified:.*a')
-        self.assertInWorkingTree(TestRemove.a)
-        self.failUnlessExists(TestRemove.a)
+        self.assertInWorkingTree('a')
+        self.failUnlessExists('a')
 
     def test_remove_deleted_files(self):
         """Check that files are removed if they don't exist any more."""
-        tree = self.getTree()
-        tree.add(TestRemove.files)
-        tree.commit("make sure files are versioned")
+        tree = self.get_committed_tree(TestRemove.files)
         for f in TestRemove.rfiles:
             osutils.delete_any(f)
         self.assertInWorkingTree(TestRemove.files)
         self.failIfExists(TestRemove.files)
 
         tree.remove(TestRemove.files, keep_files=False)
-
-        self.assertNotInWorkingTree(TestRemove.files)
-        self.failIfExists(TestRemove.files)
+        self.assertRemovedAndDeleted(TestRemove.files)
 
     def test_remove_renamed_files(self):
         """Check that files are removed even if they are renamed."""
-        tree = self.getTree()
-        tree.add(TestRemove.files)
-        tree.commit("make sure files are versioned")
+        tree = self.get_committed_tree(TestRemove.files)
 
         for f in TestRemove.rfiles:
             tree.rename_one(f,f+'x')
@@ -108,15 +116,11 @@
         self.failUnlessExists(rfilesx)
 
         tree.remove(rfilesx, keep_files=False)
-
-        self.assertNotInWorkingTree(rfilesx)
-        self.failIfExists(rfilesx)
+        self.assertRemovedAndDeleted(rfilesx)
 
     def test_remove_renamed_changed_files(self):
         """Check that files are not removed if they are renamed and changed."""
-        tree = self.getTree()
-        tree.add(TestRemove.files)
-        tree.commit("make sure files are versioned")
+        tree = self.get_committed_tree(TestRemove.files)
 
         for f in TestRemove.rfiles:
             tree.rename_one(f,f+'x')
@@ -135,18 +139,16 @@
 
     def test_force_remove_changed_files(self):
         """Check that changed files are removed and deleted when forced."""
-        tree = self.getTree()
+        tree = self.get_tree(TestRemove.files)
         tree.add(TestRemove.files)
         self.assertInWorkingTree(TestRemove.files)
 
         tree.remove(TestRemove.files, keep_files=False, force=True)
-
-        self.assertNotInWorkingTree(TestRemove.files)
-        self.failIfExists(TestRemove.files)
+        self.assertRemovedAndDeleted(TestRemove.files)
 
     def test_remove_unknown_files(self):
         """Try to delete unknown files."""
-        tree = self.getTree()
+        tree = self.get_tree(TestRemove.files)
         err = self.assertRaises(errors.BzrRemoveChangedFilesError, tree.remove,
             TestRemove.files, keep_files=False)
         self.assertContainsRe(err.changes_as_text,
@@ -154,70 +156,114 @@
 
     def test_remove_nonexisting_files(self):
         """Try to delete non-existing files."""
-        tree = self.getTree()
+        tree = self.get_tree(TestRemove.files)
         tree.remove([''], keep_files=False)
         tree.remove(['xyz', 'abc/def'], keep_files=False)
 
-    def test_remove_nonempty_directory(self):
-        """Unchanged non-empty directories should be deleted."""
-        tree = self.getTree()
-        tree.add(TestRemove.files)
-        tree.commit("make sure b is versioned")
-        self.assertInWorkingTree(TestRemove.files)
-        self.failUnlessExists(TestRemove.files)
-        tree.remove(TestRemove.b, keep_files=False)
-        self.assertNotInWorkingTree(TestRemove.b)
-        self.failIfExists(TestRemove.b)
-
-    def test_remove_nonempty_directory_with_unknowns(self):
-        """Unchanged non-empty directories should be deleted."""
-        tree = self.getTree()
-        tree.add(TestRemove.files)
-        tree.commit("make sure b is versioned")
-        self.assertInWorkingTree(TestRemove.files)
-        self.failUnlessExists(TestRemove.files)
-        self.build_tree(['b/my_unknown_file'])
-        err = self.assertRaises(errors.BzrRemoveChangedFilesError, tree.remove,
-            TestRemove.b, keep_files=False)
-        self.assertContainsRe(err.changes_as_text,
-            '(?s)unknown:.*b/my_unknown_file')
-        self.assertInWorkingTree(TestRemove.b)
-        self.failUnlessExists(TestRemove.b)
-
-    def test_force_remove_nonempty_directory(self):
-        tree = self.getTree()
-        tree.add(TestRemove.files)
-        tree.commit("make sure b is versioned")
-        self.assertInWorkingTree(TestRemove.files)
-        self.failUnlessExists(TestRemove.files)
-        tree.remove(TestRemove.b, keep_files=False, force=True)
-        self.assertNotInWorkingTree(TestRemove.b_c)
-        self.failIfExists(TestRemove.b_c)
+    def test_remove_unchanged_directory(self):
+        """Unchanged directories should be deleted."""
+        files = ['b/', 'b/c', 'b/sub_directory/', 'b/sub_directory/with_file']
+        tree = self.get_committed_tree(files)
+        tree.remove('b', keep_files=False)
+        self.assertRemovedAndDeleted('b')
+
+    def test_remove_unknown_ignored_files(self):
+        """Unknown ignored files should be deleted."""
+        tree = self.get_committed_tree(['b/'])
+        ignores.add_runtime_ignores(["*ignored*"])
+
+        self.build_tree(['unknown_ignored_file'])
+        self.assertNotEquals(None, tree.is_ignored('unknown_ignored_file'))
+        tree.remove('unknown_ignored_file', keep_files=False)
+        self.assertRemovedAndDeleted('unknown_ignored_file')
+
+        self.build_tree(['b/unknown_ignored_file', 'b/unknown_ignored_dir/'])
+        self.assertNotEquals(None, tree.is_ignored('b/unknown_ignored_file'))
+        self.assertNotEquals(None, tree.is_ignored('b/unknown_ignored_dir'))
+        tree.remove('b', keep_files=False)
+        self.assertRemovedAndDeleted('b')
+
+    def test_dont_remove_directory_with_unknowns(self):
+        """Directories with unknowns should not be deleted."""
+        directories = ['a/', 'b/', 'c/', 'c/c/']
+        tree = self.get_committed_tree(directories)
+
+        self.build_tree(['a/unknown_file'])
+        err = self.assertRaises(errors.BzrRemoveChangedFilesError, tree.remove,
+            'a', keep_files=False)
+        self.assertContainsRe(err.changes_as_text,
+            '(?s)unknown:.*a/unknown_file')
+
+        self.build_tree(['b/unknown_directory'])
+        err = self.assertRaises(errors.BzrRemoveChangedFilesError, tree.remove,
+            'b', keep_files=False)
+        self.assertContainsRe(err.changes_as_text,
+            '(?s)unknown:.*b/unknown_directory')
+
+        self.build_tree(['c/c/unknown_file'])
+        err = self.assertRaises(errors.BzrRemoveChangedFilesError, tree.remove,
+            'c/c', keep_files=False)
+        self.assertContainsRe(err.changes_as_text,
+            '(?s)unknown:.*c/c/unknown_file')
+
+        self.assertInWorkingTree(directories)
+        self.failUnlessExists(directories)
+
+    def test_force_remove_directory_with_unknowns(self):
+        """Unchanged non-empty directories should be deleted when forced."""
+        files = ['b/', 'b/c']
+        tree = self.get_committed_tree(files)
+
+        other_files = ['b/unknown_file', 'b/sub_directory/',
+            'b/sub_directory/with_file', 'b/sub_directory/sub_directory/']
+        self.build_tree(other_files)
+
+        self.assertInWorkingTree(files)
+        self.failUnlessExists(files)
+
+        tree.remove('b', keep_files=False, force=True)
+
+        self.assertRemovedAndDeleted(files)
+        self.assertRemovedAndDeleted(other_files)
 
     def test_remove_directory_with_changed_file(self):
         """Refuse to delete directories with changed files."""
-        tree = self.getTree()
-        tree.add(TestRemove.b_c)
-        tree.commit("make sure b and c are versioned")
+        files = ['b/', 'b/c']
+        tree = self.get_committed_tree(files)
         self.build_tree_contents([('b/c', "some other new content!")])
-        self.assertInWorkingTree(TestRemove.b_c)
+
         err = self.assertRaises(errors.BzrRemoveChangedFilesError, tree.remove,
-            TestRemove.b, keep_files=False)
+            'b', keep_files=False)
         self.assertContainsRe(err.changes_as_text, '(?s)modified:.*b/c')
-        self.assertInWorkingTree(TestRemove.b_c)
-        self.failUnlessExists(TestRemove.b_c)
-
-        #see if we can force it now..
-        tree.remove(TestRemove.b, keep_files=False, force=True)
-        self.assertNotInWorkingTree(TestRemove.b_c)
-        self.failIfExists(TestRemove.b_c)
-
-    def test_remove_subtree(self):
-        tree = self.make_branch_and_tree('.')
-        subtree = self.make_branch_and_tree('subtree')
-        tree.add('subtree', 'subtree-id')
-        tree.remove('subtree')
-        self.assertIs(None, tree.path2id('subtree'))
+        self.assertInWorkingTree(files)
+        self.failUnlessExists(files)
+
+        # see if we can force it now..
+        tree.remove('b', keep_files=False, force=True)
+        self.assertRemovedAndDeleted(files)
+
+    def test_remove_directory_with_renames(self):
+        """Delete directory with renames in or out."""
+
+        files = ['a/', 'a/file', 'a/directory/', 'b/']
+        files_to_move = ['a/file', 'a/directory/']
+
+        tree = self.get_committed_tree(files)
+        # move stuff from a=>b
+        tree.move(['a/file', 'a/directory'], to_dir='b')
+
+        moved_files = ['b/file', 'b/directory/']
+        self.assertRemovedAndDeleted(files_to_move)
+        self.assertInWorkingTree(moved_files)
+        self.failUnlessExists(moved_files)
+
+        # check if it works with renames out
+        tree.remove('a', keep_files=False)
+        self.assertRemovedAndDeleted(['a/'])
+
+        # check if it works with renames in
+        tree.remove('b', keep_files=False)
+        self.assertRemovedAndDeleted(['b/'])
 
     def test_non_cwd(self):
         tree = self.make_branch_and_tree('tree')
@@ -226,3 +272,4 @@
         tree.commit('add file')
         tree.remove('dir/', keep_files=False)
         self.failIfExists('tree/dir/file')
+        self.assertNotInWorkingTree('tree/dir/file', 'tree')

=== modified file 'bzrlib/workingtree.py'
--- a/bzrlib/workingtree.py	2007-08-23 00:12:35 +0000
+++ b/bzrlib/workingtree.py	2007-09-03 05:29:47 +0000
@@ -1793,29 +1793,29 @@
         :force: Delete files and directories, even if they are changed and
             even if the directories are not empty.
         """
-        ## TODO: Normalize names
-
         if isinstance(files, basestring):
             files = [files]
 
         inv_delta = []
 
         new_files=set()
-        unknown_files_in_directory=set()
+        unknown_nested_files=set()
 
         def recurse_directory_to_add_files(directory):
-            # recurse directory and add all files
+            # Recurse directory and add all files
             # so we can check if they have changed.
             for parent_info, file_infos in\
                 osutils.walkdirs(self.abspath(directory),
                     directory):
                 for relpath, basename, kind, lstat, abspath in file_infos:
-                    if kind == 'file':
-                        if self.path2id(relpath): #is it versioned?
-                            new_files.add(relpath)
-                        else:
-                            unknown_files_in_directory.add(
-                                (relpath, None, kind))
+                    # Is it versioned or ignored?
+                    if self.path2id(relpath) or self.is_ignored(relpath):
+                        # Add nested content for deletion.
+                        new_files.add(relpath)
+                    else:
+                        # Files which are not versioned and not ignored
+                        # should be treated as unknown.
+                        unknown_nested_files.add((relpath, None, kind))
 
         for filename in files:
             # Get file name into canonical form.
@@ -1825,40 +1825,48 @@
                 new_files.add(filename)
                 if osutils.isdir(abspath):
                     recurse_directory_to_add_files(filename)
-        files = [f for f in new_files]
+
+        files = list(new_files)
 
         if len(files) == 0:
             return # nothing to do
 
         # Sort needed to first handle directory content before the directory
         files.sort(reverse=True)
+
+        # Bail out if we are going to delete files we shouldn't
         if not keep_files and not force:
-            has_changed_files = len(unknown_files_in_directory) > 0
+            has_changed_files = len(unknown_nested_files) > 0
             if not has_changed_files:
                 for (file_id, path, content_change, versioned, parent_id, name,
                      kind, executable) in self._iter_changes(self.basis_tree(),
                          include_unchanged=True, require_versioned=False,
                          want_unversioned=True, specific_files=files):
-                    # check if it's unknown OR changed but not deleted:
-                    if (versioned == (False, False)
-                        or (content_change and kind[1] != None)):
+                    # Check if it's an unknown (but not ignored) OR
+                    # changed (but not deleted) :
+                    if not self.is_ignored(path[1]) and (
+                        versioned == (False, False) or
+                        content_change and kind[1] != None):
                         has_changed_files = True
                         break
 
             if has_changed_files:
-                # make delta to show ALL applicable changes in error message.
+                # Make delta show ALL applicable changes in error message.
                 tree_delta = self.changes_from(self.basis_tree(),
+                    require_versioned=False, want_unversioned=True,
                     specific_files=files)
-                for unknown_file in unknown_files_in_directory:
-                    tree_delta.unversioned.extend((unknown_file,))
+                for unknown_file in unknown_nested_files:
+                    if unknown_file not in tree_delta.unversioned:
+                        tree_delta.unversioned.extend((unknown_file,))
                 raise errors.BzrRemoveChangedFilesError(tree_delta)
 
-        # do this before any modifications
+        # Build inv_delta and delete files where applicaple,
+        # do this before any modifications to inventory.
         for f in files:
             fid = self.path2id(f)
-            message=None
+            message = None
             if not fid:
-                message="%s is not versioned." % (f,)
+                message = "%s is not versioned." % (f,)
             else:
                 if verbose:
                     # having removed it, it must be either ignored or unknown
@@ -1868,25 +1876,28 @@
                         new_status = '?'
                     textui.show_status(new_status, self.kind(fid), f,
                                        to_file=to_file)
-                # unversion file
+                # Unversion file
                 inv_delta.append((f, None, fid, None))
-                message="removed %s" % (f,)
+                message = "removed %s" % (f,)
 
             if not keep_files:
                 abs_path = self.abspath(f)
                 if osutils.lexists(abs_path):
                     if (osutils.isdir(abs_path) and
                         len(os.listdir(abs_path)) > 0):
-                        message="%s is not empty directory "\
-                            "and won't be deleted." % (f,)
+                        if force:
+                            osutils.rmtree(abs_path)
+                        else:
+                            message = "%s is not an empty directory "\
+                                "and won't be deleted." % (f,)
                     else:
                         osutils.delete_any(abs_path)
-                        message="deleted %s" % (f,)
+                        message = "deleted %s" % (f,)
                 elif message is not None:
-                    # only care if we haven't done anything yet.
-                    message="%s does not exist." % (f,)
+                    # Only care if we haven't done anything yet.
+                    message = "%s does not exist." % (f,)
 
-            # print only one message (if any) per file.
+            # Print only one message (if any) per file.
             if message is not None:
                 note(message)
         self.apply_inventory_delta(inv_delta)




More information about the bazaar-commits mailing list