Rev 5481: Merge config-editor-option into next in file:///home/vila/src/bzr/experimental/config/

[Vincent Ladeuil]

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

------------------------------------------------------------
revno: 5481 [merge]
revision-id: v.ladeuil+lp at free.fr-20110530145602-1z3ioc8ukblutxka
parent: v.ladeuil+lp at free.fr-20110525144725-te8hbkok7m1389sz
parent: v.ladeuil+lp at free.fr-20110530145600-3ljchk6w41w5bftf
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: next
timestamp: Mon 2011-05-30 16:56:02 +0200
message:
  Merge config-editor-option into next
modified:
  bzrlib/config.py               config.py-20051011043216-070c74f4e9e338e8
  bzrlib/tests/test_config.py    testconfig.py-20051011041908-742d0c15d8d8c8eb
-------------- next part --------------
=== modified file 'bzrlib/config.py'
--- a/bzrlib/config.py	2011-05-21 16:17:56 +0000
+++ b/bzrlib/config.py	2011-05-30 14:56:00 +0000
@@ -2378,9 +2378,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()
 
 
@@ -2419,11 +2419,19 @@
         # cannot exist without a branch, it's safe to make it a weakref.
         self.branch_ref = weakref.ref(branch)
 
+    def _get_branch(self):
+        b = self.branch_ref()
+        if b is None:
+            # Programmer error, a branch store can't exist if the branch it
+            # refers to is dead.
+            raise AssertionError('Dead branch ref in %r' % (self,))
+        return b
+
     def lock_write(self, token=None):
-        return self.branch_ref().lock_write(token)
+        return self._get_branch().lock_write(token)
 
     def unlock(self):
-        return self.branch_ref().unlock()
+        return self._get_branch().unlock()
 
     @needs_write_lock
     def save(self):
@@ -2625,6 +2633,12 @@
 
     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-25 14:47:25 +0000
+++ b/bzrlib/tests/test_config.py	2011-05-30 14:56:02 +0000
@@ -2125,6 +2125,8 @@
         return self.transport.has(store_basename)
 
     def test_save_empty_creates_no_file(self):
+        # FIXME: There should be a better way than relying on the test
+        # parametrization to identify branch.conf -- vila 2011-0526
         if self.store_id in ('branch', 'remote_branch'):
             raise tests.TestNotApplicable(
                 'branch.conf is *always* created when a branch is initialized')
@@ -2144,6 +2146,8 @@
         self.assertLength(0, sections)
 
     def test_save_with_content_succeeds(self):
+        # FIXME: There should be a better way than relying on the test
+        # parametrization to identify branch.conf -- vila 2011-0526
         if self.store_id in ('branch', 'remote_branch'):
             raise tests.TestNotApplicable(
                 'branch.conf is *always* created when a branch is initialized')
@@ -2251,6 +2255,21 @@
         self.assertPathExists('dir/subdir')
 
 
+class TestBranchStore(TestStore):
+
+    def test_dead_branch(self):
+        build_backing_branch(self, 'branch')
+        b = branch.Branch.open('branch')
+        store = config.BranchStore(b)
+        del b
+        # The only reliable way to trigger the error is to explicitly call the
+        # garbage collector.
+        import gc
+        gc.collect()
+        store.get_mutable_section(None).set('foo', 'bar')
+        self.assertRaises(AssertionError, store.save)
+
+
 class TestConcurrentStoreUpdates(TestStore):
 
     scenarios = [(key, {'get_stack': builder}) for key, builder
@@ -2307,14 +2326,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()
@@ -2341,15 +2360,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)