Rev 5541: Fix spurious orphan reports. in file:///home/vila/src/bzr/bugs/676608-orphan-reporting/

[Vincent Ladeuil]

At file:///home/vila/src/bzr/bugs/676608-orphan-reporting/

------------------------------------------------------------
revno: 5541
revision-id: v.ladeuil+lp at free.fr-20101119083439-7dg2iit644v586df
parent: pqm at pqm.ubuntu.com-20101116071647-ibe1m73iokndpuyf
fixes bug(s): https://launchpad.net/bugs/676608
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: 676608-orphan-reporting
timestamp: Fri 2010-11-19 09:34:39 +0100
message:
  Fix spurious orphan reports.
  
  * bzrlib/transform.py:
  (_parent_type_conflicts): Fix the comment and inline _any_contents
  for clarity (and avoid a python call).
  (TreeTransformBase._get_potential_orphans): Ignored previously
  versioned files, they will be deleted, not orphaned.
  
  * bzrlib/tests/test_transform.py:
  (_prepare_orphan): Add a versioned file in the directory and
  delete it.
  (TestOrphan.test_new_orphan_created): Check that we get a warning
  for the orphan only.
-------------- next part --------------
=== modified file 'bzrlib/tests/test_transform.py'
--- a/bzrlib/tests/test_transform.py	2010-10-15 14:21:03 +0000
+++ b/bzrlib/tests/test_transform.py	2010-11-19 08:34:39 +0000
@@ -3272,15 +3272,20 @@
                                                policy)
 
     def _prepare_orphan(self, wt):
-        self.build_tree(['dir/', 'dir/foo'])
-        wt.add(['dir'], ['dir-id'])
-        wt.commit('add dir')
+        self.build_tree(['dir/', 'dir/file', 'dir/foo'])
+        wt.add(['dir', 'dir/file'], ['dir-id', 'file-id'])
+        wt.commit('add dir and file ignoring foo')
         tt = transform.TreeTransform(wt)
         self.addCleanup(tt.finalize)
+        # dir and bar are deleted
         dir_tid = tt.trans_id_tree_path('dir')
+        file_tid = tt.trans_id_tree_path('dir/file')
         orphan_tid = tt.trans_id_tree_path('dir/foo')
+        tt.delete_contents(file_tid)
+        tt.unversion_file(file_tid)
         tt.delete_contents(dir_tid)
         tt.unversion_file(dir_tid)
+        # There should be a conflict because dir still contain foo
         raw_conflicts = tt.find_conflicts()
         self.assertLength(1, raw_conflicts)
         self.assertEqual(('missing parent', 'new-1'), raw_conflicts[0])
@@ -3290,7 +3295,13 @@
         wt = self.make_branch_and_tree('.')
         self._set_orphan_policy(wt, 'move')
         tt, orphan_tid = self._prepare_orphan(wt)
+        warnings = []
+        def warning(*args):
+            warnings.append(args[0] % args[1:])
+        self.overrideAttr(trace, 'warning', warning)
         remaining_conflicts = resolve_conflicts(tt)
+        self.assertEquals(['dir/foo has been orphaned in bzr-orphans'],
+                          warnings)
         # Yeah for resolved conflicts !
         self.assertLength(0, remaining_conflicts)
         # We have a new orphan

=== modified file 'bzrlib/transform.py'
--- a/bzrlib/transform.py	2010-11-07 15:56:46 +0000
+++ b/bzrlib/transform.py	2010-11-19 08:34:39 +0000
@@ -703,27 +703,30 @@
         return conflicts
 
     def _parent_type_conflicts(self, by_parent):
-        """parents must have directory 'contents'."""
+        """Children must have a directory parent"""
         conflicts = []
         for parent_id, children in by_parent.iteritems():
             if parent_id is ROOT_PARENT:
                 continue
-            if not self._any_contents(children):
+            existing = False
+            for child_id in children:
+                if self.final_kind(child_id) is not None:
+                    existing = True
+                    break
+            if not existing:
+                # We have no child
                 continue
+            # There is at least a child, so we need an existing directory to
+            # contain it.
             kind = self.final_kind(parent_id)
             if kind is None:
+                # The directory will be deleted
                 conflicts.append(('missing parent', parent_id))
             elif kind != "directory":
+                # Meh, we need a *directory* to put something in it
                 conflicts.append(('non-directory parent', parent_id))
         return conflicts
 
-    def _any_contents(self, trans_ids):
-        """Return true if any of the trans_ids, will have contents."""
-        for trans_id in trans_ids:
-            if self.final_kind(trans_id) is not None:
-                return True
-        return False
-
     def _set_executability(self, path, trans_id):
         """Set the executability of versioned files """
         if supports_executable():
@@ -820,9 +823,14 @@
         """
         orphans = []
         # Find the potential orphans, stop if one item should be kept
-        for c in self.by_parent()[dir_id]:
-            if self.final_file_id(c) is None:
-                orphans.append(c)
+        for child_tid in self.by_parent()[dir_id]:
+            if child_tid in self._removed_contents:
+                # The child is removed as part of the transform. Since it was
+                # versioned before, it's not an orphan
+                continue
+            elif self.final_file_id(child_tid) is None:
+                # The child is not versioned
+                orphans.append(child_tid)
             else:
                 # We have a versioned file here, searching for orphans is
                 # meaningless.