Rev 5948: (gz) Handle bug #597686, if we open() try to delete the file, in file:///home/pqm/archives/thelove/bzr/%2Btrunk/
Canonical.com Patch Queue Manager
pqm at pqm.ubuntu.com
Wed Jun 1 10:00:44 UTC 2011
At file:///home/pqm/archives/thelove/bzr/%2Btrunk/
------------------------------------------------------------
revno: 5948 [merge]
revision-id: pqm at pqm.ubuntu.com-20110601100041-xxi0zwzza9z4oz64
parent: pqm at pqm.ubuntu.com-20110601091513-vo5m9202wr24rbsl
parent: john at arbash-meinel.com-20110601090255-ucvp83wrycoaakep
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Wed 2011-06-01 10:00:41 +0000
message:
(gz) Handle bug #597686, if we open() try to delete the file,
even if there was an error.
modified:
bzrlib/tests/test_transform.py test_transaction.py-20060105172520-b3ffb3946550e6c4
bzrlib/transform.py transform.py-20060105172343-dd99e54394d91687
doc/en/release-notes/bzr-2.4.txt bzr2.4.txt-20110114053217-k7ym9jfz243fddjm-1
=== modified file 'bzrlib/tests/test_transform.py'
--- a/bzrlib/tests/test_transform.py 2011-05-21 16:44:52 +0000
+++ b/bzrlib/tests/test_transform.py 2011-06-01 08:44:37 +0000
@@ -2464,6 +2464,91 @@
self.assertPathExists('a/b')
+class TestFinalizeRobustness(tests.TestCaseWithTransport):
+ """Ensure treetransform creation errors can be safely cleaned up after"""
+
+ def _override_globals_in_method(self, instance, method_name, globals):
+ """Replace method on instance with one with updated globals"""
+ import types
+ func = getattr(instance, method_name).im_func
+ new_globals = dict(func.func_globals)
+ new_globals.update(globals)
+ new_func = types.FunctionType(func.func_code, new_globals,
+ func.func_name, func.func_defaults)
+ setattr(instance, method_name,
+ types.MethodType(new_func, instance, instance.__class__))
+ self.addCleanup(delattr, instance, method_name)
+
+ @staticmethod
+ def _fake_open_raises_before(name, mode):
+ """Like open() but raises before doing anything"""
+ raise RuntimeError
+
+ @staticmethod
+ def _fake_open_raises_after(name, mode):
+ """Like open() but raises after creating file without returning"""
+ open(name, mode).close()
+ raise RuntimeError
+
+ def create_transform_and_root_trans_id(self):
+ """Setup a transform creating a file in limbo"""
+ tree = self.make_branch_and_tree('.')
+ tt = TreeTransform(tree)
+ return tt, tt.create_path("a", tt.root)
+
+ def create_transform_and_subdir_trans_id(self):
+ """Setup a transform creating a directory containing a file in limbo"""
+ tree = self.make_branch_and_tree('.')
+ tt = TreeTransform(tree)
+ d_trans_id = tt.create_path("d", tt.root)
+ tt.create_directory(d_trans_id)
+ f_trans_id = tt.create_path("a", d_trans_id)
+ tt.adjust_path("a", d_trans_id, f_trans_id)
+ return tt, f_trans_id
+
+ def test_root_create_file_open_raises_before_creation(self):
+ tt, trans_id = self.create_transform_and_root_trans_id()
+ self._override_globals_in_method(tt, "create_file",
+ {"open": self._fake_open_raises_before})
+ self.assertRaises(RuntimeError, tt.create_file, ["contents"], trans_id)
+ path = tt._limbo_name(trans_id)
+ self.assertPathDoesNotExist(path)
+ tt.finalize()
+ self.assertPathDoesNotExist(tt._limbodir)
+
+ def test_root_create_file_open_raises_after_creation(self):
+ tt, trans_id = self.create_transform_and_root_trans_id()
+ self._override_globals_in_method(tt, "create_file",
+ {"open": self._fake_open_raises_after})
+ self.assertRaises(RuntimeError, tt.create_file, ["contents"], trans_id)
+ path = tt._limbo_name(trans_id)
+ self.assertPathExists(path)
+ tt.finalize()
+ self.assertPathDoesNotExist(path)
+ self.assertPathDoesNotExist(tt._limbodir)
+
+ def test_subdir_create_file_open_raises_before_creation(self):
+ tt, trans_id = self.create_transform_and_subdir_trans_id()
+ self._override_globals_in_method(tt, "create_file",
+ {"open": self._fake_open_raises_before})
+ self.assertRaises(RuntimeError, tt.create_file, ["contents"], trans_id)
+ path = tt._limbo_name(trans_id)
+ self.assertPathDoesNotExist(path)
+ tt.finalize()
+ self.assertPathDoesNotExist(tt._limbodir)
+
+ def test_subdir_create_file_open_raises_after_creation(self):
+ tt, trans_id = self.create_transform_and_subdir_trans_id()
+ self._override_globals_in_method(tt, "create_file",
+ {"open": self._fake_open_raises_after})
+ self.assertRaises(RuntimeError, tt.create_file, ["contents"], trans_id)
+ path = tt._limbo_name(trans_id)
+ self.assertPathExists(path)
+ tt.finalize()
+ self.assertPathDoesNotExist(path)
+ self.assertPathDoesNotExist(tt._limbodir)
+
+
class TestTransformMissingParent(tests.TestCaseWithTransport):
def make_tt_with_versioned_dir(self):
=== modified file 'bzrlib/transform.py'
--- a/bzrlib/transform.py 2011-05-21 16:43:19 +0000
+++ b/bzrlib/transform.py 2011-06-01 08:59:17 +0000
@@ -1172,11 +1172,16 @@
if self._tree is None:
return
try:
- entries = [(self._limbo_name(t), t, k) for t, k in
- self._new_contents.iteritems()]
- entries.sort(reverse=True)
- for path, trans_id, kind in entries:
- delete_any(path)
+ limbo_paths = sorted(self._limbo_files.values(), reverse=True)
+ for path in limbo_paths:
+ try:
+ delete_any(path)
+ except OSError, e:
+ if e.errno != errno.ENOENT:
+ raise
+ # XXX: warn? perhaps we just got interrupted at an
+ # inconvenient moment, but perhaps files are disappearing
+ # from under us?
try:
delete_any(self._limbodir)
except OSError:
@@ -1265,14 +1270,7 @@
name = self._limbo_name(trans_id)
f = open(name, 'wb')
try:
- try:
- unique_add(self._new_contents, trans_id, 'file')
- except:
- # Clean up the file, it never got registered so
- # TreeTransform.finalize() won't clean it up.
- f.close()
- os.unlink(name)
- raise
+ unique_add(self._new_contents, trans_id, 'file')
f.writelines(contents)
finally:
f.close()
@@ -1851,6 +1849,11 @@
self._observed_sha1s[trans_id] = (o_sha1, st)
finally:
child_pb.finished()
+ for path, trans_id in new_paths:
+ # new_paths includes stuff like workingtree conflicts. Only the
+ # stuff in new_contents actually comes from limbo.
+ if trans_id in self._limbo_files:
+ del self._limbo_files[trans_id]
self._new_contents.clear()
return modified_paths
=== modified file 'doc/en/release-notes/bzr-2.4.txt'
--- a/doc/en/release-notes/bzr-2.4.txt 2011-06-01 09:15:13 +0000
+++ b/doc/en/release-notes/bzr-2.4.txt 2011-06-01 10:00:41 +0000
@@ -39,6 +39,11 @@
* Fix a race condition for ``server_started`` hooks leading to a spurious
test failure. (Vincent Ladeuil, #789167)
+* Handle files that get created but don't get used during TreeTransform.
+ ``open()`` can create a file, and still raise an exception before it
+ returns. So anything we might have created, make sure we destroy during
+ ``finalize()``. (Martin [gz], #597686)
+
* ``pack_repo`` now uses ``Transport.move`` instead of
``Transport.rename``, deleting any existing targets even on SFTP.
(Martin von Gagern, #421776)
More information about the bazaar-commits
mailing list