Rev 5886: (vila) stacks for bazaar, locations and branch (Vincent Ladeuil) in file:///home/pqm/archives/thelove/bzr/%2Btrunk/
Canonical.com Patch Queue Manager
pqm at pqm.ubuntu.com
Wed May 18 02:47:38 UTC 2011
At file:///home/pqm/archives/thelove/bzr/%2Btrunk/
------------------------------------------------------------
revno: 5886 [merge]
revision-id: pqm at pqm.ubuntu.com-20110518024733-fktxni9qajxu2pwh
parent: pqm at pqm.ubuntu.com-20110518015517-en7p3zst2mdprrqx
parent: v.ladeuil+lp at free.fr-20110506132213-i8icnjk64njeq0fj
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Wed 2011-05-18 02:47:33 +0000
message:
(vila) stacks for bazaar, locations and branch (Vincent Ladeuil)
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
=== modified file 'bzrlib/config.py'
--- a/bzrlib/config.py 2011-05-03 16:26:11 +0000
+++ b/bzrlib/config.py 2011-05-05 10:48:13 +0000
@@ -978,25 +978,21 @@
number of path components in the section name, section is the section
name and extra_path is the difference between location and the section
name.
+
+ ``location`` will always be a local path and never a 'file://' url but the
+ section names themselves can be in either form.
"""
location_parts = location.rstrip('/').split('/')
for section in sections:
- # location is a local path if possible, so we need
- # to convert 'file://' urls to local paths if necessary.
-
- # FIXME: I don't think the above comment is still up to date,
- # LocationConfig is always instantiated with an url -- vila 2011-04-07
+ # location is a local path if possible, so we need to convert 'file://'
+ # urls in section names to local paths if necessary.
# This also avoids having file:///path be a more exact
# match than '/path'.
- # FIXME: Not sure about the above either, but since the path components
- # are compared in sync, adding two empty components (//) is likely to
- # trick the comparison and also trick the check on the number of
- # components, so we *should* take only the relevant part of the url. On
- # the other hand, this means 'file://' urls *can't* be used in sections
- # so more work is probably needed -- vila 2011-04-07
+ # FIXME: This still raises an issue if a user defines both file:///path
+ # *and* /path. Should we raise an error in this case -- vila 20110505
if section.startswith('file://'):
section_path = urlutils.local_path_from_url(section)
@@ -2270,7 +2266,11 @@
:returns: An iterable of (name, dict).
"""
# We need a loaded store
- self.load()
+ try:
+ self.load()
+ except errors.NoSuchFile:
+ # If the file doesn't exist, there is no sections
+ return
cobj = self._config_obj
if cobj.scalars:
yield self.readonly_section_class(None, cobj)
@@ -2409,18 +2409,32 @@
def __init__(self, store, location):
super(LocationMatcher, self).__init__(store)
+ if location.startswith('file://'):
+ location = urlutils.local_path_from_url(location)
self.location = location
- def get_sections(self):
- # Override the default implementation as we want to change the order
-
- # The following is a bit hackish but ensures compatibility with
- # LocationConfig by reusing the same code
- sections = list(self.store.get_sections())
+ def _get_matching_sections(self):
+ """Get all sections matching ``location``."""
+ # We slightly diverge from LocalConfig here by allowing the no-name
+ # section as the most generic one and the lower priority.
+ no_name_section = None
+ sections = []
+ # Filter out the no_name_section so _iter_for_location_by_parts can be
+ # used (it assumes all sections have a name).
+ for section in self.store.get_sections():
+ if section.id is None:
+ no_name_section = section
+ else:
+ sections.append(section)
+ # Unfortunately _iter_for_location_by_parts deals with section names so
+ # we have to resync.
filtered_sections = _iter_for_location_by_parts(
[s.id for s in sections], self.location)
iter_sections = iter(sections)
matching_sections = []
+ if no_name_section is not None:
+ matching_sections.append(
+ LocationSection(no_name_section, 0, self.location))
for section_id, extra_path, length in filtered_sections:
# a section id is unique for a given store so it's safe to iterate
# again
@@ -2428,6 +2442,11 @@
if section_id == section.id:
matching_sections.append(
LocationSection(section, length, extra_path))
+ return matching_sections
+
+ def get_sections(self):
+ # Override the default implementation as we want to change the order
+ matching_sections = self._get_matching_sections()
# We want the longest (aka more specific) locations first
sections = sorted(matching_sections,
key=lambda section: (section.length, section.id),
@@ -2476,9 +2495,10 @@
"""
# FIXME: No caching of options nor sections yet -- vila 20110503
- # Ensuring lazy loading is achieved by delaying section matching until
- # it can be avoided anymore by using callables to describe (possibly
- # empty) section lists.
+ # Ensuring lazy loading is achieved by delaying section matching (which
+ # implies querying the persistent storage) until it can't be avoided
+ # anymore by using callables to describe (possibly empty) section
+ # lists.
for section_or_callable in self.sections_def:
# Each section can expand to multiple ones when a callable is used
if callable(section_or_callable):
@@ -2498,7 +2518,7 @@
This is where we guarantee that the mutable section is lazily loaded:
this means we won't load the corresponding store before setting a value
or deleting an option. In practice the store will often be loaded but
- this allows catching some programming errors.
+ this allows helps catching some programming errors.
"""
section = self.store.get_mutable_section(self.mutable_section_name)
return section
@@ -2518,6 +2538,36 @@
return "<config.%s(%s)>" % (self.__class__.__name__, id(self))
+class GlobalStack(Stack):
+
+ def __init__(self):
+ # Get a GlobalStore
+ gstore = GlobalStore()
+ super(GlobalStack, self).__init__([gstore.get_sections], gstore)
+
+
+class LocationStack(Stack):
+
+ def __init__(self, location):
+ lstore = LocationStore()
+ matcher = LocationMatcher(lstore, location)
+ gstore = GlobalStore()
+ super(LocationStack, self).__init__(
+ [matcher.get_sections, gstore.get_sections], lstore)
+
+
+class BranchStack(Stack):
+
+ def __init__(self, branch):
+ bstore = BranchStore(branch)
+ lstore = LocationStore()
+ matcher = LocationMatcher(lstore, branch.base)
+ gstore = GlobalStore()
+ super(BranchStack, self).__init__(
+ [matcher.get_sections, bstore.get_sections, gstore.get_sections],
+ bstore)
+
+
class cmd_config(commands.Command):
__doc__ = """Display, set or remove a configuration option.
=== modified file 'bzrlib/tests/test_config.py'
--- a/bzrlib/tests/test_config.py 2011-05-13 12:51:05 +0000
+++ b/bzrlib/tests/test_config.py 2011-05-18 02:47:33 +0000
@@ -85,6 +85,14 @@
test_store_builder_registry.register(
'branch', lambda test: config.BranchStore(test.branch))
+# FIXME: Same remark as above for the following registry -- vila 20110503
+test_stack_builder_registry = registry.Registry()
+test_stack_builder_registry.register(
+ 'bazaar', lambda test: config.GlobalStack())
+test_stack_builder_registry.register(
+ 'location', lambda test: config.LocationStack('.'))
+test_stack_builder_registry.register(
+ 'branch', lambda test: config.BranchStack(test.branch))
sample_long_alias="log -r-15..-1 --line"
@@ -2177,6 +2185,23 @@
self.assertEquals(['baz', 'bar/baz'],
[section.extra_path for section in sections])
+ def test_appendpath_in_no_name_section(self):
+ # It's a bit weird to allow appendpath in a no-name section, but
+ # someone may found a use for it
+ store = self.get_store('foo.conf')
+ store._load_from_string('''
+foo=bar
+foo:policy = appendpath
+''')
+ matcher = config.LocationMatcher(store, 'dir/subdir')
+ sections = list(matcher.get_sections())
+ self.assertLength(1, sections)
+ self.assertEquals('bar/dir/subdir', sections[0].get('foo'))
+
+ def test_file_urls_are_normalized(self):
+ store = self.get_store('foo.conf')
+ matcher = config.LocationMatcher(store, 'file:///dir/subdir')
+ self.assertEquals('/dir/subdir', matcher.location)
class TestStackGet(tests.TestCase):
@@ -2215,47 +2240,61 @@
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
+class TestStackWithTransport(tests.TestCaseWithTransport):
+
+ def setUp(self):
+ super(TestStackWithTransport, self).setUp()
+ # FIXME: A more elaborate builder for the stack would avoid building a
+ # branch even for tests that don't need it.
+ self.branch = self.make_branch('branch')
+
+
+class TestStackSet(TestStackWithTransport):
+
+ scenarios = [(key, {'get_stack': builder})
+ for key, builder in test_stack_builder_registry.iteritems()]
def test_simple_set(self):
- store = config.IniFileStore(self.get_transport(), 'test.conf')
- store._load_from_string('foo=bar')
- conf = config.Stack([store.get_sections], store)
+ conf = self.get_stack(self)
+ conf.store._load_from_string('foo=bar')
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.IniFileStore(self.get_transport(), 'test.conf')
- conf = config.Stack([store.get_sections], store)
+ conf = self.get_stack(self)
conf.set('foo', 'baz')
self.assertEquals, 'baz', conf.get('foo')
-class TestStackRemove(tests.TestCaseWithTransport):
+class TestStackRemove(TestStackWithTransport):
- # FIXME: This should be parametrized for all known Stack or dedicated
- # paramerized tests created to avoid bloating -- vila 2011-04-06
+ scenarios = [(key, {'get_stack': builder})
+ for key, builder in test_stack_builder_registry.iteritems()]
def test_remove_existing(self):
- store = config.IniFileStore(self.get_transport(), 'test.conf')
- store._load_from_string('foo=bar')
- conf = config.Stack([store.get_sections], store)
+ conf = self.get_stack(self)
+ conf.store._load_from_string('foo=bar')
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.IniFileStore(self.get_transport(), 'test.conf')
- conf = config.Stack([store.get_sections], store)
+ conf = self.get_stack(self)
self.assertRaises(KeyError, conf.remove, 'I_do_not_exist')
+class TestConcreteStacks(TestStackWithTransport):
+
+ scenarios = [(key, {'get_stack': builder})
+ for key, builder in test_stack_builder_registry.iteritems()]
+
+ def test_build_stack(self):
+ stack = self.get_stack(self)
+
+
class TestConfigGetOptions(tests.TestCaseWithTransport, TestOptionsMixin):
def setUp(self):
=== modified file 'doc/developers/configuration.txt'
--- a/doc/developers/configuration.txt 2011-05-03 10:36:30 +0000
+++ b/doc/developers/configuration.txt 2011-05-04 07:56:15 +0000
@@ -95,3 +95,10 @@
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
+``GlobalStack``, ``LocationStack`` and ``BranchStack`` can be used as basis
+or examples. These classes are the only ones that should be used in code,
+``Stores`` can be used to build them but shouldn't be used otherwise, ditto
+for sections. Again, the associated tests could and should be used against the
+created stacks.
More information about the bazaar-commits
mailing list