Rev 2356: Define an explicit error when trying to grab a write lock on a readonly file. in http://bzr.arbash-meinel.com/branches/bzr/0.15-dev/locking

[John Arbash Meinel]

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

------------------------------------------------------------
revno: 2356
revision-id: john at arbash-meinel.com-20070313193927-8opc18wbi7ln0gil
parent: john at arbash-meinel.com-20070313191557-ouvms9hsnpij9xct
committer: John Arbash Meinel <john at arbash-meinel.com>
branch nick: locking
timestamp: Tue 2007-03-13 13:39:27 -0600
message:
  Define an explicit error when trying to grab a write lock on a readonly file.
  Add some future tests that ensure write and read locks exclude eachother.
  Currently fcntl locks do not exclude.
modified:
  bzrlib/errors.py               errors.py-20050309040759-20512168c4e14fbd
  bzrlib/lock.py                 lock.py-20050527050856-ec090bb51bc03349
  bzrlib/tests/test_lock.py      test_lock.py-20070313190612-mfpoa7t8kvrgrhj2-1
-------------- next part --------------
=== modified file 'bzrlib/errors.py'
--- a/bzrlib/errors.py	2007-03-12 16:46:21 +0000
+++ b/bzrlib/errors.py	2007-03-13 19:39:27 +0000
@@ -697,6 +697,15 @@
         self.obj = obj
 
 
+class ReadOnlyLockError(LockError):
+    
+    _fmt = "Cannot acquire write lock on %(fname)s. File is readonly."
+
+    def __init__(self, fname):
+        LockError.__init__(self, '')
+        self.fname = fname
+
+
 class OutSideTransaction(BzrError):
 
     _fmt = ("A transaction related operation was attempted after"

=== modified file 'bzrlib/lock.py'
--- a/bzrlib/lock.py	2007-03-01 21:56:19 +0000
+++ b/bzrlib/lock.py	2007-03-13 19:39:27 +0000
@@ -38,6 +38,7 @@
 import os
 import sys
 
+from bzrlib import errors
 from bzrlib.errors import LockError, LockContention
 from bzrlib.osutils import realpath
 from bzrlib.trace import mutter
@@ -96,7 +97,12 @@
 
         def __init__(self, filename):
             # standard IO errors get exposed directly.
-            self._open(filename, 'rb+')
+            try:
+                self._open(filename, 'rb+')
+            except IOError, e:
+                if e.errno in (errno.EACCES, errno.EPERM):
+                    raise errors.ReadOnlyLockError(e)
+                raise
             self.filename = realpath(filename)
             if self.filename in self.open_locks:
                 self._clear_f()
@@ -124,8 +130,9 @@
 
     class _fcntl_ReadLock(_fcntl_FileLock):
 
+        open_locks = {}
+
         def __init__(self, filename):
-            # standard IO errors get exposed directly.
             self._open(filename, 'rb')
             try:
                 # LOCK_NB will cause IOError to be raised if we can't grab a

=== modified file 'bzrlib/tests/test_lock.py'
--- a/bzrlib/tests/test_lock.py	2007-03-13 19:15:57 +0000
+++ b/bzrlib/tests/test_lock.py	2007-03-13 19:39:27 +0000
@@ -19,6 +19,7 @@
 from bzrlib import (
     errors,
     lock,
+    osutils,
     tests,
     )
 
@@ -55,6 +56,20 @@
         txt = a_lock.f.read()
         self.assertEqual('foo\n', txt)
 
+    def test_readonly_file(self):
+        """If the file is readonly, we can take a read lock.
+
+        But we shouldn't be able to take a write lock.
+        """
+        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.unlock()
+
+        # TODO: jam 20070313 This should be a specific subclass
+        self.assertRaises(errors.ReadOnlyLockError, lock.WriteLock, 'a-file')
+
     def test_write_lock(self):
         """Smoke test for write locks."""
         a_lock = lock.WriteLock('a-file')
@@ -80,3 +95,21 @@
         self.addCleanup(a_lock.unlock)
         # Taking out a lock on a locked file should raise LockContention
         self.assertRaises(errors.LockContention, lock.WriteLock, 'a-file')
+
+    def _disabled_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')
+        self.addCleanup(a_lock.unlock)
+        # Taking out a lock on a locked file should raise LockContention
+        self.assertRaises(errors.LockContention, lock.WriteLock, 'a-file')
+
+    def _disabled_test_write_then_read_excludes(self):
+        """If a file is write-locked, taking out a read lock should fail.
+
+        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')
+        self.addCleanup(a_lock.unlock)
+        # Taking out a lock on a locked file should raise LockContention
+        self.assertRaises(errors.LockContention, lock.ReadLock, 'a-file')