Rev 5529: Merge expand-options into doc-new-config in file:///home/vila/src/bzr/experimental/config/

Vincent Ladeuil v.ladeuil+lp at free.fr
Thu Mar 10 09:39:39 UTC 2011


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

------------------------------------------------------------
revno: 5529 [merge]
revision-id: v.ladeuil+lp at free.fr-20110310093938-jxx0h8q70fhzh2o3
parent: v.ladeuil+lp at free.fr-20110217180548-2p5ykuothxqgpqiz
parent: v.ladeuil+lp at free.fr-20110220111012-365pjabg66z4a26n
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: doc-new-config
timestamp: Thu 2011-03-10 10:39:38 +0100
message:
  Merge expand-options into doc-new-config
modified:
  bzrlib/bugtracker.py           bugtracker.py-20070410073305-vu1vu1qosjurg8kb-1
  bzrlib/config.py               config.py-20051011043216-070c74f4e9e338e8
  bzrlib/errors.py               errors.py-20050309040759-20512168c4e14fbd
  bzrlib/help_topics/en/configuration.txt configuration.txt-20060314161707-868350809502af01
  bzrlib/tests/__init__.py       selftest.py-20050531073622-8d0e3c8845c97a64
  bzrlib/tests/per_branch/test_config.py test_config.py-20100513163053-tufhixqa9nn7lsp2-1
  bzrlib/tests/test_config.py    testconfig.py-20051011041908-742d0c15d8d8c8eb
  doc/en/release-notes/bzr-2.4.txt bzr2.4.txt-20110114053217-k7ym9jfz243fddjm-1
  doc/en/whats-new/whats-new-in-2.4.txt whatsnewin2.4.txt-20110114044330-nipk1og7j729fy89-1
-------------- next part --------------
=== modified file 'bzrlib/bugtracker.py'
--- a/bzrlib/bugtracker.py	2011-02-17 17:51:13 +0000
+++ b/bzrlib/bugtracker.py	2011-02-18 11:24:21 +0000
@@ -247,7 +247,7 @@
     def get(self, abbreviation, branch):
         config = branch.get_config()
         url = config.get_user_option(
-            "%s_%s_url" % (self.type_name, abbreviation), interpolate=False)
+            "%s_%s_url" % (self.type_name, abbreviation), expand=False)
         if url is None:
             return None
         self._base_url = url

=== modified file 'bzrlib/config.py'
--- a/bzrlib/config.py	2011-02-17 18:04:59 +0000
+++ b/bzrlib/config.py	2011-02-20 11:07:56 +0000
@@ -133,7 +133,7 @@
 class ConfigObj(configobj.ConfigObj):
 
     def __init__(self, infile=None, **kwargs):
-        # We define our own interpolation mechanism
+        # We define our own interpolation mechanism calling it option expansion
         super(ConfigObj, self).__init__(infile=infile,
                                         interpolation=False,
                                         **kwargs)
@@ -152,6 +152,28 @@
         return self[section][name]
 
 
+# FIXME: Until we can guarantee that each config file is loaded once and 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,
+# yell at mgz^W vila and the RM if this is still present at that time
+# -- vila 20110219
+_expand_default_value = None
+def _get_expand_default_value():
+    global _expand_default_value
+    if _expand_default_value is not None:
+        return _expand_default_value
+    conf = GlobalConfig()
+    # Note that we must not use None for the expand value below or we'll run
+    # into infinite recursion. Using False really would be quite silly ;)
+    expand = conf.get_user_option_as_bool('bzr.config.expand', expand=True)
+    if expand is None:
+        # This is an opt-in feature, you *really* need to clearly say you want
+        # to activate it !
+        expand = False
+    _expand_default_value = expand
+    return expand
+
 
 class Config(object):
     """A configuration policy - what username, editor, gpg needs etc."""
@@ -194,20 +216,20 @@
 
     option_ref_re = None
 
-    def interpolate(self, string, env=None):
-        """Interpolate the string in the configuration context.
+    def expand_options(self, string, env=None):
+        """Expand option references in the string in the configuration context.
 
-        :param string: The string to interpolate
+        :param string: The string containing option to expand.
 
         :param env: An option dict defining additional configuration options or
             overriding existing ones.
 
-        :returns: The interpolated string.
+        :returns: The expanded string.
         """
-        return self._interpolate_string(string, env)
+        return self._expand_options_in_string(string, env)
 
-    def _interpolate_list(self, slist, env=None, _ref_stack=None):
-        """Interpolate a list of strings in the configuration context.
+    def _expand_options_in_list(self, slist, env=None, _ref_stack=None):
+        """Expand options in  a list of strings in the configuration context.
 
         :param slist: A list of strings.
 
@@ -215,35 +237,35 @@
             overriding existing ones.
 
         :param _ref_stack: Private list containing the options being
-            interpolated to detect loops.
+            expanded to detect loops.
 
-        :returns: The flatten list of interpolated strings.
+        :returns: The flatten list of expanded strings.
         """
-        # interpolate each value separately flattening lists
+        # expand options in each value separately flattening lists
         result = []
         for s in slist:
-            value = self._interpolate_string(s, env, _ref_stack)
+            value = self._expand_options_in_string(s, env, _ref_stack)
             if isinstance(value, list):
                 result.extend(value)
             else:
                 result.append(value)
         return result
 
-    def _interpolate_string(self, string, env=None, _ref_stack=None):
-        """Interpolate the string in the configuration context.
+    def _expand_options_in_string(self, string, env=None, _ref_stack=None):
+        """Expand options in the string in the configuration context.
 
-        :param string: The string to interpolate
+        :param string: The string to be expanded.
 
         :param env: An option dict defining additional configuration options or
             overriding existing ones.
 
         :param _ref_stack: Private list containing the options being
-            interpolated to detect loops.
+            expanded to detect loops.
 
-        :returns: The interpolated string.
+        :returns: The expanded string.
         """
         if string is None:
-            # Not much to interpolate there
+            # Not much to expand there
             return None
         if _ref_stack is None:
             # What references are currently resolved (to detect loops)
@@ -271,17 +293,18 @@
             for chunk in raw_chunks:
                 if not chunk_is_ref:
                     if chunk:
-                        # Keep only non-empty strings
+                        # Keep only non-empty strings (or we get bogus empty
+                        # slots when a list value is involved).
                         chunks.append(chunk)
                     chunk_is_ref = True
                 else:
                     name = chunk[1:-1]
                     if name in _ref_stack:
-                        raise errors.InterpolationLoop(string, _ref_stack)
+                        raise errors.OptionExpansionLoop(string, _ref_stack)
                     _ref_stack.append(name)
-                    value = self._interpolate_option(name, env, _ref_stack)
+                    value = self._expand_option(name, env, _ref_stack)
                     if value is None:
-                        raise errors.InterpolationUnknownOption(name, string)
+                        raise errors.ExpandingUnknownOption(name, string)
                     if isinstance(value, list):
                         list_value = True
                         chunks.extend(value)
@@ -290,55 +313,68 @@
                     _ref_stack.pop()
                     chunk_is_ref = False
             if list_value:
-                # Once a list appears as the result of an interpolation, all
+                # Once a list appears as the result of an expansion, all
                 # callers will get a list result. This allows a consistent
-                # behavior even when some options in the interpolation chain
-                # may be seen defined as strings even if their interpolated
-                # value is a list.
-                return self._interpolate_list(chunks, env, _ref_stack)
+                # behavior even when some options in the expansion chain
+                # defined as strings (no comma in their value) but their
+                # expanded value is a list.
+                return self._expand_options_in_list(chunks, env, _ref_stack)
             else:
                 result = ''.join(chunks)
         return result
 
-    def _interpolate_option(self, name, env, _ref_stack):
+    def _expand_option(self, name, env, _ref_stack):
         if env is not None and name in env:
             # Special case, values provided in env takes precedence over
             # anything else
             value = env[name]
         else:
-            # FIXME: This is a limited implementation, what we really need
-            # is a way to query the bzr config for the value of an option,
-            # respecting the scope rules -- vila 20101222
-            value = self.get_user_option(name, interpolate=False)
+            # FIXME: This is a limited implementation, what we really need is a
+            # way to query the bzr config for the value of an option,
+            # respecting the scope rules (That is, once we implement fallback
+            # configs, getting the option value should restart from the top
+            # config, not the current one) -- vila 20101222
+            value = self.get_user_option(name, expand=False)
             if isinstance(value, list):
-                value = self._interpolate_list(value, env, _ref_stack)
+                value = self._expand_options_in_list(value, env, _ref_stack)
             else:
-                value = self._interpolate_string(value, env, _ref_stack)
+                value = self._expand_options_in_string(value, env, _ref_stack)
         return value
 
     def _get_user_option(self, option_name):
         """Template method to provide a user option."""
         return None
 
-    def get_user_option(self, option_name, interpolate=True):
-        """Get a generic option - no special process, no default."""
+    def get_user_option(self, option_name, expand=None):
+        """Get a generic option - no special process, no default.
+
+        :param option_name: The queried option.
+
+        :param expand: Whether options references should be expanded.
+
+        :returns: The value of the option.
+        """
+        if expand is None:
+            expand = _get_expand_default_value()
         value = self._get_user_option(option_name)
-        if interpolate:
+        if expand:
             if isinstance(value, list):
-                value = self._interpolate_list(value)
+                value = self._expand_options_in_list(value)
             elif isinstance(value, dict):
-                raise ValueError('Dicts do not support interpolation')
+                trace.warning('Cannot expand "%s":'
+                              ' Dicts do not support option expansion'
+                              % (option_name,))
             else:
-                value = self._interpolate_string(value)
+                value = self._expand_options_in_string(value)
         return value
 
-    def get_user_option_as_bool(self, option_name):
+    def get_user_option_as_bool(self, option_name, expand=None):
         """Get a generic option as a boolean - no special process, no default.
 
         :return None if the option doesn't exist or its value can't be
             interpreted as a boolean. Returns True or False otherwise.
         """
-        s = self._get_user_option(option_name)
+        s = self.get_user_option(option_name, expand=expand)
         if s is None:
             # The option doesn't exist
             return None
@@ -349,13 +385,13 @@
                           s, option_name)
         return val
 
-    def get_user_option_as_list(self, option_name):
+    def get_user_option_as_list(self, option_name, expand=None):
         """Get a generic option as a list - no special process, no default.
 
         :return None if the option doesn't exist. Returns the value as a list
             otherwise.
         """
-        l = self._get_user_option(option_name)
+        l = self.get_user_option(option_name, expand=expand)
         if isinstance(l, (str, unicode)):
             # A single value, most probably the user forgot (or didn't care to
             # add) the final ','
@@ -509,7 +545,7 @@
         # This should be done through the proposed config defaults mechanism
         # when it becomes available in the future.
         command_line = (self.get_user_option('bzr.mergetool.%s' % name,
-                                             interpolate=False)
+                                             expand=False)
                         or mergetools.known_merge_tools.get(name, None))
         return command_line
 

=== modified file 'bzrlib/errors.py'
--- a/bzrlib/errors.py	2011-02-17 00:07:11 +0000
+++ b/bzrlib/errors.py	2011-02-18 11:24:21 +0000
@@ -3224,18 +3224,18 @@
 
 # FIXME: I would prefer to define the config related exception classes in
 # config.py but the lazy import mechanism proscribes this -- vila 20101222
-class InterpolationLoop(BzrError):
+class OptionExpansionLoop(BzrError):
 
-    _fmt = 'Loop involving %(refs)r while evaluating "%(string)s".'
+    _fmt = 'Loop involving %(refs)r while expanding "%(string)s".'
 
     def __init__(self, string, refs):
         self.string = string
         self.refs = '->'.join(refs)
 
 
-class InterpolationUnknownOption(BzrError):
+class ExpandingUnknownOption(BzrError):
 
-    _fmt = 'Option %(name)s is not defined while interpolating "%(string)s".'
+    _fmt = 'Option %(name)s is not defined while expanding "%(string)s".'
 
     def __init__(self, name, string):
         self.name = name

=== modified file 'bzrlib/help_topics/en/configuration.txt'
--- a/bzrlib/help_topics/en/configuration.txt	2011-01-16 01:12:01 +0000
+++ b/bzrlib/help_topics/en/configuration.txt	2011-02-18 10:29:04 +0000
@@ -258,6 +258,13 @@
     email            = John Doe <jdoe at isp.com>
     check_signatures = require
 
+A variable can reference other variables **in the same configuration file** by
+enclosing them in curly brackets::
+
+    my_branch_name = feature_x
+    my_server      = bzr+ssh://example.com
+    push_location   = {my_server}/project/{my_branch_name}
+
 
 Variable policies
 ^^^^^^^^^^^^^^^^^

=== modified file 'bzrlib/tests/__init__.py'
--- a/bzrlib/tests/__init__.py	2011-02-11 17:12:35 +0000
+++ b/bzrlib/tests/__init__.py	2011-02-20 11:07:56 +0000
@@ -954,6 +954,10 @@
         # between tests.  We should get rid of this altogether: bug 656694. --
         # mbp 20101008
         self.overrideAttr(bzrlib.trace, '_verbosity_level', 0)
+        # Isolate config option expansion until its default value for bzrlib is
+        # settled on or a the FIXME associated with _get_expand_default_value
+        # is addressed -- vila 20110219
+        self.overrideAttr(config, '_expand_default_value', None)
 
     def debug(self):
         # debug a frame up.

=== modified file 'bzrlib/tests/per_branch/test_config.py'
--- a/bzrlib/tests/per_branch/test_config.py	2011-02-17 17:51:13 +0000
+++ b/bzrlib/tests/per_branch/test_config.py	2011-02-18 11:24:21 +0000
@@ -33,8 +33,6 @@
         value_dict = {
             'ascii': 'abcd', u'unicode \N{WATCH}': u'foo \N{INTERROBANG}'}
         config.set_user_option('name', value_dict.copy())
-        self.assertEqual(value_dict, config.get_user_option('name',
-                                                            interpolate=False))
-        self.assertRaises(ValueError, config.get_user_option, 'name')
+        self.assertEqual(value_dict, config.get_user_option('name'))
 
 

=== modified file 'bzrlib/tests/test_config.py'
--- a/bzrlib/tests/test_config.py	2011-02-17 16:51:03 +0000
+++ b/bzrlib/tests/test_config.py	2011-02-20 11:07:56 +0000
@@ -528,15 +528,72 @@
         self.assertFileEqual(content, 'test.conf')
 
 
-class TestIniConfigInterpolation(tests.TestCase):
-    """Test interpolation from the IniConfig level.
+class TestIniConfigOptionExpansionDefaultValue(tests.TestCaseInTempDir):
+    """What is the default value of expand for config options.
+
+    This is an opt-in beta feature used to evaluate whether or not option
+    references can appear in dangerous place raising exceptions, disapearing
+    (and as such corrupting data) or if it's safe to activate the option by
+    default.
+
+    Note that these tests relies on config._expand_default_value being already
+    overwritten in the parent class setUp.
+    """
+
+    def setUp(self):
+        super(TestIniConfigOptionExpansionDefaultValue, self).setUp()
+        self.config = None
+        self.warnings = []
+        def warning(*args):
+            self.warnings.append(args[0] % args[1:])
+        self.overrideAttr(trace, 'warning', warning)
+
+    def get_config(self, expand):
+        c = config.GlobalConfig.from_string('bzr.config.expand=%s' % (expand,),
+                                            save=True)
+        return c
+
+    def assertExpandIs(self, expected):
+        actual = config._get_expand_default_value()
+        #self.config.get_user_option_as_bool('bzr.config.expand')
+        self.assertEquals(expected, actual)
+
+    def test_default_is_None(self):
+        self.assertEquals(None, config._expand_default_value)
+
+    def test_default_is_False_even_if_None(self):
+        self.config = self.get_config(None)
+        self.assertExpandIs(False)
+
+    def test_default_is_False_even_if_invalid(self):
+        self.config = self.get_config('<your choice>')
+        self.assertExpandIs(False)
+        # ...
+        # Huh ? My choice is False ? Thanks, always happy to hear that :D
+        # Wait, you've been warned !
+        self.assertLength(1, self.warnings)
+        self.assertEquals(
+            'Value "<your choice>" is not a boolean for "bzr.config.expand"',
+            self.warnings[0])
+
+    def test_default_is_True(self):
+        self.config = self.get_config(True)
+        self.assertExpandIs(True)
+        
+    def test_default_is_False(self):
+        self.config = self.get_config(False)
+        self.assertExpandIs(False)
+        
+
+class TestIniConfigOptionExpansion(tests.TestCase):
+    """Test option expansion from the IniConfig level.
 
     What we really want here is to test the Config level, but the class being
     abstract as far as storing values is concerned, this can't be done
     properly (yet).
     """
     # FIXME: This should be rewritten when all configs share a storage
-    # implementation.
+    # implementation -- vila 2011-02-18
 
     def get_config(self, string=None):
         if string is None:
@@ -544,55 +601,55 @@
         c = config.IniBasedConfig.from_string(string)
         return c
 
-    def assertInterpolate(self, expected, conf, string, env=None):
-        self.assertEquals(expected, conf.interpolate(string, env))
+    def assertExpansion(self, expected, conf, string, env=None):
+        self.assertEquals(expected, conf.expand_options(string, env))
 
-    def test_no_interpolation(self):
+    def test_no_expansion(self):
         c = self.get_config('')
-        self.assertInterpolate('foo', c, 'foo')
+        self.assertExpansion('foo', c, 'foo')
 
     def test_env_adding_options(self):
         c = self.get_config('')
-        self.assertInterpolate('bar', c, '{foo}', {'foo': 'bar'})
+        self.assertExpansion('bar', c, '{foo}', {'foo': 'bar'})
 
     def test_env_overriding_options(self):
         c = self.get_config('foo=baz')
-        self.assertInterpolate('bar', c, '{foo}', {'foo': 'bar'})
+        self.assertExpansion('bar', c, '{foo}', {'foo': 'bar'})
 
     def test_simple_ref(self):
         c = self.get_config('foo=xxx')
-        self.assertInterpolate('xxx', c, '{foo}')
+        self.assertExpansion('xxx', c, '{foo}')
 
     def test_unknown_ref(self):
         c = self.get_config('')
-        self.assertRaises(errors.InterpolationUnknownOption,
-                          c.interpolate, '{foo}')
+        self.assertRaises(errors.ExpandingUnknownOption,
+                          c.expand_options, '{foo}')
 
     def test_indirect_ref(self):
         c = self.get_config('''
 foo=xxx
 bar={foo}
 ''')
-        self.assertInterpolate('xxx', c, '{bar}')
+        self.assertExpansion('xxx', c, '{bar}')
 
     def test_embedded_ref(self):
         c = self.get_config('''
 foo=xxx
 bar=foo
 ''')
-        self.assertInterpolate('xxx', c, '{{bar}}')
+        self.assertExpansion('xxx', c, '{{bar}}')
 
     def test_simple_loop(self):
         c = self.get_config('foo={foo}')
-        self.assertRaises(errors.InterpolationLoop, c.interpolate, '{foo}')
+        self.assertRaises(errors.OptionExpansionLoop, c.expand_options, '{foo}')
 
     def test_indirect_loop(self):
         c = self.get_config('''
 foo={bar}
 bar={baz}
 baz={foo}''')
-        e = self.assertRaises(errors.InterpolationLoop,
-                              c.interpolate, '{foo}')
+        e = self.assertRaises(errors.OptionExpansionLoop,
+                              c.expand_options, '{foo}')
         self.assertEquals('foo->bar->baz', e.refs)
         self.assertEquals('{foo}', e.string)
 
@@ -604,7 +661,7 @@
 list={foo},{bar},{baz}
 ''')
         self.assertEquals(['start', 'middle', 'end'],
-                           conf.get_user_option('list'))
+                           conf.get_user_option('list', expand=True))
 
     def test_cascading_list(self):
         conf = self.get_config('''
@@ -614,7 +671,7 @@
 list={foo}
 ''')
         self.assertEquals(['start', 'middle', 'end'],
-                           conf.get_user_option('list'))
+                           conf.get_user_option('list', expand=True))
 
     def test_pathological_hidden_list(self):
         conf = self.get_config('''
@@ -628,7 +685,39 @@
         # Nope, it's either a string or a list, and the list wins as soon as a
         # ',' appears, so the string concatenation never occur.
         self.assertEquals(['{foo', '}', '{', 'bar}'],
-                          conf.get_user_option('hidden'))
+                          conf.get_user_option('hidden', expand=True))
+
+class TestLocationConfigOptionExpansion(tests.TestCaseInTempDir):
+
+    def get_config(self, location, string=None):
+        if string is None:
+            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.LocationConfig.from_string(string, location)
+        return c
+
+    def test_dont_cross_unrelated_section(self):
+        c = self.get_config('/another/branch/path','''
+[/one/branch/path]
+foo = hello
+bar = {foo}/2
+
+[/another/branch/path]
+bar = {foo}/2
+''')
+        self.assertRaises(errors.ExpandingUnknownOption,
+                          c.get_user_option, 'bar', expand=True)
+
+    def test_cross_related_sections(self):
+        c = self.get_config('/project/branch/path','''
+[/project]
+foo = qu
+
+[/project/branch/path]
+bar = {foo}ux
+''')
+        self.assertEquals('quux', c.get_user_option('bar', expand=True))
 
 
 class TestIniBaseConfigOnDisk(tests.TestCaseInTempDir):

=== modified file 'doc/en/release-notes/bzr-2.4.txt'
--- a/doc/en/release-notes/bzr-2.4.txt	2011-02-16 17:20:10 +0000
+++ b/doc/en/release-notes/bzr-2.4.txt	2011-02-20 11:10:12 +0000
@@ -26,6 +26,14 @@
 * External merge tools can now be configured in bazaar.conf. See
   ``bzr help configuration`` for more information.  (Gordon Tyler, #489915)
 
+* Configuration options can now use references to other options in the same
+  file by enclosing them with curly brackets (``{other_opt}``). This makes it
+  possible to use, for example,
+  ``push_location=lp:~vila/bzr/config-{nickname}`` in ``branch.conf`` when
+  using a loom. During the beta period, the default behaviour is to disable
+  this feature. It can be activated by declaring ``bzr.config.expand = True``
+  in ``bazaar.conf``. (Vincent Ladeuil)
+
 Improvements
 ************
 

=== modified file 'doc/en/whats-new/whats-new-in-2.4.txt'
--- a/doc/en/whats-new/whats-new-in-2.4.txt	2011-02-07 04:29:44 +0000
+++ b/doc/en/whats-new/whats-new-in-2.4.txt	2011-02-20 11:10:12 +0000
@@ -32,6 +32,14 @@
 revisions from tags will always be present, so that operations like ``bzr
 log -r tag:foo`` will always work.
 
+Configuration files
+*******************
+
+Option values can now refer to other options in the same configuration file by
+enclosing them in curly brackets (``{option}``). This is an opt-in feature
+during the beta period controlled by the ``bzr.config.expand`` option that
+should be declared in ``bazaar.conf`` and no other file.
+
 Further information
 *******************
 



More information about the bazaar-commits mailing list