Rev 5943: (vila) Support config remote branch config file. (Vincent Ladeuil) in file:///home/pqm/archives/thelove/bzr/%2Btrunk/
Canonical.com Patch Queue Manager
pqm at pqm.ubuntu.com
Tue May 31 19:43:30 UTC 2011
At file:///home/pqm/archives/thelove/bzr/%2Btrunk/
------------------------------------------------------------
revno: 5943 [merge]
revision-id: pqm at pqm.ubuntu.com-20110531194327-pjelx43boom8r26y
parent: pqm at pqm.ubuntu.com-20110531190047-n1zhm73cb1dnp5hn
parent: v.ladeuil+lp at free.fr-20110531172946-jn4n75b9mhymgmte
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Tue 2011-05-31 19:43:27 +0000
message:
(vila) Support config remote branch config file. (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 19:00:47 +0000
+++ b/bzrlib/config.py 2011-05-31 19:43:27 +0000
@@ -2167,6 +2167,15 @@
"""
raise NotImplementedError(self._load_from_string)
+ def unload(self):
+ """Unloads the Store.
+
+ This should make is_loaded() return False. This is used when the caller
+ knows that the persistent storage has changed or may have change since
+ the last load.
+ """
+ raise NotImplementedError(self.unload)
+
def save(self):
"""Saves the Store to persistent storage."""
raise NotImplementedError(self.save)
@@ -2220,6 +2229,9 @@
def is_loaded(self):
return self._config_obj != None
+ def unload(self):
+ self._config_obj = None
+
def load(self):
"""Load the store from the associated file."""
if self.is_loaded():
@@ -2370,14 +2382,25 @@
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
+ # 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
def lock_write(self, token=None):
- return self.branch.lock_write(token)
+ return self._get_branch().lock_write(token)
def unlock(self):
- return self.branch.unlock()
+ return self._get_branch().unlock()
@needs_write_lock
def save(self):
@@ -2578,8 +2601,8 @@
"""
def set(self, name, value):
- # Force a reload (assuming we use a LockableIniFileStore)
- self.store._config_obj = None
+ # Force a reload
+ self.store.unload()
super(_CompatibleStack, self).set(name, value)
# Force a write to persistent storage
self.store.save()
@@ -2602,7 +2625,6 @@
super(LocationStack, self).__init__(
[matcher.get_sections, gstore.get_sections], lstore)
-# FIXME: See BranchStore, same remarks -- vila 20110512
class BranchStack(_CompatibleStack):
def __init__(self, branch):
@@ -2613,6 +2635,7 @@
super(BranchStack, self).__init__(
[matcher.get_sections, bstore.get_sections, gstore.get_sections],
bstore)
+ self.branch = branch
class cmd_config(commands.Command):
@@ -2794,7 +2817,7 @@
# object.
test_store_builder_registry = registry.Registry()
-# Thre registered object should be a callable receiving a test instance
+# The registered object should be a callable receiving a test instance
# parameter (inheriting from tests.TestCaseWithTransport) and returning a Stack
# object.
test_stack_builder_registry = registry.Registry()
=== modified file 'bzrlib/tests/test_config.py'
--- a/bzrlib/tests/test_config.py 2011-05-31 19:00:47 +0000
+++ b/bzrlib/tests/test_config.py 2011-05-31 19:43:27 +0000
@@ -41,6 +41,7 @@
trace,
transport,
)
+from bzrlib.transport import remote
from bzrlib.tests import (
features,
TestSkipped,
@@ -72,33 +73,99 @@
config.test_store_builder_registry.register(
'location', lambda test: config.LocationStore())
+
+def build_backing_branch(test, relpath,
+ transport_class=None, server_class=None):
+ """Test helper to create a backing branch only once.
+
+ Some tests needs multiple stores/stacks to check concurrent update
+ behaviours. As such, they need to build different branch *objects* even if
+ they share the branch on disk.
+
+ :param relpath: The relative path to the branch. (Note that the helper
+ should always specify the same relpath).
+
+ :param transport_class: The Transport class the test needs to use.
+
+ :param server_class: The server associated with the ``transport_class``
+ above.
+
+ Either both or neither of ``transport_class`` and ``server_class`` should
+ be specified.
+ """
+ if transport_class is not None and server_class is not None:
+ test.transport_class = transport_class
+ test.transport_server = server_class
+ elif not (transport_class is None and server_class is None):
+ raise AssertionError('Specify both ``transport_class`` and '
+ '``server_class`` or neither of them')
+ if getattr(test, 'backing_branch', None) is None:
+ # First call, let's build the branch on disk
+ 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):
- if getattr(test, 'branch', None) is None:
- test.branch = test.make_branch('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'))
+ 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)
+def build_remote_branch_store(test):
+ # There is only one permutation (but we won't be able to handle more with
+ # this design anyway)
+ (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)
+
+
config.test_stack_builder_registry.register(
'bazaar', lambda test: config.GlobalStack())
config.test_stack_builder_registry.register(
'location', lambda test: config.LocationStack('.'))
+
def build_branch_stack(test):
- if getattr(test, 'branch', None) is None:
- test.branch = test.make_branch('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'))
+ 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)
+def build_remote_branch_stack(test):
+ # There is only one permutation (but we won't be able to handle more with
+ # this design anyway)
+ (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)
+
+
sample_long_alias="log -r-15..-1 --line"
sample_config_text = u"""
[DEFAULT]
@@ -642,11 +709,11 @@
def test_default_is_True(self):
self.config = self.get_config(True)
self.assertExpandIs(True)
-
+
def test_default_is_False(self):
self.config = self.get_config(False)
self.assertExpandIs(False)
-
+
class TestIniConfigOptionExpansion(tests.TestCase):
"""Test option expansion from the IniConfig level.
@@ -1940,7 +2007,6 @@
def setUp(self):
super(TestReadonlyStore, self).setUp()
- self.branch = self.make_branch('branch')
def test_building_delays_load(self):
store = self.get_store(self)
@@ -1988,7 +2054,9 @@
return self.transport.has(store_basename)
def test_save_empty_creates_no_file(self):
- if self.store_id == 'branch':
+ # 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')
store = self.get_store(self)
@@ -2007,7 +2075,9 @@
self.assertLength(0, sections)
def test_save_with_content_succeeds(self):
- if self.store_id == 'branch':
+ # 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')
store = self.get_store(self)
@@ -2114,7 +2184,28 @@
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.
+
+ New Store implementation may fail some of these tests but until such
+ implementations exist it's hard to properly filter them from the scenarios
+ applied here. If you encounter such a case, contact the bzr devs.
+ """
scenarios = [(key, {'get_stack': builder}) for key, builder
in config.test_stack_builder_registry.iteritems()]
More information about the bazaar-commits
mailing list