Rev 5404: (vila) Cleanups in bzrlib.transform.py (Vincent Ladeuil) in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

Canonical.com Patch Queue Manager pqm at pqm.ubuntu.com
Wed Sep 1 19:06:46 BST 2010


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

------------------------------------------------------------
revno: 5404 [merge]
revision-id: pqm at pqm.ubuntu.com-20100901180619-u0q01om0gjzxxy2i
parent: pqm at pqm.ubuntu.com-20100901112740-kt9s91eujt7ayp3m
parent: v.ladeuil+lp at free.fr-20100901164201-naaaweke048ix3kj
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Wed 2010-09-01 19:06:19 +0100
message:
  (vila) Cleanups in bzrlib.transform.py (Vincent Ladeuil)
modified:
  NEWS                           NEWS-20050323055033-4e00b5db738777ff
  bzrlib/merge.py                merge.py-20050513021216-953b65a438527106
  bzrlib/tests/test_transform.py test_transaction.py-20060105172520-b3ffb3946550e6c4
  bzrlib/transform.py            transform.py-20060105172343-dd99e54394d91687
=== modified file 'NEWS'
--- a/NEWS	2010-09-01 11:27:40 +0000
+++ b/NEWS	2010-09-01 18:06:19 +0000
@@ -20,6 +20,11 @@
   is now named "msg" instead of earlier "message".
   (Parth Malwankar, #603461)
 
+* ``bzrlib.transform.TreeTransformBase.final_kind``,
+  ``bzrlib.transform.TreeTransform.tree_kind`` and
+  ``bzrlib.transform.TransformPreview.tree_kind`` now return None instead
+  of raising NoSuchFile.  (Vincent Ladeuil)
+
 * `decode` parameter to get() method in FtpTransport and GioTransport classes
   is deprecated. (Alexander Belchenko)
 

=== modified file 'bzrlib/merge.py'
--- a/bzrlib/merge.py	2010-07-29 03:56:54 +0000
+++ b/bzrlib/merge.py	2010-08-25 13:02:32 +0000
@@ -1085,9 +1085,7 @@
         return result
 
     def fix_root(self):
-        try:
-            self.tt.final_kind(self.tt.root)
-        except errors.NoSuchFile:
+        if self.tt.final_kind(self.tt.root) is None:
             self.tt.cancel_deletion(self.tt.root)
         if self.tt.final_file_id(self.tt.root) is None:
             self.tt.version_file(self.tt.tree_file_id(self.tt.root),
@@ -1102,10 +1100,9 @@
             # the other tree's root is a non-root in the current tree (as when
             # a previously unrelated branch is merged into another)
             return
-        try:
-            self.tt.final_kind(other_root)
+        if self.tt.final_kind(other_root) is not None:
             other_root_is_present = True
-        except errors.NoSuchFile:
+        else:
             # other_root doesn't have a physical representation. We still need
             # to move any references to the actual root of the tree.
             other_root_is_present = False
@@ -1115,15 +1112,10 @@
         for thing, child in self.other_tree.inventory.root.children.iteritems():
             trans_id = self.tt.trans_id_file_id(child.file_id)
             if not other_root_is_present:
-                # FIXME: Make final_kind returns None instead of raising
-                # NoSuchFile to avoid the ugly construct below -- vila 20100402
-                try:
-                    self.tt.final_kind(trans_id)
+                if self.tt.final_kind(trans_id) is not None:
                     # The item exist in the final tree and has a defined place
                     # to go already.
                     continue
-                except errors.NoSuchFile, e:
-                    pass
             # Move the item into the root
             self.tt.adjust_path(self.tt.final_name(trans_id),
                                 self.tt.root, trans_id)
@@ -1405,10 +1397,7 @@
             self.tt.version_file(file_id, trans_id)
         # The merge has been performed, so the old contents should not be
         # retained.
-        try:
-            self.tt.delete_contents(trans_id)
-        except errors.NoSuchFile:
-            pass
+        self.tt.delete_contents(trans_id)
         return result
 
     def _default_other_winner_merge(self, merge_hook_params):
@@ -1586,10 +1575,7 @@
         if winner == 'this' and file_status != "modified":
             return
         trans_id = self.tt.trans_id_file_id(file_id)
-        try:
-            if self.tt.final_kind(trans_id) != "file":
-                return
-        except errors.NoSuchFile:
+        if self.tt.final_kind(trans_id) != "file":
             return
         if winner == "this":
             executability = this_executable

=== modified file 'bzrlib/tests/test_transform.py'
--- a/bzrlib/tests/test_transform.py	2010-07-21 10:38:31 +0000
+++ b/bzrlib/tests/test_transform.py	2010-08-25 10:20:41 +0000
@@ -123,12 +123,12 @@
         imaginary_id = transform.trans_id_tree_path('imaginary')
         imaginary_id2 = transform.trans_id_tree_path('imaginary/')
         self.assertEqual(imaginary_id, imaginary_id2)
-        self.assertEqual(transform.get_tree_parent(imaginary_id), root)
-        self.assertEqual(transform.final_kind(root), 'directory')
-        self.assertEqual(transform.final_file_id(root), self.wt.get_root_id())
+        self.assertEqual(root, transform.get_tree_parent(imaginary_id))
+        self.assertEqual('directory', transform.final_kind(root))
+        self.assertEqual(self.wt.get_root_id(), transform.final_file_id(root))
         trans_id = transform.create_path('name', root)
         self.assertIs(transform.final_file_id(trans_id), None)
-        self.assertRaises(NoSuchFile, transform.final_kind, trans_id)
+        self.assertIs(None, transform.final_kind(trans_id))
         transform.create_file('contents', trans_id)
         transform.set_executability(True, trans_id)
         transform.version_file('my_pretties', trans_id)

=== modified file 'bzrlib/transform.py'
--- a/bzrlib/transform.py	2010-08-06 19:54:45 +0000
+++ b/bzrlib/transform.py	2010-08-27 07:54:21 +0000
@@ -315,10 +315,9 @@
 
     def delete_contents(self, trans_id):
         """Schedule the contents of a path entry for deletion"""
-        # Ensure that the object exists in the WorkingTree, this will raise an
-        # exception if there is a problem
-        self.tree_kind(trans_id)
-        self._removed_contents.add(trans_id)
+        kind = self.tree_kind(trans_id)
+        if kind is not None:
+            self._removed_contents.add(trans_id)
 
     def cancel_deletion(self, trans_id):
         """Cancel a scheduled deletion"""
@@ -389,22 +388,22 @@
         changed_kind = set(self._removed_contents)
         changed_kind.intersection_update(self._new_contents)
         changed_kind.difference_update(new_ids)
-        changed_kind = (t for t in changed_kind if self.tree_kind(t) !=
-                        self.final_kind(t))
+        changed_kind = (t for t in changed_kind
+                        if self.tree_kind(t) != self.final_kind(t))
         new_ids.update(changed_kind)
         return sorted(FinalPaths(self).get_paths(new_ids))
 
     def final_kind(self, trans_id):
         """Determine the final file kind, after any changes applied.
 
-        Raises NoSuchFile if the file does not exist/has no contents.
-        (It is conceivable that a path would be created without the
-        corresponding contents insertion command)
+        :return: None if the file does not exist/has no contents.  (It is
+            conceivable that a path would be created without the corresponding
+            contents insertion command)
         """
         if trans_id in self._new_contents:
             return self._new_contents[trans_id]
         elif trans_id in self._removed_contents:
-            raise NoSuchFile(None)
+            return None
         else:
             return self.tree_kind(trans_id)
 
@@ -596,9 +595,8 @@
         """
         conflicts = []
         for trans_id in self._new_id.iterkeys():
-            try:
-                kind = self.final_kind(trans_id)
-            except NoSuchFile:
+            kind = self.final_kind(trans_id)
+            if kind is None:
                 conflicts.append(('versioning no contents', trans_id))
                 continue
             if not InventoryEntry.versionable_kind(kind):
@@ -618,11 +616,7 @@
             if self.final_file_id(trans_id) is None:
                 conflicts.append(('unversioned executability', trans_id))
             else:
-                try:
-                    non_file = self.final_kind(trans_id) != "file"
-                except NoSuchFile:
-                    non_file = True
-                if non_file is True:
+                if self.final_kind(trans_id) != "file":
                     conflicts.append(('non-file executability', trans_id))
         return conflicts
 
@@ -630,9 +624,7 @@
         """Check for overwrites (not permitted on Win32)"""
         conflicts = []
         for trans_id in self._new_contents:
-            try:
-                self.tree_kind(trans_id)
-            except NoSuchFile:
+            if self.tree_kind(trans_id) is None:
                 continue
             if trans_id not in self._removed_contents:
                 conflicts.append(('overwrite', trans_id,
@@ -652,10 +644,7 @@
             last_name = None
             last_trans_id = None
             for name, trans_id in name_ids:
-                try:
-                    kind = self.final_kind(trans_id)
-                except NoSuchFile:
-                    kind = None
+                kind = self.final_kind(trans_id)
                 file_id = self.final_file_id(trans_id)
                 if kind is None and file_id is None:
                     continue
@@ -687,15 +676,7 @@
                 continue
             if not self._any_contents(children):
                 continue
-            for child in children:
-                try:
-                    self.final_kind(child)
-                except NoSuchFile:
-                    continue
-            try:
-                kind = self.final_kind(parent_id)
-            except NoSuchFile:
-                kind = None
+            kind = self.final_kind(parent_id)
             if kind is None:
                 conflicts.append(('missing parent', parent_id))
             elif kind != "directory":
@@ -705,11 +686,8 @@
     def _any_contents(self, trans_ids):
         """Return true if any of the trans_ids, will have contents."""
         for trans_id in trans_ids:
-            try:
-                kind = self.final_kind(trans_id)
-            except NoSuchFile:
-                continue
-            return True
+            if self.final_kind(trans_id) is not None:
+                return True
         return False
 
     def _set_executability(self, path, trans_id):
@@ -845,10 +823,7 @@
         Return a (name, parent, kind, executable) tuple
         """
         to_name = self.final_name(to_trans_id)
-        try:
-            to_kind = self.final_kind(to_trans_id)
-        except NoSuchFile:
-            to_kind = None
+        to_kind = self.final_kind(to_trans_id)
         to_parent = self.final_file_id(self.final_parent(to_trans_id))
         if to_trans_id in self._new_executability:
             to_executable = self._new_executability[to_trans_id]
@@ -1419,18 +1394,15 @@
     def tree_kind(self, trans_id):
         """Determine the file kind in the working tree.
 
-        Raises NoSuchFile if the file does not exist
+        :returns: The file kind or None if the file does not exist
         """
         path = self._tree_id_paths.get(trans_id)
         if path is None:
-            raise NoSuchFile(None)
+            return None
         try:
             return file_kind(self._tree.abspath(path))
-        except OSError, e:
-            if e.errno != errno.ENOENT:
-                raise
-            else:
-                raise NoSuchFile(path)
+        except errors.NoSuchFile:
+            return None
 
     def _set_mode(self, trans_id, mode_id, typefunc):
         """Set the mode of new file contents.
@@ -1605,9 +1577,8 @@
                 if file_id is None:
                     continue
                 needs_entry = False
-                try:
-                    kind = self.final_kind(trans_id)
-                except NoSuchFile:
+                kind = self.final_kind(trans_id)
+                if kind is None:
                     kind = self._tree.stored_kind(file_id)
                 parent_trans_id = self.final_parent(trans_id)
                 parent_file_id = new_path_file_ids.get(parent_trans_id)
@@ -1725,9 +1696,12 @@
     def tree_kind(self, trans_id):
         path = self._tree_id_paths.get(trans_id)
         if path is None:
-            raise NoSuchFile(None)
+            return None
         file_id = self._tree.path2id(path)
-        return self._tree.kind(file_id)
+        try:
+            return self._tree.kind(file_id)
+        except errors.NoSuchFile:
+            return None
 
     def _set_mode(self, trans_id, mode_id, typefunc):
         """Set the mode of new file contents.
@@ -1929,9 +1903,8 @@
             if (specific_file_ids is not None
                 and file_id not in specific_file_ids):
                 continue
-            try:
-                kind = self._transform.final_kind(trans_id)
-            except NoSuchFile:
+            kind = self._transform.final_kind(trans_id)
+            if kind is None:
                 kind = self._transform._tree.stored_kind(file_id)
             new_entry = inventory.make_entry(
                 kind,
@@ -2169,10 +2142,10 @@
                 path_from_root = self._final_paths.get_path(child_id)
                 basename = self._transform.final_name(child_id)
                 file_id = self._transform.final_file_id(child_id)
-                try:
-                    kind = self._transform.final_kind(child_id)
+                kind  = self._transform.final_kind(child_id)
+                if kind is not None:
                     versioned_kind = kind
-                except NoSuchFile:
+                else:
                     kind = 'unknown'
                     versioned_kind = self._transform._tree.stored_kind(file_id)
                 if versioned_kind == 'directory':




More information about the bazaar-commits mailing list