[RFC] bug #111758: Bazaar does not indicate which file already exists with Error 17

Alexander Belchenko bialix at ukr.net
Mon Nov 26 15:08:26 GMT 2007


Aaron,

Probably you can help me with my question re bug #111758.

I'm starting to write the test for this bug and eventually stumble
with my question.

Here is my test draft:

=== modified file 'bzrlib/tests/test_transform.py'
--- bzrlib/tests/test_transform.py 23.11.2007 10:31:24
+++ bzrlib/tests/test_transform.py 26.11.2007 16:49:07
@@ -983,8 +983,21 @@
          self.callDeprecated([txt], change_entry, None, None, None, 
None, None,
              None, None, None)

+    def test_case_insensitive_clash(self):
+        def tt_helper():
+            wt = self.make_branch_and_tree('.')
+            tt = TreeTransform(wt)  # TreeTransform obtains write lock
+            try:
+                tt.new_file('foo', tt.root, 'bar')
+                tt.new_file('Foo', tt.root, 'spam')
+                tt.apply()
+            finally:
+                wt.unlock()
+        tt_helper()
+

As expected this test produce exception:

Traceback (most recent call last):
   File 
"C:\work\Bazaar\devel\bug.111758\bzrlib\tests\test_transform.py", line 
996, in test_case_insensitive_clash
     tt_helper()
   File 
"C:\work\Bazaar\devel\bug.111758\bzrlib\tests\test_transform.py", line 
993, in tt_helper
     tt.apply()
   File "C:\work\Bazaar\devel\bug.111758\bzrlib\transform.py", line 894, 
in apply
     mover)
   File "C:\work\Bazaar\devel\bug.111758\bzrlib\transform.py", line 
1000, in _apply_insertions
     mover.rename(self._limbo_name(trans_id), full_path)
   File "C:\work\Bazaar\devel\bug.111758\bzrlib\transform.py", line 
1867, in rename
     os.rename(from_, to)
OSError: [Errno 17] File exists


Last line in the traceback raise a bit of suspicion inside me.
You're using os.rename to rename [probably] over the top of file?
Or you're assume that file 'to' is not exist yet?
If your code allows to rename over the top then we should use
osutils.fancy_rename. If TreeTransform code makes check before
invoking os.rename then probably it's OK. IMO there is should
be plain os.rename otherwise we can't catch case-insensitive
error.

Is I understand correctly that method rename() in class _FileMover
is appropriate place to catch case-insensitive clash? IMO, I should
convert OSError/WindowsError to custom BzrError as I did before
for symlinks. Is it correct way to fix the bug?

Alexander



More information about the bazaar mailing list