Rev 5797: Forget weakref for branch <-> config. in file:///home/vila/src/bzr/experimental/config/

[Vincent Ladeuil]

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

------------------------------------------------------------
revno: 5797
revision-id: v.ladeuil+lp at free.fr-20110601090106-j6576yr7gv1xs1fs
parent: v.ladeuil+lp at free.fr-20110531210804-svtczt2ouwi7luae
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: config-locks
timestamp: Wed 2011-06-01 11:01:06 +0200
message:
  Forget weakref for branch <-> config.
-------------- next part --------------
=== modified file 'bzrlib/config.py'
--- a/bzrlib/config.py	2011-05-31 20:22:24 +0000
+++ b/bzrlib/config.py	2011-06-01 09:01:06 +0000
@@ -2409,25 +2409,13 @@
     def __init__(self, branch):
         super(BranchStore, self).__init__(branch.control_transport,
                                           'branch.conf')
-        # We don't want to create a cycle here when the BranchStore becomes
-        # part of an object (roughly a Stack, directly or indirectly) that is
-        # an attribute of the branch object itself. Since the BranchStore
-        # 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
+        self.branch = branch
 
     def lock_write(self, token=None):
-        return self._get_branch().lock_write(token)
+        return self.branch.lock_write(token)
 
     def unlock(self):
-        return self._get_branch().unlock()
+        return self.branch.unlock()
 
     @needs_write_lock
     def save(self):

=== modified file 'bzrlib/tests/test_config.py'
--- a/bzrlib/tests/test_config.py	2011-05-31 20:22:24 +0000
+++ b/bzrlib/tests/test_config.py	2011-06-01 09:01:06 +0000
@@ -104,26 +104,9 @@
         test.backing_branch = test.make_branch(relpath)
 
 
-def keep_branch_alive(test, b):
-    """Keep a branch alive for the duration of a test.
-
-    :param tests: the test that should hold the branch alive.
-
-    :param b: the branch that should be kept alive.
-
-    Several tests need to keep a reference to a branch object as they are
-    testing a Store which uses a weak reference. This is achieved by embedding
-    a reference to the branch object in a lambda passed to a cleanup. When the
-    test finish the cleanup method is deleted and so does the reference to the
-    branch.
-    """
-    test.addCleanup(lambda : b)
-
-
 def build_branch_store(test):
     build_backing_branch(test, 'branch')
     b = branch.Branch.open('branch')
-    keep_branch_alive(test, b)
     return config.BranchStore(b)
 config.test_store_builder_registry.register('branch', build_branch_store)
 
@@ -134,7 +117,6 @@
     (transport_class, server_class) = remote.get_test_permutations()[0]
     build_backing_branch(test, 'branch', transport_class, server_class)
     b = branch.Branch.open(test.get_url('branch'))
-    keep_branch_alive(test, b)
     return config.BranchStore(b)
 config.test_store_builder_registry.register('remote_branch',
                                             build_remote_branch_store)
@@ -149,7 +131,6 @@
 def build_branch_stack(test):
     build_backing_branch(test, 'branch')
     b = branch.Branch.open('branch')
-    keep_branch_alive(test, b)
     return config.BranchStack(b)
 config.test_stack_builder_registry.register('branch', build_branch_stack)
 
@@ -160,7 +141,6 @@
     (transport_class, server_class) = remote.get_test_permutations()[0]
     build_backing_branch(test, 'branch', transport_class, server_class)
     b = branch.Branch.open(test.get_url('branch'))
-    keep_branch_alive(test, b)
     return config.BranchStack(b)
 config.test_stack_builder_registry.register('remote_branch',
                                             build_remote_branch_stack)
@@ -2240,21 +2220,6 @@
         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):
     """Test that Stores properly handle conccurent updates.