Rev 5192: (lifeless) Wrap os.rename to get better errors on failure. (Martin Pool) in file:///home/pqm/archives/thelove/bzr/%2Btrunk/
Canonical.com Patch Queue Manager
pqm at pqm.ubuntu.com
Wed Apr 28 23:03:03 BST 2010
At file:///home/pqm/archives/thelove/bzr/%2Btrunk/
------------------------------------------------------------
revno: 5192 [merge]
revision-id: pqm at pqm.ubuntu.com-20100428220257-fwkbybtzi147j8re
parent: pqm at pqm.ubuntu.com-20100428115252-dj0jg0on85ww9qoj
parent: mbp at sourcefrog.net-20100428075334-mbrtapod09d7v6ea
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Wed 2010-04-28 23:02:57 +0100
message:
(lifeless) Wrap os.rename to get better errors on failure. (Martin Pool)
modified:
NEWS NEWS-20050323055033-4e00b5db738777ff
bzrlib/osutils.py osutils.py-20050309040759-eeaff12fbf77ac86
bzrlib/tests/blackbox/test_filesystem_cicp.py test_filesystem_cicp-20081028010456-vclkg401m81keaxc-1
bzrlib/tests/blackbox/test_join.py test_join.py-20060928210902-95dkqa6boh8uq92b-1
bzrlib/tests/blackbox/test_switch.py test_switch.py-20071122111948-0c5en6uz92bwl76h-1
bzrlib/tests/test_osutils.py test_osutils.py-20051201224856-e48ee24c12182989
bzrlib/transform.py transform.py-20060105172343-dd99e54394d91687
bzrlib/transport/local.py local_transport.py-20050711165921-9b1f142bfe480c24
=== modified file 'NEWS'
--- a/NEWS 2010-04-28 10:33:44 +0000
+++ b/NEWS 2010-04-28 22:02:57 +0000
@@ -51,6 +51,10 @@
* Reduce peak memory by one copy of compressed text.
(John Arbash Meinel, #566940)
+* Show the filenames when a file rename fails so that the error will be
+ more comprehensible.
+ (Martin Pool, #491763)
+
Improvements
************
=== modified file 'bzrlib/osutils.py'
--- a/bzrlib/osutils.py 2010-04-24 09:08:27 +0000
+++ b/bzrlib/osutils.py 2010-04-27 07:52:08 +0000
@@ -362,6 +362,14 @@
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.
@@ -369,7 +377,7 @@
and then deleted.
"""
try:
- fancy_rename(old, new, rename_func=os.rename, unlink_func=os.unlink)
+ fancy_rename(old, new, rename_func=_wrapped_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
@@ -380,6 +388,16 @@
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())
@@ -390,8 +408,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/blackbox/test_filesystem_cicp.py'
--- a/bzrlib/tests/blackbox/test_filesystem_cicp.py 2009-08-26 09:06:02 +0000
+++ b/bzrlib/tests/blackbox/test_filesystem_cicp.py 2010-04-28 07:53:34 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2008 Canonical Ltd
+# Copyright (C) 2008, 2010 Canonical Ltd
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
@@ -19,6 +19,8 @@
import os
+from bzrlib import (osutils,
+ )
from bzrlib.tests.blackbox import ExternalBase
from bzrlib.tests import CaseInsCasePresFilenameFeature, KnownFailure
from bzrlib.osutils import canonical_relpath, pathjoin
@@ -89,7 +91,7 @@
self.build_tree(['MixedCase'])
self.check_output('adding MixedCase\n', 'add MixedCase')
# 'accidently' rename the file on disk
- os.rename('MixedCase', 'mixedcase')
+ osutils.rename('MixedCase', 'mixedcase')
self.check_empty_output('add mixedcase')
def test_re_add_dir(self):
@@ -103,7 +105,7 @@
'adding MixedCaseParent/MixedCase\n',
'add MixedCaseParent')
# 'accidently' rename the directory on disk
- os.rename('MixedCaseParent', 'mixedcaseparent')
+ osutils.rename('MixedCaseParent', 'mixedcaseparent')
self.check_empty_output('add mixedcaseparent')
def test_add_not_found(self):
@@ -130,7 +132,7 @@
wt = self._make_mixed_case_tree()
self.run_bzr('add')
self.run_bzr('ci -m message')
- os.rename('CamelCaseParent/CamelCase', 'CamelCaseParent/NewCamelCase')
+ osutils.rename('CamelCaseParent/CamelCase', 'CamelCaseParent/NewCamelCase')
# In this case we can specify the incorrect case for the destination,
# as we use --after, so the file-system is sniffed.
@@ -157,7 +159,7 @@
# Remove the source and create a destination file on disk with a different case.
# bzr should report that the filename is already versioned.
os.unlink('CamelCaseParent/CamelCase')
- os.rename('lowercaseparent/lowercase', 'lowercaseparent/LOWERCASE')
+ osutils.rename('lowercaseparent/lowercase', 'lowercaseparent/LOWERCASE')
ex = 'bzr: ERROR: Could not move CamelCase => lowercase: lowercaseparent/lowercase is already versioned.\n'
self.check_error_output(3, ex, 'mv --after camelcaseparent/camelcase LOWERCASEPARENT/LOWERCASE')
@@ -173,7 +175,7 @@
wt = self._make_mixed_case_tree()
self.run_bzr('add')
self.run_bzr('ci -m message')
- os.rename('CamelCaseParent', 'NewCamelCaseParent')
+ osutils.rename('CamelCaseParent', 'NewCamelCaseParent')
# In this case we can specify the incorrect case for the destination,
# as we use --after, so the file-system is sniffed.
@@ -199,7 +201,7 @@
# perform a mv to the new case - we must ensure the file-system has the
# new case first.
- os.rename('CamelCaseParent/CamelCase', 'CamelCaseParent/camelCase')
+ osutils.rename('CamelCaseParent/CamelCase', 'CamelCaseParent/camelCase')
self.check_output('CamelCaseParent/CamelCase => CamelCaseParent/camelCase\n',
'mv --after camelcaseparent/camelcase camelcaseparent/camelCase')
# bzr should not have renamed the file to a different case
=== modified file 'bzrlib/tests/blackbox/test_join.py'
--- a/bzrlib/tests/blackbox/test_join.py 2009-03-23 14:59:43 +0000
+++ b/bzrlib/tests/blackbox/test_join.py 2010-04-28 07:53:34 +0000
@@ -1,4 +1,4 @@
-# Copyright (C) 2006 Canonical Ltd
+# Copyright (C) 2006, 2010 Canonical Ltd
#
# This program is free software; you can redistribute it and/or modify
# it under the terms of the GNU General Public License as published by
@@ -17,7 +17,13 @@
import os
-from bzrlib import bzrdir, repository, tests, workingtree
+from bzrlib import (
+ bzrdir,
+ osutils,
+ repository,
+ tests,
+ workingtree,
+ )
class TestJoin(tests.TestCaseWithTransport):
@@ -50,7 +56,7 @@
def test_join_error(self):
base_tree, sub_tree = self.make_trees()
os.mkdir('tree/subtree2')
- os.rename('tree/subtree', 'tree/subtree2/subtree')
+ osutils.rename('tree/subtree', 'tree/subtree2/subtree')
self.run_bzr_error(
('Cannot join .*subtree. Parent directory is not versioned',),
'join tree/subtree2/subtree')
=== modified file 'bzrlib/tests/blackbox/test_switch.py'
--- a/bzrlib/tests/blackbox/test_switch.py 2010-03-26 07:08:56 +0000
+++ b/bzrlib/tests/blackbox/test_switch.py 2010-04-28 07:53:34 +0000
@@ -20,6 +20,7 @@
import os
+from bzrlib import osutils
from bzrlib.workingtree import WorkingTree
from bzrlib.tests.blackbox import ExternalBase
from bzrlib.directory_service import directories
@@ -167,7 +168,7 @@
def prepare_lightweight_switch(self):
branch = self.make_branch('branch')
branch.create_checkout('tree', lightweight=True)
- os.rename('branch', 'branch1')
+ osutils.rename('branch', 'branch1')
def test_switch_lightweight_after_branch_moved(self):
self.prepare_lightweight_switch()
=== modified file 'bzrlib/tests/test_osutils.py'
--- a/bzrlib/tests/test_osutils.py 2010-04-24 13:48:58 +0000
+++ b/bzrlib/tests/test_osutils.py 2010-04-27 07:52:08 +0000
@@ -184,6 +184,13 @@
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/transform.py'
--- a/bzrlib/transform.py 2010-04-01 12:53:53 +0000
+++ b/bzrlib/transform.py 2010-04-28 07:53:34 +0000
@@ -1160,7 +1160,7 @@
if trans_id not in self._new_contents:
continue
new_path = self._limbo_name(trans_id)
- os.rename(old_path, new_path)
+ osutils.rename(old_path, new_path)
for descendant in self._limbo_descendants(trans_id):
desc_path = self._limbo_files[descendant]
desc_path = new_path + desc_path[len(old_path):]
@@ -2898,9 +2898,9 @@
self.pending_deletions = []
def rename(self, from_, to):
- """Rename a file from one path to another. Functions like os.rename"""
+ """Rename a file from one path to another."""
try:
- os.rename(from_, to)
+ osutils.rename(from_, to)
except OSError, e:
if e.errno in (errno.EEXIST, errno.ENOTEMPTY):
raise errors.FileExists(to, str(e))
@@ -2920,7 +2920,7 @@
def rollback(self):
"""Reverse all renames that have been performed"""
for from_, to in reversed(self.past_renames):
- os.rename(to, from_)
+ osutils.rename(to, from_)
# after rollback, don't reuse _FileMover
past_renames = None
pending_deletions = None
=== modified file 'bzrlib/transport/local.py'
--- a/bzrlib/transport/local.py 2010-03-11 04:18:33 +0000
+++ b/bzrlib/transport/local.py 2010-04-28 07:53:34 +0000
@@ -399,13 +399,18 @@
def rename(self, rel_from, rel_to):
path_from = self._abspath(rel_from)
+ path_to = self._abspath(rel_to)
try:
# *don't* call bzrlib.osutils.rename, because we want to
- # detect errors on rename
- os.rename(path_from, self._abspath(rel_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
+ os.rename(path_from, path_to)
except (IOError, OSError),e:
# TODO: What about path_to?
- self._translate_error(e, path_from)
+ self._translate_error(
+ osutils._add_rename_error_details(e, path_from, path_to),
+ 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