Rev 4626: Fixed as per Andrew's review comments. in file:///home/vila/src/bzr/bugs/529968-content-conflict-rename-on-resolve/

[Vincent Ladeuil]

At file:///home/vila/src/bzr/bugs/529968-content-conflict-rename-on-resolve/

------------------------------------------------------------
revno: 4626
revision-id: v.ladeuil+lp at free.fr-20100302075853-6mpnlk1p7ezlfnli
parent: v.ladeuil+lp at free.fr-20100302073815-0ymcuompxllxvvd0
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: 529968-content-conflict-rename-on-resolve
timestamp: Tue 2010-03-02 08:58:53 +0100
message:
  Fixed as per Andrew's review comments.
  
  * bzrlib/conflicts.py:
  (ContentsConflict._take_it_with_cleanups): Use
  OperationWithCleanups to ensure a clean error handling.
-------------- next part --------------
=== modified file 'bzrlib/conflicts.py'
--- a/bzrlib/conflicts.py	2010-03-02 07:30:27 +0000
+++ b/bzrlib/conflicts.py	2010-03-02 07:58:53 +0000
@@ -26,6 +26,7 @@
 
 from bzrlib import (
     builtins,
+    cleanup,
     commands,
     errors,
     osutils,
@@ -479,41 +480,43 @@
     def associated_filenames(self):
         return [self.path + suffix for suffix in ('.BASE', '.OTHER')]
 
-    def _take_it(self, tree, suffix_to_remove):
+    def _take_it(self, tt, suffix_to_remove):
         """Resolve the conflict.
 
-        :param tree: The working tree where the conflict is resolved.
+        :param tt: The TreeTransform where the conflict is resolved.
         :param suffix_to_remove: Either 'THIS' or 'OTHER'
 
         The resolution is symmetric, when taking THIS, OTHER is deleted and
         item.THIS is renamed into item and vice-versa.
         """
+        try:
+            # Delete 'item.THIS' or 'item.OTHER' depending on
+            # suffix_to_remove
+            tt.delete_contents(
+                tt.trans_id_tree_path(self.path + '.' + suffix_to_remove))
+        except errors.NoSuchFile:
+            # There are valid cases where 'item.suffix_to_remove' either
+            # never existed or was already deleted (including the case
+            # where the user deleted it)
+            pass
+        # Rename 'item.suffix_to_remove' (note that if
+        # 'item.suffix_to_remove' has been deleted, this is a no-op)
+        this_tid = tt.trans_id_file_id(self.file_id)
+        parent_tid = tt.get_tree_parent(this_tid)
+        tt.adjust_path(self.path, parent_tid, this_tid)
+        tt.apply()
+
+    def _take_it_with_cleanups(self, tree, suffix_to_remove):
         tt = transform.TreeTransform(tree)
-        try:
-            try:
-                # Delete 'item.THIS' or 'item.OTHER' depending on
-                # suffix_to_remove
-                tt.delete_contents(
-                    tt.trans_id_tree_path(self.path + '.' + suffix_to_remove))
-            except errors.NoSuchFile:
-                # There are valid cases where 'item.suffix_to_remove' either
-                # never existed or was already deleted (including the case
-                # where the user deleted it)
-                pass
-            # Rename 'item.suffix_to_remove' (note that if
-            # 'item.suffix_to_remove' has been deleted, this is a no-op)
-            this_tid = tt.trans_id_file_id(self.file_id)
-            parent_tid = tt.get_tree_parent(this_tid)
-            tt.adjust_path(self.path, parent_tid, this_tid)
-            tt.apply()
-        finally:
-            tt.finalize()
+        op = cleanup.OperationWithCleanups(self._take_it)
+        op.add_cleanup(tt.finalize)
+        op.run_simple(tt, suffix_to_remove)
 
     def action_take_this(self, tree):
-        self._take_it(tree, 'OTHER')
+        self._take_it_with_cleanups(tree, 'OTHER')
 
     def action_take_other(self, tree):
-        self._take_it(tree, 'THIS')
+        self._take_it_with_cleanups(tree, 'THIS')
 
 
 # FIXME: TextConflict is about a single file-id, there never is a conflict_path