[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