Rev 3596: (robertc) Fix bug 150438 - dirstate corruption due to invalid in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

Canonical.com Patch Queue Manager pqm at pqm.ubuntu.com
Wed Jul 30 22:31:15 BST 2008


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

------------------------------------------------------------
revno: 3596
revision-id:pqm at pqm.ubuntu.com-20080730213059-hcremsawwvcqjcj1
parent: pqm at pqm.ubuntu.com-20080730113809-meoemzm18iybvd4k
parent: robertc at robertcollins.net-20080730095022-4tc7ij34c0tmejb5
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Wed 2008-07-30 22:30:59 +0100
message:
  (robertc) Fix bug 150438 - dirstate corruption due to invalid
  	inventory delta objects. (Robert Collins)
modified:
  NEWS                           NEWS-20050323055033-4e00b5db738777ff
  bzrlib/dirstate.py             dirstate.py-20060728012006-d6mvoihjb3je9peu-1
  bzrlib/inventory.py            inventory.py-20050309040759-6648b84ca2005b37
  bzrlib/osutils.py              osutils.py-20050309040759-eeaff12fbf77ac86
  bzrlib/tests/__init__.py       selftest.py-20050531073622-8d0e3c8845c97a64
  bzrlib/tests/blackbox/test_status.py teststatus.py-20050712014354-508855eb9f29f7dc
  bzrlib/tests/workingtree_implementations/test_remove.py test_remove.py-20070413183901-rvnp85rtc0q0sclp-1
  bzrlib/workingtree.py          workingtree.py-20050511021032-29b6ec0a681e02e3
    ------------------------------------------------------------
    revno: 3585.2.4
    revision-id:robertc at robertcollins.net-20080730095022-4tc7ij34c0tmejb5
    parent: robertc at robertcollins.net-20080730085510-1qnn946mganbu2bd
    committer: Robert Collins <robertc at robertcollins.net>
    branch nick: 150438
    timestamp: Wed 2008-07-30 19:50:22 +1000
    message:
       * Deleting directories by hand before running ``bzr rm`` will not
         cause subsequent errors in ``bzr st`` and ``bzr commit``.
         (Robert Collins, #150438)
    modified:
      NEWS                           NEWS-20050323055033-4e00b5db738777ff
      bzrlib/osutils.py              osutils.py-20050309040759-eeaff12fbf77ac86
      bzrlib/workingtree.py          workingtree.py-20050511021032-29b6ec0a681e02e3
    ------------------------------------------------------------
    revno: 3585.2.3
    revision-id:robertc at robertcollins.net-20080730085510-1qnn946mganbu2bd
    parent: robertc at robertcollins.net-20080730082020-vff6erv12li5x0en
    committer: Robert Collins <robertc at robertcollins.net>
    branch nick: 150438
    timestamp: Wed 2008-07-30 18:55:10 +1000
    message:
      Teach dirstate.update_by_delta to detect corrupt deltas which do not remove children explicitly.
    modified:
      bzrlib/dirstate.py             dirstate.py-20060728012006-d6mvoihjb3je9peu-1
    ------------------------------------------------------------
    revno: 3585.2.2
    revision-id:robertc at robertcollins.net-20080730082020-vff6erv12li5x0en
    parent: robertc at robertcollins.net-20080730075222-azbmg0knc7u9mkpg
    committer: Robert Collins <robertc at robertcollins.net>
    branch nick: 150438
    timestamp: Wed 2008-07-30 18:20:20 +1000
    message:
      Cause apply_inventory_delta on Inventory objects to fail with deltas that leave dangling children.
    modified:
      bzrlib/inventory.py            inventory.py-20050309040759-6648b84ca2005b37
    ------------------------------------------------------------
    revno: 3585.2.1
    revision-id:robertc at robertcollins.net-20080730075222-azbmg0knc7u9mkpg
    parent: pqm at pqm.ubuntu.com-20080729005846-o7t0ck17azx0xddl
    committer: Robert Collins <robertc at robertcollins.net>
    branch nick: 150438
    timestamp: Wed 2008-07-30 17:52:22 +1000
    message:
      Create acceptance test for bug 150438.
    modified:
      bzrlib/tests/__init__.py       selftest.py-20050531073622-8d0e3c8845c97a64
      bzrlib/tests/blackbox/test_status.py teststatus.py-20050712014354-508855eb9f29f7dc
      bzrlib/tests/workingtree_implementations/test_remove.py test_remove.py-20070413183901-rvnp85rtc0q0sclp-1
=== modified file 'NEWS'
--- a/NEWS	2008-07-30 04:34:59 +0000
+++ b/NEWS	2008-07-30 21:30:59 +0000
@@ -57,6 +57,9 @@
       smart server protocol to or from Windows.
       (Andrew Bennetts, #246180)
 
+    * Deleting directories by hand before running ``bzr rm`` will not
+      cause subsequent errors in ``bzr st`` and ``bzr commit``.
+      (Robert Collins, #150438)
 
   DOCUMENTATION:
 

=== modified file 'bzrlib/dirstate.py'
--- a/bzrlib/dirstate.py	2008-06-05 21:47:39 +0000
+++ b/bzrlib/dirstate.py	2008-07-30 08:55:10 +0000
@@ -1199,6 +1199,7 @@
                     fingerprint = ''
                 insertions[file_id] = (key, minikind, inv_entry.executable,
                                        fingerprint, new_path)
+            # Transform moves into delete+add pairs
             if None not in (old_path, new_path):
                 for child in self._iter_child_entries(0, old_path):
                     if child[0][2] in insertions or child[0][2] in removals:
@@ -1228,6 +1229,21 @@
                 self._get_block_entry_index(dirname, basename, 0)
             entry = self._dirblocks[block_i][1][entry_i]
             self._make_absent(entry)
+            # See if we have a malformed delta: deleting a directory must not
+            # leave crud behind. This increases the number of bisects needed
+            # substantially, but deletion or renames of large numbers of paths
+            # is rare enough it shouldn't be an issue (famous last words?) RBC
+            # 20080730.
+            block_i, entry_i, d_present, f_present = \
+                self._get_block_entry_index(path, '', 0)
+            if d_present:
+                # The dir block is still present in the dirstate; this could
+                # be due to it being in a parent tree, or a corrupt delta.
+                for child_entry in self._dirblocks[block_i][1]:
+                    if child_entry[1][0][0] not in ('r', 'a'):
+                        raise errors.InconsistentDelta(path, entry[0][2],
+                            "The file id was deleted but its children were "
+                            "not deleted.")
 
     def _apply_insertions(self, adds):
         for key, minikind, executable, fingerprint, path_utf8 in sorted(adds):

=== modified file 'bzrlib/inventory.py'
--- a/bzrlib/inventory.py	2008-07-25 02:39:08 +0000
+++ b/bzrlib/inventory.py	2008-07-30 21:30:59 +0000
@@ -815,7 +815,9 @@
                 # adds come later
                 continue
             # Preserve unaltered children of file_id for later reinsertion.
-            children[file_id] = getattr(self[file_id], 'children', {})
+            file_id_children = getattr(self[file_id], 'children', {})
+            if len(file_id_children):
+                children[file_id] = file_id_children
             # Remove file_id and the unaltered children. If file_id is not
             # being deleted it will be reinserted back later.
             self.remove_recursive_id(file_id)
@@ -827,8 +829,16 @@
         for new_path, new_entry in sorted((np, e) for op, np, f, e in
                                           delta if np is not None):
             if new_entry.kind == 'directory':
-                new_entry.children = children.get(new_entry.file_id, {})
+                # Pop the child which to allow detection of children whose
+                # parents were deleted and which were not reattached to a new
+                # parent.
+                new_entry.children = children.pop(new_entry.file_id, {})
             self.add(new_entry)
+        if len(children):
+            # Get the parent id that was deleted
+            parent_id, children = children.popitem()
+            raise errors.InconsistentDelta("<deleted>", parent_id,
+                "The file id was deleted but its children were not deleted.")
 
     def _set_root(self, ie):
         self.root = ie

=== modified file 'bzrlib/osutils.py'
--- a/bzrlib/osutils.py	2008-07-17 20:16:43 +0000
+++ b/bzrlib/osutils.py	2008-07-30 09:50:22 +0000
@@ -1168,11 +1168,20 @@
 
         dirblock = []
         append = dirblock.append
-        for name in sorted(_listdir(top)):
-            abspath = top_slash + name
-            statvalue = _lstat(abspath)
-            kind = _kind_from_mode(statvalue.st_mode & 0170000, 'unknown')
-            append((relprefix + name, name, kind, statvalue, abspath))
+        try:
+            names = sorted(_listdir(top))
+        except OSError, e:
+            if getattr(e, 'errno', None) == errno.ENOTDIR:
+                # We have been asked to examine a file, this is fine.
+                pass
+            else:
+                raise
+        else:
+            for name in names:
+                abspath = top_slash + name
+                statvalue = _lstat(abspath)
+                kind = _kind_from_mode(statvalue.st_mode & 0170000, 'unknown')
+                append((relprefix + name, name, kind, statvalue, abspath))
         yield (relroot, top), dirblock
 
         # push the user specified dirs from dirblock

=== modified file 'bzrlib/tests/__init__.py'
--- a/bzrlib/tests/__init__.py	2008-07-22 19:20:52 +0000
+++ b/bzrlib/tests/__init__.py	2008-07-30 21:30:59 +0000
@@ -2138,7 +2138,7 @@
             tree = workingtree.WorkingTree.open(root_path)
         if not isinstance(path, basestring):
             for p in path:
-                self.assertInWorkingTree(p,tree=tree)
+                self.assertInWorkingTree(p, tree=tree)
         else:
             self.assertIsNot(tree.path2id(path), None,
                 path+' not in working tree.')

=== modified file 'bzrlib/tests/blackbox/test_status.py'
--- a/bzrlib/tests/blackbox/test_status.py	2008-05-01 20:10:42 +0000
+++ b/bzrlib/tests/blackbox/test_status.py	2008-07-30 07:52:22 +0000
@@ -438,13 +438,11 @@
         b_tree.add('b')
         b_tree.commit('b')
 
-        chdir('a')
-        self.run_bzr('merge ../b')
-        out, err = self.run_bzr('status --no-pending')
+        self.run_bzr('merge ../b', working_dir='a')
+        out, err = self.run_bzr('status --no-pending', working_dir='a')
         self.assertEquals(out, "added:\n  b\n")
 
 
-
 class TestStatusEncodings(TestCaseWithTransport):
     
     def setUp(self):

=== modified file 'bzrlib/tests/workingtree_implementations/test_remove.py'
--- a/bzrlib/tests/workingtree_implementations/test_remove.py	2007-11-29 18:06:55 +0000
+++ b/bzrlib/tests/workingtree_implementations/test_remove.py	2008-07-30 07:52:22 +0000
@@ -67,9 +67,9 @@
     def test_remove_unchanged_files(self):
         """Check that unchanged files are removed and deleted."""
         tree = self.get_committed_tree(TestRemove.files)
-
         tree.remove(TestRemove.files, keep_files=False)
         self.assertRemovedAndDeleted(TestRemove.files)
+        tree._validate()
 
     def test_remove_added_files(self):
         """Removal of newly added files must fail."""
@@ -82,6 +82,7 @@
             '(?s)added:.*a.*b/.*b/c.*d/')
         self.assertInWorkingTree(TestRemove.files)
         self.failUnlessExists(TestRemove.files)
+        tree._validate()
 
     def test_remove_changed_file(self):
         """Removal of a changed files must fail."""
@@ -93,6 +94,7 @@
         self.assertContainsRe(err.changes_as_text, '(?s)modified:.*a')
         self.assertInWorkingTree('a')
         self.failUnlessExists('a')
+        tree._validate()
 
     def test_remove_deleted_files(self):
         """Check that files are removed if they don't exist any more."""
@@ -101,9 +103,9 @@
             osutils.delete_any(f)
         self.assertInWorkingTree(TestRemove.files)
         self.failIfExists(TestRemove.files)
-
         tree.remove(TestRemove.files, keep_files=False)
         self.assertRemovedAndDeleted(TestRemove.files)
+        tree._validate()
 
     def test_remove_renamed_files(self):
         """Check that files are removed even if they are renamed."""
@@ -117,6 +119,7 @@
 
         tree.remove(rfilesx, keep_files=False)
         self.assertRemovedAndDeleted(rfilesx)
+        tree._validate()
 
     def test_remove_renamed_changed_files(self):
         """Check that files are not removed if they are renamed and changed."""
@@ -136,6 +139,7 @@
             '(?s)modified:.*ax.*bx/cx')
         self.assertInWorkingTree(rfilesx)
         self.failUnlessExists(rfilesx)
+        tree._validate()
 
     def test_force_remove_changed_files(self):
         """Check that changed files are removed and deleted when forced."""
@@ -145,6 +149,7 @@
 
         tree.remove(TestRemove.files, keep_files=False, force=True)
         self.assertRemovedAndDeleted(TestRemove.files)
+        tree._validate()
 
     def test_remove_unknown_files(self):
         """Try to delete unknown files."""
@@ -153,12 +158,14 @@
             TestRemove.files, keep_files=False)
         self.assertContainsRe(err.changes_as_text,
             '(?s)unknown:.*d/.*b/c.*b/.*a.*')
+        tree._validate()
 
     def test_remove_nonexisting_files(self):
         """Try to delete non-existing files."""
         tree = self.get_tree(TestRemove.files)
         tree.remove([''], keep_files=False)
         tree.remove(['xyz', 'abc/def'], keep_files=False)
+        tree._validate()
 
     def test_remove_unchanged_directory(self):
         """Unchanged directories should be deleted."""
@@ -166,6 +173,16 @@
         tree = self.get_committed_tree(files)
         tree.remove('b', keep_files=False)
         self.assertRemovedAndDeleted('b')
+        tree._validate()
+
+    def test_remove_absent_directory(self):
+        """Removing a absent directory succeeds without corruption (#150438)."""
+        paths = ['a/', 'a/b']
+        tree = self.get_committed_tree(paths)
+        self.get_transport('.').delete_tree('a')
+        tree.remove(['a'])
+        self.assertRemovedAndDeleted('b')
+        tree._validate()
 
     def test_remove_unknown_ignored_files(self):
         """Unknown ignored files should be deleted."""
@@ -182,6 +199,7 @@
         self.assertNotEquals(None, tree.is_ignored('b/unknown_ignored_dir'))
         tree.remove('b', keep_files=False)
         self.assertRemovedAndDeleted('b')
+        tree._validate()
 
     def test_remove_changed_ignored_files(self):
         """Changed ignored files should not be deleted."""
@@ -196,6 +214,7 @@
         self.assertContainsRe(err.changes_as_text,
             '(?s)added:.*' + files[0])
         self.assertInWorkingTree(files)
+        tree._validate()
 
     def test_dont_remove_directory_with_unknowns(self):
         """Directories with unknowns should not be deleted."""
@@ -222,6 +241,7 @@
 
         self.assertInWorkingTree(directories)
         self.failUnlessExists(directories)
+        tree._validate()
 
     def test_force_remove_directory_with_unknowns(self):
         """Unchanged non-empty directories should be deleted when forced."""
@@ -239,6 +259,7 @@
 
         self.assertRemovedAndDeleted(files)
         self.assertRemovedAndDeleted(other_files)
+        tree._validate()
 
     def test_remove_directory_with_changed_file(self):
         """Refuse to delete directories with changed files."""
@@ -255,6 +276,7 @@
         # see if we can force it now..
         tree.remove('b', keep_files=False, force=True)
         self.assertRemovedAndDeleted(files)
+        tree._validate()
 
     def test_remove_directory_with_renames(self):
         """Delete directory with renames in or out."""
@@ -278,6 +300,7 @@
         # check if it works with renames in
         tree.remove('b', keep_files=False)
         self.assertRemovedAndDeleted(['b/'])
+        tree._validate()
 
     def test_non_cwd(self):
         tree = self.make_branch_and_tree('tree')
@@ -287,6 +310,7 @@
         tree.remove('dir/', keep_files=False)
         self.failIfExists('tree/dir/file')
         self.assertNotInWorkingTree('tree/dir/file', 'tree')
+        tree._validate()
 
     def test_remove_uncommitted_removed_file(self):
         # As per bug #152811
@@ -294,6 +318,7 @@
         tree.remove('a', keep_files=False)
         tree.remove('a', keep_files=False)
         self.failIfExists('a')
+        tree._validate()
 
     def test_remove_file_and_containing_dir(self):
         tree = self.get_committed_tree(['config/', 'config/file'])
@@ -301,3 +326,4 @@
         tree.remove('config', keep_files=False)
         self.failIfExists('config/file')
         self.failIfExists('config')
+        tree._validate()

=== modified file 'bzrlib/workingtree.py'
--- a/bzrlib/workingtree.py	2008-07-08 14:55:19 +0000
+++ b/bzrlib/workingtree.py	2008-07-30 09:50:22 +0000
@@ -1859,9 +1859,8 @@
             # 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:
+                self.walkdirs(directory):
+                for relpath, basename, kind, lstat, fileid, kind in file_infos:
                     # Is it versioned or ignored?
                     if self.path2id(relpath) or self.is_ignored(relpath):
                         # Add nested content for deletion.
@@ -1877,8 +1876,7 @@
             filename = self.relpath(abspath)
             if len(filename) > 0:
                 new_files.add(filename)
-                if osutils.isdir(abspath):
-                    recurse_directory_to_add_files(filename)
+                recurse_directory_to_add_files(filename)
 
         files = list(new_files)
 
@@ -2418,10 +2416,11 @@
                 relroot = ""
             # FIXME: stash the node in pending
             entry = inv[top_id]
-            for name, child in entry.sorted_children():
-                dirblock.append((relroot + name, name, child.kind, None,
-                    child.file_id, child.kind
-                    ))
+            if entry.kind == 'directory':
+                for name, child in entry.sorted_children():
+                    dirblock.append((relroot + name, name, child.kind, None,
+                        child.file_id, child.kind
+                        ))
             yield (currentdir[0], entry.file_id), dirblock
             # push the user specified dirs from dirblock
             for dir in reversed(dirblock):




More information about the bazaar-commits mailing list