Rev 5942: (vila) Implement branch config locking. (Vincent Ladeuil) in file:///home/pqm/archives/thelove/bzr/%2Btrunk/

[Canonical.com Patch Queue Manager]

At file:///home/pqm/archives/thelove/bzr/%2Btrunk/

------------------------------------------------------------
revno: 5942 [merge]
revision-id: pqm at pqm.ubuntu.com-20110531190047-n1zhm73cb1dnp5hn
parent: pqm at pqm.ubuntu.com-20110531181932-l8lk4840z7dty5sa
parent: v.ladeuil+lp at free.fr-20110531081404-vxfc2zzr38grt2nf
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Tue 2011-05-31 19:00:47 +0000
message:
  (vila) Implement branch config locking. (Vincent Ladeuil)
modified:
  bzrlib/config.py               config.py-20051011043216-070c74f4e9e338e8
  bzrlib/tests/test_config.py    testconfig.py-20051011041908-742d0c15d8d8c8eb
=== modified file 'bzrlib/config.py'
--- a/bzrlib/config.py	2011-05-31 18:19:32 +0000
+++ b/bzrlib/config.py	2011-05-31 19:00:47 +0000
@@ -65,6 +65,7 @@
 import os
 import string
 import sys
+import weakref
 
 from bzrlib import commands
 from bzrlib.decorators import needs_write_lock
@@ -2364,14 +2365,28 @@
         super(LocationStore, self).__init__(t, 'locations.conf')
 
 
-# FIXME: We should rely on the branch itself to be locked (possibly checking
-# that even) but we shouldn't lock ourselves. This may make `bzr config` is
-# a bit trickier though but I punt for now -- vila 20110512
-class BranchStore(LockableIniFileStore):
+class BranchStore(IniFileStore):
 
     def __init__(self, branch):
         super(BranchStore, self).__init__(branch.control_transport,
                                           'branch.conf')
+        # FIXME: This creates a cycle -- vila 2011-05-27
+        self.branch = branch
+
+    def lock_write(self, token=None):
+        return self.branch.lock_write(token)
+
+    def unlock(self):
+        return self.branch.unlock()
+
+    @needs_write_lock
+    def save(self):
+        # We need to be able to override the undecorated implementation
+        self.save_without_locking()
+
+    def save_without_locking(self):
+        super(BranchStore, self).save()
+
 
 class SectionMatcher(object):
     """Select sections into a given Store.

=== modified file 'bzrlib/tests/test_config.py'
--- a/bzrlib/tests/test_config.py	2011-05-31 18:19:32 +0000
+++ b/bzrlib/tests/test_config.py	2011-05-31 19:00:47 +0000
@@ -75,7 +75,11 @@
 def build_branch_store(test):
     if getattr(test, 'branch', None) is None:
         test.branch = test.make_branch('branch')
-    return config.BranchStore(test.branch)
+    # Since we can be called to create different stores, we need to build them
+    # from different branch *objects*, even if they point to the same branch on
+    # disk, otherwise tests about conccurent updates won't be able to trigger
+    # LockContention
+    return config.BranchStore(branch.Branch.open('branch'))
 config.test_store_builder_registry.register('branch', build_branch_store)
 
 
@@ -87,7 +91,11 @@
 def build_branch_stack(test):
     if getattr(test, 'branch', None) is None:
         test.branch = test.make_branch('branch')
-    return config.BranchStack(test.branch)
+    # Since we can be called to create different stacks, we need to build them
+    # from different branch *objects*, even if they point to the same branch on
+    # disk, otherwise tests about conccurent updates won't be able to trigger
+    # LockContention
+    return config.BranchStack(branch.Branch.open('branch'))
 config.test_stack_builder_registry.register('branch', build_branch_stack)
 
 
@@ -2180,7 +2188,6 @@
         self.addCleanup(after_writing.set)
         t1.start()
         before_writing.wait()
-        self.assertTrue(c1.store._lock.is_held)
         self.assertRaises(errors.LockContention,
                           c2.set, 'one', 'c2')
         self.assertEquals('c1', c1.get('one'))
@@ -2216,7 +2223,6 @@
        t1.start()
        # Ensure the thread is ready to write
        ready_to_write.wait()
-       self.assertTrue(c1.store._lock.is_held)
        self.assertEquals('c1', c1.get('one'))
        # If we read during the write, we get the old value
        c2 = self.get_stack(self)