Rev 5795: Address poolie's review comments. in file:///home/vila/src/bzr/experimental/config/

[Vincent Ladeuil]

At file:///home/vila/src/bzr/experimental/config/

------------------------------------------------------------
revno: 5795
revision-id: v.ladeuil+lp at free.fr-20110530131748-ukk0e2sj0xjk63ha
parent: v.ladeuil+lp at free.fr-20110525144711-ypu8k8yg9m1rtjh7
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: config-locks
timestamp: Mon 2011-05-30 15:17:48 +0200
message:
  Address poolie's review comments.
-------------- next part --------------
=== modified file 'bzrlib/config.py'
--- a/bzrlib/config.py	2011-05-18 08:34:58 +0000
+++ b/bzrlib/config.py	2011-05-30 13:17:48 +0000
@@ -2332,9 +2332,9 @@
     @needs_write_lock
     def save(self):
         # We need to be able to override the undecorated implementation
-        self._save()
+        self.save_without_locking()
 
-    def _save(self):
+    def save_without_locking(self):
         super(LockableIniFileStore, self).save()
 
 
@@ -2364,7 +2364,7 @@
 
 # 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
-# abit trickier though but I punt for now -- vila 20110512
+# a bit trickier though but I punt for now -- vila 20110512
 class BranchStore(LockableIniFileStore):
 
     def __init__(self, branch):
@@ -2547,11 +2547,17 @@
 class _CompatibleStack(Stack):
     """Place holder for compatibility with previous design.
 
-    This intended to ease the transition from the Config-based design to the
+    This is intended to ease the transition from the Config-based design to the
     Stack-based design and should not be used nor relied upon by plugins.
 
     One assumption made here is that the daughter classes will all use Stores
     derived from LockableIniFileStore).
+
+    It implements set() by re-loading the store before applying the
+    modification and saving it.
+
+    The long term plan being to implement a single write by store to save
+    all modifications, this class should not be used in the interim.
     """
 
     def set(self, name, value):

=== modified file 'bzrlib/tests/test_config.py'
--- a/bzrlib/tests/test_config.py	2011-05-18 09:20:18 +0000
+++ b/bzrlib/tests/test_config.py	2011-05-30 13:17:48 +0000
@@ -2162,14 +2162,14 @@
         before_writing = threading.Event()
         after_writing = threading.Event()
         writing_done = threading.Event()
-        c1_save_orig = c1.store._save
-        def c1_save():
+        c1_save_without_locking_orig = c1.store.save_without_locking
+        def c1_save_without_locking():
             before_writing.set()
-            c1_save_orig()
+            c1_save_without_locking_orig()
             # The lock is held. We wait for the main thread to decide when to
             # continue
             after_writing.wait()
-        c1.store._save = c1_save
+        c1.store.save_without_locking = c1_save_without_locking
         def c1_set():
             c1.set('one', 'c1')
             writing_done.set()
@@ -2197,15 +2197,15 @@
        do_writing = threading.Event()
        writing_done = threading.Event()
        # We override the _save implementation so we know the store is locked
-       c1_orig = c1.store._save
-       def c1_save():
+       c1_save_without_locking_orig = c1.store.save_without_locking
+       def c1_save_without_locking():
            ready_to_write.set()
            # The lock is held. We wait for the main thread to decide when to
            # continue
            do_writing.wait()
-           c1_orig()
+           c1_save_without_locking_orig()
            writing_done.set()
-       c1.store._save = c1_save
+       c1.store.save_without_locking = c1_save_without_locking
        def c1_set():
            c1.set('one', 'c1')
        t1 = threading.Thread(target=c1_set)