Rev 5810: Merge config-lock-branch into config-lock-remote in file:///home/vila/src/bzr/experimental/config/

Vincent Ladeuil v.ladeuil+lp at free.fr
Tue Jun 14 13:57:22 UTC 2011


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

------------------------------------------------------------
revno: 5810 [merge]
revision-id: v.ladeuil+lp at free.fr-20110614135722-csm1rzi6fl347g7l
parent: v.ladeuil+lp at free.fr-20110608095151-17uu395nv9cmox6u
parent: v.ladeuil+lp at free.fr-20110614135721-epfz87h5ucov9psu
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: config-lock-remote
timestamp: Tue 2011-06-14 15:57:22 +0200
message:
  Merge config-lock-branch into config-lock-remote
modified:
  bzrlib/config.py               config.py-20051011043216-070c74f4e9e338e8
  bzrlib/hooks.py                hooks.py-20070325015548-ix4np2q0kd8452au-1
  bzrlib/remote.py               remote.py-20060720103555-yeeg2x51vn0rbtdp-1
  bzrlib/tests/__init__.py       selftest.py-20050531073622-8d0e3c8845c97a64
  bzrlib/tests/test_config.py    testconfig.py-20051011041908-742d0c15d8d8c8eb
  doc/en/release-notes/bzr-2.4.txt bzr2.4.txt-20110114053217-k7ym9jfz243fddjm-1
-------------- next part --------------
=== modified file 'bzrlib/config.py'
--- a/bzrlib/config.py	2011-06-01 12:31:04 +0000
+++ b/bzrlib/config.py	2011-06-14 10:07:16 +0000
@@ -65,9 +65,8 @@
 import os
 import string
 import sys
-import weakref
-
-from bzrlib import commands
+
+
 from bzrlib.decorators import needs_write_lock
 from bzrlib.lazy_import import lazy_import
 lazy_import(globals(), """
@@ -95,6 +94,8 @@
 from bzrlib.util.configobj import configobj
 """)
 from bzrlib import (
+    commands,
+    hooks,
     registry,
     )
 from bzrlib.symbol_versioning import (
@@ -159,7 +160,7 @@
         return self[section][name]
 
 
-# FIXME: Until we can guarantee that each config file is loaded once and and
+# FIXME: Until we can guarantee that each config file is loaded once and
 # only once for a given bzrlib session, we don't want to re-read the file every
 # time we query for an option so we cache the value (bad ! watch out for tests
 # needing to restore the proper value).This shouldn't be part of 2.4.0 final,
@@ -370,6 +371,8 @@
                               % (option_name,))
             else:
                 value = self._expand_options_in_string(value)
+        for hook in OldConfigHooks['get']:
+            hook(self, option_name, value)        
         return value
 
     def get_user_option_as_bool(self, option_name, expand=None):
@@ -554,6 +557,76 @@
         return command_line
 
 
+class _ConfigHooks(hooks.Hooks):
+    """A dict mapping hook names and a list of callables for configs.
+    """
+
+    def __init__(self):
+        """Create the default hooks.
+
+        These are all empty initially, because by default nothing should get
+        notified.
+        """
+        super(_ConfigHooks, self).__init__('bzrlib.config', 'ConfigHooks')
+        self.add_hook('load',
+                      'Invoked when a config store is loaded.'
+                      ' The signature is (store).',
+                      (2, 4))
+        self.add_hook('save',
+                      'Invoked when a config store is saved.'
+                      ' The signature is (store).',
+                      (2, 4))
+        # The hooks for config options
+        self.add_hook('get',
+                      'Invoked when a config option is read.'
+                      ' The signature is (stack, name, value).',
+                      (2, 4))
+        self.add_hook('set',
+                      'Invoked when a config option is set.'
+                      ' The signature is (stack, name, value).',
+                      (2, 4))
+        self.add_hook('remove',
+                      'Invoked when a config option is removed.'
+                      ' The signature is (stack, name).',
+                      (2, 4))
+ConfigHooks = _ConfigHooks()
+
+
+class _OldConfigHooks(hooks.Hooks):
+    """A dict mapping hook names and a list of callables for configs.
+    """
+
+    def __init__(self):
+        """Create the default hooks.
+
+        These are all empty initially, because by default nothing should get
+        notified.
+        """
+        super(_OldConfigHooks, self).__init__('bzrlib.config', 'OldConfigHooks')
+        self.add_hook('load',
+                      'Invoked when a config store is loaded.'
+                      ' The signature is (config).',
+                      (2, 4))
+        self.add_hook('save',
+                      'Invoked when a config store is saved.'
+                      ' The signature is (config).',
+                      (2, 4))
+        # The hooks for config options
+        self.add_hook('get',
+                      'Invoked when a config option is read.'
+                      ' The signature is (config, name, value).',
+                      (2, 4))
+        self.add_hook('set',
+                      'Invoked when a config option is set.'
+                      ' The signature is (config, name, value).',
+                      (2, 4))
+        self.add_hook('remove',
+                      'Invoked when a config option is removed.'
+                      ' The signature is (config, name).',
+                      (2, 4))
+OldConfigHooks = _OldConfigHooks()
+
+
 class IniBasedConfig(Config):
     """A configuration policy that draws from ini files."""
 
@@ -621,6 +694,8 @@
             raise errors.ParseConfigError(e.errors, e.config.filename)
         # Make sure self.reload() will use the right file name
         self._parser.filename = self.file_name
+        for hook in OldConfigHooks['load']:
+            hook(self)
         return self._parser
 
     def reload(self):
@@ -629,6 +704,8 @@
             raise AssertionError('We need a file name to reload the config')
         if self._parser is not None:
             self._parser.reload()
+        for hook in ConfigHooks['load']:
+            hook(self)
 
     def _get_matching_sections(self):
         """Return an ordered list of (section_name, extra_path) pairs.
@@ -806,6 +883,8 @@
         except KeyError:
             raise errors.NoSuchConfigOption(option_name)
         self._write_config_file()
+        for hook in OldConfigHooks['remove']:
+            hook(self, option_name)
 
     def _write_config_file(self):
         if self.file_name is None:
@@ -817,6 +896,8 @@
         atomic_file.commit()
         atomic_file.close()
         osutils.copy_ownership_from_path(self.file_name)
+        for hook in OldConfigHooks['save']:
+            hook(self)
 
 
 class LockableConfig(IniBasedConfig):
@@ -950,7 +1031,8 @@
         self.reload()
         self._get_parser().setdefault(section, {})[option] = value
         self._write_config_file()
-
+        for hook in OldConfigHooks['set']:
+            hook(self, option, value)
 
     def _get_sections(self, name=None):
         """See IniBasedConfig._get_sections()."""
@@ -1152,6 +1234,8 @@
         # the allowed values of store match the config policies
         self._set_option_policy(location, option, store)
         self._write_config_file()
+        for hook in OldConfigHooks['set']:
+            hook(self, option, value)
 
 
 class BranchConfig(Config):
@@ -2054,7 +2138,10 @@
                 section_obj = configobj[section]
             except KeyError:
                 return default
-        return section_obj.get(name, default)
+        value = section_obj.get(name, default)
+        for hook in OldConfigHooks['get']:
+            hook(self, name, value)        
+        return value
 
     def set_option(self, value, name, section=None):
         """Set the value associated with a named option.
@@ -2068,6 +2155,8 @@
             configobj[name] = value
         else:
             configobj.setdefault(section, {})[name] = value
+        for hook in OldConfigHooks['set']:
+            hook(self, name, value)
         self._set_configobj(configobj)
 
     def remove_option(self, option_name, section_name=None):
@@ -2076,11 +2165,16 @@
             del configobj[option_name]
         else:
             del configobj[section_name][option_name]
+        for hook in OldConfigHooks['remove']:
+            hook(self, option_name)
         self._set_configobj(configobj)
 
     def _get_config_file(self):
         try:
-            return StringIO(self._transport.get_bytes(self._filename))
+            f = StringIO(self._transport.get_bytes(self._filename))
+            for hook in OldConfigHooks['load']:
+                hook(self)
+            return f
         except errors.NoSuchFile:
             return StringIO()
 
@@ -2096,6 +2190,8 @@
         configobj.write(out_file)
         out_file.seek(0)
         self._transport.put_file(self._filename, out_file)
+        for hook in OldConfigHooks['save']:
+            hook(self)
 
 
 class Option(object):
@@ -2271,6 +2367,8 @@
             return
         content = self.transport.get_bytes(self.file_name)
         self._load_from_string(content)
+        for hook in ConfigHooks['load']:
+            hook(self)
 
     def _load_from_string(self, str_or_unicode):
         """Create a config store from a string.
@@ -2298,6 +2396,8 @@
         out = StringIO()
         self._config_obj.write(out)
         self.transport.put_bytes(self.file_name, out.getvalue())
+        for hook in ConfigHooks['save']:
+            hook(self)
 
     def external_url(self):
         # FIXME: external_url should really accepts an optional relpath
@@ -2587,6 +2687,8 @@
                 opt = None
             if opt is not None:
                 value = opt.get_default()
+        for hook in ConfigHooks['get']:
+            hook(self, name, value)
         return value
 
     def _get_mutable_section(self):
@@ -2604,11 +2706,15 @@
         """Set a new value for the option."""
         section = self._get_mutable_section()
         section.set(name, value)
+        for hook in ConfigHooks['set']:
+            hook(self, name, value)
 
     def remove(self, name):
         """Remove an existing option."""
         section = self._get_mutable_section()
         section.remove(name)
+        for hook in ConfigHooks['remove']:
+            hook(self, name)
 
     def __repr__(self):
         # Mostly for debugging use

=== modified file 'bzrlib/hooks.py'
--- a/bzrlib/hooks.py	2011-04-07 10:36:24 +0000
+++ b/bzrlib/hooks.py	2011-06-14 10:07:16 +0000
@@ -72,6 +72,7 @@
     ('bzrlib.branch', 'Branch.hooks', 'BranchHooks'),
     ('bzrlib.bzrdir', 'BzrDir.hooks', 'BzrDirHooks'),
     ('bzrlib.commands', 'Command.hooks', 'CommandHooks'),
+    ('bzrlib.config', 'ConfigHooks', '_ConfigHooks'),
     ('bzrlib.info', 'hooks', 'InfoHooks'),
     ('bzrlib.lock', 'Lock.hooks', 'LockHooks'),
     ('bzrlib.merge', 'Merger.hooks', 'MergeHooks'),
@@ -256,8 +257,7 @@
         try:
             uninstall = getattr(hook, "uninstall")
         except AttributeError:
-            raise errors.UnsupportedOperation(self.install_named_hook_lazy,
-                self)
+            raise errors.UnsupportedOperation(self.uninstall_named_hook, self)
         else:
             uninstall(label)
 

=== modified file 'bzrlib/remote.py'
--- a/bzrlib/remote.py	2011-05-20 13:28:35 +0000
+++ b/bzrlib/remote.py	2011-06-14 10:07:16 +0000
@@ -3101,22 +3101,32 @@
         """
         try:
             configobj = self._get_configobj()
+            section_obj = None
             if section is None:
                 section_obj = configobj
             else:
                 try:
                     section_obj = configobj[section]
                 except KeyError:
-                    return default
-            return section_obj.get(name, default)
+                    pass
+            if section_obj is None:
+                value = default
+            else:
+                value = section_obj.get(name, default)
         except errors.UnknownSmartMethod:
-            return self._vfs_get_option(name, section, default)
+            value = self._vfs_get_option(name, section, default)
+        for hook in config.OldConfigHooks['get']:
+            hook(self, name, value)
+        return value
 
     def _response_to_configobj(self, response):
         if len(response[0]) and response[0][0] != 'ok':
             raise errors.UnexpectedSmartServerResponse(response)
         lines = response[1].read_body_bytes().splitlines()
-        return config.ConfigObj(lines, encoding='utf-8')
+        conf = config.ConfigObj(lines, encoding='utf-8')
+        for hook in config.OldConfigHooks['load']:
+            hook(self)
+        return conf
 
 
 class RemoteBranchConfig(RemoteConfig):

=== modified file 'bzrlib/tests/__init__.py'
--- a/bzrlib/tests/__init__.py	2011-06-01 05:21:43 +0000
+++ b/bzrlib/tests/__init__.py	2011-06-14 13:15:33 +0000
@@ -989,6 +989,9 @@
         # is addressed -- vila 20110219
         self.overrideAttr(config, '_expand_default_value', None)
         self._log_files = set()
+        self._init_counters()
+        if 'config_stats' in selftest_debug_flags:
+            self._install_config_stats_hooks()
 
     def debug(self):
         # debug a frame up.
@@ -1011,6 +1014,48 @@
         if name in details:
             del details[name]
 
+    def _init_counters(self):
+        """Setup a dict to collect various counters.
+
+        Each key in the dict holds a value for a different counter. When the
+        test ends, use addDetail subunit API to record the counter values.
+        """
+        self._counters = {}
+        def add_counter_details():
+            for k, v in self._counters.iteritems():
+                self.addDetail('%s' % (k,), content.text_content('%s' % (v,)))
+            self._counters = None
+        self.addCleanup(add_counter_details)
+
+    def _install_config_stats_hooks(self):
+        """Install config hooks to count hook calls.
+
+        """
+        def install_counter_hook(hooks, prefix, hook_name):
+            """Create a counter and install its associated hook"""
+            counter_name = '%s.%s' % (prefix, hook_name)
+            self._counters[counter_name] = 0
+            def increment_counter(name): self._counters[name] += 1
+            def create_hook_point(attr_name):
+                # Force the lambda creation at the right time so we refer to
+                # the right counter name
+                return lambda *args: increment_counter(attr_name)
+            label = 'count %s.%s calls' % (prefix, hook_name)
+            hooks.install_named_hook(hook_name, create_hook_point(counter_name),
+                                     label)
+            self.addCleanup(hooks.uninstall_named_hook, hook_name, label)
+
+        for hook_name in ('get', 'set', 'remove', 'load', 'save'):
+            install_counter_hook(config.ConfigHooks, 'config', hook_name)
+
+        # The OldConfigHooks are private and need special handling to protect
+        # against recursive tests (tests that run other tests), so we just do
+        # manually what registering them into _builtin_known_hooks will provide
+        # us.
+        self.overrideAttr(config, 'OldConfigHooks', config._OldConfigHooks())
+        for hook_name in ('get', 'set', 'remove', 'load', 'save'):
+            install_counter_hook(config.OldConfigHooks, 'old_config', hook_name)
+
     def _clear_debug_flags(self):
         """Prevent externally set debug flags affecting tests.
 
@@ -3510,6 +3555,8 @@
 #                           with proper exclusion rules.
 #   -Ethreads               Will display thread ident at creation/join time to
 #                           help track thread leaks
+#   -Econfig_stats          Will collect statistics using the subunit addDetail
+#                           API
 selftest_debug_flags = set()
 
 

=== modified file 'bzrlib/tests/test_config.py'
--- a/bzrlib/tests/test_config.py	2011-06-01 12:31:04 +0000
+++ b/bzrlib/tests/test_config.py	2011-06-14 10:07:16 +0000
@@ -37,6 +37,7 @@
     ui,
     urlutils,
     registry,
+    remote,
     tests,
     trace,
     transport,
@@ -45,11 +46,11 @@
     deprecated_in,
     deprecated_method,
     )
-from bzrlib.transport import remote
+from bzrlib.transport import remote as transport_remote
 from bzrlib.tests import (
     features,
-    TestSkipped,
     scenarios,
+    test_server,
     )
 from bzrlib.util.configobj import configobj
 
@@ -118,7 +119,8 @@
 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]
+    (transport_class,
+     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.BranchStore(b)
@@ -142,7 +144,8 @@
 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]
+    (transport_class,
+     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.BranchStack(b)
@@ -1906,6 +1909,247 @@
         self.assertIs(None, bzrdir_config.get_default_stack_on())
 
 
+class TestOldConfigHooks(tests.TestCaseWithTransport):
+
+    def setUp(self):
+        super(TestOldConfigHooks, self).setUp()
+        create_configs_with_file_option(self)
+
+    def assertGetHook(self, conf, name, value):
+        calls = []
+        def hook(*args):
+            calls.append(args)
+        config.OldConfigHooks.install_named_hook('get', hook, None)
+        self.addCleanup(
+            config.OldConfigHooks.uninstall_named_hook, 'get', None)
+        self.assertLength(0, calls)
+        actual_value = conf.get_user_option(name)
+        self.assertEquals(value, actual_value)
+        self.assertLength(1, calls)
+        self.assertEquals((conf, name, value), calls[0])
+
+    def test_get_hook_bazaar(self):
+        self.assertGetHook(self.bazaar_config, 'file', 'bazaar')
+
+    def test_get_hook_locations(self):
+        self.assertGetHook(self.locations_config, 'file', 'locations')
+
+    def test_get_hook_branch(self):
+        # Since locations masks branch, we define a different option
+        self.branch_config.set_user_option('file2', 'branch')
+        self.assertGetHook(self.branch_config, 'file2', 'branch')
+
+    def assertSetHook(self, conf, name, value):
+        calls = []
+        def hook(*args):
+            calls.append(args)
+        config.OldConfigHooks.install_named_hook('set', hook, None)
+        self.addCleanup(
+            config.OldConfigHooks.uninstall_named_hook, 'set', None)
+        self.assertLength(0, calls)
+        conf.set_user_option(name, value)
+        self.assertLength(1, calls)
+        # We can't assert the conf object below as different configs use
+        # different means to implement set_user_option and we care only about
+        # coverage here.
+        self.assertEquals((name, value), calls[0][1:])
+
+    def test_set_hook_bazaar(self):
+        self.assertSetHook(self.bazaar_config, 'foo', 'bazaar')
+
+    def test_set_hook_locations(self):
+        self.assertSetHook(self.locations_config, 'foo', 'locations')
+
+    def test_set_hook_branch(self):
+        self.assertSetHook(self.branch_config, 'foo', 'branch')
+
+    def assertRemoveHook(self, conf, name, section_name=None):
+        calls = []
+        def hook(*args):
+            calls.append(args)
+        config.OldConfigHooks.install_named_hook('remove', hook, None)
+        self.addCleanup(
+            config.OldConfigHooks.uninstall_named_hook, 'remove', None)
+        self.assertLength(0, calls)
+        conf.remove_user_option(name, section_name)
+        self.assertLength(1, calls)
+        # We can't assert the conf object below as different configs use
+        # different means to implement remove_user_option and we care only about
+        # coverage here.
+        self.assertEquals((name,), calls[0][1:])
+
+    def test_remove_hook_bazaar(self):
+        self.assertRemoveHook(self.bazaar_config, 'file')
+
+    def test_remove_hook_locations(self):
+        self.assertRemoveHook(self.locations_config, 'file',
+                              self.locations_config.location)
+
+    def test_remove_hook_branch(self):
+        self.assertRemoveHook(self.branch_config, 'file')
+
+    def assertLoadHook(self, name, conf_class, *conf_args):
+        calls = []
+        def hook(*args):
+            calls.append(args)
+        config.OldConfigHooks.install_named_hook('load', hook, None)
+        self.addCleanup(
+            config.OldConfigHooks.uninstall_named_hook, 'load', None)
+        self.assertLength(0, calls)
+        # Build a config
+        conf = conf_class(*conf_args)
+        # Access an option to trigger a load
+        conf.get_user_option(name)
+        self.assertLength(1, calls)
+        # Since we can't assert about conf, we just use the number of calls ;-/
+
+    def test_load_hook_bazaar(self):
+        self.assertLoadHook('file', config.GlobalConfig)
+
+    def test_load_hook_locations(self):
+        self.assertLoadHook('file', config.LocationConfig, self.tree.basedir)
+
+    def test_load_hook_branch(self):
+        self.assertLoadHook('file', config.BranchConfig, self.tree.branch)
+
+    def assertSaveHook(self, conf):
+        calls = []
+        def hook(*args):
+            calls.append(args)
+        config.OldConfigHooks.install_named_hook('save', hook, None)
+        self.addCleanup(
+            config.OldConfigHooks.uninstall_named_hook, 'save', None)
+        self.assertLength(0, calls)
+        # Setting an option triggers a save
+        conf.set_user_option('foo', 'bar')
+        self.assertLength(1, calls)
+        # Since we can't assert about conf, we just use the number of calls ;-/
+
+    def test_save_hook_bazaar(self):
+        self.assertSaveHook(self.bazaar_config)
+
+    def test_save_hook_locations(self):
+        self.assertSaveHook(self.locations_config)
+
+    def test_save_hook_branch(self):
+        self.assertSaveHook(self.branch_config)
+
+
+class TestOldConfigHooksForRemote(tests.TestCaseWithTransport):
+    """Tests config hooks for remote configs.
+
+    No tests for the remove hook as this is not implemented there.
+    """
+
+    def setUp(self):
+        super(TestOldConfigHooksForRemote, self).setUp()
+        self.transport_server = test_server.SmartTCPServer_for_testing
+        create_configs_with_file_option(self)
+
+    def assertGetHook(self, conf, name, value):
+        calls = []
+        def hook(*args):
+            calls.append(args)
+        config.OldConfigHooks.install_named_hook('get', hook, None)
+        self.addCleanup(
+            config.OldConfigHooks.uninstall_named_hook, 'get', None)
+        self.assertLength(0, calls)
+        actual_value = conf.get_option(name)
+        self.assertEquals(value, actual_value)
+        self.assertLength(1, calls)
+        self.assertEquals((conf, name, value), calls[0])
+
+    def test_get_hook_remote_branch(self):
+        remote_branch = branch.Branch.open(self.get_url('tree'))
+        self.assertGetHook(remote_branch._get_config(), 'file', 'branch')
+
+    def test_get_hook_remote_bzrdir(self):
+        remote_bzrdir = bzrdir.BzrDir.open(self.get_url('tree'))
+        conf = remote_bzrdir._get_config()
+        conf.set_option('remotedir', 'file')
+        self.assertGetHook(conf, 'file', 'remotedir')
+
+    def assertSetHook(self, conf, name, value):
+        calls = []
+        def hook(*args):
+            calls.append(args)
+        config.OldConfigHooks.install_named_hook('set', hook, None)
+        self.addCleanup(
+            config.OldConfigHooks.uninstall_named_hook, 'set', None)
+        self.assertLength(0, calls)
+        conf.set_option(value, name)
+        self.assertLength(1, calls)
+        # We can't assert the conf object below as different configs use
+        # different means to implement set_user_option and we care only about
+        # coverage here.
+        self.assertEquals((name, value), calls[0][1:])
+
+    def test_set_hook_remote_branch(self):
+        remote_branch = branch.Branch.open(self.get_url('tree'))
+        self.addCleanup(remote_branch.lock_write().unlock)
+        self.assertSetHook(remote_branch._get_config(), 'file', 'remote')
+
+    def test_set_hook_remote_bzrdir(self):
+        remote_branch = branch.Branch.open(self.get_url('tree'))
+        self.addCleanup(remote_branch.lock_write().unlock)
+        remote_bzrdir = bzrdir.BzrDir.open(self.get_url('tree'))
+        self.assertSetHook(remote_bzrdir._get_config(), 'file', 'remotedir')
+
+    def assertLoadHook(self, expected_nb_calls, name, conf_class, *conf_args):
+        calls = []
+        def hook(*args):
+            calls.append(args)
+        config.OldConfigHooks.install_named_hook('load', hook, None)
+        self.addCleanup(
+            config.OldConfigHooks.uninstall_named_hook, 'load', None)
+        self.assertLength(0, calls)
+        # Build a config
+        conf = conf_class(*conf_args)
+        # Access an option to trigger a load
+        conf.get_option(name)
+        self.assertLength(expected_nb_calls, calls)
+        # Since we can't assert about conf, we just use the number of calls ;-/
+
+    def test_load_hook_remote_branch(self):
+        remote_branch = branch.Branch.open(self.get_url('tree'))
+        self.assertLoadHook(1, 'file', remote.RemoteBranchConfig, remote_branch)
+
+    def test_load_hook_remote_bzrdir(self):
+        remote_bzrdir = bzrdir.BzrDir.open(self.get_url('tree'))
+        # The config file doesn't exist, set an option to force its creation
+        conf = remote_bzrdir._get_config()
+        conf.set_option('remotedir', 'file')
+        # We get one call for the server and one call for the client, this is
+        # caused by the differences in implementations betwen
+        # SmartServerBzrDirRequestConfigFile (in smart/bzrdir.py) and
+        # SmartServerBranchGetConfigFile (in smart/branch.py)
+        self.assertLoadHook(2 ,'file', remote.RemoteBzrDirConfig, remote_bzrdir)
+
+    def assertSaveHook(self, conf):
+        calls = []
+        def hook(*args):
+            calls.append(args)
+        config.OldConfigHooks.install_named_hook('save', hook, None)
+        self.addCleanup(
+            config.OldConfigHooks.uninstall_named_hook, 'save', None)
+        self.assertLength(0, calls)
+        # Setting an option triggers a save
+        conf.set_option('foo', 'bar')
+        self.assertLength(1, calls)
+        # Since we can't assert about conf, we just use the number of calls ;-/
+
+    def test_save_hook_remote_branch(self):
+        remote_branch = branch.Branch.open(self.get_url('tree'))
+        self.addCleanup(remote_branch.lock_write().unlock)
+        self.assertSaveHook(remote_branch._get_config())
+
+    def test_save_hook_remote_bzrdir(self):
+        remote_branch = branch.Branch.open(self.get_url('tree'))
+        self.addCleanup(remote_branch.lock_write().unlock)
+        remote_bzrdir = bzrdir.BzrDir.open(self.get_url('tree'))
+        self.assertSaveHook(remote_bzrdir._get_config())
+
+
 class TestOption(tests.TestCase):
 
     def test_default_value(self):
@@ -2167,6 +2411,36 @@
         self.assertLength(1, sections)
         self.assertSectionContent(('baz', {'foo': 'bar'}), sections[0])
 
+    def test_load_hook(self):
+        # We first needs to ensure that the store exists
+        store = self.get_store(self)
+        section = store.get_mutable_section('baz')
+        section.set('foo', 'bar')
+        store.save()
+        # Now we can try to load it
+        store = self.get_store(self)
+        calls = []
+        def hook(*args):
+            calls.append(args)
+        config.ConfigHooks.install_named_hook('load', hook, None)
+        self.assertLength(0, calls)
+        store.load()
+        self.assertLength(1, calls)
+        self.assertEquals((store,), calls[0])
+
+    def test_save_hook(self):
+        calls = []
+        def hook(*args):
+            calls.append(args)
+        config.ConfigHooks.install_named_hook('save', hook, None)
+        self.assertLength(0, calls)
+        store = self.get_store(self)
+        section = store.get_mutable_section('baz')
+        section.set('foo', 'bar')
+        store.save()
+        self.assertLength(1, calls)
+        self.assertEquals((store,), calls[0])
+
 
 class TestIniFileStore(TestStore):
 
@@ -2494,10 +2768,6 @@
         conf_stack = config.Stack([conf1, conf2])
         self.assertEquals('baz', conf_stack.get('foo'))
 
-    def test_get_for_empty_stack(self):
-        conf_stack = config.Stack([])
-        self.assertEquals(None, conf_stack.get('foo'))
-
     def test_get_for_empty_section_callable(self):
         conf_stack = config.Stack([lambda : []])
         self.assertEquals(None, conf_stack.get('foo'))
@@ -2521,6 +2791,26 @@
         stack = self.get_stack(self)
 
 
+class TestStackGet(TestStackWithTransport):
+
+    def test_get_for_empty_stack(self):
+        conf = self.get_stack(self)
+        self.assertEquals(None, conf.get('foo'))
+
+    def test_get_hook(self):
+        conf = self.get_stack(self)
+        conf.store._load_from_string('foo=bar')
+        calls = []
+        def hook(*args):
+            calls.append(args)
+        config.ConfigHooks.install_named_hook('get', hook, None)
+        self.assertLength(0, calls)
+        value = conf.get('foo')
+        self.assertEquals('bar', value)
+        self.assertLength(1, calls)
+        self.assertEquals((conf, 'foo', 'bar'), calls[0])
+
+
 class TestStackSet(TestStackWithTransport):
 
     def test_simple_set(self):
@@ -2536,6 +2826,17 @@
         conf.set('foo', 'baz')
         self.assertEquals, 'baz', conf.get('foo')
 
+    def test_set_hook(self):
+        calls = []
+        def hook(*args):
+            calls.append(args)
+        config.ConfigHooks.install_named_hook('set', hook, None)
+        self.assertLength(0, calls)
+        conf = self.get_stack(self)
+        conf.set('foo', 'bar')
+        self.assertLength(1, calls)
+        self.assertEquals((conf, 'foo', 'bar'), calls[0])
+
 
 class TestStackRemove(TestStackWithTransport):
 
@@ -2551,6 +2852,18 @@
         conf = self.get_stack(self)
         self.assertRaises(KeyError, conf.remove, 'I_do_not_exist')
 
+    def test_remove_hook(self):
+        calls = []
+        def hook(*args):
+            calls.append(args)
+        config.ConfigHooks.install_named_hook('remove', hook, None)
+        self.assertLength(0, calls)
+        conf = self.get_stack(self)
+        conf.store._load_from_string('foo=bar')
+        conf.remove('foo')
+        self.assertLength(1, calls)
+        self.assertEquals((conf, 'foo'), calls[0])
+
 
 class TestConfigGetOptions(tests.TestCaseWithTransport, TestOptionsMixin):
 
@@ -3216,7 +3529,8 @@
         to be able to choose a user name with no configuration.
         """
         if sys.platform == 'win32':
-            raise TestSkipped("User name inference not implemented on win32")
+            raise tests.TestSkipped(
+                "User name inference not implemented on win32")
         realname, address = config._auto_user_id()
         if os.path.exists('/etc/mailname'):
             self.assertIsNot(None, realname)

=== modified file 'doc/en/release-notes/bzr-2.4.txt'
--- a/doc/en/release-notes/bzr-2.4.txt	2011-06-07 18:39:38 +0000
+++ b/doc/en/release-notes/bzr-2.4.txt	2011-06-14 09:58:01 +0000
@@ -20,9 +20,15 @@
 
 .. New commands, options, etc that users may wish to try out.
 
+* Hooks have been added for config stacks: ``get``, ``set`` and ``remove``
+  are called when an option is repsectively read, modified or deleted. Also
+  added ``load`` and ``save`` hooks for config stores, called when the
+  stores are loaded or saved.  (Vincent Ladeuil)
+
 * New hook server_exception in bzrlib.smart.server to catch any
-  exception caused while running bzr serve.  (Jonathan Riddell,
-  #274578)
+  exception caused while running bzr serve.
+  (Jonathan Riddell, #274578)
+
 
 Improvements
 ************



More information about the bazaar-commits mailing list