Rev 6058: Backport the fix for bug #855155 to bzr-2.4 in http://bazaar.launchpad.net/~jameinel/bzr/2.4-dirstate-invalid-delta-855155

[John Arbash Meinel]

At http://bazaar.launchpad.net/~jameinel/bzr/2.4-dirstate-invalid-delta-855155

------------------------------------------------------------
revno: 6058
revision-id: john at arbash-meinel.com-20130528081113-azyyvad54c3am2oq
parent: v.ladeuil+lp at free.fr-20111027132043-io8o52tmhchqywg9
fixes bug: https://launchpad.net/bugs/855155
author: Brian de Alwis <bsd at acm.org>, John Arbash Meinel <john at arbash-meinel.com>
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: 2.4-dirstate-invalid-delta-855155
timestamp: Tue 2013-05-28 09:11:13 +0100
message:
  Backport the fix for bug #855155 to bzr-2.4
  
  Dirstate.update_basis_by_delta had a logic flaw where if it got a delta that added files to
  an otherwise empty directory, it would notice that the dirblock didn't exist,
  and it would claim the delta was invalid, rather than noticing the parent
  directory did exist, it just had an empty (thus nonexistent) block.
  This never triggered in commit, because you always have the files in 'current'
  which forces the block to exist. But things like uncommit can change the basis
  without touching current.
-------------- next part --------------
=== modified file 'bzrlib/dirstate.py'
--- a/bzrlib/dirstate.py	2011-09-28 15:41:38 +0000
+++ b/bzrlib/dirstate.py	2013-05-28 08:11:13 +0000
@@ -1690,9 +1690,18 @@
             entry_key = st(dirname, basename, file_id)
             block_index, present = self._find_block_index_from_key(entry_key)
             if not present:
-                self._raise_invalid(new_path, file_id,
-                    "Unable to find block for this record."
-                    " Was the parent added?")
+                # The block where we want to put the file is not present.
+                # However, it might have just been an empty directory. Look for
+                # the parent in the basis-so-far before throwing an error.
+                parent_dir, parent_base = osutils.split(dirname)
+                parent_block_idx, parent_entry_idx, _, parent_present = \
+                    self._get_block_entry_index(parent_dir, parent_base, 1)
+                if not parent_present:
+                    self._raise_invalid(new_path, file_id,
+                        "Unable to find block for this record."
+                        " Was the parent added?")
+                self._ensure_block(parent_block_idx, parent_entry_idx, dirname)
+
             block = self._dirblocks[block_index][1]
             entry_index, present = self._find_entry_index(entry_key, block)
             if real_add:

=== modified file 'bzrlib/tests/blackbox/test_uncommit.py'
--- a/bzrlib/tests/blackbox/test_uncommit.py	2010-09-15 09:35:42 +0000
+++ b/bzrlib/tests/blackbox/test_uncommit.py	2013-05-28 08:11:13 +0000
@@ -280,3 +280,20 @@
         tree.commit(u'\u1234 message')
         out, err = self.run_bzr('uncommit --force tree', encoding='ascii')
         self.assertContainsRe(out, r'\? message')
+
+
+class TestInconsistentDelta(TestCaseWithTransport):
+    # See https://bugs.launchpad.net/bzr/+bug/855155
+    # See https://bugs.launchpad.net/bzr/+bug/1100385
+    # bzr uncommit may result in error
+    # 'An inconsistent delta was supplied involving'
+
+    def test_inconsistent_delta(self):
+        # Script taken from https://bugs.launchpad.net/bzr/+bug/855155/comments/26
+        wt = self.make_branch_and_tree('test')
+        self.build_tree(['test/a/', 'test/a/b', 'test/a/c'])
+        wt.add(['a', 'a/b', 'a/c'])
+        wt.commit('initial commit', rev_id='a1')
+        wt.remove(['a/b', 'a/c'])
+        wt.commit('remove b and c', rev_id='a2')
+        self.run_bzr("uncommit --force test")

=== modified file 'bzrlib/tests/per_workingtree/test_parents.py'
--- a/bzrlib/tests/per_workingtree/test_parents.py	2011-05-08 16:02:52 +0000
+++ b/bzrlib/tests/per_workingtree/test_parents.py	2013-05-28 08:11:13 +0000
@@ -449,7 +449,7 @@
             self.add_dir(new_shape, new_revid, 'root-id', None, '')
 
     def assertTransitionFromBasisToShape(self, basis_shape, basis_revid,
-        new_shape, new_revid, extra_parent=None):
+        new_shape, new_revid, extra_parent=None, set_current_inventory=True):
         # set the inventory revision ids.
         basis_shape.revision_id = basis_revid
         new_shape.revision_id = new_revid
@@ -464,8 +464,9 @@
                 parents.append(extra_parent)
             tree.set_parent_ids(parents)
         self.fake_up_revision(tree, new_revid, new_shape)
-        # give tree an inventory of new_shape
-        tree._write_inventory(new_shape)
+        if set_current_inventory:
+            # give tree an inventory of new_shape
+            tree._write_inventory(new_shape)
         self.assertDeltaApplicationResultsInExpectedBasis(tree, new_revid,
             delta, new_shape)
         # The tree should be internally consistent; while this is a moderately
@@ -756,3 +757,17 @@
         self.add_link(new_shape, old_revid, 'link-id-C', 'dir-id-B', 'C', 'D')
         self.assertTransitionFromBasisToShape(basis_shape, old_revid,
             new_shape, new_revid)
+
+    def test_add_files_to_empty_directory(self):
+        old_revid = 'old-parent'
+        basis_shape = Inventory(root_id=None)
+        self.add_dir(basis_shape, old_revid, 'root-id', None, '')
+        self.add_dir(basis_shape, old_revid, 'dir-id-A', 'root-id', 'A')
+        new_revid = 'new-parent'
+        new_shape = Inventory(root_id=None)
+        self.add_new_root(new_shape, old_revid, new_revid)
+        self.add_dir(new_shape, old_revid, 'dir-id-A', 'root-id', 'A')
+        self.add_file(new_shape, new_revid, 'file-id-B', 'dir-id-A', 'B',
+            '1' * 32, 24)
+        self.assertTransitionFromBasisToShape(basis_shape, old_revid,
+                new_shape, new_revid, set_current_inventory=False)