Rev 2765: Add rollback to TreeTransform in file:///home/pqm/archives/thelove/bzr/%2Btrunk/
Canonical.com Patch Queue Manager
pqm at pqm.ubuntu.com
Wed Aug 29 07:10:09 BST 2007
At file:///home/pqm/archives/thelove/bzr/%2Btrunk/
------------------------------------------------------------
revno: 2765
revision-id: pqm at pqm.ubuntu.com-20070829061006-7zz2j5icnc8do0ru
parent: pqm at pqm.ubuntu.com-20070829051658-yvj0n3mvso1v8uz3
parent: aaron.bentley at utoronto.ca-20070829053005-1x7fnhe3jx0c1ak5
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Wed 2007-08-29 07:10:06 +0100
message:
Add rollback to TreeTransform
modified:
NEWS NEWS-20050323055033-4e00b5db738777ff
bzrlib/errors.py errors.py-20050309040759-20512168c4e14fbd
bzrlib/tests/test_transform.py test_transaction.py-20060105172520-b3ffb3946550e6c4
bzrlib/transform.py transform.py-20060105172343-dd99e54394d91687
------------------------------------------------------------
revno: 2733.2.12
merged: aaron.bentley at utoronto.ca-20070829053005-1x7fnhe3jx0c1ak5
parent: aaron.bentley at utoronto.ca-20070829052221-27rynnk5yws2619u
committer: Aaron Bentley <aaron.bentley at utoronto.ca>
branch nick: transform-rollback
timestamp: Wed 2007-08-29 01:30:05 -0400
message:
Updates from review
------------------------------------------------------------
revno: 2733.2.11
merged: aaron.bentley at utoronto.ca-20070829052221-27rynnk5yws2619u
parent: aaron.bentley at utoronto.ca-20070829044841-p5jqkr7ncva9jjgp
committer: Aaron Bentley <aaron.bentley at utoronto.ca>
branch nick: transform-rollback
timestamp: Wed 2007-08-29 01:22:21 -0400
message:
Detect irregularities with the pending-deletion directory
------------------------------------------------------------
revno: 2733.2.10
merged: aaron.bentley at utoronto.ca-20070829044841-p5jqkr7ncva9jjgp
parent: abentley at panoramicfeedback.com-20070821152659-bs4ko8872xdigr11
parent: pqm at pqm.ubuntu.com-20070829001914-yd1js1ms7eg7g38g
committer: Aaron Bentley <aaron.bentley at utoronto.ca>
branch nick: transform-rollback
timestamp: Wed 2007-08-29 00:48:41 -0400
message:
Merge bzr.dev
------------------------------------------------------------
revno: 2733.2.9
merged: abentley at panoramicfeedback.com-20070821152659-bs4ko8872xdigr11
parent: abentley at panoramicfeedback.com-20070821151740-cnad1x134tkxicwj
committer: Aaron Bentley <abentley at panoramicfeedback.com>
branch nick: transform-rollback
timestamp: Tue 2007-08-21 11:26:59 -0400
message:
Update docstrings
------------------------------------------------------------
revno: 2733.2.8
merged: abentley at panoramicfeedback.com-20070821151740-cnad1x134tkxicwj
parent: abentley at panoramicfeedback.com-20070821150857-qr11vyi8c0dpqspj
committer: Aaron Bentley <abentley at panoramicfeedback.com>
branch nick: transform-rollback
timestamp: Tue 2007-08-21 11:17:40 -0400
message:
Update NEWS
------------------------------------------------------------
revno: 2733.2.7
merged: abentley at panoramicfeedback.com-20070821150857-qr11vyi8c0dpqspj
parent: abentley at panoramicfeedback.com-20070821150326-br7qk4twfmx62su8
parent: pqm at pqm.ubuntu.com-20070821044713-ttnupbvhlsbwh1he
committer: Aaron Bentley <abentley at panoramicfeedback.com>
branch nick: transform-rollback
timestamp: Tue 2007-08-21 11:08:57 -0400
message:
Merge bzr.dev
------------------------------------------------------------
revno: 2733.2.6
merged: abentley at panoramicfeedback.com-20070821150326-br7qk4twfmx62su8
parent: abentley at panoramicfeedback.com-20070821144630-zxutc2pzhissxcv0
committer: Aaron Bentley <abentley at panoramicfeedback.com>
branch nick: transform-rollback
timestamp: Tue 2007-08-21 11:03:26 -0400
message:
Make TreeTransform commits rollbackable
------------------------------------------------------------
revno: 2733.2.5
merged: abentley at panoramicfeedback.com-20070821144630-zxutc2pzhissxcv0
parent: abentley at panoramicfeedback.com-20070821135235-b550623c3zvm71re
committer: Aaron Bentley <abentley at panoramicfeedback.com>
branch nick: transform-rollback
timestamp: Tue 2007-08-21 10:46:30 -0400
message:
Implement FileMover.pre_delete and FileMover.apply_deletions
------------------------------------------------------------
revno: 2733.2.4
merged: abentley at panoramicfeedback.com-20070821135235-b550623c3zvm71re
parent: abentley at panoramicfeedback.com-20070821134608-s8a6y3u2pk7q5bt9
committer: Aaron Bentley <abentley at panoramicfeedback.com>
branch nick: transform-rollback
timestamp: Tue 2007-08-21 09:52:35 -0400
message:
Test transform rollback when renaming into place
------------------------------------------------------------
revno: 2733.2.3
merged: abentley at panoramicfeedback.com-20070821134608-s8a6y3u2pk7q5bt9
parent: abentley at panoramicfeedback.com-20070820211031-rhqjt9mxt1dbzwrn
committer: Aaron Bentley <abentley at panoramicfeedback.com>
branch nick: transform-rollback
timestamp: Tue 2007-08-21 09:46:08 -0400
message:
Test tranform rollback
------------------------------------------------------------
revno: 2733.2.2
merged: abentley at panoramicfeedback.com-20070820211031-rhqjt9mxt1dbzwrn
parent: abentley at panoramicfeedback.com-20070820205037-z3yvwisr73r8jqff
committer: Aaron Bentley <abentley at panoramicfeedback.com>
branch nick: transform-rollback
timestamp: Mon 2007-08-20 17:10:31 -0400
message:
Use FileMover to perform renames in TT
------------------------------------------------------------
revno: 2733.2.1
merged: abentley at panoramicfeedback.com-20070820205037-z3yvwisr73r8jqff
parent: pqm at pqm.ubuntu.com-20070820141921-kmbpaev7g9epuy08
committer: Aaron Bentley <abentley at panoramicfeedback.com>
branch nick: transform-rollback
timestamp: Mon 2007-08-20 16:50:37 -0400
message:
Implement FileMover, to support TreeTransform rollback
=== modified file 'NEWS'
--- a/NEWS 2007-08-29 05:16:58 +0000
+++ b/NEWS 2007-08-29 06:10:06 +0000
@@ -63,6 +63,9 @@
instead of claiming there is no help for said alias. (Daniel Watkins,
#133548)
+ * TreeTransform-based operations, like pull, merge, revert, and branch,
+ now roll back if they encounter an error. (Aaron Bentley, #67699)
+
IMPROVEMENTS:
* ``pull`` and ``merge`` are much faster at installing bundle format 4.
=== modified file 'bzrlib/errors.py'
--- a/bzrlib/errors.py 2007-08-23 18:02:53 +0000
+++ b/bzrlib/errors.py 2007-08-29 05:22:21 +0000
@@ -1845,6 +1845,16 @@
self.limbo_dir = limbo_dir
+class ExistingPendingDeletion(BzrError):
+
+ _fmt = """This tree contains left-over files from a failed operation.
+ Please examine %(pending_deletion)s to see if it contains any files you
+ wish to keep, and delete it when you are done."""
+
+ def __init__(self, pending_deletion):
+ BzrError.__init__(self, pending_deletion=pending_deletion)
+
+
class ImmortalLimbo(BzrError):
_fmt = """Unable to delete transform temporary directory $(limbo_dir)s.
@@ -1856,6 +1866,16 @@
self.limbo_dir = limbo_dir
+class ImmortalPendingDeletion(BzrError):
+
+ _fmt = """Unable to delete transform temporary directory
+ %(pending_deletion)s. Please examine %(pending_deletions)s to see if it
+ contains any files you wish to keep, and delete it when you are done."""
+
+ def __init__(self, pending_deletion):
+ BzrError.__init__(self, pending_deletion=pending_deletion)
+
+
class OutOfDateTree(BzrError):
_fmt = "Working tree is out of date, please run 'bzr update'."
=== modified file 'bzrlib/tests/test_transform.py'
--- a/bzrlib/tests/test_transform.py 2007-08-09 03:39:31 +0000
+++ b/bzrlib/tests/test_transform.py 2007-08-29 05:30:05 +0000
@@ -31,14 +31,15 @@
from bzrlib.errors import (DuplicateKey, MalformedTransform, NoSuchFile,
ReusingTransform, CantMoveRoot,
PathsNotVersionedError, ExistingLimbo,
- ImmortalLimbo, LockError)
+ ExistingPendingDeletion, ImmortalLimbo,
+ ImmortalPendingDeletion, LockError)
from bzrlib.osutils import file_kind, has_symlinks, pathjoin
from bzrlib.merge import Merge3Merger
from bzrlib.tests import TestCaseInTempDir, TestSkipped, TestCase
from bzrlib.transform import (TreeTransform, ROOT_PARENT, FinalPaths,
resolve_conflicts, cook_conflicts,
find_interesting, build_tree, get_backup_name,
- change_entry)
+ change_entry, _FileMover)
class TestTreeTransform(tests.TestCaseWithTransport):
@@ -54,9 +55,9 @@
return transform, transform.root
def test_existing_limbo(self):
- limbo_name = urlutils.local_path_from_url(
- self.wt._control_files.controlfilename('limbo'))
transform, root = self.get_transform()
+ limbo_name = transform._limbodir
+ deletion_path = transform._deletiondir
os.mkdir(pathjoin(limbo_name, 'hehe'))
self.assertRaises(ImmortalLimbo, transform.apply)
self.assertRaises(LockError, self.wt.unlock)
@@ -64,9 +65,19 @@
self.assertRaises(LockError, self.wt.unlock)
os.rmdir(pathjoin(limbo_name, 'hehe'))
os.rmdir(limbo_name)
+ os.rmdir(deletion_path)
transform, root = self.get_transform()
transform.apply()
+ def test_existing_pending_deletion(self):
+ transform, root = self.get_transform()
+ deletion_path = self._limbodir = urlutils.local_path_from_url(
+ transform._tree._control_files.controlfilename('pending-deletion'))
+ os.mkdir(pathjoin(deletion_path, 'blocking-directory'))
+ self.assertRaises(ImmortalPendingDeletion, transform.apply)
+ self.assertRaises(LockError, self.wt.unlock)
+ self.assertRaises(ExistingPendingDeletion, self.get_transform)
+
def test_build(self):
transform, root = self.get_transform()
self.assertIs(transform.get_tree_parent(root), ROOT_PARENT)
@@ -1338,3 +1349,122 @@
self.assertEqual(name, 'name.~1~')
name = get_backup_name(MockEntry(), {'a':['1', '2', '3']}, 'a', tt)
self.assertEqual(name, 'name.~4~')
+
+
+class TestFileMover(tests.TestCaseWithTransport):
+
+ def test_file_mover(self):
+ self.build_tree(['a/', 'a/b', 'c/', 'c/d'])
+ mover = _FileMover()
+ mover.rename('a', 'q')
+ self.failUnlessExists('q')
+ self.failIfExists('a')
+ self.failUnlessExists('q/b')
+ self.failUnlessExists('c')
+ self.failUnlessExists('c/d')
+
+ def test_pre_delete_rollback(self):
+ self.build_tree(['a/'])
+ mover = _FileMover()
+ mover.pre_delete('a', 'q')
+ self.failUnlessExists('q')
+ self.failIfExists('a')
+ mover.rollback()
+ self.failIfExists('q')
+ self.failUnlessExists('a')
+
+ def test_apply_deletions(self):
+ self.build_tree(['a/', 'b/'])
+ mover = _FileMover()
+ mover.pre_delete('a', 'q')
+ mover.pre_delete('b', 'r')
+ self.failUnlessExists('q')
+ self.failUnlessExists('r')
+ self.failIfExists('a')
+ self.failIfExists('b')
+ mover.apply_deletions()
+ self.failIfExists('q')
+ self.failIfExists('r')
+ self.failIfExists('a')
+ self.failIfExists('b')
+
+ def test_file_mover_rollback(self):
+ self.build_tree(['a/', 'a/b', 'c/', 'c/d/', 'c/e/'])
+ mover = _FileMover()
+ mover.rename('c/d', 'c/f')
+ mover.rename('c/e', 'c/d')
+ try:
+ mover.rename('a', 'c')
+ except OSError, e:
+ mover.rollback()
+ self.failUnlessExists('a')
+ self.failUnlessExists('c/d')
+
+
+class Bogus(Exception):
+ pass
+
+
+class TestTransformRollback(tests.TestCaseWithTransport):
+
+ class ExceptionFileMover(_FileMover):
+
+ def __init__(self, bad_source=None, bad_target=None):
+ _FileMover.__init__(self)
+ self.bad_source = bad_source
+ self.bad_target = bad_target
+
+ def rename(self, source, target):
+ if (self.bad_source is not None and
+ source.endswith(self.bad_source)):
+ raise Bogus
+ elif (self.bad_target is not None and
+ target.endswith(self.bad_target)):
+ raise Bogus
+ else:
+ _FileMover.rename(self, source, target)
+
+ def test_rollback_rename(self):
+ tree = self.make_branch_and_tree('.')
+ self.build_tree(['a/', 'a/b'])
+ tt = TreeTransform(tree)
+ self.addCleanup(tt.finalize)
+ a_id = tt.trans_id_tree_path('a')
+ tt.adjust_path('c', tt.root, a_id)
+ tt.adjust_path('d', a_id, tt.trans_id_tree_path('a/b'))
+ self.assertRaises(Bogus, tt.apply,
+ _mover=self.ExceptionFileMover(bad_source='a'))
+ self.failUnlessExists('a')
+ self.failUnlessExists('a/b')
+ tt.apply()
+ self.failUnlessExists('c')
+ self.failUnlessExists('c/d')
+
+ def test_rollback_rename_into_place(self):
+ tree = self.make_branch_and_tree('.')
+ self.build_tree(['a/', 'a/b'])
+ tt = TreeTransform(tree)
+ self.addCleanup(tt.finalize)
+ a_id = tt.trans_id_tree_path('a')
+ tt.adjust_path('c', tt.root, a_id)
+ tt.adjust_path('d', a_id, tt.trans_id_tree_path('a/b'))
+ self.assertRaises(Bogus, tt.apply,
+ _mover=self.ExceptionFileMover(bad_target='c/d'))
+ self.failUnlessExists('a')
+ self.failUnlessExists('a/b')
+ tt.apply()
+ self.failUnlessExists('c')
+ self.failUnlessExists('c/d')
+
+ def test_rollback_deletion(self):
+ tree = self.make_branch_and_tree('.')
+ self.build_tree(['a/', 'a/b'])
+ tt = TreeTransform(tree)
+ self.addCleanup(tt.finalize)
+ a_id = tt.trans_id_tree_path('a')
+ tt.delete_contents(a_id)
+ tt.adjust_path('d', tt.root, tt.trans_id_tree_path('a/b'))
+ self.assertRaises(Bogus, tt.apply,
+ _mover=self.ExceptionFileMover(bad_target='d'))
+ self.failUnlessExists('a')
+ self.failUnlessExists('a/b')
=== modified file 'bzrlib/transform.py'
--- a/bzrlib/transform.py 2007-08-16 05:37:08 +0000
+++ b/bzrlib/transform.py 2007-08-29 05:30:05 +0000
@@ -108,6 +108,14 @@
except OSError, e:
if e.errno == errno.EEXIST:
raise ExistingLimbo(self._limbodir)
+ self._deletiondir = urlutils.local_path_from_url(
+ control_files.controlfilename('pending-deletion'))
+ try:
+ os.mkdir(self._deletiondir)
+ except OSError, e:
+ if e.errno == errno.EEXIST:
+ raise errors.ExistingPendingDeletion(self._deletiondir)
+
except:
self._tree.unlock()
raise
@@ -170,6 +178,10 @@
except OSError:
# We don't especially care *why* the dir is immortal.
raise ImmortalLimbo(self._limbodir)
+ try:
+ os.rmdir(self._deletiondir)
+ except OSError:
+ raise errors.ImmortalPendingDeletion(self._deletiondir)
finally:
self._tree.unlock()
self._tree = None
@@ -789,7 +801,7 @@
return True
return False
- def apply(self, no_conflicts=False):
+ def apply(self, no_conflicts=False, _mover=None):
"""Apply all changes to the inventory and filesystem.
If filesystem or inventory conflicts are present, MalformedTransform
@@ -799,6 +811,7 @@
:param no_conflicts: if True, the caller guarantees there are no
conflicts, so no check is made.
+ :param _mover: Supply an alternate FileMover, for testing
"""
if not no_conflicts:
conflicts = self.find_conflicts()
@@ -808,10 +821,21 @@
inventory_delta = []
child_pb = bzrlib.ui.ui_factory.nested_progress_bar()
try:
- child_pb.update('Apply phase', 0, 2)
- self._apply_removals(inv, inventory_delta)
- child_pb.update('Apply phase', 1, 2)
- modified_paths = self._apply_insertions(inv, inventory_delta)
+ if _mover is None:
+ mover = _FileMover()
+ else:
+ mover = _mover
+ try:
+ child_pb.update('Apply phase', 0, 2)
+ self._apply_removals(inv, inventory_delta, mover)
+ child_pb.update('Apply phase', 1, 2)
+ modified_paths = self._apply_insertions(inv, inventory_delta,
+ mover)
+ except:
+ mover.rollback()
+ raise
+ else:
+ mover.apply_deletions()
finally:
child_pb.finished()
self._tree.apply_inventory_delta(inventory_delta)
@@ -852,7 +876,7 @@
self._limbo_files[trans_id] = limbo_name
return limbo_name
- def _apply_removals(self, inv, inventory_delta):
+ def _apply_removals(self, inv, inventory_delta, mover):
"""Perform tree operations that remove directory/inventory names.
That is, delete files that are to be deleted, and put any files that
@@ -868,11 +892,12 @@
child_pb.update('removing file', num, len(tree_paths))
full_path = self._tree.abspath(path)
if trans_id in self._removed_contents:
- delete_any(full_path)
+ mover.pre_delete(full_path, os.path.join(self._deletiondir,
+ trans_id))
elif trans_id in self._new_name or trans_id in \
self._new_parent:
try:
- os.rename(full_path, self._limbo_name(trans_id))
+ mover.rename(full_path, self._limbo_name(trans_id))
except OSError, e:
if e.errno != errno.ENOENT:
raise
@@ -888,7 +913,7 @@
finally:
child_pb.finished()
- def _apply_insertions(self, inv, inventory_delta):
+ def _apply_insertions(self, inv, inventory_delta, mover):
"""Perform tree operations that insert directory/inventory names.
That is, create any files that need to be created, and restore from
@@ -911,7 +936,7 @@
full_path = self._tree.abspath(path)
if trans_id in self._needs_rename:
try:
- os.rename(self._limbo_name(trans_id), full_path)
+ mover.rename(self._limbo_name(trans_id), full_path)
except OSError, e:
# We may be renaming a dangling inventory id
if e.errno != errno.ENOENT:
@@ -1763,3 +1788,42 @@
file_id=modified_id,
conflict_path=conflicting_path,
conflict_file_id=conflicting_id)
+
+
+class _FileMover(object):
+ """Moves and deletes files for TreeTransform, tracking operations"""
+
+ def __init__(self):
+ self.past_renames = []
+ self.pending_deletions = []
+
+ def rename(self, from_, to):
+ """Rename a file from one path to another. Functions like os.rename"""
+ os.rename(from_, to)
+ self.past_renames.append((from_, to))
+
+ def pre_delete(self, from_, to):
+ """Rename a file out of the way and mark it for deletion.
+
+ Unlike os.unlink, this works equally well for files and directories.
+ :param from_: The current file path
+ :param to: A temporary path for the file
+ """
+ self.rename(from_, to)
+ self.pending_deletions.append(to)
+
+ def rollback(self):
+ """Reverse all renames that have been performed"""
+ for from_, to in reversed(self.past_renames):
+ os.rename(to, from_)
+ # after rollback, don't reuse _FileMover
+ past_renames = None
+ pending_deletions = None
+
+ def apply_deletions(self):
+ """Apply all marked deletions"""
+ for path in self.pending_deletions:
+ delete_any(path)
+ # after apply_deletions, don't reuse _FileMover
+ past_renames = None
+ pending_deletions = None
More information about the bazaar-commits
mailing list