Rev 5228: (mbp) (mbp) better message when rename fails inside TreeTransform (Martin in file:///home/pqm/archives/thelove/bzr/%2Btrunk/
Canonical.com Patch Queue Manager
pqm at pqm.ubuntu.com
Thu May 13 18:33:02 BST 2010
At file:///home/pqm/archives/thelove/bzr/%2Btrunk/
------------------------------------------------------------
revno: 5228 [merge]
revision-id: pqm at pqm.ubuntu.com-20100513173255-x3kt3mczwhphdm5y
parent: pqm at pqm.ubuntu.com-20100512135749-8b0et7ntvh8dgm2t
parent: mbp at canonical.com-20100513161131-jk7lzmv1qa1a81m0
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Thu 2010-05-13 18:32:55 +0100
message:
(mbp) (mbp) better message when rename fails inside TreeTransform (Martin
Pool)
modified:
bzrlib/errors.py errors.py-20050309040759-20512168c4e14fbd
bzrlib/osutils.py osutils.py-20050309040759-eeaff12fbf77ac86
bzrlib/tests/test_errors.py test_errors.py-20060210110251-41aba2deddf936a8
bzrlib/tests/test_osutils.py test_osutils.py-20051201224856-e48ee24c12182989
bzrlib/tests/test_transform.py test_transaction.py-20060105172520-b3ffb3946550e6c4
bzrlib/transform.py transform.py-20060105172343-dd99e54394d91687
bzrlib/transport/local.py local_transport.py-20050711165921-9b1f142bfe480c24
=== modified file 'bzrlib/errors.py'
--- a/bzrlib/errors.py 2010-05-05 14:11:13 +0000
+++ b/bzrlib/errors.py 2010-05-13 17:32:55 +0000
@@ -1923,6 +1923,17 @@
class CantMoveRoot(BzrError):
_fmt = "Moving the root directory is not supported at this time"
+
+
+class TransformRenameFailed(BzrError):
+
+ _fmt = "Failed to rename %(from_path)s to %(to_path)s: %(why)s"
+
+ def __init__(self, from_path, to_path, why, errno):
+ self.from_path = from_path
+ self.to_path = to_path
+ self.why = why
+ self.errno = errno
class BzrMoveFailedError(BzrError):
=== modified file 'bzrlib/osutils.py'
--- a/bzrlib/osutils.py 2010-05-12 08:34:47 +0000
+++ b/bzrlib/osutils.py 2010-05-13 17:32:55 +0000
@@ -361,14 +361,6 @@
return _win32_fixdrive(tempfile.mkdtemp(*args, **kwargs).replace('\\', '/'))
-def _add_rename_error_details(e, old, new):
- new_e = OSError(e.errno, "failed to rename %s to %s: %s"
- % (old, new, e.strerror))
- new_e.filename = old
- new_e.to_filename = new
- return new_e
-
-
def _win32_rename(old, new):
"""We expect to be able to atomically replace 'new' with old.
@@ -376,7 +368,7 @@
and then deleted.
"""
try:
- fancy_rename(old, new, rename_func=_wrapped_rename, unlink_func=os.unlink)
+ fancy_rename(old, new, rename_func=os.rename, unlink_func=os.unlink)
except OSError, e:
if e.errno in (errno.EPERM, errno.EACCES, errno.EBUSY, errno.EINVAL):
# If we try to rename a non-existant file onto cwd, we get
@@ -387,16 +379,6 @@
raise
-def _wrapped_rename(old, new):
- """Rename a file or directory"""
- try:
- os.rename(old, new)
- except (IOError, OSError), e:
- # this is eventually called by all rename-like functions, so should
- # catch all of them
- raise _add_rename_error_details(e, old, new)
-
-
def _mac_getcwd():
return unicodedata.normalize('NFC', os.getcwdu())
@@ -407,8 +389,8 @@
realpath = _posix_realpath
pathjoin = os.path.join
normpath = os.path.normpath
-rename = _wrapped_rename # overridden below on win32
getcwd = os.getcwdu
+rename = os.rename
dirname = os.path.dirname
basename = os.path.basename
split = os.path.split
=== modified file 'bzrlib/tests/test_errors.py'
--- a/bzrlib/tests/test_errors.py 2010-04-22 18:36:13 +0000
+++ b/bzrlib/tests/test_errors.py 2010-04-30 09:05:29 +0000
@@ -713,3 +713,9 @@
e = errors.FileTimestampUnavailable("/path/foo")
self.assertEquals("The filestamp for /path/foo is not available.",
str(e))
+
+ def test_transform_rename_failed(self):
+ e = errors.TransformRenameFailed(u"from", u"to", "readonly file", 2)
+ self.assertEquals(
+ u"Failed to rename from to to: readonly file",
+ str(e))
=== modified file 'bzrlib/tests/test_osutils.py'
--- a/bzrlib/tests/test_osutils.py 2010-05-05 14:11:13 +0000
+++ b/bzrlib/tests/test_osutils.py 2010-05-13 17:32:55 +0000
@@ -184,13 +184,6 @@
shape = sorted(os.listdir('.'))
self.assertEquals(['A', 'B'], shape)
- def test_rename_error(self):
- # We wrap os.rename to make it give an error including the filenames
- # https://bugs.launchpad.net/bzr/+bug/491763
- err = self.assertRaises(OSError, osutils.rename,
- 'nonexistent', 'target')
- self.assertContainsString(str(err), 'nonexistent')
-
class TestRandChars(tests.TestCase):
=== modified file 'bzrlib/tests/test_transform.py'
--- a/bzrlib/tests/test_transform.py 2010-05-10 16:25:47 +0000
+++ b/bzrlib/tests/test_transform.py 2010-05-13 17:32:55 +0000
@@ -852,6 +852,32 @@
rename.set_executability(True, myfile)
rename.apply()
+ def test_rename_fails(self):
+ # see https://bugs.launchpad.net/bzr/+bug/491763
+ create, root_id = self.get_transform()
+ first_dir = create.new_directory('first-dir', root_id, 'first-id')
+ myfile = create.new_file('myfile', root_id, 'myfile-text',
+ 'myfile-id')
+ create.apply()
+ # make the file and directory readonly in the hope this will prevent
+ # renames
+ osutils.make_readonly(self.wt.abspath('first-dir'))
+ osutils.make_readonly(self.wt.abspath('myfile'))
+ # now transform to rename
+ rename_transform, root_id = self.get_transform()
+ file_trans_id = rename_transform.trans_id_file_id('myfile-id')
+ dir_id = rename_transform.trans_id_file_id('first-id')
+ rename_transform.adjust_path('newname', dir_id, file_trans_id)
+ e = self.assertRaises(errors.TransformRenameFailed,
+ rename_transform.apply)
+ # Looks like:
+ # "Failed to rename .../work/.bzr/checkout/limbo/new-1
+ # to .../first-dir/newname: [Errno 13] Permission denied"
+ # so the first filename is not visible in it; we expect a strerror but
+ # it may vary per OS and language so it's not checked here
+ self.assertContainsRe(str(e),
+ "Failed to rename .*first-dir.newname:")
+
def test_set_executability_order(self):
"""Ensure that executability behaves the same, no matter what order.
=== modified file 'bzrlib/transform.py'
--- a/bzrlib/transform.py 2010-05-11 08:44:59 +0000
+++ b/bzrlib/transform.py 2010-05-13 17:32:55 +0000
@@ -1657,7 +1657,7 @@
or trans_id in self._new_parent):
try:
mover.rename(full_path, self._limbo_name(trans_id))
- except OSError, e:
+ except errors.TransformRenameFailed, e:
if e.errno != errno.ENOENT:
raise
else:
@@ -1688,7 +1688,7 @@
if trans_id in self._needs_rename:
try:
mover.rename(self._limbo_name(trans_id), full_path)
- except OSError, e:
+ except errors.TransformRenameFailed, e:
# We may be renaming a dangling inventory id
if e.errno != errno.ENOENT:
raise
@@ -2926,10 +2926,12 @@
"""Rename a file from one path to another."""
try:
osutils.rename(from_, to)
- except OSError, e:
+ except (IOError, OSError), e:
if e.errno in (errno.EEXIST, errno.ENOTEMPTY):
raise errors.FileExists(to, str(e))
- raise
+ # normal OSError doesn't include filenames so it's hard to see where
+ # the problem is, see https://bugs.launchpad.net/bzr/+bug/491763
+ raise errors.TransformRenameFailed(from_, to, str(e), e.errno)
self.past_renames.append((from_, to))
def pre_delete(self, from_, to):
@@ -2945,7 +2947,10 @@
def rollback(self):
"""Reverse all renames that have been performed"""
for from_, to in reversed(self.past_renames):
- osutils.rename(to, from_)
+ try:
+ osutils.rename(to, from_)
+ except (OSError, IOError), e:
+ raise errors.TransformRenameFailed(to, from_, str(e), e.errno)
# after rollback, don't reuse _FileMover
past_renames = None
pending_deletions = None
=== modified file 'bzrlib/transport/local.py'
--- a/bzrlib/transport/local.py 2010-04-28 07:53:34 +0000
+++ b/bzrlib/transport/local.py 2010-04-30 09:10:00 +0000
@@ -403,14 +403,11 @@
try:
# *don't* call bzrlib.osutils.rename, because we want to
# detect conflicting names on rename, and osutils.rename tries to
- # mask cross-platform differences there; however we do update the
- # exception to include the filenames
+ # mask cross-platform differences there
os.rename(path_from, path_to)
except (IOError, OSError),e:
# TODO: What about path_to?
- self._translate_error(
- osutils._add_rename_error_details(e, path_from, path_to),
- path_from)
+ self._translate_error(e, path_from)
def move(self, rel_from, rel_to):
"""Move the item at rel_from to the location at rel_to"""
More information about the bazaar-commits
mailing list