Rev 5773: Merge config-stack into config-concrete-stacks resolving conflicts in file:///home/vila/src/bzr/experimental/config/
Vincent Ladeuil
v.ladeuil+lp at free.fr
Tue May 3 12:11:21 UTC 2011
At file:///home/vila/src/bzr/experimental/config/
------------------------------------------------------------
revno: 5773 [merge]
revision-id: v.ladeuil+lp at free.fr-20110503121121-zaps1ohfx9oy4ryl
parent: v.ladeuil+lp at free.fr-20110502080547-3x1l9qq8ggqfp3so
parent: v.ladeuil+lp at free.fr-20110503103630-1jp0uxb5tjd7yjsg
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: config-concrete-stacks
timestamp: Tue 2011-05-03 14:11:21 +0200
message:
Merge config-stack into config-concrete-stacks resolving conflicts
modified:
bzrlib/config.py config.py-20051011043216-070c74f4e9e338e8
bzrlib/tests/test_config.py testconfig.py-20051011041908-742d0c15d8d8c8eb
doc/developers/configuration.txt configuration.txt-20110408142435-korjxxnskvq44sta-1
doc/en/user-guide/configuring_bazaar.txt configuring_bazaar.t-20071128000722-ncxiua259xwbdbg7-1
-------------- next part --------------
=== modified file 'bzrlib/config.py'
--- a/bzrlib/config.py 2011-04-12 11:23:00 +0000
+++ b/bzrlib/config.py 2011-05-03 12:11:21 +0000
@@ -2094,11 +2094,12 @@
self._transport.put_file(self._filename, out_file)
-class ReadOnlySection(object):
+class Section(object):
"""A section defines a dict of options.
This is merely a read-only dict which can add some knowledge about the
- options.
+ options. It is *not* a python dict object though and doesn't try to mimic
+ its API.
"""
def __init__(self, section_id, options):
@@ -2109,12 +2110,16 @@
def get(self, name, default=None):
return self.options.get(name, default)
+ def __repr__(self):
+ # Mostly for debugging use
+ return "<config.%s id=%s>" % (self.__class__.__name__, self.id)
+
_NewlyCreatedOption = object()
"""Was the option created during the MutableSection lifetime"""
-class MutableSection(ReadOnlySection):
+class MutableSection(Section):
"""A section allowing changes and keeping track of the original values."""
def __init__(self, section_id, options):
@@ -2138,14 +2143,19 @@
class Store(object):
"""Abstract interface to persistent storage for configuration options."""
- readonly_section_class = ReadOnlySection
+ readonly_section_class = Section
mutable_section_class = MutableSection
- @property
- def loaded(self):
- raise NotImplementedError(self.loaded)
+ def is_loaded(self):
+ """Returns True if the Store has been loaded.
+
+ This is used to implement lazy loading and ensure the persistent
+ storage is queried only when needed.
+ """
+ raise NotImplementedError(self.is_loaded)
def load(self):
+ """Loads the Store from persistent storage."""
raise NotImplementedError(self.load)
def _load_from_string(self, str_or_unicode):
@@ -2160,6 +2170,7 @@
raise NotImplementedError(self._load_from_string)
def save(self):
+ """Saves the Store to persistent storage."""
raise NotImplementedError(self.save)
def external_url(self):
@@ -2179,8 +2190,22 @@
"""
raise NotImplementedError(self.get_mutable_section)
-
-class ConfigObjStore(Store):
+ def __repr__(self):
+ # Mostly for debugging use
+ return "<config.%s(%s)>" % (self.__class__.__name__,
+ self.external_url())
+
+
+class IniFileStore(Store):
+ """A config Store using ConfigObj for storage.
+
+ :ivar transport: The transport object where the config file is located.
+
+ :ivar file_name: The config file basename in the transport directory.
+
+ :ivar _config_obj: Private member to hold the ConfigObj instance used to
+ serialize/deserialize the config file.
+ """
def __init__(self, transport, file_name):
"""A config Store using ConfigObj for storage.
@@ -2189,18 +2214,17 @@
:param file_name: The config file basename in the transport directory.
"""
- super(ConfigObjStore, self).__init__()
+ super(IniFileStore, self).__init__()
self.transport = transport
self.file_name = file_name
self._config_obj = None
- @property
- def loaded(self):
+ def is_loaded(self):
return self._config_obj != None
def load(self):
"""Load the store from the associated file."""
- if self.loaded:
+ if self.is_loaded():
return
content = self.transport.get_bytes(self.file_name)
self._load_from_string(content)
@@ -2214,7 +2238,7 @@
This is for tests and should not be used in production unless a
convincing use case can be demonstrated :)
"""
- if self.loaded:
+ if self.is_loaded():
raise AssertionError('Already loaded: %r' % (self._config_obj,))
co_input = StringIO(str_or_unicode.encode('utf-8'))
try:
@@ -2225,7 +2249,7 @@
raise errors.ParseConfigError(e.errors, self.external_url())
def save(self):
- if not self.loaded:
+ if not self.is_loaded():
# Nothing to save
return
out = StringIO()
@@ -2238,7 +2262,7 @@
# The following will do in the interim but maybe we don't want to
# expose a path here but rather a config ID and its associated
# object </hand wawe>.
- return os.path.join(self.transport.external_url(), self.file_name)
+ return urlutils.join(self.transport.external_url(), self.file_name)
def get_sections(self):
"""Get the configobj section in the file order.
@@ -2273,7 +2297,7 @@
# they may face the same issue.
-class LockableConfigObjStore(ConfigObjStore):
+class LockableIniFileStore(IniFileStore):
"""A ConfigObjStore using locks on save to ensure store integrity."""
def __init__(self, transport, file_name, lock_dir_name=None):
@@ -2286,7 +2310,7 @@
if lock_dir_name is None:
lock_dir_name = 'lock'
self.lock_dir_name = lock_dir_name
- super(LockableConfigObjStore, self).__init__(transport, file_name)
+ super(LockableIniFileStore, self).__init__(transport, file_name)
self._lock = lockdir.LockDir(self.transport, self.lock_dir_name)
def lock_write(self, token=None):
@@ -2308,13 +2332,18 @@
@needs_write_lock
def save(self):
- super(LockableConfigObjStore, self).save()
+ super(LockableIniFileStore, self).save()
# FIXME: global, bazaar, shouldn't that be 'user' instead or even
# 'user_defaults' as opposed to 'user_overrides', 'system_defaults'
# (/etc/bzr/bazaar.conf) and 'system_overrides' ? -- vila 2011-04-05
-class GlobalStore(LockableConfigObjStore):
+
+# FIXME: Moreover, we shouldn't need classes for these stores either, factory
+# functions or a registry will make it easier and clearer for tests, focusing
+# on the relevant parts of the API that needs testing -- vila 20110503 (based
+# on a poolie's remark)
+class GlobalStore(LockableIniFileStore):
def __init__(self, possible_transports=None):
t = transport.get_transport(config_dir(),
@@ -2322,7 +2351,7 @@
super(GlobalStore, self).__init__(t, 'bazaar.conf')
-class LocationStore(LockableConfigObjStore):
+class LocationStore(LockableIniFileStore):
def __init__(self, possible_transports=None):
t = transport.get_transport(config_dir(),
@@ -2330,7 +2359,7 @@
super(LocationStore, self).__init__(t, 'locations.conf')
-class BranchStore(ConfigObjStore):
+class BranchStore(IniFileStore):
def __init__(self, branch):
super(BranchStore, self).__init__(branch.control_transport,
@@ -2359,7 +2388,7 @@
raise NotImplementedError(self.match)
-class LocationSection(ReadOnlySection):
+class LocationSection(Section):
def __init__(self, section, length, extra_path):
super(LocationSection, self).__init__(section.id, section.options)
@@ -2415,7 +2444,7 @@
yield section
-class ConfigStack(object):
+class Stack(object):
"""A stack of configurations where an option can be defined"""
def __init__(self, sections=None, get_mutable_section=None):
@@ -2477,6 +2506,10 @@
section = self.get_mutable_section()
section.remove(name)
+ def __repr__(self):
+ # Mostly for debugging use
+ return "<config.%s(%s)>" % (self.__class__.__name__, id(self))
+
class GlobalStack(ConfigStack):
=== modified file 'bzrlib/tests/test_config.py'
--- a/bzrlib/tests/test_config.py 2011-05-02 08:05:47 +0000
+++ b/bzrlib/tests/test_config.py 2011-05-03 12:11:21 +0000
@@ -63,15 +63,21 @@
load_tests = scenarios.load_tests_apply_scenarios
-# We need adpaters that can build a config store in a test context. Test
+# We need adapters that can build a config store in a test context. Test
# classes, based on TestCaseWithTransport, can use the registry to parametrize
# themselves. The builder will receive a test instance and should return a
# ready-to-use store. Plugins that defines new stores can also register
# themselves here to be tested against the tests defined below.
+
+# FIXME: plugins should *not* need to import test_config to register their
+# helpers (or selftest -s xxx will be broken), the following registry should be
+# moved to bzrlib.config instead so that selftest -s bt.test_config also runs
+# the plugin specific tests (selftest -s bp.xxx won't, that would be against
+# the spirit of '-s') -- vila 20110503
test_store_builder_registry = registry.Registry()
test_store_builder_registry.register(
- 'configobj', lambda test: config.ConfigObjStore(test.get_transport(),
- 'configobj.conf'))
+ 'configobj', lambda test: config.IniFileStore(test.get_transport(),
+ 'configobj.conf'))
test_store_builder_registry.register(
'bazaar', lambda test: config.GlobalStore())
test_store_builder_registry.register(
@@ -1832,29 +1838,29 @@
self.assertIs(None, bzrdir_config.get_default_stack_on())
-class TestConfigReadOnlySection(tests.TestCase):
+class TestSection(tests.TestCase):
# FIXME: Parametrize so that all sections produced by Stores run these
# tests -- vila 2011-04-01
def test_get_a_value(self):
a_dict = dict(foo='bar')
- section = config.ReadOnlySection('myID', a_dict)
+ section = config.Section('myID', a_dict)
self.assertEquals('bar', section.get('foo'))
- def test_get_unkown_option(self):
+ def test_get_unknown_option(self):
a_dict = dict()
- section = config.ReadOnlySection(None, a_dict)
+ section = config.Section(None, a_dict)
self.assertEquals('out of thin air',
section.get('foo', 'out of thin air'))
def test_options_is_shared(self):
a_dict = dict()
- section = config.ReadOnlySection(None, a_dict)
+ section = config.Section(None, a_dict)
self.assertIs(a_dict, section.options)
-class TestConfigMutableSection(tests.TestCase):
+class TestMutableSection(tests.TestCase):
# FIXME: Parametrize so that all sections (including os.environ and the
# ones produced by Stores) run these tests -- vila 2011-04-01
@@ -1926,9 +1932,9 @@
def test_building_delays_load(self):
store = self.get_store(self)
- self.assertEquals(False, store.loaded)
+ self.assertEquals(False, store.is_loaded())
store._load_from_string('')
- self.assertEquals(True, store.loaded)
+ self.assertEquals(True, store.is_loaded())
def test_get_no_sections_for_empty(self):
store = self.get_store(self)
@@ -2036,28 +2042,28 @@
self.assertSectionContent(('baz', {'foo': 'bar'}), sections[0])
-class TestConfigObjStore(TestStore):
+class TestIniFileStore(TestStore):
def test_loading_unknown_file_fails(self):
- store = config.ConfigObjStore(self.get_transport(), 'I-do-not-exist')
+ store = config.IniFileStore(self.get_transport(), 'I-do-not-exist')
self.assertRaises(errors.NoSuchFile, store.load)
def test_invalid_content(self):
- store = config.ConfigObjStore(self.get_transport(), 'foo.conf', )
- self.assertEquals(False, store.loaded)
+ store = config.IniFileStore(self.get_transport(), 'foo.conf', )
+ self.assertEquals(False, store.is_loaded())
exc = self.assertRaises(
errors.ParseConfigError, store._load_from_string,
'this is invalid !')
self.assertEndsWith(exc.filename, 'foo.conf')
# And the load failed
- self.assertEquals(False, store.loaded)
+ self.assertEquals(False, store.is_loaded())
def test_get_embedded_sections(self):
# A more complicated example (which also shows that section names and
# option names share the same name space...)
# FIXME: This should be fixed by forbidding dicts as values ?
# -- vila 2011-04-05
- store = config.ConfigObjStore(self.get_transport(), 'foo.conf', )
+ store = config.IniFileStore(self.get_transport(), 'foo.conf', )
store._load_from_string('''
foo=bar
l=1,2
@@ -2086,17 +2092,17 @@
sections[3])
-class TestLockableConfigObjStore(TestStore):
+class TestLockableIniFileStore(TestStore):
def test_create_store_in_created_dir(self):
t = self.get_transport('dir/subdir')
- store = config.LockableConfigObjStore(t, 'foo.conf')
+ store = config.LockableIniFileStore(t, 'foo.conf')
store.get_mutable_section(None).set('foo', 'bar')
store.save()
# FIXME: We should adapt the tests in TestLockableConfig about concurrent
# writes. Since this requires a clearer rewrite, I'll just rely on using
- # the same code in LockableConfigObjStore (copied from LockableConfig, but
+ # the same code in LockableIniFileStore (copied from LockableConfig, but
# trivial enough, the main difference is that we add @needs_write_lock on
# save() instead of set_user_option() and remove_user_option()). The intent
# is to ensure that we always get a valid content for the store even when
@@ -2110,7 +2116,7 @@
scenarios = [('location', {'matcher': config.LocationMatcher})]
def get_store(self, file_name):
- return config.ConfigObjStore(self.get_readonly_transport(), file_name)
+ return config.IniFileStore(self.get_readonly_transport(), file_name)
def test_no_matches_for_empty_stores(self):
store = self.get_store('foo.conf')
@@ -2121,13 +2127,13 @@
def test_build_doesnt_load_store(self):
store = self.get_store('foo.conf')
matcher = self.matcher(store, '/bar')
- self.assertFalse(store.loaded)
+ self.assertFalse(store.is_loaded())
class TestLocationSection(tests.TestCase):
def get_section(self, options, extra_path):
- section = config.ReadOnlySection('foo', options)
+ section = config.Section('foo', options)
# We don't care about the length so we use '0'
return config.LocationSection(section, 0, extra_path)
@@ -2150,7 +2156,7 @@
class TestLocationMatcher(TestStore):
def get_store(self, file_name):
- return config.ConfigObjStore(self.get_readonly_transport(), file_name)
+ return config.IniFileStore(self.get_readonly_transport(), file_name)
def test_more_specific_sections_first(self):
store = self.get_store('foo.conf')
@@ -2172,79 +2178,80 @@
[section.extra_path for section in sections])
-class TestConfigStackGet(tests.TestCase):
+class TestStackGet(tests.TestCase):
- # FIXME: This should be parametrized for all known ConfigStack or dedicated
+ # FIXME: This should be parametrized for all known Stack or dedicated
# paramerized tests created to avoid bloating -- vila 2011-03-31
def test_single_config_get(self):
conf = dict(foo='bar')
- conf_stack = config.ConfigStack([conf])
+ conf_stack = config.Stack([conf])
self.assertEquals('bar', conf_stack.get('foo'))
def test_get_first_definition(self):
conf1 = dict(foo='bar')
conf2 = dict(foo='baz')
- conf_stack = config.ConfigStack([conf1, conf2])
+ conf_stack = config.Stack([conf1, conf2])
self.assertEquals('bar', conf_stack.get('foo'))
def test_get_embedded_definition(self):
conf1 = dict(yy='12')
- conf2 = config.ConfigStack([dict(xx='42'), dict(foo='baz')])
- conf_stack = config.ConfigStack([conf1, conf2])
+ conf2 = config.Stack([dict(xx='42'), dict(foo='baz')])
+ conf_stack = config.Stack([conf1, conf2])
self.assertEquals('baz', conf_stack.get('foo'))
def test_get_for_empty_stack(self):
- conf_stack = config.ConfigStack()
+ conf_stack = config.Stack()
self.assertEquals(None, conf_stack.get('foo'))
def test_get_for_empty_section_callable(self):
- conf_stack = config.ConfigStack([lambda : []])
+ conf_stack = config.Stack([lambda : []])
self.assertEquals(None, conf_stack.get('foo'))
-
-class TestConfigStackSet(tests.TestCaseWithTransport):
-
- # FIXME: This should be parametrized for all known ConfigStack or dedicated
+ def test_get_for_broken_callable(self):
+ # Trying to use and invalid callable raises an exception on first use
+ conf_stack = config.Stack([lambda : object()])
+ self.assertRaises(TypeError, conf_stack.get, 'foo')
+
+
+class TestStackSet(tests.TestCaseWithTransport):
+
+ # FIXME: This should be parametrized for all known Stack or dedicated
# paramerized tests created to avoid bloating -- vila 2011-04-05
def test_simple_set(self):
- store = config.ConfigObjStore(self.get_transport(), 'test.conf')
+ store = config.IniFileStore(self.get_transport(), 'test.conf')
store._load_from_string('foo=bar')
- conf = config.ConfigStack(
- [store.get_sections], store.get_mutable_section)
+ conf = config.Stack([store.get_sections], store.get_mutable_section)
self.assertEquals('bar', conf.get('foo'))
conf.set('foo', 'baz')
# Did we get it back ?
self.assertEquals('baz', conf.get('foo'))
def test_set_creates_a_new_section(self):
- store = config.ConfigObjStore(self.get_transport(), 'test.conf')
- conf = config.ConfigStack(
- [store.get_sections], store.get_mutable_section)
+ store = config.IniFileStore(self.get_transport(), 'test.conf')
+ conf = config.Stack([store.get_sections], store.get_mutable_section)
conf.set('foo', 'baz')
self.assertEquals, 'baz', conf.get('foo')
-class TestConfigStackRemove(tests.TestCaseWithTransport):
+class TestStackRemove(tests.TestCaseWithTransport):
- # FIXME: This should be parametrized for all known ConfigStack or dedicated
+ # FIXME: This should be parametrized for all known Stack or dedicated
# paramerized tests created to avoid bloating -- vila 2011-04-06
def test_remove_existing(self):
- store = config.ConfigObjStore(self.get_transport(), 'test.conf')
+ store = config.IniFileStore(self.get_transport(), 'test.conf')
store._load_from_string('foo=bar')
- conf = config.ConfigStack(
- [store.get_sections], store.get_mutable_section)
+ conf = config.Stack([store.get_sections], store.get_mutable_section)
self.assertEquals('bar', conf.get('foo'))
conf.remove('foo')
# Did we get it back ?
self.assertEquals(None, conf.get('foo'))
def test_remove_unknown(self):
- store = config.ConfigObjStore(self.get_transport(), 'test.conf')
- conf = config.ConfigStack(
- [store.get_sections], store.get_mutable_section)
+ store = config.IniFileStore(self.get_transport(), 'test.conf')
+ conf = config.Stack([store.get_sections], store.get_mutable_section)
self.assertRaises(KeyError, conf.remove, 'I_do_not_exist')
=== modified file 'doc/developers/configuration.txt'
--- a/doc/developers/configuration.txt 2011-04-08 16:30:42 +0000
+++ b/doc/developers/configuration.txt 2011-05-03 12:11:21 +0000
@@ -18,21 +18,21 @@
- you can get, set and remove an option,
- the value is a unicode string.
-MutableSection are needed to set or remove an option, ReadOnlySection should
+MutableSection is needed to set or remove an option, ReadOnlySection should
be used otherwise.
Stores
------
-Options can persistent in which case they are saved into Stores.
+Options can be persistent in which case they are saved into Stores.
``config.Store`` defines the abstract interface that all stores should
implement.
-This object doesn't provide a direct access to the options, it only provides
-access to Sections. This is deliberate to ensure that sections can be properly
-shared by reusing the same underlying objects. Accessing options should be
-done via the ``Section`` objects.
+This object doesn't provide direct access to the options, it only provides
+access to Sections. This is deliberate to ensure that sections can be
+properly shared by reusing the same underlying objects. Accessing options
+should be done via the ``Section`` objects.
A ``Store`` can contain one or more sections, each section is uniquely
identified by a unicode string.
@@ -88,11 +88,13 @@
option but these sections shouldn't be modified lightly as modifying an option
used for different contexts will indeed be seen by all these contexts.
-Default values in configuration files are defined by users, developers
+Default values in configuration files are defined by users. Developers
shouldn't have to modify them, as such, no mechanism nor heuristics are used
-to find which section (or sections) should be modified, a ``Stack`` defines a
-mutable section when there is no ambiguity, if there is one, then the *user*
-should be able to decide and this case a new ``Stack`` can be created cheaply.
+to find which section (or sections) should be modified.
+
+A ``Stack`` defines a mutable section when there is no ambiguity. If there
+is one, then the *user* should be able to decide and in this case a new
+``Stack`` can be created cheaply.
Different stacks can be created for different purposes, the existing
``Global.Stack``, ``LocationStack`` and ``BranchStack`` can be used as basis
=== modified file 'doc/en/user-guide/configuring_bazaar.txt'
--- a/doc/en/user-guide/configuring_bazaar.txt 2011-04-08 14:59:29 +0000
+++ b/doc/en/user-guide/configuring_bazaar.txt 2011-05-02 08:15:59 +0000
@@ -41,19 +41,19 @@
-------------------------
As shown in the example above, there are various ways to
-configure Bazaar, they all share some common properties though,
-an option has:
+configure Bazaar, they all share some common properties though.
+An option has:
- a name which is generally a valid python identifier,
-- a value which is a string. In some cases, Bazzar will be able
+- a value which is a string. In some cases, Bazaar will be able
to recognize special values like 'True', 'False' to infer a
boolean type, but basically, as a user, you will always specify
a value as a string.
-Options are grouped in various contexts so their name uniquely
-identify them in this context. When needed, options can be made
-persistent by recording them in a configuration file.
+Options are grouped in various contexts so the option name
+uniquely identifies it in this context. When needed, options can
+be made persistent by recording them in a configuration file.
Configuration files
More information about the bazaar-commits
mailing list