Rev 2930: (John Arbash Meinel) Fix bug #114615 by telling WT4.unversion() to skip renamed entries. in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

Canonical.com Patch Queue Manager pqm at pqm.ubuntu.com
Wed Oct 24 00:01:28 BST 2007


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

------------------------------------------------------------
revno: 2930
revision-id: pqm at pqm.ubuntu.com-20071023230126-g4h0l6g1dz8e3d57
parent: pqm at pqm.ubuntu.com-20071023082111-h6u34i4gvlb2nwch
parent: john at arbash-meinel.com-20071023214830-5mlzu2mjcmnx7znz
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Wed 2007-10-24 00:01:26 +0100
message:
  (John Arbash Meinel) Fix bug #114615 by telling WT4.unversion() to skip renamed entries.
modified:
  bzrlib/dirstate.py             dirstate.py-20060728012006-d6mvoihjb3je9peu-1
  bzrlib/tests/workingtree_implementations/test_commit.py test_commit.py-20060421013633-1610ec2331c8190f
  bzrlib/tests/workingtree_implementations/test_unversion.py test_unversion.py-20060907074408-bygh2y28jz8u0cg7-1
  bzrlib/workingtree_4.py        workingtree_4.py-20070208044105-5fgpc5j3ljlh5q6c-1
    ------------------------------------------------------------
    revno: 2922.2.6
    merged: john at arbash-meinel.com-20071023214830-5mlzu2mjcmnx7znz
    parent: john at arbash-meinel.com-20071023213503-xrvfwqsaah4jmfn2
    committer: John Arbash Meinel <john at arbash-meinel.com>
    branch nick: bogus_removal_114615
    timestamp: Tue 2007-10-23 16:48:30 -0500
    message:
      Add another direct test in unversion, because the other test
      did not trigger the 'delete a file in another directory' bug.
    ------------------------------------------------------------
    revno: 2922.2.5
    merged: john at arbash-meinel.com-20071023213503-xrvfwqsaah4jmfn2
    parent: john at arbash-meinel.com-20071023211052-ru4vxnhwb0vpemz4
    committer: John Arbash Meinel <john at arbash-meinel.com>
    branch nick: bogus_removal_114615
    timestamp: Tue 2007-10-23 16:35:03 -0500
    message:
      Add a direct test for WT.unversion()
    ------------------------------------------------------------
    revno: 2922.2.4
    merged: john at arbash-meinel.com-20071023211052-ru4vxnhwb0vpemz4
    parent: john at arbash-meinel.com-20071023195147-echbf8brs44uifo4
    committer: John Arbash Meinel <john at arbash-meinel.com>
    branch nick: bogus_removal_114615
    timestamp: Tue 2007-10-23 16:10:52 -0500
    message:
      Fix bug #114615 by teaching unversion() to not touch renamed entries.
    ------------------------------------------------------------
    revno: 2922.2.3
    merged: john at arbash-meinel.com-20071023195147-echbf8brs44uifo4
    parent: john at arbash-meinel.com-20071023194449-wb8fafx5unde63wq
    committer: John Arbash Meinel <john at arbash-meinel.com>
    branch nick: bogus_removal_114615
    timestamp: Tue 2007-10-23 14:51:47 -0500
    message:
      Add a test which shows why the previous fix is broken.
      Basically, it causes files which are renamed *out* of that directory to
      be considered deleted. So instead just remove the assertion that
      the target should not be absent.
    ------------------------------------------------------------
    revno: 2922.2.2
    merged: john at arbash-meinel.com-20071023194449-wb8fafx5unde63wq
    parent: john at arbash-meinel.com-20071023181218-in3x181fnemz3vfj
    committer: John Arbash Meinel <john at arbash-meinel.com>
    branch nick: bogus_removal_114615
    timestamp: Tue 2007-10-23 14:44:49 -0500
    message:
      If THIS is considered renamed, _make_absent needs to remove the renamed target.
    ------------------------------------------------------------
    revno: 2922.2.1
    merged: john at arbash-meinel.com-20071023181218-in3x181fnemz3vfj
    parent: pqm at pqm.ubuntu.com-20071022014712-mpln8namgmsywr75
    committer: John Arbash Meinel <john at arbash-meinel.com>
    branch nick: bogus_removal_114615
    timestamp: Tue 2007-10-23 13:12:18 -0500
    message:
      Add failing tests exposing part of bug #114615
=== modified file 'bzrlib/dirstate.py'
--- a/bzrlib/dirstate.py	2007-10-22 20:05:12 +0000
+++ b/bzrlib/dirstate.py	2007-10-23 23:01:26 +0000
@@ -2019,7 +2019,7 @@
             if self._id_index is not None:
                 self._id_index[current_old[0][2]].remove(current_old[0])
         # update all remaining keys for this id to record it as absent. The
-        # existing details may either be the record we are making as deleted
+        # existing details may either be the record we are marking as deleted
         # (if there were other trees with the id present at this path), or may
         # be relocations.
         for update_key in all_remaining_keys:

=== modified file 'bzrlib/tests/workingtree_implementations/test_commit.py'
--- a/bzrlib/tests/workingtree_implementations/test_commit.py	2007-09-20 07:59:48 +0000
+++ b/bzrlib/tests/workingtree_implementations/test_commit.py	2007-10-23 21:10:52 +0000
@@ -21,7 +21,9 @@
 from bzrlib import (
     branch,
     bzrdir,
+    conflicts,
     errors,
+    osutils,
     revision as _mod_revision,
     ui,
     uncommit,
@@ -88,6 +90,110 @@
 
 class TestCommit(TestCaseWithWorkingTree):
 
+    def test_autodelete_renamed(self):
+        tree_a = self.make_branch_and_tree('a')
+        self.build_tree(['a/dir/', 'a/dir/f1', 'a/dir/f2'])
+        tree_a.add(['dir', 'dir/f1', 'dir/f2'], ['dir-id', 'f1-id', 'f2-id'])
+        rev_id1 = tree_a.commit('init')
+        # Start off by renaming entries,
+        # but then actually auto delete the whole tree
+        # https://bugs.launchpad.net/bzr/+bug/114615
+        tree_a.rename_one('dir/f1', 'dir/a')
+        tree_a.rename_one('dir/f2', 'dir/z')
+        osutils.rmtree('a/dir')
+        tree_a.commit('autoremoved')
+
+        tree_a.lock_read()
+        try:
+            root_id = tree_a.inventory.root.file_id
+            paths = [(path, ie.file_id)
+                     for path, ie in tree_a.iter_entries_by_dir()]
+        finally:
+            tree_a.unlock()
+        # The only paths left should be the root
+        self.assertEqual([('', root_id)], paths)
+
+    def test_no_autodelete_renamed_away(self):
+        tree_a = self.make_branch_and_tree('a')
+        self.build_tree(['a/dir/', 'a/dir/f1', 'a/dir/f2', 'a/dir2/'])
+        tree_a.add(['dir', 'dir/f1', 'dir/f2', 'dir2'],
+                   ['dir-id', 'f1-id', 'f2-id', 'dir2-id'])
+        rev_id1 = tree_a.commit('init')
+        # Rename one entry out of this directory
+        tree_a.rename_one('dir/f1', 'dir2/a')
+        osutils.rmtree('a/dir')
+        tree_a.commit('autoremoved')
+
+        tree_a.lock_read()
+        try:
+            root_id = tree_a.inventory.root.file_id
+            paths = [(path, ie.file_id)
+                     for path, ie in tree_a.iter_entries_by_dir()]
+        finally:
+            tree_a.unlock()
+        # The only paths left should be the root
+        self.assertEqual([('', root_id), ('dir2', 'dir2-id'),
+                          ('dir2/a', 'f1-id'),
+                         ], paths)
+
+    def test_no_autodelete_alternate_renamed(self):
+        # Test for bug #114615
+        tree_a = self.make_branch_and_tree('A')
+        self.build_tree(['A/a/', 'A/a/m', 'A/a/n'])
+        tree_a.add(['a', 'a/m', 'a/n'], ['a-id', 'm-id', 'n-id'])
+        tree_a.commit('init')
+
+        tree_a.lock_read()
+        try:
+            root_id = tree_a.inventory.root.file_id
+        finally:
+            tree_a.unlock()
+
+        tree_b = tree_a.bzrdir.sprout('B').open_workingtree()
+        self.build_tree(['B/xyz/'])
+        tree_b.add(['xyz'], ['xyz-id'])
+        tree_b.rename_one('a/m', 'xyz/m')
+        osutils.rmtree('B/a')
+        tree_b.commit('delete in B')
+
+        paths = [(path, ie.file_id)
+                 for path, ie in tree_b.iter_entries_by_dir()]
+        self.assertEqual([('', root_id),
+                          ('xyz', 'xyz-id'),
+                          ('xyz/m', 'm-id'),
+                         ], paths)
+
+        self.build_tree_contents([('A/a/n', 'new contents for n\n')])
+        tree_a.commit('change n in A')
+
+        # Merging from A should introduce conflicts because 'n' was modified
+        # and removed, so 'a' needs to be restored.
+        num_conflicts = tree_b.merge_from_branch(tree_a.branch)
+        self.assertEqual(3, num_conflicts)
+        paths = [(path, ie.file_id)
+                 for path, ie in tree_b.iter_entries_by_dir()]
+        self.assertEqual([('', root_id),
+                          ('a', 'a-id'),
+                          ('xyz', 'xyz-id'),
+                          ('a/n.OTHER', 'n-id'),
+                          ('xyz/m', 'm-id'),
+                         ], paths)
+        osutils.rmtree('B/a')
+        try:
+            # bzr resolve --all
+            tree_b.set_conflicts(conflicts.ConflictList())
+        except errors.UnsupportedOperation:
+            # On WT2, set_conflicts is unsupported, but the rmtree has the same
+            # effect.
+            pass
+        tree_b.commit('autoremove a, without touching xyz/m')
+        paths = [(path, ie.file_id)
+                 for path, ie in tree_b.iter_entries_by_dir()]
+        self.assertEqual([('', root_id),
+                          ('xyz', 'xyz-id'),
+                          ('xyz/m', 'm-id'),
+                         ], paths)
+
     def test_commit_sets_last_revision(self):
         tree = self.make_branch_and_tree('tree')
         committed_id = tree.commit('foo', rev_id='foo')

=== modified file 'bzrlib/tests/workingtree_implementations/test_unversion.py'
--- a/bzrlib/tests/workingtree_implementations/test_unversion.py	2007-03-01 01:56:28 +0000
+++ b/bzrlib/tests/workingtree_implementations/test_unversion.py	2007-10-23 21:48:30 +0000
@@ -16,7 +16,10 @@
 
 """Tests of the WorkingTree.unversion API."""
 
-from bzrlib import errors
+from bzrlib import (
+    errors,
+    osutils,
+    )
 from bzrlib.tests.workingtree_implementations import TestCaseWithWorkingTree
 
 
@@ -99,3 +102,91 @@
             self.assertTrue(tree.has_filename('d'))
         finally:
             tree.unlock()
+
+    def test_unversion_renamed(self):
+        tree = self.make_branch_and_tree('a')
+        self.build_tree(['a/dir/', 'a/dir/f1', 'a/dir/f2', 'a/dir/f3',
+                         'a/dir2/'])
+        tree.add(['dir', 'dir/f1', 'dir/f2', 'dir/f3', 'dir2'],
+                 ['dir-id', 'f1-id', 'f2-id', 'f3-id', 'dir2-id'])
+        rev_id1 = tree.commit('init')
+        # Start off by renaming entries, and then unversion a bunch of entries
+        # https://bugs.launchpad.net/bzr/+bug/114615
+        tree.rename_one('dir/f1', 'dir/a')
+        tree.rename_one('dir/f2', 'dir/z')
+        tree.move(['dir/f3'], 'dir2')
+
+        tree.lock_read()
+        try:
+            root_id = tree.inventory.root.file_id
+            paths = [(path, ie.file_id)
+                     for path, ie in tree.iter_entries_by_dir()]
+        finally:
+            tree.unlock()
+        self.assertEqual([('', root_id),
+                          ('dir', 'dir-id'),
+                          ('dir2', 'dir2-id'),
+                          ('dir/a', 'f1-id'),
+                          ('dir/z', 'f2-id'),
+                          ('dir2/f3', 'f3-id'),
+                         ], paths)
+
+        tree.unversion(set(['dir-id']))
+        paths = [(path, ie.file_id)
+                 for path, ie in tree.iter_entries_by_dir()]
+
+        self.assertEqual([('', root_id),
+                          ('dir2', 'dir2-id'),
+                          ('dir2/f3', 'f3-id'),
+                         ], paths)
+
+    def test_unversion_after_conflicted_merge(self):
+        # Test for bug #114615
+        tree_a = self.make_branch_and_tree('A')
+        self.build_tree(['A/a/', 'A/a/m', 'A/a/n'])
+        tree_a.add(['a', 'a/m', 'a/n'], ['a-id', 'm-id', 'n-id'])
+        tree_a.commit('init')
+
+        tree_a.lock_read()
+        try:
+            root_id = tree_a.inventory.root.file_id
+        finally:
+            tree_a.unlock()
+
+        tree_b = tree_a.bzrdir.sprout('B').open_workingtree()
+        self.build_tree(['B/xyz/'])
+        tree_b.add(['xyz'], ['xyz-id'])
+        tree_b.rename_one('a/m', 'xyz/m')
+        tree_b.unversion(['a-id'])
+        tree_b.commit('delete in B')
+
+        paths = [(path, ie.file_id)
+                 for path, ie in tree_b.iter_entries_by_dir()]
+        self.assertEqual([('', root_id),
+                          ('xyz', 'xyz-id'),
+                          ('xyz/m', 'm-id'),
+                         ], paths)
+
+        self.build_tree_contents([('A/a/n', 'new contents for n\n')])
+        tree_a.commit('change n in A')
+
+        # Merging from A should introduce conflicts because 'n' was modified
+        # and removed, so 'a' needs to be restored. We also have a conflict
+        # because 'a' is still an existing directory
+        num_conflicts = tree_b.merge_from_branch(tree_a.branch)
+        self.assertEqual(4, num_conflicts)
+        paths = [(path, ie.file_id)
+                 for path, ie in tree_b.iter_entries_by_dir()]
+        self.assertEqual([('', root_id),
+                          ('a', 'a-id'),
+                          ('xyz', 'xyz-id'),
+                          ('a/n.OTHER', 'n-id'),
+                          ('xyz/m', 'm-id'),
+                         ], paths)
+        tree_b.unversion(['a-id'])
+        paths = [(path, ie.file_id)
+                 for path, ie in tree_b.iter_entries_by_dir()]
+        self.assertEqual([('', root_id),
+                          ('xyz', 'xyz-id'),
+                          ('xyz/m', 'm-id'),
+                         ], paths)

=== modified file 'bzrlib/workingtree_4.py'
--- a/bzrlib/workingtree_4.py	2007-10-17 17:03:06 +0000
+++ b/bzrlib/workingtree_4.py	2007-10-23 21:10:52 +0000
@@ -1189,7 +1189,8 @@
                     # Mark this file id as having been removed
                     entry = block[1][entry_index]
                     ids_to_unversion.discard(entry[0][2])
-                    if (entry[1][0][0] == 'a'
+                    if (entry[1][0][0] in 'ar' # don't remove absent or renamed
+                                               # entries
                         or not state._make_absent(entry)):
                         entry_index += 1
                 # go to the next block. (At the moment we dont delete empty




More information about the bazaar-commits mailing list