Merge lp:~vila/bzr/832036-stack-registry into lp:bzr

[Vincent Ladeuil]

This introduces a stack registry (config.stacks) needed to:
- get a single point where stacks are obtained,
- allow plugins to override/enrich the stacks or add their own ones.

The registry really provides stack *factories*, each factory
defining its own context which callers should provide via the
parameters.

I've replaced the *Stack classes with their moral equivalent as
factories (suggested by Martin in a previous review).

I first implemented the registry so that the callers use:

  - c = config.GlobalStack()
  + c = config.stacks.get('bazaar')()

  - return config.BranchStack(b)
  + return config.stacks.get('branch')(b)

But I then switched to:

  - c = config.stacks.get('bazaar')()
  + c = config.stacks.get('bazaar')

  - return config.stacks.get('branch')(b)
  + return config.stacks.get('branch', b)

which avoid the additional () and should be easier to use and let
the caller focus on getting a stack, not on implementation
details (i.e. *how* the stack is built is of little interest to
the caller as long as he get the right stack).

There is still one piece missing here: a way to centralize access
to the stores so we can guarantee unique reads/writes. This
registry will make it easier to pass some info to the factories
to get these stores.

I'm not sold on the factory signatures, Martin mentioned using a
dict (instead of args in the current implementation), that may be
better. In fact, turning these args into a dict will allow
injecting the store access means to all factories (i.e. 'stores'
or 'possible_stores' object can be passed from the registry in
this dict and will provide a single access point to get Store
objects shared across all stacks but this is probably better
addressed as a separate mp)...

I've fixed a small bug in the OptionRegistry where the option
help was acquired at registration, only to be ignored in
get_help(). The same attribute is used in both cases so there is
no point in acquiring it at registration.

Known issues not addressed here but IMHO not strictly required to
consider bug #832036 fixed:

* composing specific stacks should be easier and the mandatory
  parts should be reusable (option values from the command-line
  should be respected by all stacks (or not ? do we really want
  to allow this feature for 'control-only' which defines only
  'default_stack_on' and ignore 'locations.conf' for example ?)).

* the stack registry is not yet plugged into the library state (I
  need some teddy bear to discuss how this should work though).

[Jelmer Vernooij (jelmer)]

Can you explain a bit what the stacks make possible, and what use cases they cover? I don't really understand why this all needs to be in a registry rather than e.g. just having the relevant context objects provide their own stack (Branch.get_config_stack()?), like we do for config at the moment.

review: Needs Information

[Vincent Ladeuil (vila)]

> Can you explain a bit what the stacks make possible, and what use cases they
> cover?

Pfew, quite a lot already, see
http://doc.bazaar.canonical.com/devnotes/configuration.html for
the full story ;)

But talking about already available features: they are associated
with an optional MutableStore and a given section where the
modification can occur. They are also associated with a list of
sections across multiple stores where option values can be
defined. Each stack can define its own rules about selection and
ordering for these sections.

As such, they already support the old GlobalConfig and
BranchConfig (except for the point you raised about bzr-svn
injecting its own branch config to allow subversion.conf sections
to be supported).

They can also support more config files/contexts depending on
what we decide:

- config stacks for repositories and working trees,
- system-wide config file,
- defaults and overrides config files (the later being
  locations.conf today, the former existing for UUIDs svn repos
  via subversion.conf only today),
- etc

I didn't push for more features nor make the existing ones
covering more than what was needed for the migration, but they
already offer a greater flexibility than the existing ones.

> I don't really understand why this all needs to be in a registry rather
> than e.g. just having the relevant context objects provide their own stack
> (Branch.get_config_stack()?), like we do for config at the moment.

Several reasons:

- the actual design is a mix of transport/local files APIs, that
  makes it hard to have a consistent behavior for all config
  files (some existing classes don't even provide a way to remove
  an option for example).

- creating config objects on the fly, makes it hard to share the
  some config files as far as IOs is concerned (which means you
  generally need to read bazaar.conf, locations.conf and
  branch.conf each time you want to query the config for a given
  option),

- using classes instead of a registry didn't scale well in the
  past in other areas.

But in the end, using a registry mean you can implement
Branch.get_config_stack() by getting the stack from the registry ;)

[Vincent Ladeuil (vila)]

Let's try another way to answer.

> Can you explain a bit what the stacks make possible, and what
> use cases they cover?

Stacks allow a more uniform way to declare where option values
could be find for a given context:

* an ordered list of config files should be queried,

* for each config file a set of sections (depending on the
  context) should be selected,

* a single (optional) config file a single section in this file
  should be used to *modify* option values. If this file/section is
  not provided, the stack is read-only.

* additionally some global behaviors can be taken into account

** option values can be overridden from the command-line

** a given config file should be read and written only once for a
   given bzr operation.

> I don't really understand why this all needs to be in a registry rather
> than e.g. just having the relevant context objects provide their own stack
> (Branch.get_config_stack()?), like we do for config at the moment.

The actual implementation creates a new config object for every
option query.

The global behaviors mentioned above require that all stores
(config files) are handled in a central place to be able to share
them between the various stacks.

Using a stack registry gives a single place to control these
global behaviors without forcing all stack implementors to care
about it.

@jelmer: Did that answer your questions ?

[Vincent Ladeuil (vila)]

ping ?

[Martin Pool (mbp)]

I'm fine I think Jelmer has a reasonable point that just asking eg the Branch for its configuration seems like a pretty nice api for normal use. It also gives an easy place to hold a reference to the configuration object so that it can be reused.

So, why wouldn't that work?
 * it's quite conceivable we need to ask about the configuration that would apply to a branch, before the branch exists? but, really, you can't ask about data from the branch.conf, until there's at least a nascent branch.
 * we need a way to ask about say locations that don't have any clear owner. perhaps they could go on the library state.

As vila says, we can always provide this on top of some lower-level code that actually constructs the stacks.

I think stacks are a good way to say that the configuration relevant to, say, a branch is in order the location, the branch.conf, the global config.

I think the main thing I would question about this is why we actually have a registry rather than just the factory functions. Registries are a way of handling lookup by name and dynamic lazy loading, neither of which seem especially relevant here as far as I can tell. I would just tell people to call eg branch_stack(), or more to the point have Branch.get_config_stack() call that.

review: Needs Information

[Martin Pool (mbp)]

I'm -1 on this until I understand what it actually improves.

If there was a bug saying "plugins want to do X and can only do it by monkeypatching" that would be a good reason to add something like a registry (though it wouldn't necessarily be urgent.) A bug like 832036 isn't very helpful because it specifies a purported solution not a problem.

[Vincent Ladeuil (vila)]

> Registries are a way of handling lookup by name and dynamic lazy loading, neither of which seem especially relevant here as far as I can tell.

Of course they are relevant, one important target is to never load a config file before it's strictly required which is what lazy loading is about no ?

I'm not saying that's the *onlY* possible solution but it builds on known concepts and code so I see little point in reinventing a wheel here.

Anyway, the discussion on IRC went ahead so enough on that.

Unmerged revisions

6129. By Vincent Ladeuil on 2011-09-07

Implement a config stack registry and use it.

6128. By Vincent Ladeuil on 2011-09-06

Make config.stacks.get() build the stack and accept context parameters but _get_factory for tests and unforeseen uses.

6127. By Vincent Ladeuil on 2011-09-06

Introduce a stack factory registry and use it instead of the *Stack classses (now deleted).

6126. By Vincent Ladeuil on 2011-09-05

Implements a stack registry with basic tests.

6125. By Canonical.com Patch Queue Manager <email address hidden> on 2011-09-02

(jelmer) Mention the number of tags that was pushed/pull,
 if any. (Jelmer Vernooij)

6124. By Canonical.com Patch Queue Manager <email address hidden> on 2011-09-02

(jelmer) Add a ``bugtracker`` configuration option. (Jelmer Vernooij)

=== modified file 'bzrlib/bzrdir.py'
--- bzrlib/bzrdir.py	2011-08-17 09:27:29 +0000
+++ bzrlib/bzrdir.py	2011-09-07 09:07:25 +0000
@@ -2288,7 +2288,7 @@
     help='Same as 2a.')
 
 # The current format that is made on 'bzr init'.
-format_name = config.GlobalStack().get('default_format')
+format_name = config.stacks.get('bazaar').get('default_format')
 controldir.format_registry.set_default(format_name)
 
 # XXX 2010-08-20 JRV: There is still a lot of code relying on

=== modified file 'bzrlib/config.py'
--- bzrlib/config.py	2011-09-06 11:53:48 +0000
+++ bzrlib/config.py	2011-09-07 09:07:25 +0000
@@ -414,7 +414,7 @@
             # add) the final ','
             l = [l]
         return l
-        
+
     def get_user_option_as_int_from_SI(self,  option_name,  default=None):
         """Get a generic option from a human readable size in SI units, e.g 10MB
         
@@ -452,7 +452,6 @@
             except TypeError:
                 val = default
         return val
-        
 
     def gpg_signing_command(self):
         """What program should be used to sign signatures?"""
@@ -2468,8 +2467,7 @@
 
         :param option: The option to register. Its name is used as the key.
         """
-        super(OptionRegistry, self).register(option.name, option,
-                                             help=option.help)
+        super(OptionRegistry, self).register(option.name, option)
 
     def register_lazy(self, key, module_name, member_name):
         """Register a new option to be loaded on request.
@@ -3214,63 +3212,111 @@
         self.store.save()
 
 
-class GlobalStack(_CompatibleStack):
-    """Global options only stack."""
-
-    def __init__(self):
-        # Get a GlobalStore
-        gstore = GlobalStore()
-        super(GlobalStack, self).__init__([gstore.get_sections], gstore)
-
-
-class LocationStack(_CompatibleStack):
-    """Per-location options falling back to global options stack."""
-
-    def __init__(self, location):
-        """Make a new stack for a location and global configuration.
-        
-        :param location: A URL prefix to """
-        lstore = LocationStore()
-        matcher = LocationMatcher(lstore, location)
-        gstore = GlobalStore()
-        super(LocationStack, self).__init__(
-            [matcher.get_sections, gstore.get_sections], lstore)
-
-
-class BranchStack(_CompatibleStack):
-    """Per-location options falling back to branch then global options 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)
-        self.branch = branch
-
-
-class RemoteControlStack(_CompatibleStack):
-    """Remote control-only options stack."""
-
-    def __init__(self, bzrdir):
-        cstore = ControlStore(bzrdir)
-        super(RemoteControlStack, self).__init__(
+def global_stack():
+    """Create a stack for the global options."""
+    # Get a GlobalStore
+    gstore = GlobalStore()
+    stack = _CompatibleStack([gstore.get_sections], gstore)
+    return stack
+
+
+def location_stack(location):
+    """Create a stack for the options applying to a location.
+
+    This includes the matching sections from LocationStore and falls back to
+    the section in the GlobalStore.
+
+    :param location: A URL prefix to match sections from the LocationStore.
+    """
+    lstore = LocationStore()
+    matcher = LocationMatcher(lstore, location)
+    gstore = GlobalStore()
+    stack = _CompatibleStack([matcher.get_sections, gstore.get_sections],
+                             lstore)
+    return stack
+
+
+def branch_stack(branch):
+    """Create a stack for the branch options.
+
+    This includes the section in the BranchStore, falls back the sections in
+    LocationStore (using the branch URL) and finally to the section in the
+    GlobalStore.
+
+    :param branch: The related branch object.
+    """
+    bstore = BranchStore(branch)
+    lstore = LocationStore()
+    matcher = LocationMatcher(lstore, branch.base)
+    gstore = GlobalStore()
+    stack = _CompatibleStack(
+        [matcher.get_sections, bstore.get_sections, gstore.get_sections],
+        bstore)
+    return stack
+
+# Neeed for 'default_stack_on location' which is the *only* option defined in
+# 'control.conf'. This stack is not yet used in bzrdir.py -- vila 2011-09-06
+def control_only_stack(bzrdir):
+    """Create a stack for the control options *only*.
+
+    This includes only the section in the ControlStore.
+
+    :param bzrdir: The related bzrdir object.
+    """
+    cstore = ControlStore(bzrdir)
+    stack = _CompatibleStack(
             [cstore.get_sections],
             cstore)
-        self.bzrdir = bzrdir
-
-
-class RemoteBranchStack(_CompatibleStack):
-    """Remote branch-only options stack."""
-
-    def __init__(self, branch):
-        bstore = BranchStore(branch)
-        super(RemoteBranchStack, self).__init__(
+    return stack
+
+
+# Needed for 'stacked_on_location' *only* which use a hack that could be
+# removed when each config file is read only once. This stack is not yet used
+# in branch.py -- vila 2011-09-06
+def branch_only_stack(branch):
+    """Create a stack for the branch options *only*.
+
+    This includes only the section in the BranchStore.
+
+    :param branch: The related branch object.
+    """
+    bstore = BranchStore(branch)
+    stack = _CompatibleStack(
             [bstore.get_sections],
             bstore)
-        self.branch = branch
+    return stack
+
+
+class StackRegistry(registry.Registry):
+    """Registry of Stack factories.
+
+    This registry is queried for a given key and provides a factory that accept
+    a context and build the appropriate Stack for this context.
+    """
+
+    def get(self, key, *args, **kwargs):
+        return self._get_factory(key)(*args, **kwargs)
+
+    def _get_factory(self, key):
+        return super(StackRegistry, self).get(key)
+
+    def get_help(self, key=None):
+        """Get the help text associated with the given key"""
+        factory = self._get_factory(key)
+        the_help = factory.__doc__
+        if callable(the_help):
+            return the_help(self, key)
+        return the_help
+
+
+stacks = StackRegistry()
+stacks.register('bazaar', global_stack)
+stacks.register('locations', location_stack)
+stacks.register('branch', branch_stack)
+
+# Not used yet, may change in the future, don't use -- vila 2011-09-06
+stacks.register('control-only', control_only_stack)
+stacks.register('branch-only', branch_only_stack)
 
 
 class cmd_config(commands.Command):

=== modified file 'bzrlib/debug.py'
--- bzrlib/debug.py	2011-08-20 07:49:15 +0000
+++ bzrlib/debug.py	2011-09-07 09:07:25 +0000
@@ -32,7 +32,7 @@
 
     from bzrlib import config
 
-    c = config.GlobalStack()
+    c = config.stacks.get('bazaar')
     for f in c.get('debug_flags'):
         debug_flags.add(f)
 

=== modified file 'bzrlib/dirstate.py'
--- bzrlib/dirstate.py	2011-07-25 07:11:56 +0000
+++ bzrlib/dirstate.py	2011-09-07 09:07:25 +0000
@@ -450,12 +450,11 @@
         self._known_hash_changes = set()
         # How many hash changed entries can we have without saving
         self._worth_saving_limit = worth_saving_limit
-        self._config_stack = config.LocationStack(urlutils.local_path_to_url(
-            path))
+        self._config_stack = config.stacks.get(
+            'locations', urlutils.local_path_to_url(path))
 
     def __repr__(self):
-        return "%s(%r)" % \
-            (self.__class__.__name__, self._filename)
+        return "%s(%r)" % (self.__class__.__name__, self._filename)
 
     def _mark_modified(self, hash_changed_entries=None, header_modified=False):
         """Mark this dirstate as modified.

=== modified file 'bzrlib/i18n.py'
--- bzrlib/i18n.py	2011-08-31 10:56:58 +0000
+++ bzrlib/i18n.py	2011-09-07 09:07:25 +0000
@@ -136,7 +136,7 @@
 def _get_current_locale():
     if not os.environ.get('LANGUAGE'):
         from bzrlib import config
-        lang = config.GlobalStack().get('language')
+        lang = config.stacks.get('bazaar').get('language')
         if lang:
             os.environ['LANGUAGE'] = lang
             return lang

=== modified file 'bzrlib/library_state.py'
--- bzrlib/library_state.py	2011-03-29 09:50:04 +0000
+++ bzrlib/library_state.py	2011-09-07 09:07:25 +0000
@@ -69,8 +69,7 @@
         return self # This is bound to the 'as' clause in a with statement.
 
     def _start(self):
-        """Do all initialization.
-        """        
+        """Do all initialization."""
         # NB: This function tweaks so much global state it's hard to test it in
         # isolation within the same interpreter.  It's not reached on normal
         # in-process run_bzr calls.  If it's broken, we expect that
@@ -108,4 +107,4 @@
         bzrlib.ui.ui_factory = self._orig_ui
         global global_state
         global_state = self.saved_state
-        return False # propogate exceptions.
+        return False # propagate exceptions.

=== modified file 'bzrlib/lockdir.py'
--- bzrlib/lockdir.py	2011-08-12 12:11:44 +0000
+++ bzrlib/lockdir.py	2011-09-07 09:07:25 +0000
@@ -708,7 +708,7 @@
         # XXX: This really should also use the locationconfig at least, but
         # that seems a bit hard to hook up at the moment. -- mbp 20110329
         # FIXME: The above is still true ;) -- vila 20110811
-        return config.GlobalStack()
+        return config.stacks.get('bazaar')
 
 
 class LockHeldInfo(object):

=== modified file 'bzrlib/msgeditor.py'
--- bzrlib/msgeditor.py	2011-07-23 16:33:38 +0000
+++ bzrlib/msgeditor.py	2011-09-07 09:07:25 +0000
@@ -41,7 +41,7 @@
     except KeyError:
         pass
 
-    e = config.GlobalStack().get('editor')
+    e = config.stacks.get('bazaar').get('editor')
     if e is not None:
         yield e, config.config_filename()
 

=== modified file 'bzrlib/osutils.py'
--- bzrlib/osutils.py	2011-08-24 23:55:03 +0000
+++ bzrlib/osutils.py	2011-09-07 09:07:25 +0000
@@ -986,7 +986,7 @@
 def report_extension_load_failures():
     if not _extension_load_failures:
         return
-    if config.GlobalStack().get('ignore_missing_extensions'):
+    if config.stacks.get('bazaar').get('ignore_missing_extensions'):
         return
     # the warnings framework should by default show this only once
     from bzrlib.trace import warning

=== modified file 'bzrlib/repofmt/pack_repo.py'
--- bzrlib/repofmt/pack_repo.py	2011-07-25 07:51:35 +0000
+++ bzrlib/repofmt/pack_repo.py	2011-09-07 09:07:25 +0000
@@ -831,7 +831,7 @@
                 set(all_combined).difference([combined_idx]))
         # resumed packs
         self._resumed_packs = []
-        self.config_stack = config.LocationStack(self.transport.base)
+        self.config_stack = config.stacks.get('locations', self.transport.base)
 
     def __repr__(self):
         return '%s(%r)' % (self.__class__.__name__, self.repo)

=== modified file 'bzrlib/tests/per_workingtree/test_workingtree.py'
--- bzrlib/tests/per_workingtree/test_workingtree.py	2011-08-31 22:52:43 +0000
+++ bzrlib/tests/per_workingtree/test_workingtree.py	2011-09-07 09:07:25 +0000
@@ -1161,7 +1161,7 @@
 
     def test_set_in_branch(self):
         wt = self.make_wt_with_worth_saving_limit()
-        conf = config.BranchStack(wt.branch)
+        conf = config.stacks.get('branch', wt.branch)
         conf.set('bzr.workingtree.worth_saving_limit', '20')
         self.assertEqual(20, wt._worth_saving_limit())
         ds = wt.current_dirstate()
@@ -1169,7 +1169,7 @@
 
     def test_invalid(self):
         wt = self.make_wt_with_worth_saving_limit()
-        conf = config.BranchStack(wt.branch)
+        conf = config.stacks.get('branch', wt.branch)
         conf.set('bzr.workingtree.worth_saving_limit', 'a')
         # If the config entry is invalid, default to 10
         warnings = []

=== modified file 'bzrlib/tests/test_config.py'
--- bzrlib/tests/test_config.py	2011-09-06 11:53:48 +0000
+++ bzrlib/tests/test_config.py	2011-09-07 09:07:25 +0000
@@ -132,15 +132,15 @@
 
 
 config.test_stack_builder_registry.register(
-    'bazaar', lambda test: config.GlobalStack())
+    'bazaar', lambda test: config.stacks.get('bazaar'))
 config.test_stack_builder_registry.register(
-    'location', lambda test: config.LocationStack('.'))
+    'location', lambda test: config.stacks.get('locations', '.'))
 
 
 def build_branch_stack(test):
     build_backing_branch(test, 'branch')
     b = branch.Branch.open('branch')
-    return config.BranchStack(b)
+    return config.stacks.get('branch', b)
 config.test_stack_builder_registry.register('branch', build_branch_stack)
 
 
@@ -151,7 +151,7 @@
      server_class) = transport_remote.get_test_permutations()[0]
     build_backing_branch(test, 'branch', transport_class, server_class)
     b = branch.Branch.open(test.get_url('branch'))
-    return config.RemoteBranchStack(b)
+    return config.stacks.get('branch', b)
 config.test_stack_builder_registry.register('remote_branch',
                                             build_remote_branch_stack)
 
@@ -164,7 +164,7 @@
     # creating a dedicated helper to create only the bzrdir
     build_backing_branch(test, 'branch', transport_class, server_class)
     b = branch.Branch.open(test.get_url('branch'))
-    return config.RemoteControlStack(b.bzrdir)
+    return config.stacks.get('control-only', b.bzrdir)
 config.test_stack_builder_registry.register('remote_control',
                                             build_remote_control_stack)
 
@@ -2529,7 +2529,6 @@
 
     def test_help_is_set(self):
         option_help = self.registry.get_help(self.option_name)
-        self.assertNotEquals(None, option_help)
         # Come on, think about the user, he really wants to know what the
         # option is about
         self.assertIsNot(None, option_help)
@@ -3181,6 +3180,54 @@
         self.assertEquals(expected_location, matcher.location)
 
 
+class TestStackRegistry(tests.TestCase):
+
+    def setUp(self):
+        super(TestStackRegistry, self).setUp()
+        self.registry = config.StackRegistry()
+
+    def test_register_remove(self):
+        self.registry.register('test', config.Stack)
+        self.assertIs(config.Stack, self.registry._get_factory('test'))
+        self.registry.remove('test')
+        self.assertRaises(KeyError, self.registry._get_factory, 'test')
+
+    lazy_stack = config.Stack
+
+    def test_register_lazy_remove(self):
+        self.registry.register_lazy('test', self.__module__,
+                                    'TestStackRegistry.lazy_stack')
+        self.assertIs(self.lazy_stack, self.registry._get_factory('test'))
+        self.registry.remove('test')
+        self.assertRaises(KeyError, self.registry._get_factory, 'test')
+
+    def test_register_override(self):
+        self.registry.register('test', config.Stack)
+        self.assertIs(config.Stack, self.registry._get_factory('test'))
+        self.registry.register('test', self.lazy_stack, override_existing=True)
+        self.assertIs(self.lazy_stack, self.registry._get_factory('test'))
+
+
+class TestRegisteredStacks(tests.TestCase):
+
+    scenarios = [(key, {'stack_name': key, 'stack_factory': factory})
+                 for key, factory in config.stacks.iteritems()]
+
+    def setUp(self):
+        super(TestRegisteredStacks, self).setUp()
+        self.registry = config.stacks
+
+    def test_factory_callable(self):
+        self.assertTrue(callable(self.stack_factory))
+
+    def test_help_is_set(self):
+        stack_help = self.registry.get_help(self.stack_name)
+        # Come on, think about the user, he really wants to know what the
+        # stack is about
+        self.assertIsNot(None, stack_help)
+        self.assertNotEquals('', stack_help)
+
+
 class TestStackGet(tests.TestCase):
 
     # FIXME: This should be parametrized for all known Stack or dedicated
@@ -3506,7 +3553,7 @@
             string = ''
         # Since we don't save the config we won't strictly require to inherit
         # from TestCaseInTempDir, but an error occurs so quickly...
-        c = config.LocationStack(location)
+        c = config.stacks.get('locations', location)
         c.store._load_from_string(string)
         return c
 

=== modified file 'bzrlib/tests/test_debug.py'
--- bzrlib/tests/test_debug.py	2011-08-12 16:25:56 +0000
+++ bzrlib/tests/test_debug.py	2011-09-07 09:07:25 +0000
@@ -33,7 +33,7 @@
         self.assertDebugFlags(['hpss'], 'debug_flags = hpss\n')
 
     def assertDebugFlags(self, expected_flags, conf_bytes):
-        conf = config.GlobalStack()
+        conf = config.stacks.get('bazaar')
         conf.store._load_from_string(conf_bytes)
         conf.store.save()
         self.overrideAttr(debug, 'debug_flags', set())

=== modified file 'bzrlib/tests/test_selftest.py'
--- bzrlib/tests/test_selftest.py	2011-08-19 22:34:02 +0000
+++ bzrlib/tests/test_selftest.py	2011-09-07 09:07:25 +0000
@@ -3162,7 +3162,7 @@
         result.stopTestRun()
         self.assertEqual(result._tests_leaking_threads_count, 0)
         self.assertEqual(result.leaks, [])
-        
+
     def test_thread_leak(self):
         """Ensure a thread that outlives the running of a test is reported
 

=== modified file 'bzrlib/ui/__init__.py'
--- bzrlib/ui/__init__.py	2011-08-08 15:57:57 +0000
+++ bzrlib/ui/__init__.py	2011-09-07 09:07:25 +0000
@@ -248,7 +248,7 @@
         """
         # XXX: is the caller supposed to close the resulting object?
         if encoding is None:
-            encoding = config.GlobalStack().get('output_encoding')
+            encoding = config.stacks.get('bazaar').get('output_encoding')
         if encoding is None:
             encoding = osutils.get_terminal_encoding(trace=True)
         if encoding_type is None:

=== modified file 'bzrlib/workingtree_4.py'
--- bzrlib/workingtree_4.py	2011-08-12 14:47:42 +0000
+++ bzrlib/workingtree_4.py	2011-09-07 09:07:25 +0000
@@ -251,7 +251,7 @@
         :return: an integer. -1 means never save.
         """
         # FIXME: We want a WorkingTreeStack here -- vila 20110812
-        conf = config.BranchStack(self.branch)
+        conf = config.stacks.get('branch', self.branch)
         return conf.get('bzr.workingtree.worth_saving_limit')
 
     def filter_unversioned_files(self, paths):

=== modified file 'doc/developers/configuration.txt'
--- doc/developers/configuration.txt	2011-08-19 14:10:32 +0000
+++ doc/developers/configuration.txt	2011-09-07 09:07:25 +0000
@@ -122,9 +122,27 @@
 is one, then the *user* should be able to decide and in this case a new
 ``Stack`` can be created cheaply.
 
+StackRegistry
+-------------
+
+All config stacks should be obtained from ``config.stacks`` which provides
+stack factories for various contexts.
+
+Each factory is registered under a name so plugins can override them as they
+see fit.
+
+Each factory defines its own context as a set of parameters that should be
+provided to build a stack.
+
 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.
+``global_stack``, ``location_stack``, ``branch_stack`` and so on can be used
+as basis or examples. 
+
+Such factory methods can use ``Stores``, ``SectionMatcher`` or ``Sections``
+objects to build their stack (they are the only intended consumer) but these
+objects shouldn't be used otherwise.
+
+There are parameterized tests intended to exercise these factories and their
+resulting stacks (see ``config.test_stack_builder_registry`` and the tests
+that use them in ``bzrlib/tests/test_config.py``). Using them should help
+implementing such factories.

=== modified file 'doc/en/release-notes/bzr-2.5.txt'
--- doc/en/release-notes/bzr-2.5.txt	2011-09-06 13:35:07 +0000
+++ doc/en/release-notes/bzr-2.5.txt	2011-09-07 09:07:25 +0000
@@ -92,6 +92,9 @@
 * bzr now ships with translations for command help.  (Jonathan
   Riddell, INADA Naoki, #83941)
 
+* The ``config.stacks`` registry provides a way to get a config stack
+  factory for various contexts. (Vincent Ladeuil, #832036)
+
 Improvements
 ************