Rev 6394: Merge pre-requisite in file:///home/vila/src/bzr/bugs/907268-bazaar-DEFAULT/
Vincent Ladeuil
v.ladeuil+lp at free.fr
Wed Dec 21 10:51:52 UTC 2011
At file:///home/vila/src/bzr/bugs/907268-bazaar-DEFAULT/
------------------------------------------------------------
revno: 6394 [merge]
revision-id: v.ladeuil+lp at free.fr-20111221105152-1xdfd4lmx33pu8aq
parent: pqm at pqm.ubuntu.com-20111221000749-jnw2o645cdjr961f
parent: v.ladeuil+lp at free.fr-20111221104234-ejo8i4yzwi1jdoui
committer: Vincent Ladeuil <v.ladeuil+lp at free.fr>
branch nick: 907268-bazaar-DEFAULT
timestamp: Wed 2011-12-21 11:51:52 +0100
message:
Merge pre-requisite
modified:
bzrlib/config.py config.py-20051011043216-070c74f4e9e338e8
bzrlib/plugins/po_merge/po_merge.py po_merge.py-20111123180440-la918t6t068pzacx-4
bzrlib/tests/blackbox/test_config.py test_config.py-20100927150753-x6rf54uibd08r636-1
bzrlib/tests/test_config.py testconfig.py-20051011041908-742d0c15d8d8c8eb
doc/developers/configuration.txt configuration.txt-20110408142435-korjxxnskvq44sta-1
doc/en/release-notes/bzr-2.5.txt bzr2.5.txt-20110708125756-587p0hpw7oke4h05-1
-------------- next part --------------
=== modified file 'bzrlib/config.py'
--- a/bzrlib/config.py 2011-12-20 18:47:35 +0000
+++ b/bzrlib/config.py 2011-12-21 10:42:34 +0000
@@ -2344,7 +2344,8 @@
"""
def __init__(self, name, default=None, default_from_env=None,
- help=None, from_unicode=None, invalid=None):
+ help=None, from_unicode=None, invalid=None,
+ unquote=True):
"""Build an option definition.
:param name: the name used to refer to the option.
@@ -2372,6 +2373,11 @@
TypeError. Accepted values are: None (ignore invalid values),
'warning' (emit a warning), 'error' (emit an error message and
terminates).
+
+ :param unquote: should the unicode value be unquoted before conversion.
+ This should be used only when the store providing the values cannot
+ safely unquote them (see http://pad.lv/906897). It is provided so
+ daughter classes can handle the quoting themselves.
"""
if default_from_env is None:
default_from_env = []
@@ -2398,11 +2404,14 @@
self.default_from_env = default_from_env
self.help = help
self.from_unicode = from_unicode
+ self.unquote = unquote
if invalid and invalid not in ('warning', 'error'):
raise AssertionError("%s not supported for 'invalid'" % (invalid,))
self.invalid = invalid
- def convert_from_unicode(self, unicode_value):
+ def convert_from_unicode(self, store, unicode_value):
+ if self.unquote and store is not None and unicode_value is not None:
+ unicode_value = store.unquote(unicode_value)
if self.from_unicode is None or unicode_value is None:
# Don't convert or nothing to convert
return unicode_value
@@ -2497,28 +2506,41 @@
{}, encoding='utf-8', list_values=True, interpolation=False)
-def list_from_store(unicode_str):
- if not isinstance(unicode_str, basestring):
- raise TypeError
- # Now inject our string directly as unicode. All callers got their value
- # from configobj, so values that need to be quoted are already properly
- # quoted.
- _list_converter_config.reset()
- _list_converter_config._parse([u"list=%s" % (unicode_str,)])
- maybe_list = _list_converter_config['list']
- # ConfigObj return '' instead of u''. Use 'str' below to catch all cases.
- if isinstance(maybe_list, basestring):
- if maybe_list:
- # A single value, most probably the user forgot (or didn't care to
- # add) the final ','
- l = [maybe_list]
+class ListOption(Option):
+
+ def __init__(self, name, default=None, default_from_env=None,
+ help=None, invalid=None):
+ """A list Option definition.
+
+ This overrides the base class so the conversion from a unicode string
+ can take quoting into account.
+ """
+ super(ListOption, self).__init__(
+ name, default=default, default_from_env=default_from_env,
+ from_unicode=self.from_unicode, help=help,
+ invalid=invalid, unquote=False)
+
+ def from_unicode(self, unicode_value):
+ if not isinstance(unicode_value, basestring):
+ raise TypeError
+ # Now inject our string directly as unicode. All callers got their
+ # value from configobj, so values that need to be quoted are already
+ # properly quoted.
+ _list_converter_config.reset()
+ _list_converter_config._parse([u"list=%s" % (unicode_value,)])
+ maybe_list = _list_converter_config['list']
+ if isinstance(maybe_list, basestring):
+ if maybe_list:
+ # A single value, most probably the user forgot (or didn't care
+ # to add) the final ','
+ l = [maybe_list]
+ else:
+ # The empty string, convert to empty list
+ l = []
else:
- # The empty string, convert to empty list
- l = []
- else:
- # We rely on ConfigObj providing us with a list already
- l = maybe_list
- return l
+ # We rely on ConfigObj providing us with a list already
+ l = maybe_list
+ return l
class OptionRegistry(registry.Registry):
@@ -2574,8 +2596,8 @@
existing mainline of the branch.
'''))
option_registry.register(
- Option('acceptable_keys',
- default=None, from_unicode=list_from_store,
+ ListOption('acceptable_keys',
+ default=None,
help="""\
List of GPG key patterns which are acceptable for verification.
"""))
@@ -2636,7 +2658,7 @@
should not be lost if the machine crashes. See also repository.fdatasync.
'''))
option_registry.register(
- Option('debug_flags', default=[], from_unicode=list_from_store,
+ ListOption('debug_flags', default=[],
help='Debug flags to activate.'))
option_registry.register(
Option('default_format', default='2a',
@@ -2851,6 +2873,20 @@
"""
raise NotImplementedError(self.unload)
+ def quote(self, value):
+ """Quote a configuration option value for storing purposes.
+
+ This allows Stacks to present values as they will be stored.
+ """
+ return value
+
+ def unquote(self, value):
+ """Unquote a configuration option value into unicode.
+
+ The received value is quoted as stored.
+ """
+ return value
+
def save(self):
"""Saves the Store to persistent storage."""
raise NotImplementedError(self.save)
@@ -3023,6 +3059,20 @@
section = self._config_obj.setdefault(section_id, {})
return self.mutable_section_class(section_id, section)
+ def quote(self, value):
+ try:
+ # configobj conflates automagical list values and quoting
+ self._config_obj.list_values = True
+ return self._config_obj._quote(value)
+ finally:
+ self._config_obj.list_values = False
+
+ def unquote(self, value):
+ if value:
+ # _unquote doesn't handle None nor empty strings
+ value = self._config_obj._unquote(value)
+ return value
+
class TransportIniFileStore(IniFileStore):
"""IniFileStore that loads files from a transport.
@@ -3357,10 +3407,12 @@
# implies querying the persistent storage) until it can't be avoided
# anymore by using callables to describe (possibly empty) section
# lists.
+ found_store = None # Where the option value has been found
for sections in self.sections_def:
for store, section in sections():
value = section.get(name)
if value is not None:
+ found_store = store
break
if value is not None:
break
@@ -3382,8 +3434,10 @@
trace.warning('Cannot expand "%s":'
' %s does not support option expansion'
% (name, type(val)))
- if opt is not None:
- val = opt.convert_from_unicode(val)
+ if opt is None:
+ val = found_store.unquote(val)
+ else:
+ val = opt.convert_from_unicode(found_store, val)
return val
value = expand_and_convert(value)
if opt is not None and value is None:
@@ -3467,19 +3521,20 @@
or deleting an option. In practice the store will often be loaded but
this helps catching some programming errors.
"""
- section = self.store.get_mutable_section(self.mutable_section_id)
- return section
+ store = self.store
+ section = store.get_mutable_section(self.mutable_section_id)
+ return store, section
def set(self, name, value):
"""Set a new value for the option."""
- section = self._get_mutable_section()
- section.set(name, value)
+ store, section = self._get_mutable_section()
+ section.set(name, store.quote(value))
for hook in ConfigHooks['set']:
hook(self, name, value)
def remove(self, name):
"""Remove an existing option."""
- section = self._get_mutable_section()
+ _, section = self._get_mutable_section()
section.remove(name)
for hook in ConfigHooks['remove']:
hook(self, name)
@@ -3641,10 +3696,11 @@
bstore)
self.branch = branch
+
# Use a an empty dict to initialize an empty configobj avoiding all
# parsing and encoding checks
_quoting_config = configobj.ConfigObj(
- {}, encoding='utf-8', interpolation=False)
+ {}, encoding='utf-8', interpolation=False, list_values=True)
class cmd_config(commands.Command):
__doc__ = """Display, set or remove a configuration option.
@@ -3777,6 +3833,13 @@
self.outf.write(' [%s]\n' % (section.id,))
cur_section = section.id
value = section.get(oname, expand=False)
+ # Since we don't use the stack, we need to restore a
+ # proper quoting.
+ try:
+ opt = option_registry.get(oname)
+ value = opt.convert_from_unicode(store, value)
+ except KeyError:
+ value = store.unquote(value)
value = _quoting_config._quote(value)
self.outf.write(' %s = %s\n' % (oname, value))
=== modified file 'bzrlib/plugins/po_merge/po_merge.py'
--- a/bzrlib/plugins/po_merge/po_merge.py 2011-12-19 13:23:58 +0000
+++ b/bzrlib/plugins/po_merge/po_merge.py 2011-12-21 10:42:34 +0000
@@ -57,9 +57,8 @@
''')
-po_dirs_option = config.Option(
+po_dirs_option = config.ListOption(
'po_merge.po_dirs', default='po,debian/po',
- from_unicode=config.list_from_store,
help='List of dirs containing .po files that the hook applies to.')
=== modified file 'bzrlib/tests/blackbox/test_config.py'
--- a/bzrlib/tests/blackbox/test_config.py 2011-12-14 12:15:44 +0000
+++ b/bzrlib/tests/blackbox/test_config.py 2011-12-21 08:52:41 +0000
@@ -95,22 +95,22 @@
''')
def test_list_all_values(self):
- # FIXME: we should register the option as a list or it's displayed as
- # astring and as such, quoted.
+ config.option_registry.register(config.ListOption('list'))
+ self.addCleanup(config.option_registry.remove, 'list')
self.bazaar_config.set_user_option('list', [1, 'a', 'with, a comma'])
script.run_script(self, '''\
$ bzr config -d tree
bazaar:
- list = '1, a, "with, a comma"'
+ list = 1, a, "with, a comma"
''')
def test_list_value_only(self):
- # FIXME: we should register the option as a list or it's displayed as
- # astring and as such, quoted.
+ config.option_registry.register(config.ListOption('list'))
+ self.addCleanup(config.option_registry.remove, 'list')
self.bazaar_config.set_user_option('list', [1, 'a', 'with, a comma'])
script.run_script(self, '''\
$ bzr config -d tree list
- '1, a, "with, a comma"'
+ 1, a, "with, a comma"
''')
def test_bazaar_config(self):
@@ -142,6 +142,7 @@
hello = world
''')
+
class TestConfigDisplayWithPolicy(tests.TestCaseWithTransport):
def test_location_with_policy(self):
=== modified file 'bzrlib/tests/test_config.py'
--- a/bzrlib/tests/test_config.py 2011-12-16 16:49:57 +0000
+++ b/bzrlib/tests/test_config.py 2011-12-21 10:42:34 +0000
@@ -21,7 +21,7 @@
import sys
import threading
-
+import testtools
from testtools import matchers
#import bzrlib specific imports here
@@ -2366,15 +2366,14 @@
class TestOptionConverterMixin(object):
def assertConverted(self, expected, opt, value):
- self.assertEquals(expected, opt.convert_from_unicode(value),
- 'Expecting %s, got %s' % (expected, value,))
+ self.assertEquals(expected, opt.convert_from_unicode(None, value))
def assertWarns(self, opt, value):
warnings = []
def warning(*args):
warnings.append(args[0] % args[1:])
self.overrideAttr(trace, 'warning', warning)
- self.assertEquals(None, opt.convert_from_unicode(value))
+ self.assertEquals(None, opt.convert_from_unicode(None, value))
self.assertLength(1, warnings)
self.assertEquals(
'Value "%s" is not valid for "%s"' % (value, opt.name),
@@ -2382,12 +2381,11 @@
def assertErrors(self, opt, value):
self.assertRaises(errors.ConfigOptionValueError,
- opt.convert_from_unicode, value)
+ opt.convert_from_unicode, None, value)
def assertConvertInvalid(self, opt, invalid_value):
opt.invalid = None
- self.assertEquals(None, opt.convert_from_unicode(invalid_value),
- '%s is not None' % (invalid_value,))
+ self.assertEquals(None, opt.convert_from_unicode(None, invalid_value))
opt.invalid = 'warning'
self.assertWarns(opt, invalid_value)
opt.invalid = 'error'
@@ -2457,105 +2455,10 @@
self.assertConverted(100, opt, u'100')
-class TestOptionWithListConverter(tests.TestCase, TestOptionConverterMixin):
-
- def get_option(self):
- return config.Option('foo', help='A list.',
- from_unicode=config.list_from_store)
-
- def test_convert_invalid(self):
- # No string is invalid as all forms can be converted to a list
- pass
-
- def test_convert_valid(self):
- opt = self.get_option()
- # An empty string is an empty list
- self.assertConverted([], opt, '') # Using a bare str() just in case
- self.assertConverted([], opt, u'')
- # A boolean
- self.assertConverted([u'True'], opt, u'True')
- # An integer
- self.assertConverted([u'42'], opt, u'42')
- # A single string
- self.assertConverted([u'bar'], opt, u'bar')
- # A list remains a list (configObj will turn a string containing commas
- # into a list, but that's not what we're testing here)
- self.assertConverted([u'foo', u'1', u'True'],
- opt, [u'foo', u'1', u'True'])
-
-
-class TestOptionConverterMixin(object):
-
- def assertConverted(self, expected, opt, value):
- self.assertEquals(expected, opt.convert_from_unicode(value))
-
- def assertWarns(self, opt, value):
- warnings = []
- def warning(*args):
- warnings.append(args[0] % args[1:])
- self.overrideAttr(trace, 'warning', warning)
- self.assertEquals(None, opt.convert_from_unicode(value))
- self.assertLength(1, warnings)
- self.assertEquals(
- 'Value "%s" is not valid for "%s"' % (value, opt.name),
- warnings[0])
-
- def assertErrors(self, opt, value):
- self.assertRaises(errors.ConfigOptionValueError,
- opt.convert_from_unicode, value)
-
- def assertConvertInvalid(self, opt, invalid_value):
- opt.invalid = None
- self.assertEquals(None, opt.convert_from_unicode(invalid_value))
- opt.invalid = 'warning'
- self.assertWarns(opt, invalid_value)
- opt.invalid = 'error'
- self.assertErrors(opt, invalid_value)
-
-
-class TestOptionWithBooleanConverter(tests.TestCase, TestOptionConverterMixin):
-
- def get_option(self):
- return config.Option('foo', help='A boolean.',
- from_unicode=config.bool_from_store)
-
- def test_convert_invalid(self):
- opt = self.get_option()
- # A string that is not recognized as a boolean
- self.assertConvertInvalid(opt, u'invalid-boolean')
- # A list of strings is never recognized as a boolean
- self.assertConvertInvalid(opt, [u'not', u'a', u'boolean'])
-
- def test_convert_valid(self):
- opt = self.get_option()
- self.assertConverted(True, opt, u'True')
- self.assertConverted(True, opt, u'1')
- self.assertConverted(False, opt, u'False')
-
-
-class TestOptionWithIntegerConverter(tests.TestCase, TestOptionConverterMixin):
-
- def get_option(self):
- return config.Option('foo', help='An integer.',
- from_unicode=config.int_from_store)
-
- def test_convert_invalid(self):
- opt = self.get_option()
- # A string that is not recognized as an integer
- self.assertConvertInvalid(opt, u'forty-two')
- # A list of strings is never recognized as an integer
- self.assertConvertInvalid(opt, [u'a', u'list'])
-
- def test_convert_valid(self):
- opt = self.get_option()
- self.assertConverted(16, opt, u'16')
-
-
-class TestOptionWithListConverter(tests.TestCase, TestOptionConverterMixin):
-
- def get_option(self):
- return config.Option('foo', help='A list.',
- from_unicode=config.list_from_store)
+class TestListOption(tests.TestCase, TestOptionConverterMixin):
+
+ def get_option(self):
+ return config.ListOption('foo', help='A list.')
def test_convert_invalid(self):
opt = self.get_option()
@@ -2711,6 +2614,7 @@
def setUp(self):
super(TestCommandLineStore, self).setUp()
self.store = config.CommandLineStore()
+ self.overrideAttr(config, 'option_registry', config.OptionRegistry())
def get_section(self):
"""Get the unique section for the command line overrides."""
@@ -2731,12 +2635,15 @@
self.assertEqual('b', section.get('a'))
def test_list_override(self):
+ opt = config.ListOption('l')
+ config.option_registry.register(opt)
self.store._from_cmdline(['l=1,2,3'])
val = self.get_section().get('l')
self.assertEqual('1,2,3', val)
# Reminder: lists should be registered as such explicitely, otherwise
# the conversion needs to be done afterwards.
- self.assertEqual(['1', '2', '3'], config.list_from_store(val))
+ self.assertEqual(['1', '2', '3'],
+ opt.convert_from_unicode(self.store, val))
def test_multiple_overrides(self):
self.store._from_cmdline(['a=b', 'x=y'])
@@ -2796,6 +2703,63 @@
self.assertRaises(AssertionError, store._load_from_string, 'bar=baz')
+class TestStoreQuoting(TestStore):
+
+ scenarios = [(key, {'get_store': builder}) for key, builder
+ in config.test_store_builder_registry.iteritems()]
+
+ def setUp(self):
+ super(TestStoreQuoting, self).setUp()
+ self.store = self.get_store(self)
+ # We need a loaded store but any content will do
+ self.store._load_from_string('')
+
+ def assertIdempotent(self, s):
+ """Assert that quoting an unquoted string is a no-op and vice-versa.
+
+ What matters here is that option values, as they appear in a store, can
+ be safely round-tripped out of the store and back.
+
+ :param s: A string, quoted if required.
+ """
+ self.assertEquals(s, self.store.quote(self.store.unquote(s)))
+ self.assertEquals(s, self.store.unquote(self.store.quote(s)))
+
+ def test_empty_string(self):
+ if isinstance(self.store, config.IniFileStore):
+ # configobj._quote doesn't handle empty values
+ with testtools.ExpectedException(AssertionError):
+ self.assertIdempotent('')
+ else:
+ self.assertIdempotent('')
+ # But quoted empty strings are ok
+ self.assertIdempotent('""')
+
+ def test_embedded_spaces(self):
+ self.assertIdempotent('" a b c "')
+
+ def test_embedded_commas(self):
+ self.assertIdempotent('" a , b c "')
+
+ def test_simple_comma(self):
+ if isinstance(self.store, config.IniFileStore):
+ # configobj requires that lists are special-cased
+ with testtools.ExpectedException(AssertionError):
+ self.assertIdempotent(',')
+ else:
+ self.assertIdempotent(',')
+ # When a single comma is required, quoting is also required
+ self.assertIdempotent('","')
+
+ def test_list(self):
+ if isinstance(self.store, config.IniFileStore):
+ # configobj requires that lists are special-cased
+ with testtools.ExpectedException(AssertionError):
+ self.assertIdempotent('a,b')
+ else:
+ self.assertIdempotent('a,b')
+
+
class TestIniFileStoreContent(tests.TestCaseWithTransport):
"""Simulate loading a config store with content of various encodings.
@@ -3007,6 +2971,25 @@
self.assertEquals((store,), calls[0])
+class TestQuotingIniFileStore(tests.TestCaseWithTransport):
+
+ def get_store(self):
+ return config.TransportIniFileStore(self.get_transport(), 'foo.conf')
+
+ def test_get_quoted_string(self):
+ store = self.get_store()
+ store._load_from_string('foo= " abc "')
+ stack = config.Stack([store.get_sections])
+ self.assertEquals(' abc ', stack.get('foo'))
+
+ def test_set_quoted_string(self):
+ store = self.get_store()
+ stack = config.Stack([store.get_sections], store)
+ stack.set('foo', ' a b c ')
+ store.save()
+ self.assertFileEqual('foo = " a b c "\n', 'foo.conf')
+
+
class TestTransportIniFileStore(TestStore):
def test_loading_unknown_file_fails(self):
@@ -3520,9 +3503,8 @@
self.assertEquals(12, self.conf.get('foo'))
def register_list_option(self, name, default=None, default_from_env=None):
- l = config.Option(name, help='A list.',
- default=default, default_from_env=default_from_env,
- from_unicode=config.list_from_store)
+ l = config.ListOption(name, help='A list.', default=default,
+ default_from_env=default_from_env)
self.registry.register(l)
def test_get_default_list_None(self):
@@ -3675,7 +3657,7 @@
list={foo},{bar},{baz}
''')
self.registry.register(
- config.Option('list', from_unicode=config.list_from_store))
+ config.ListOption('list'))
self.assertEquals(['start', 'middle', 'end'],
self.conf.get('list', expand=True))
@@ -3687,7 +3669,7 @@
list={foo}
''')
self.registry.register(
- config.Option('list', from_unicode=config.list_from_store))
+ config.ListOption('list'))
self.assertEquals(['start', 'middle', 'end'],
self.conf.get('list', expand=True))
@@ -3702,8 +3684,7 @@
''')
# What matters is what the registration says, the conversion happens
# only after all expansions have been performed
- self.registry.register(
- config.Option('hidden', from_unicode=config.list_from_store))
+ self.registry.register(config.ListOption('hidden'))
self.assertEquals(['bin', 'go'],
self.conf.get('hidden', expand=True))
=== modified file 'doc/developers/configuration.txt'
--- a/doc/developers/configuration.txt 2011-12-14 16:30:09 +0000
+++ b/doc/developers/configuration.txt 2011-12-21 10:01:12 +0000
@@ -128,8 +128,6 @@
Adding a new store
------------------
-
-
The following stores are used by ``bzr`` in ways that illustrate various
uses of sections.
@@ -207,7 +205,10 @@
The value of an option is a unicode string or ``None`` if it's not
defined. By using ``from_unicode`` you can turn this string into a more
-appropriate representation (a list of unicode strings for example).
+appropriate representation.
+
+If you need a list value, you should use ``ListOption`` instead.
+
Sections
--------
@@ -251,6 +252,16 @@
places to inherit from the existing basic tests and add their own specific
ones.
+A ``Store`` defines how option values are stored, this includes:
+
+* defining the sections where the options are grouped,
+
+* defining how the values are quoted/unquoted for storage purposes. Stacks
+ use the unquoted values internally (default value handling and option
+ expansion are simpler this way) and ``bzr config`` quote them when they
+ need to be displayed.
+
+
Filtering sections
------------------
=== modified file 'doc/en/release-notes/bzr-2.5.txt'
--- a/doc/en/release-notes/bzr-2.5.txt 2011-12-21 00:07:49 +0000
+++ b/doc/en/release-notes/bzr-2.5.txt 2011-12-21 10:42:34 +0000
@@ -53,6 +53,10 @@
* Allow configuration option default value to be a python callable at
registration. (Vincent Ladeuil, #832064)
+* Configuration stores can now provides a specific quoting mechanism. This
+ is required to workaround ``configobj`` conflating quoting and list values
+ automatic conversion. (Vincent Ladeuil, #906897)
+
* Create obsolete_packs directory when repacking if it does not
exist. (Jonathan Riddell, Jelmer Vernooij, #314314)
More information about the bazaar-commits
mailing list