Rev 2363: [merge] LockCleanup changes. in http://bzr.arbash-meinel.com/branches/bzr/0.15-dev/locking

John Arbash Meinel john at arbash-meinel.com
Wed Mar 14 22:31:58 GMT 2007


At http://bzr.arbash-meinel.com/branches/bzr/0.15-dev/locking

------------------------------------------------------------
revno: 2363
revision-id: john at arbash-meinel.com-20070314223138-wfmodqhruo0g5l0e
parent: john at arbash-meinel.com-20070313233914-bigtq3yxnmf90d9s
parent: john at arbash-meinel.com-20070314205703-taih3n87g9n4afdv
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: locking
timestamp: Wed 2007-03-14 17:31:38 -0500
message:
  [merge] LockCleanup changes.
added:
  bzrlib/tests/per_lock/         bzrlibtestsper_lock-20070314195914-llb0phfp2laomqb3-1
  bzrlib/tests/per_lock/__init__.py __init__.py-20070314201444-u92yjsqrkh2m3qcb-1
renamed:
  bzrlib/tests/test_lock.py => bzrlib/tests/per_lock/test_lock.py test_lock.py-20070313190612-mfpoa7t8kvrgrhj2-1
modified:
  bzrlib/errors.py               errors.py-20050309040759-20512168c4e14fbd
  bzrlib/lock.py                 lock.py-20050527050856-ec090bb51bc03349
  bzrlib/tests/__init__.py       selftest.py-20050531073622-8d0e3c8845c97a64
  bzrlib/tests/test_errors.py    test_errors.py-20060210110251-41aba2deddf936a8
  bzrlib/tests/per_lock/test_lock.py test_lock.py-20070313190612-mfpoa7t8kvrgrhj2-1
    ------------------------------------------------------------
    revno: 2361.1.3
    merged: john at arbash-meinel.com-20070314205703-taih3n87g9n4afdv
    parent: john at arbash-meinel.com-20070314204717-htynwogv97fqr22a
    committer: John Arbash Meinel <john at arbash-meinel.com>
    branch nick: lock_cleanup
    timestamp: Wed 2007-03-14 15:57:03 -0500
    message:
      Code cleanup
    ------------------------------------------------------------
    revno: 2361.1.2
    merged: john at arbash-meinel.com-20070314204717-htynwogv97fqr22a
    parent: john at arbash-meinel.com-20070314201552-bjtfua57456dviep
    committer: John Arbash Meinel <john at arbash-meinel.com>
    branch nick: lock_cleanup
    timestamp: Wed 2007-03-14 15:47:17 -0500
    message:
      Cleanup errors, and change ReadOnlyLockError to pass around more details.
    ------------------------------------------------------------
    revno: 2361.1.1
    merged: john at arbash-meinel.com-20070314201552-bjtfua57456dviep
    parent: john at arbash-meinel.com-20070313231812-h74g1zz32v12lv6s
    committer: John Arbash Meinel <john at arbash-meinel.com>
    branch nick: lock_cleanup
    timestamp: Wed 2007-03-14 15:15:52 -0500
    message:
      Update the lock code and test code so that if more than one
      lock implementation is available, they will both be tested.
      
      It is quite a bit of overhead, for a case where we are likely to only have 1
      real lock implementation per platform, but hey, for now we have 2.
-------------- next part --------------
=== added directory 'bzrlib/tests/per_lock'
=== added file 'bzrlib/tests/per_lock/__init__.py'
--- a/bzrlib/tests/per_lock/__init__.py	1970-01-01 00:00:00 +0000
+++ b/bzrlib/tests/per_lock/__init__.py	2007-03-14 20:15:52 +0000
@@ -0,0 +1,74 @@
+# Copyright (C) 2007 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
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program; if not, write to the Free Software
+# Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA  02111-1307  USA
+
+"""OS Lock implementation tests for bzr.
+
+These test the conformance of all the lock variations to the expected API.
+"""
+
+from copy import deepcopy
+
+from bzrlib import (
+    lock,
+    tests,
+    )
+
+
+class TestCaseWithLock(tests.TestCaseWithTransport):
+
+    write_lock = None
+    read_lock = None
+
+
+class LockTestProviderAdapter(object):
+    """A tool to generate a suite testing multiple lock formats at once.
+
+    This is done by copying the test once for each lock and injecting the
+    read_lock and write_lock classes.
+    They are also given a new test id.
+    """
+
+    def __init__(self, lock_classes):
+        self._lock_classes = lock_classes
+
+    def _clone_test(self, test, write_lock, read_lock, variation):
+        """Clone test for adaption."""
+        new_test = deepcopy(test)
+        new_test.write_lock = write_lock
+        new_test.read_lock = read_lock
+        def make_new_test_id():
+            new_id = "%s(%s)" % (test.id(), variation)
+            return lambda: new_id
+        new_test.id = make_new_test_id()
+        return new_test
+
+    def adapt(self, test):
+        result = tests.TestSuite()
+        for name, write_lock, read_lock in self._lock_classes:
+            new_test = self._clone_test(test, write_lock, read_lock, name)
+            result.addTest(new_test)
+        return result
+
+
+def test_suite():
+    result = tests.TestSuite()
+    test_lock_implementations = [
+        'bzrlib.tests.per_lock.test_lock',
+        ]
+    adapter = LockTestProviderAdapter(lock._lock_classes)
+    loader = tests.TestLoader()
+    tests.adapt_modules(test_lock_implementations, adapter, loader, result)
+    return result

=== renamed file 'bzrlib/tests/test_lock.py' => 'bzrlib/tests/per_lock/test_lock.py'
--- a/bzrlib/tests/test_lock.py	2007-03-13 23:39:14 +0000
+++ b/bzrlib/tests/per_lock/test_lock.py	2007-03-14 22:31:38 +0000
@@ -18,13 +18,13 @@
 
 from bzrlib import (
     errors,
-    lock,
     osutils,
-    tests,
     )
 
-
-class TestLock(tests.TestCaseInTempDir):
+from bzrlib.tests.per_lock import TestCaseWithLock
+
+
+class TestLock(TestCaseWithLock):
 
     def setUp(self):
         super(TestLock, self).setUp()
@@ -32,7 +32,7 @@
 
     def test_read_lock(self):
         """Smoke test for read locks."""
-        a_lock = lock.ReadLock('a-file')
+        a_lock = self.read_lock('a-file')
         self.addCleanup(a_lock.unlock)
         # The lock file should be opened for reading
         txt = a_lock.f.read()
@@ -40,14 +40,14 @@
 
     def test_create_if_needed_read(self):
         """We will create the file if it doesn't exist yet."""
-        a_lock = lock.ReadLock('other-file')
+        a_lock = self.read_lock('other-file')
         self.addCleanup(a_lock.unlock)
         txt = a_lock.f.read()
         self.assertEqual('', txt)
 
     def test_create_if_needed_write(self):
         """We will create the file if it doesn't exist yet."""
-        a_lock = lock.WriteLock('other-file')
+        a_lock = self.write_lock('other-file')
         self.addCleanup(a_lock.unlock)
         txt = a_lock.f.read()
         self.assertEqual('', txt)
@@ -64,15 +64,15 @@
         osutils.make_readonly('a-file')
         # Make sure the file is read-only (on all platforms)
         self.assertRaises(IOError, open, 'a-file', 'rb+')
-        a_lock = lock.ReadLock('a-file')
+        a_lock = self.read_lock('a-file')
         a_lock.unlock()
 
         # TODO: jam 20070313 This should be a specific subclass
-        self.assertRaises(errors.ReadOnlyLockError, lock.WriteLock, 'a-file')
+        self.assertRaises(errors.ReadOnlyLockError, self.write_lock, 'a-file')
 
     def test_write_lock(self):
         """Smoke test for write locks."""
-        a_lock = lock.WriteLock('a-file')
+        a_lock = self.write_lock('a-file')
         self.addCleanup(a_lock.unlock)
         # You should be able to read and write to the lock file.
         txt = a_lock.f.read()
@@ -87,30 +87,30 @@
 
     def test_multiple_read_locks(self):
         """You can take out more than one read lock on the same file."""
-        a_lock = lock.ReadLock('a-file')
+        a_lock = self.read_lock('a-file')
         self.addCleanup(a_lock.unlock)
-        b_lock = lock.ReadLock('a-file')
+        b_lock = self.read_lock('a-file')
         self.addCleanup(b_lock.unlock)
 
     def test_multiple_write_locks_exclude(self):
         """Taking out more than one write lock should fail."""
-        a_lock = lock.WriteLock('a-file')
+        a_lock = self.write_lock('a-file')
         self.addCleanup(a_lock.unlock)
         # Taking out a lock on a locked file should raise LockContention
-        self.assertRaises(errors.LockContention, lock.WriteLock, 'a-file')
+        self.assertRaises(errors.LockContention, self.write_lock, 'a-file')
 
     def test_read_then_write_excludes(self):
         """If a file is read-locked, taking out a write lock should fail."""
-        a_lock = lock.ReadLock('a-file')
+        a_lock = self.read_lock('a-file')
         self.addCleanup(a_lock.unlock)
         # Taking out a lock on a locked file should raise LockContention
-        self.assertRaises(errors.LockContention, lock.WriteLock, 'a-file')
+        self.assertRaises(errors.LockContention, self.write_lock, 'a-file')
 
     def test_read_unlock_write(self):
         """Make sure that unlocking allows us to lock write"""
-        a_lock = lock.ReadLock('a-file')
+        a_lock = self.read_lock('a-file')
         a_lock.unlock()
-        a_lock = lock.WriteLock('a-file')
+        a_lock = self.write_lock('a-file')
         a_lock.unlock()
 
     def test_write_then_read_excludes(self):
@@ -119,31 +119,31 @@
         The file is exclusively owned by the write lock, so we shouldn't be
         able to take out a shared read lock.
         """
-        a_lock = lock.WriteLock('a-file')
+        a_lock = self.write_lock('a-file')
         self.addCleanup(a_lock.unlock)
         # Taking out a lock on a locked file should raise LockContention
-        self.assertRaises(errors.LockContention, lock.ReadLock, 'a-file')
+        self.assertRaises(errors.LockContention, self.read_lock, 'a-file')
 
     def test_write_unlock_read(self):
         """If we have removed the write lock, we can grab a read lock."""
-        a_lock = lock.WriteLock('a-file')
+        a_lock = self.write_lock('a-file')
         a_lock.unlock()
-        a_lock = lock.ReadLock('a-file')
+        a_lock = self.read_lock('a-file')
         a_lock.unlock()
 
     def test_multiple_read_unlock_write(self):
         """We can only grab a write lock if all read locks are done."""
         a_lock = b_lock = c_lock = None
         try:
-            a_lock = lock.ReadLock('a-file')
-            b_lock = lock.ReadLock('a-file')
-            self.assertRaises(errors.LockContention, lock.WriteLock, 'a-file')
+            a_lock = self.read_lock('a-file')
+            b_lock = self.read_lock('a-file')
+            self.assertRaises(errors.LockContention, self.write_lock, 'a-file')
             a_lock.unlock()
             a_lock = None
-            self.assertRaises(errors.LockContention, lock.WriteLock, 'a-file')
+            self.assertRaises(errors.LockContention, self.write_lock, 'a-file')
             b_lock.unlock()
             b_lock = None
-            c_lock = lock.WriteLock('a-file')
+            c_lock = self.write_lock('a-file')
             c_lock.unlock()
             c_lock = None
         finally:

=== modified file 'bzrlib/errors.py'
--- a/bzrlib/errors.py	2007-03-13 19:39:27 +0000
+++ b/bzrlib/errors.py	2007-03-14 20:47:17 +0000
@@ -698,12 +698,13 @@
 
 
 class ReadOnlyLockError(LockError):
-    
-    _fmt = "Cannot acquire write lock on %(fname)s. File is readonly."
-
-    def __init__(self, fname):
+
+    _fmt = "Cannot acquire write lock on %(fname)s. %(msg)s"
+
+    def __init__(self, fname, msg):
         LockError.__init__(self, '')
         self.fname = fname
+        self.msg = msg
 
 
 class OutSideTransaction(BzrError):

=== modified file 'bzrlib/lock.py'
--- a/bzrlib/lock.py	2007-03-13 23:39:14 +0000
+++ b/bzrlib/lock.py	2007-03-14 22:31:38 +0000
@@ -26,7 +26,7 @@
 
 It is not specified whether these locks are reentrant (i.e. can be
 taken repeatedly by a single process) or whether they exclude
-different threads in a single process.  That reentrancy is provided by 
+different threads in a single process.  That reentrancy is provided by
 LockableFiles.
 
 This defines two classes: ReadLock and WriteLock, which can be
@@ -35,34 +35,35 @@
 """
 
 import errno
-import os
-import sys
 
-from bzrlib import errors
-from bzrlib.errors import LockError, LockContention
-from bzrlib.osutils import realpath
-from bzrlib.trace import mutter
+from bzrlib import (
+    errors,
+    osutils,
+    trace,
+    )
 
 
 class _base_Lock(object):
 
     def __init__(self):
         self.f = None
+        self.filename = None
 
     def _open(self, filename, filemode):
+        self.filename = osutils.realpath(filename)
         try:
-            self.f = open(filename, filemode)
+            self.f = open(self.filename, filemode)
             return self.f
         except IOError, e:
             if e.errno in (errno.EACCES, errno.EPERM):
-                raise errors.ReadOnlyLockError(e)
+                raise errors.ReadOnlyLockError(self.filename, str(e))
             if e.errno != errno.ENOENT:
                 raise
 
             # maybe this is an old branch (before may 2005)
-            mutter("trying to create missing branch lock %r", filename)
-            
-            self.f = open(filename, 'wb+')
+            trace.mutter("trying to create missing lock %r", self.filename)
+
+            self.f = open(self.filename, 'wb+')
             return self.f
 
     def _clear_f(self):
@@ -76,26 +77,29 @@
             from warnings import warn
             warn("lock on %r not released" % self.f)
             self.unlock()
-            
+
     def unlock(self):
         raise NotImplementedError()
 
 
-have_ctypes = have_pywin32 = have_fcntl = False
 try:
     import fcntl
     have_fcntl = True
 except ImportError:
-    try:
-        import win32con, win32file, pywintypes, winerror, msvcrt
-        have_pywin32 = True
-    except ImportError:
-        try:
-            import ctypes, msvcrt
-            have_ctypes = True
-        except ImportError:
-            raise NotImplementedError("please write a locking method "
-                                      "for platform %r" % sys.platform)
+    have_fcntl = False
+try:
+    import win32con, win32file, pywintypes, winerror, msvcrt
+    have_pywin32 = True
+except ImportError:
+    have_pywin32 = False
+try:
+    import ctypes, msvcrt
+    have_ctypes = True
+except ImportError:
+    have_ctypes = False
+
+
+_lock_classes = []
 
 
 if have_fcntl:
@@ -116,15 +120,15 @@
         _open_locks = set()
 
         def __init__(self, filename):
-            # standard IO errors get exposed directly.
             super(_fcntl_WriteLock, self).__init__()
-            self.filename = realpath(filename)
+            # Check we can grab a lock before we actually open the file.
+            self.filename = osutils.realpath(filename)
             if (self.filename in _fcntl_WriteLock._open_locks
                 or self.filename in _fcntl_ReadLock._open_locks):
                 self._clear_f()
                 raise errors.LockContention(self.filename)
 
-            self._open(filename, 'rb+')
+            self._open(self.filename, 'rb+')
             # reserve a slot for this lock - even if the lockf call fails,
             # at thisi point unlock() will be called, because self.f is set.
             # TODO: make this fully threadsafe, if we decide we care.
@@ -152,7 +156,7 @@
 
         def __init__(self, filename):
             super(_fcntl_ReadLock, self).__init__()
-            self.filename = realpath(filename)
+            self.filename = osutils.realpath(filename)
             if self.filename in _fcntl_WriteLock._open_locks:
                 raise errors.LockContention(self.filename)
             _fcntl_ReadLock._open_locks.setdefault(self.filename, 0)
@@ -176,10 +180,10 @@
             self._unlock()
 
 
-    WriteLock = _fcntl_WriteLock
-    ReadLock = _fcntl_ReadLock
-
-elif have_pywin32:
+    _lock_classes.append(('fcntl', _fcntl_WriteLock, _fcntl_ReadLock))
+
+
+if have_pywin32:
     LOCK_SH = 0 # the default
     LOCK_EX = win32con.LOCKFILE_EXCLUSIVE_LOCK
     LOCK_NB = win32con.LOCKFILE_FAIL_IMMEDIATELY
@@ -203,7 +207,7 @@
                 raise
             except Exception, e:
                 self._clear_f()
-                raise LockError(e)
+                raise errors.LockError(e)
 
         def unlock(self):
             overlapped = pywintypes.OVERLAPPED()
@@ -211,7 +215,7 @@
                 win32file.UnlockFileEx(self.hfile, 0, 0x7fff0000, overlapped)
                 self._clear_f()
             except Exception, e:
-                raise LockError(e)
+                raise errors.LockError(e)
 
 
     class _w32c_ReadLock(_w32c_FileLock):
@@ -219,15 +223,17 @@
             super(_w32c_ReadLock, self).__init__()
             self._lock(filename, 'rb', LOCK_SH + LOCK_NB)
 
+
     class _w32c_WriteLock(_w32c_FileLock):
         def __init__(self, filename):
             super(_w32c_WriteLock, self).__init__()
             self._lock(filename, 'rb+', LOCK_EX + LOCK_NB)
 
-    WriteLock = _w32c_WriteLock
-    ReadLock = _w32c_ReadLock
-else:
-    assert have_ctypes, "We should have ctypes installed"
+
+    _lock_classes.append(('pywin32', _w32c_WriteLock, _w32c_ReadLock))
+
+
+if have_ctypes:
     # These constants were copied from the win32con.py module.
     LOCKFILE_FAIL_IMMEDIATELY = 1
     LOCKFILE_EXCLUSIVE_LOCK = 2
@@ -254,7 +260,7 @@
     #     PVOID Pointer;
     #   };
     #   HANDLE hEvent;
-    # } OVERLAPPED, 
+    # } OVERLAPPED,
 
     class _inner_struct(ctypes.Structure):
         _fields_ = [('Offset', ctypes.c_uint), # DWORD
@@ -318,11 +324,23 @@
             super(_ctypes_ReadLock, self).__init__()
             self._lock(filename, 'rb', LOCK_SH + LOCK_NB)
 
+
     class _ctypes_WriteLock(_ctypes_FileLock):
         def __init__(self, filename):
             super(_ctypes_WriteLock, self).__init__()
             self._lock(filename, 'rb+', LOCK_EX + LOCK_NB)
 
-    WriteLock = _ctypes_WriteLock
-    ReadLock = _ctypes_ReadLock
+
+    _lock_classes.append(('ctypes', _ctypes_WriteLock, _ctypes_ReadLock))
+
+
+if len(_lock_classes) == 0:
+    raise NotImplementedError(
+        "We must have one of fcntl, pywin32, or ctypes available"
+        " to support OS locking."
+        )
+
+
+# We default to using the first available lock class.
+_lock_type, WriteLock, ReadLock = _lock_classes[0]
 

=== modified file 'bzrlib/tests/__init__.py'
--- a/bzrlib/tests/__init__.py	2007-03-13 19:11:05 +0000
+++ b/bzrlib/tests/__init__.py	2007-03-14 20:15:52 +0000
@@ -120,6 +120,7 @@
     import bzrlib.tests.interrepository_implementations
     import bzrlib.tests.interversionedfile_implementations
     import bzrlib.tests.intertree_implementations
+    import bzrlib.tests.per_lock
     import bzrlib.tests.repository_implementations
     import bzrlib.tests.revisionstore_implementations
     import bzrlib.tests.tree_implementations
@@ -132,6 +133,7 @@
             bzrlib.tests.interrepository_implementations,
             bzrlib.tests.interversionedfile_implementations,
             bzrlib.tests.intertree_implementations,
+            bzrlib.tests.per_lock,
             bzrlib.tests.repository_implementations,
             bzrlib.tests.revisionstore_implementations,
             bzrlib.tests.tree_implementations,
@@ -1950,7 +1952,6 @@
                    'bzrlib.tests.test_knit',
                    'bzrlib.tests.test_lazy_import',
                    'bzrlib.tests.test_lazy_regex',
-                   'bzrlib.tests.test_lock',
                    'bzrlib.tests.test_lockdir',
                    'bzrlib.tests.test_lockable_files',
                    'bzrlib.tests.test_log',

=== modified file 'bzrlib/tests/test_errors.py'
--- a/bzrlib/tests/test_errors.py	2007-02-26 22:08:14 +0000
+++ b/bzrlib/tests/test_errors.py	2007-03-14 20:47:17 +0000
@@ -105,6 +105,11 @@
             "to be.",
             str(error))
 
+    def test_read_only_lock_error(self):
+        error = errors.ReadOnlyLockError('filename', 'error message')
+        self.assertEqualDiff("Cannot acquire write lock on filename."
+                             " error message", str(error))
+
     def test_too_many_concurrent_requests(self):
         error = errors.TooManyConcurrentRequests("a medium")
         self.assertEqualDiff("The medium 'a medium' has reached its concurrent "



More information about the bazaar-commits mailing list