Merge lp:~bialix/bzr/lock-unicode-win32 into lp:~bzr/bzr/trunk-old

[Alexander Belchenko]

[Alexander Belchenko (bialix)]

This patch fixes regression in OS locks @ win32 re unicode paths.

Please, merge it to 1.17.

[John A Meinel (jameinel)]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Alexander Belchenko wrote:
> Alexander Belchenko has proposed merging lp:~bialix/bzr/lock-unicode-win32 into lp:bzr.
>
> Requested reviews:
> John A Meinel (jameinel)
> bzr-core (bzr-core)
>
> This patch fixes regression in OS locks @ win32 re unicode paths.
>
> Please, merge it to 1.17.
>

  Review: approve

This seems fine, though I'll note that we haven't use 'os.path.sup..' in
the past to determine W versus A. Why not use:

   # for Windows NT/2K/XP/etc vs Windows 98
   win32file_CreateFile = getattr(win32file, 'CreateFileW',
      win32file.CreateFile)

Also, I thought we used ctypes preferentially to win32file, since that
was "already present in python 2.5", do we need fixes on the ctypes side
as well?

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org

iEYEARECAAYFAkpTUB8ACgkQJdeBCYSNAAPt4wCgyg9SKrgwgdy3KpHIT5B6smPP
k9MAoKG5xtvbcKDmDidPAYOQUoEH+QGu
=Ggvf
-----END PGP SIGNATURE-----

[Alexander Belchenko (bialix)]

John A Meinel пишет:
> Review: approve

Can you merge it, please?

> This seems fine, though I'll note that we haven't use 'os.path.sup..' in
> the past to determine W versus A. Why not use:
>
> # for Windows NT/2K/XP/etc vs Windows 98
> win32file_CreateFile = getattr(win32file, 'CreateFileW',
> win32file.CreateFile)

Because 'CreateFileW' function present even on Windows 98, but it cannot
be actually called.

os.path.supports_unicode_filenames is uised in lock.py in the code
related to ctypes-based locks. So my patch is basically do the same for
win32file-based locks as it works today (in bzr.dev) for ctypes-based locks.

> Also, I thought we used ctypes preferentially to win32file, since that
> was "already present in python 2.5", do we need fixes on the ctypes side
> as well?

As I could see we prefer win32file-based locks, because by default first
defined locks implementation is used by bzrlib API, and win32file-based
is first implementation for win32. IIUC bzr prefer win32file because
it's a C extension around Win32 API, while ctypes is universal FFI and
therefore slower (at least from theory).

Also ctypes-based implementation uses unicode API on Windows NT, and it
successfully passed new unicode tests.

[John A Meinel (jameinel)]

review: Approve

[Martin Pool (mbp)]

I'll merge it

=== modified file 'bzrlib/lock.py'
--- bzrlib/lock.py	2009-06-19 10:04:02 +0000
+++ bzrlib/lock.py	2009-07-07 17:35:09 +0000
@@ -301,13 +301,19 @@
 
 
 if have_pywin32 and sys.platform == 'win32':
+    if os.path.supports_unicode_filenames:
+        # for Windows NT/2K/XP/etc
+        win32file_CreateFile = win32file.CreateFileW
+    else:
+        # for Windows 98
+        win32file_CreateFile = win32file.CreateFile
 
     class _w32c_FileLock(_OSLock):
 
         def _open(self, filename, access, share, cflags, pymode):
             self.filename = osutils.realpath(filename)
             try:
-                self._handle = win32file.CreateFile(filename, access, share,
+                self._handle = win32file_CreateFile(filename, access, share,
                     None, win32file.OPEN_ALWAYS,
                     win32file.FILE_ATTRIBUTE_NORMAL, None)
             except pywintypes.error, e:

=== modified file 'bzrlib/tests/per_lock/__init__.py'
--- bzrlib/tests/per_lock/__init__.py	2009-03-23 14:59:43 +0000
+++ bzrlib/tests/per_lock/__init__.py	2009-07-07 17:35:09 +0000
@@ -39,7 +39,6 @@
     return result
 
 
-
 def load_tests(standard_tests, module, loader):
     submod_tests = loader.loadTestsFromModuleNames([
         'bzrlib.tests.per_lock.test_lock',

=== modified file 'bzrlib/tests/per_lock/test_lock.py'
--- bzrlib/tests/per_lock/test_lock.py	2009-03-23 14:59:43 +0000
+++ bzrlib/tests/per_lock/test_lock.py	2009-07-07 17:35:10 +0000
@@ -21,6 +21,7 @@
     osutils,
     )
 
+from bzrlib.tests import UnicodeFilenameFeature
 from bzrlib.tests.per_lock import TestCaseWithLock
 
 
@@ -160,3 +161,18 @@
                 b_lock.unlock()
             if c_lock is not None:
                 c_lock.unlock()
+
+
+class TestLockUnicodePath(TestCaseWithLock):
+
+    _test_needs_features = [UnicodeFilenameFeature]
+
+    def test_read_lock(self):
+        self.build_tree([u'\u1234'])
+        u_lock = self.read_lock(u'\u1234')
+        self.addCleanup(u_lock.unlock)
+
+    def test_write_lock(self):
+        self.build_tree([u'\u1234'])
+        u_lock = self.write_lock(u'\u1234')
+        self.addCleanup(u_lock.unlock)