Rev 6554: (vila) Share and cache local config files (Vincent Ladeuil) in file:///srv/pqm.bazaar-vcs.org/archives/thelove/bzr/%2Btrunk/

Patch Queue Manager pqm at pqm.ubuntu.com
Thu Aug 23 14:24:39 UTC 2012


At file:///srv/pqm.bazaar-vcs.org/archives/thelove/bzr/%2Btrunk/

------------------------------------------------------------
revno: 6554 [merge]
revision-id: pqm at pqm.ubuntu.com-20120823142438-804xd3yql622ahhp
parent: pqm at pqm.ubuntu.com-20120809092037-lwa17t65fwlv7hus
parent: v.ladeuil+lp at free.fr-20120823135807-ck1skxzx3gn9exb7
committer: Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Thu 2012-08-23 14:24:38 +0000
message:
  (vila) Share and cache local config files (Vincent Ladeuil)
modified:
  bzrlib/config.py               config.py-20051011043216-070c74f4e9e338e8
  bzrlib/library_state.py        library_state.py-20100625053036-962zdkiik8k6m5jx-1
  bzrlib/lockdir.py              lockdir.py-20060220222025-98258adf27fbdda3
  bzrlib/plugins/launchpad/test_register.py test_register.py-20060315182712-40f5dda945c829a8
  bzrlib/remote.py               remote.py-20060720103555-yeeg2x51vn0rbtdp-1
  bzrlib/tests/__init__.py       selftest.py-20050531073622-8d0e3c8845c97a64
  bzrlib/tests/blackbox/test_init.py test_init.py-20060309032856-a292116204d86eb7
  bzrlib/tests/blackbox/test_serve.py test_serve.py-20060913064329-8t2pvmsikl4s3xhl-1
  bzrlib/tests/test_config.py    testconfig.py-20051011041908-742d0c15d8d8c8eb
  bzrlib/tests/test_debug.py     test_debug.py-20090303053802-01e8mlv24odmpgix-1
  bzrlib/tests/test_lockdir.py   test_lockdir.py-20060220222025-33d4221569a3d600
  bzrlib/tests/test_ui.py        test_ui.py-20051130162854-458e667a7414af09
  doc/en/release-notes/bzr-2.6.txt bzr2.6.txt-20120116134316-8w1xxom1c7vcu1t5-1
=== modified file 'bzrlib/config.py'
--- a/bzrlib/config.py	2012-08-01 09:27:40 +0000
+++ b/bzrlib/config.py	2012-08-23 13:58:07 +0000
@@ -3288,6 +3288,7 @@
         # anyway.
         return 'In-Process Store, no URL'
 
+
 class TransportIniFileStore(IniFileStore):
     """IniFileStore that loads files from a transport.
 
@@ -3383,6 +3384,10 @@
 # on the relevant parts of the API that needs testing -- vila 20110503 (based
 # on a poolie's remark)
 class GlobalStore(LockableIniFileStore):
+    """A config store for global options.
+
+    There is a single GlobalStore for a given process.
+    """
 
     def __init__(self, possible_transports=None):
         t = transport.get_transport_from_path(
@@ -3392,6 +3397,10 @@
 
 
 class LocationStore(LockableIniFileStore):
+    """A config store for global options.
+
+    There is a single GlobalStore for a given process.
+    """
 
     def __init__(self, possible_transports=None):
         t = transport.get_transport_from_path(
@@ -3401,6 +3410,10 @@
 
 
 class BranchStore(TransportIniFileStore):
+    """A config store for branch options.
+
+    There is a single BranchStore for a given branch.
+    """
 
     def __init__(self, branch):
         super(BranchStore, self).__init__(branch.control_transport,
@@ -3623,6 +3636,10 @@
         yield is_ref, chunk
         is_ref = not is_ref
 
+# FIXME: _shared_stores should be an attribute of a library state once a
+# library_state object is always available.
+_shared_stores = {}
+_shared_stores_at_exit_installed = False
 
 class Stack(object):
     """A stack of configurations where an option can be defined"""
@@ -3817,11 +3834,47 @@
         return "<config.%s(%s)>" % (self.__class__.__name__, id(self))
 
     def _get_overrides(self):
-        # Hack around library_state.initialize never called
+        # FIXME: Hack around library_state.initialize never called
         if bzrlib.global_state is not None:
             return bzrlib.global_state.cmdline_overrides.get_sections()
         return []
 
+    def get_shared_store(self, store, state=None):
+        """Get a known shared store.
+
+        Store urls uniquely identify them and are used to ensure a single copy
+        is shared across all users.
+
+        :param store: The store known to the caller.
+
+        :param state: The library state where the known stores are kept.
+
+        :returns: The store received if it's not a known one, an already known
+            otherwise.
+        """
+        if state is None:
+            state = bzrlib.global_state
+        if state is None:
+            global _shared_stores_at_exit_installed
+            stores = _shared_stores
+            def save_config_changes():
+                for k, store in stores.iteritems():
+                    store.save_changes()
+            if not _shared_stores_at_exit_installed:
+                # FIXME: Ugly hack waiting for library_state to always be
+                # available. -- vila 20120731
+                import atexit
+                atexit.register(save_config_changes)
+                _shared_stores_at_exit_installed = True
+        else:
+            stores = state.config_stores
+        url = store.external_url()
+        try:
+            return stores[url]
+        except KeyError:
+            stores[url] = store
+            return store
+
 
 class MemoryStack(Stack):
     """A configuration stack defined from a string.
@@ -3877,7 +3930,7 @@
         self.store.save()
 
 
-class GlobalStack(_CompatibleStack):
+class GlobalStack(Stack):
     """Global options only stack.
 
     The following sections are queried:
@@ -3891,14 +3944,14 @@
     """
 
     def __init__(self):
-        gstore = GlobalStore()
+        gstore = self.get_shared_store(GlobalStore())
         super(GlobalStack, self).__init__(
             [self._get_overrides,
              NameMatcher(gstore, 'DEFAULT').get_sections],
             gstore, mutable_section_id='DEFAULT')
 
 
-class LocationStack(_CompatibleStack):
+class LocationStack(Stack):
     """Per-location options falling back to global options stack.
 
 
@@ -3920,10 +3973,10 @@
         """Make a new stack for a location and global configuration.
 
         :param location: A URL prefix to """
-        lstore = LocationStore()
+        lstore = self.get_shared_store(LocationStore())
         if location.startswith('file://'):
             location = urlutils.local_path_from_url(location)
-        gstore = GlobalStore()
+        gstore = self.get_shared_store(GlobalStore())
         super(LocationStack, self).__init__(
             [self._get_overrides,
              LocationMatcher(lstore, location).get_sections,
@@ -3951,9 +4004,9 @@
     """
 
     def __init__(self, branch):
-        lstore = LocationStore()
+        lstore = self.get_shared_store(LocationStore())
         bstore = branch._get_config_store()
-        gstore = GlobalStore()
+        gstore = self.get_shared_store(GlobalStore())
         super(BranchStack, self).__init__(
             [self._get_overrides,
              LocationMatcher(lstore, branch.base).get_sections,
@@ -3981,7 +4034,7 @@
         # unlock saves all the changes.
 
 
-class RemoteControlStack(_CompatibleStack):
+class RemoteControlStack(Stack):
     """Remote control-only options stack."""
 
     # FIXME 2011-11-22 JRV This should probably be renamed to avoid confusion

=== modified file 'bzrlib/library_state.py'
--- a/bzrlib/library_state.py	2011-12-19 13:23:58 +0000
+++ b/bzrlib/library_state.py	2012-07-31 09:22:02 +0000
@@ -76,6 +76,8 @@
         # There is no overrides by default, they are set later when the command
         # arguments are parsed.
         self.cmdline_overrides = config.CommandLineStore()
+        # No config stores are cached to start with
+        self.config_stores = {} # By url
         self.started = False
 
     def __enter__(self):
@@ -106,6 +108,10 @@
         self.started = True
 
     def __exit__(self, exc_type, exc_val, exc_tb):
+        if exc_type is None:
+            # Save config changes
+            for k, store in self.config_stores.iteritems():
+                store.save_changes()
         self.cleanups.cleanup_now()
         trace._flush_stdout_stderr()
         trace._flush_trace()

=== modified file 'bzrlib/lockdir.py'
--- a/bzrlib/lockdir.py	2011-12-19 13:23:58 +0000
+++ b/bzrlib/lockdir.py	2012-08-02 11:30:36 +0000
@@ -858,6 +858,6 @@
     as it gives some clue who the user is.
     """
     try:
-        return config.GlobalConfig().username()
+        return config.GlobalStack().get('email')
     except errors.NoWhoami:
         return osutils.getuser_unicode()

=== modified file 'bzrlib/plugins/launchpad/test_register.py'
--- a/bzrlib/plugins/launchpad/test_register.py	2012-02-14 17:22:37 +0000
+++ b/bzrlib/plugins/launchpad/test_register.py	2012-07-31 12:59:58 +0000
@@ -331,6 +331,9 @@
         service = LaunchpadService()
         g_conf = config.GlobalStack()
         g_conf.set('email', 'Test User <test at user.com>')
+        g_conf.store.save()
+        # FIXME: auth_path base dir exists only because bazaar.conf has just
+        # been saved, brittle... -- vila 20120731
         f = open(auth_path, 'wb')
         try:
             scheme, hostinfo = urlparse.urlsplit(service.service_url)[:2]
@@ -352,6 +355,7 @@
         self.assertIs(None, service.registrant_password)
         g_conf = config.GlobalStack()
         g_conf.set('email', 'Test User <test at user.com>')
+        g_conf.store.save()
         stdout = tests.StringIOWrapper()
         stderr = tests.StringIOWrapper()
         ui.ui_factory = tests.TestUIFactory(stdin='userpass\n',

=== modified file 'bzrlib/remote.py'
--- a/bzrlib/remote.py	2012-07-19 18:28:25 +0000
+++ b/bzrlib/remote.py	2012-07-31 13:01:03 +0000
@@ -384,7 +384,7 @@
             self._real_store = _mod_config.ControlStore(self.bzrdir)
 
     def external_url(self):
-        return self.bzrdir.user_url
+        return urlutils.join(self.branch.user_url, 'control.conf')
 
     def _load_content(self):
         medium = self.bzrdir._client._medium
@@ -3251,7 +3251,7 @@
         self._real_store = None
 
     def external_url(self):
-        return self.branch.user_url
+        return urlutils.join(self.branch.user_url, 'branch.conf')
 
     def _load_content(self):
         path = self.branch._remote_path()

=== modified file 'bzrlib/tests/__init__.py'
--- a/bzrlib/tests/__init__.py	2012-08-03 09:35:46 +0000
+++ b/bzrlib/tests/__init__.py	2012-08-03 14:23:05 +0000
@@ -1001,6 +1001,8 @@
     def setUp(self):
         super(TestCase, self).setUp()
 
+        # At this point we're still accessing the config files in $BZR_HOME (as
+        # set by the user running selftest).
         timeout = config.GlobalStack().get('selftest.timeout')
         if timeout:
             timeout_fixture = fixtures.TimeoutFixture(timeout)
@@ -2447,6 +2449,34 @@
         self.transport_readonly_server = None
         self.__vfs_server = None
 
+    def setUp(self):
+        super(TestCaseWithMemoryTransport, self).setUp()
+
+        def _add_disconnect_cleanup(transport):
+            """Schedule disconnection of given transport at test cleanup
+
+            This needs to happen for all connected transports or leaks occur.
+
+            Note reconnections may mean we call disconnect multiple times per
+            transport which is suboptimal but seems harmless.
+            """
+            self.addCleanup(transport.disconnect)
+
+        _mod_transport.Transport.hooks.install_named_hook('post_connect',
+            _add_disconnect_cleanup, None)
+
+        self._make_test_root()
+        self.addCleanup(os.chdir, os.getcwdu())
+        self.makeAndChdirToTestDir()
+        self.overrideEnvironmentForTesting()
+        self.__readonly_server = None
+        self.__server = None
+        self.reduceLockdirTimeout()
+        # Each test may use its own config files even if the local config files
+        # don't actually exist. They'll rightly fail if they try to create them
+        # though.
+        self.overrideAttr(config, '_shared_stores', {})
+
     def get_transport(self, relpath=None):
         """Return a writeable transport.
 
@@ -2733,30 +2763,6 @@
         self.overrideEnv('HOME', test_home_dir)
         self.overrideEnv('BZR_HOME', test_home_dir)
 
-    def setUp(self):
-        super(TestCaseWithMemoryTransport, self).setUp()
-
-        def _add_disconnect_cleanup(transport):
-            """Schedule disconnection of given transport at test cleanup
-
-            This needs to happen for all connected transports or leaks occur.
-
-            Note reconnections may mean we call disconnect multiple times per
-            transport which is suboptimal but seems harmless.
-            """
-            self.addCleanup(transport.disconnect)
- 
-        _mod_transport.Transport.hooks.install_named_hook('post_connect',
-            _add_disconnect_cleanup, None)
-
-        self._make_test_root()
-        self.addCleanup(os.chdir, os.getcwdu())
-        self.makeAndChdirToTestDir()
-        self.overrideEnvironmentForTesting()
-        self.__readonly_server = None
-        self.__server = None
-        self.reduceLockdirTimeout()
-
     def setup_smart_server_with_call_log(self):
         """Sets up a smart server as the transport server with a call log."""
         self.transport_server = test_server.SmartTCPServer_for_testing
@@ -2949,6 +2955,10 @@
     readwrite one must both define get_url() as resolving to os.getcwd().
     """
 
+    def setUp(self):
+        super(TestCaseWithTransport, self).setUp()
+        self.__vfs_server = None
+
     def get_vfs_only_server(self):
         """See TestCaseWithMemoryTransport.
 
@@ -3037,17 +3047,13 @@
         self.assertFalse(differences.has_changed(),
             "Trees %r and %r are different: %r" % (left, right, differences))
 
-    def setUp(self):
-        super(TestCaseWithTransport, self).setUp()
-        self.__vfs_server = None
-
     def disable_missing_extensions_warning(self):
         """Some tests expect a precise stderr content.
 
         There is no point in forcing them to duplicate the extension related
         warning.
         """
-        config.GlobalConfig().set_user_option('ignore_missing_extensions', True)
+        config.GlobalStack().set('ignore_missing_extensions', True)
 
 
 class ChrootedTestCase(TestCaseWithTransport):

=== modified file 'bzrlib/tests/blackbox/test_init.py'
--- a/bzrlib/tests/blackbox/test_init.py	2012-08-04 14:27:47 +0000
+++ b/bzrlib/tests/blackbox/test_init.py	2012-08-23 14:24:38 +0000
@@ -168,10 +168,12 @@
 
     def test_init_default_format_option(self):
         """bzr init should read default format from option default_format"""
-        conf = _mod_config.GlobalConfig.from_string('''
+        g_store = _mod_config.GlobalStore()
+        g_store._load_from_string('''
 [DEFAULT]
 default_format = 1.9
-''', save=True)
+''')
+        g_store.save()
         out, err = self.run_bzr_subprocess('init')
         self.assertContainsRe(out, '1.9')
 

=== modified file 'bzrlib/tests/blackbox/test_serve.py'
--- a/bzrlib/tests/blackbox/test_serve.py	2012-03-21 13:58:39 +0000
+++ b/bzrlib/tests/blackbox/test_serve.py	2012-08-03 11:23:55 +0000
@@ -274,6 +274,8 @@
     def test_bzr_serve_supports_configurable_timeout(self):
         gs = config.GlobalStack()
         gs.set('serve.client_timeout', 0.2)
+        # Save the config as the subprocess will use it
+        gs.store.save()
         process, url = self.start_server_port()
         self.build_tree_contents([('a_file', 'contents\n')])
         # We can connect and issue a request
@@ -281,9 +283,6 @@
         self.assertEqual('contents\n', t.get_bytes('a_file'))
         # However, if we just wait for more content from the server, it will
         # eventually disconnect us.
-        # TODO: Use something like signal.alarm() so that if the server doesn't
-        #       properly handle the timeout, we end up failing the test instead
-        #       of hanging forever.
         m = t.get_smart_medium()
         m.read_bytes(1)
         # Now, we wait for timeout to trigger

=== modified file 'bzrlib/tests/test_config.py'
--- a/bzrlib/tests/test_config.py	2012-07-28 15:19:25 +0000
+++ b/bzrlib/tests/test_config.py	2012-08-01 14:05:11 +0000
@@ -4347,6 +4347,15 @@
         self.assertSectionNames(['ALIASES'], self.bazaar_config, 'ALIASES')
 
 
+class TestSharedStores(tests.TestCaseInTempDir):
+
+    def test_bazaar_conf_shared(self):
+        g1 = config.GlobalStack()
+        g2 = config.GlobalStack()
+        # The two stacks share the same store
+        self.assertIs(g1.store, g2.store)
+
+
 class TestAuthenticationConfigFile(tests.TestCase):
     """Test the authentication.conf file matching"""
 

=== modified file 'bzrlib/tests/test_debug.py'
--- a/bzrlib/tests/test_debug.py	2011-11-16 17:19:13 +0000
+++ b/bzrlib/tests/test_debug.py	2012-07-31 09:17:34 +0000
@@ -26,11 +26,14 @@
 
 class TestDebugFlags(tests.TestCaseInTempDir):
 
-    def test_set_debug_flags_from_config(self):
-        # test both combinations because configobject automatically splits up
-        # comma-separated lists
+    def test_set_no_debug_flags_from_config(self):
+        self.assertDebugFlags([], '')
+
+    def test_set_single_debug_flags_from_config(self):
+        self.assertDebugFlags(['hpss'], 'debug_flags = hpss\n')
+
+    def test_set_multiple_debug_flags_from_config(self):
         self.assertDebugFlags(['hpss', 'error'], 'debug_flags = hpss, error\n')
-        self.assertDebugFlags(['hpss'], 'debug_flags = hpss\n')
 
     def assertDebugFlags(self, expected_flags, conf_bytes):
         conf = config.GlobalStack()

=== modified file 'bzrlib/tests/test_lockdir.py'
--- a/bzrlib/tests/test_lockdir.py	2012-01-27 22:09:19 +0000
+++ b/bzrlib/tests/test_lockdir.py	2012-08-02 11:45:26 +0000
@@ -734,7 +734,7 @@
             lambda: 'aproperhostname')
         # This is off by default at present; see the discussion in the bug.
         # If you change the default, don't forget to update the docs.
-        config.GlobalConfig().set_user_option('locks.steal_dead', True)
+        config.GlobalStack().set('locks.steal_dead', True)
         # Create a lock pretending to come from a different nonexistent
         # process on the same machine.
         l1 = LockDir(self.get_transport(), 'a',

=== modified file 'bzrlib/tests/test_ui.py'
--- a/bzrlib/tests/test_ui.py	2012-04-30 08:59:57 +0000
+++ b/bzrlib/tests/test_ui.py	2012-08-02 11:45:26 +0000
@@ -56,8 +56,7 @@
 
     def test_output_encoding_configuration(self):
         enc = fixtures.generate_unicode_encodings().next()
-        config.GlobalConfig().set_user_option('output_encoding',
-            enc)
+        config.GlobalStack().set('output_encoding', enc)
         ui = tests.TestUIFactory(stdin=None,
             stdout=tests.StringIOWrapper(),
             stderr=tests.StringIOWrapper())

=== modified file 'doc/en/release-notes/bzr-2.6.txt'
--- a/doc/en/release-notes/bzr-2.6.txt	2012-07-28 20:20:38 +0000
+++ b/doc/en/release-notes/bzr-2.6.txt	2012-08-03 12:22:25 +0000
@@ -28,13 +28,17 @@
 Improvements
 ************
 
-``bzr lp-find-proposal`` now only cares about the revision-id that is
-specified, not the branch you use.  This was enabled by a new API call in
-Launchpad's web service.  (Aaron Bentley)
+* ``bzr lp-find-proposal`` now only cares about the revision-id that is
+  specified, not the branch you use.  This was enabled by a new API call in
+  Launchpad's web service.  (Aaron Bentley)
 
 * Implement authentication.conf password obfuscation, the password_encoding
   option can now be set to base64. (Florian Dorn)
 
+* Local configurations files (i.e. accessed on the local file system like
+  ``bazaar.conf`` and ``locations.conf``) are now shared, reducing the
+  number of IOs when querying a configuation option. (Vincent Ladeuil, #832042)
+
 Bug Fixes
 *********
 




More information about the bazaar-commits mailing list