Rev 5684: (vila) Add the opt-in ability to expand options in config files (Vincent in file:///home/pqm/archives/thelove/bzr/%2Btrunk/
Canonical.com Patch Queue Manager
pqm at pqm.ubuntu.com
Thu Feb 24 13:55:37 UTC 2011
At file:///home/pqm/archives/thelove/bzr/%2Btrunk/
------------------------------------------------------------
revno: 5684 [merge]
revision-id: pqm at pqm.ubuntu.com-20110224135532-b5zu78in53r1ptc1
parent: pqm at pqm.ubuntu.com-20110223174127-wwpmy437d2g2e1ny
parent: v.ladeuil+lp at free.fr-20110220111012-365pjabg66z4a26n
committer: Canonical.com Patch Queue Manager <pqm at pqm.ubuntu.com>
branch nick: +trunk
timestamp: Thu 2011-02-24 13:55:32 +0000
message:
(vila) Add the opt-in ability to expand options in config files (Vincent
Ladeuil)
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/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
=== modified file 'bzrlib/bugtracker.py'
--- a/bzrlib/bugtracker.py 2010-09-25 00:42:22 +0000
+++ b/bzrlib/bugtracker.py 2011-02-18 11:24:21 +0000
@@ -231,7 +231,8 @@
tracker_registry.register('gnome',
- UniqueIntegerBugTracker('gnome', 'http://bugzilla.gnome.org/show_bug.cgi?id='))
+ UniqueIntegerBugTracker('gnome',
+ 'http://bugzilla.gnome.org/show_bug.cgi?id='))
class URLParametrizedBugTracker(BugTracker):
@@ -246,7 +247,7 @@
def get(self, abbreviation, branch):
config = branch.get_config()
url = config.get_user_option(
- "%s_%s_url" % (self.type_name, abbreviation))
+ "%s_%s_url" % (self.type_name, abbreviation), expand=False)
if url is None:
return None
self._base_url = url
@@ -261,9 +262,12 @@
return urlutils.join(self._base_url, self._bug_area) + str(bug_id)
-class URLParametrizedIntegerBugTracker(IntegerBugTracker, URLParametrizedBugTracker):
- """A type of bug tracker that can be found on a variety of different sites,
- and thus needs to have the base URL configured, but only allows integer bug IDs.
+class URLParametrizedIntegerBugTracker(IntegerBugTracker,
+ URLParametrizedBugTracker):
+ """A type of bug tracker that only allows integer bug IDs.
+
+ This can be found on a variety of different sites, and thus needs to have
+ the base URL configured.
Looks for a config setting in the form '<type_name>_<abbreviation>_url'.
`type_name` is the name of the type of tracker (e.g. 'bugzilla' or 'trac')
=== modified file 'bzrlib/config.py'
--- a/bzrlib/config.py 2011-01-20 04:44:14 +0000
+++ b/bzrlib/config.py 2011-02-20 11:07:56 +0000
@@ -129,25 +129,50 @@
STORE_BRANCH = 3
STORE_GLOBAL = 4
-_ConfigObj = None
-def ConfigObj(*args, **kwargs):
- global _ConfigObj
- if _ConfigObj is None:
- class ConfigObj(configobj.ConfigObj):
-
- def get_bool(self, section, key):
- return self[section].as_bool(key)
-
- def get_value(self, section, name):
- # Try [] for the old DEFAULT section.
- if section == "DEFAULT":
- try:
- return self[name]
- except KeyError:
- pass
- return self[section][name]
- _ConfigObj = ConfigObj
- return _ConfigObj(*args, **kwargs)
+
+class ConfigObj(configobj.ConfigObj):
+
+ def __init__(self, infile=None, **kwargs):
+ # We define our own interpolation mechanism calling it option expansion
+ super(ConfigObj, self).__init__(infile=infile,
+ interpolation=False,
+ **kwargs)
+
+
+ def get_bool(self, section, key):
+ return self[section].as_bool(key)
+
+ def get_value(self, section, name):
+ # Try [] for the old DEFAULT section.
+ if section == "DEFAULT":
+ try:
+ return self[name]
+ except KeyError:
+ pass
+ 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):
@@ -189,21 +214,167 @@
def _get_signing_policy(self):
"""Template method to override signature creation policy."""
+ option_ref_re = None
+
+ def expand_options(self, string, env=None):
+ """Expand option references in the string in the configuration context.
+
+ :param string: The string containing option to expand.
+
+ :param env: An option dict defining additional configuration options or
+ overriding existing ones.
+
+ :returns: The expanded string.
+ """
+ return self._expand_options_in_string(string, env)
+
+ 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.
+
+ :param env: An option dict defining additional configuration options or
+ overriding existing ones.
+
+ :param _ref_stack: Private list containing the options being
+ expanded to detect loops.
+
+ :returns: The flatten list of expanded strings.
+ """
+ # expand options in each value separately flattening lists
+ result = []
+ for s in slist:
+ value = self._expand_options_in_string(s, env, _ref_stack)
+ if isinstance(value, list):
+ result.extend(value)
+ else:
+ result.append(value)
+ return result
+
+ 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 be expanded.
+
+ :param env: An option dict defining additional configuration options or
+ overriding existing ones.
+
+ :param _ref_stack: Private list containing the options being
+ expanded to detect loops.
+
+ :returns: The expanded string.
+ """
+ if string is None:
+ # Not much to expand there
+ return None
+ if _ref_stack is None:
+ # What references are currently resolved (to detect loops)
+ _ref_stack = []
+ if self.option_ref_re is None:
+ # We want to match the most embedded reference first (i.e. for
+ # '{{foo}}' we will get '{foo}',
+ # for '{bar{baz}}' we will get '{baz}'
+ self.option_ref_re = re.compile('({[^{}]+})')
+ result = string
+ # We need to iterate until no more refs appear ({{foo}} will need two
+ # iterations for example).
+ while True:
+ try:
+ raw_chunks = self.option_ref_re.split(result)
+ except TypeError:
+ import pdb; pdb.set_trace()
+ if len(raw_chunks) == 1:
+ # Shorcut the trivial case: no refs
+ return result
+ chunks = []
+ list_value = False
+ # Split will isolate refs so that every other chunk is a ref
+ chunk_is_ref = False
+ for chunk in raw_chunks:
+ if not chunk_is_ref:
+ if chunk:
+ # 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.OptionExpansionLoop(string, _ref_stack)
+ _ref_stack.append(name)
+ value = self._expand_option(name, env, _ref_stack)
+ if value is None:
+ raise errors.ExpandingUnknownOption(name, string)
+ if isinstance(value, list):
+ list_value = True
+ chunks.extend(value)
+ else:
+ chunks.append(value)
+ _ref_stack.pop()
+ chunk_is_ref = False
+ if list_value:
+ # 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 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 _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 (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._expand_options_in_list(value, env, _ref_stack)
+ else:
+ 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):
- """Get a generic option - no special process, no default."""
- return self._get_user_option(option_name)
-
- def get_user_option_as_bool(self, option_name):
+ 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 expand:
+ if isinstance(value, list):
+ value = self._expand_options_in_list(value)
+ elif isinstance(value, dict):
+ trace.warning('Cannot expand "%s":'
+ ' Dicts do not support option expansion'
+ % (option_name,))
+ else:
+ value = self._expand_options_in_string(value)
+ return value
+
+ 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
@@ -214,15 +385,16 @@
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 the final ','
+ # A single value, most probably the user forgot (or didn't care to
+ # add) the final ','
l = [l]
return l
@@ -372,8 +544,9 @@
# be found in the known_merge_tools if it's not found in the config.
# 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) or
- mergetools.known_merge_tools.get(name, None))
+ command_line = (self.get_user_option('bzr.mergetool.%s' % name,
+ expand=False)
+ or mergetools.known_merge_tools.get(name, None))
return command_line
@@ -672,6 +845,11 @@
def __init__(self, file_name):
super(LockableConfig, self).__init__(file_name=file_name)
self.dir = osutils.dirname(osutils.safe_unicode(self.file_name))
+ # FIXME: It doesn't matter that we don't provide possible_transports
+ # below since this is currently used only for local config files ;
+ # local transports are not shared. But if/when we start using
+ # LockableConfig for other kind of transports, we will need to reuse
+ # whatever connection is already established -- vila 20100929
self.transport = transport.get_transport(self.dir)
self._lock = lockdir.LockDir(self.transport, 'lock')
=== modified file 'bzrlib/errors.py'
--- a/bzrlib/errors.py 2011-01-26 20:02:52 +0000
+++ b/bzrlib/errors.py 2011-02-18 11:24:21 +0000
@@ -3221,3 +3221,22 @@
def __init__(self, branch_url):
self.branch_url = branch_url
+
+# FIXME: I would prefer to define the config related exception classes in
+# config.py but the lazy import mechanism proscribes this -- vila 20101222
+class OptionExpansionLoop(BzrError):
+
+ _fmt = 'Loop involving %(refs)r while expanding "%(string)s".'
+
+ def __init__(self, string, refs):
+ self.string = string
+ self.refs = '->'.join(refs)
+
+
+class ExpandingUnknownOption(BzrError):
+
+ _fmt = 'Option %(name)s is not defined while expanding "%(string)s".'
+
+ def __init__(self, name, string):
+ self.name = name
+ self.string = string
=== 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/test_config.py'
--- a/bzrlib/tests/test_config.py 2011-02-22 13:51:26 +0000
+++ b/bzrlib/tests/test_config.py 2011-02-24 13:55:32 +0000
@@ -540,6 +540,7 @@
' Use IniBasedConfig(_content=xxx) instead.'],
conf._get_parser, file=config_file)
+
class TestIniConfigSaving(tests.TestCaseInTempDir):
def test_cant_save_without_a_file_name(self):
@@ -553,6 +554,198 @@
self.assertFileEqual(content, 'test.conf')
+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 -- vila 2011-02-18
+
+ def get_config(self, string=None):
+ if string is None:
+ string = ''
+ c = config.IniBasedConfig.from_string(string)
+ return c
+
+ def assertExpansion(self, expected, conf, string, env=None):
+ self.assertEquals(expected, conf.expand_options(string, env))
+
+ def test_no_expansion(self):
+ c = self.get_config('')
+ self.assertExpansion('foo', c, 'foo')
+
+ def test_env_adding_options(self):
+ c = self.get_config('')
+ self.assertExpansion('bar', c, '{foo}', {'foo': 'bar'})
+
+ def test_env_overriding_options(self):
+ c = self.get_config('foo=baz')
+ self.assertExpansion('bar', c, '{foo}', {'foo': 'bar'})
+
+ def test_simple_ref(self):
+ c = self.get_config('foo=xxx')
+ self.assertExpansion('xxx', c, '{foo}')
+
+ def test_unknown_ref(self):
+ c = self.get_config('')
+ self.assertRaises(errors.ExpandingUnknownOption,
+ c.expand_options, '{foo}')
+
+ def test_indirect_ref(self):
+ c = self.get_config('''
+foo=xxx
+bar={foo}
+''')
+ self.assertExpansion('xxx', c, '{bar}')
+
+ def test_embedded_ref(self):
+ c = self.get_config('''
+foo=xxx
+bar=foo
+''')
+ self.assertExpansion('xxx', c, '{{bar}}')
+
+ def test_simple_loop(self):
+ c = self.get_config('foo={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.OptionExpansionLoop,
+ c.expand_options, '{foo}')
+ self.assertEquals('foo->bar->baz', e.refs)
+ self.assertEquals('{foo}', e.string)
+
+ def test_list(self):
+ conf = self.get_config('''
+foo=start
+bar=middle
+baz=end
+list={foo},{bar},{baz}
+''')
+ self.assertEquals(['start', 'middle', 'end'],
+ conf.get_user_option('list', expand=True))
+
+ def test_cascading_list(self):
+ conf = self.get_config('''
+foo=start,{bar}
+bar=middle,{baz}
+baz=end
+list={foo}
+''')
+ self.assertEquals(['start', 'middle', 'end'],
+ conf.get_user_option('list', expand=True))
+
+ def test_pathological_hidden_list(self):
+ conf = self.get_config('''
+foo=bin
+bar=go
+start={foo
+middle=},{
+end=bar}
+hidden={start}{middle}{end}
+''')
+ # 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', 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):
def test_cannot_reload_without_name(self):
@@ -1011,7 +1204,7 @@
conf = self._get_empty_config()
cmdline = conf.find_merge_tool('kdiff3')
self.assertEquals('kdiff3 {base} {this} {other} -o {result}', cmdline)
-
+
def test_find_merge_tool_override_known(self):
conf = self._get_empty_config()
conf.set_user_option('bzr.mergetool.kdiff3', 'kdiff3 blah')
=== modified file 'doc/en/release-notes/bzr-2.4.txt'
--- a/doc/en/release-notes/bzr-2.4.txt 2011-02-23 17:41:27 +0000
+++ b/doc/en/release-notes/bzr-2.4.txt 2011-02-24 13:55:32 +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