[RFC] bug #111758: Bazaar does not indicate which file already exists with Error 17
Aaron Bentley
aaron.bentley at utoronto.ca
Mon Nov 26 15:28:57 GMT 2007
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1
Alexander Belchenko wrote:
> 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.
> === 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()
This is a test for what happens when TT tries to create two files that
are case-insensitively the same. As I read it, bug 111758 is about
errno 17, not case-insensitivity. There are more ways to get error 17
than just case-insensitivity. Unicode normalization, for example.
If you're checking what happens when TT causes error 17, you should
create two directories with the same name, and then run
tt.apply(no_conflicts=True). This will cause errno 17 on Linux, and
presumably also on Windows & Mac.
> 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?
I'm operating under the belief that 'to' does 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.
It checks whether the file already exists, in
TreeTransform._duplicate_entries.
> IMO there is should
> be plain os.rename otherwise we can't catch case-insensitive
> error.
I don't understand this comment.
> Is I understand correctly that method rename() in class _FileMover
> is appropriate place to catch case-insensitive clash?
No. _duplicate_entries should do case-insensitive comparison on
case-insensitive filesystems. But you should catch errnor 17 in _FileMover.
I am waiting for your case-sensitivity detection patch to be merged
before I implement this.
> IMO, I should
> convert OSError/WindowsError to custom BzrError as I did before
> for symlinks. Is it correct way to fix the bug?
Yes, I think so.
Aaron
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.6 (GNU/Linux)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org
iD8DBQFHSuY50F+nu1YWqI0RAqDxAJ9x0udqy0JAGtfTPsGOKuJHl+saZwCfWfr4
JPSeaycQ6k3IPBqKCQPwjsI=
=80J8
-----END PGP SIGNATURE-----
More information about the bazaar
mailing list