Merge lp:~jelmer/brz/move-errors-config into lp:brz

[Jelmer Vernooij]

Move config-related errors from breezy.errors to breezy.config.

Move config-related errors from breezy.errors to breezy.config.

[Martin Packman (gz)]

See inline comment, otherwise looks sane.

review: Approve

=== modified file 'breezy/config.py'
--- breezy/config.py	2017-07-04 20:03:11 +0000
+++ breezy/config.py	2017-07-15 16:04:48 +0000
@@ -92,7 +92,6 @@
     controldir,
     debug,
     directory_service,
-    errors,
     lazy_regex,
     library_state,
     lockdir,
@@ -108,6 +107,7 @@
 """)
 from . import (
     commands,
+    errors,
     hooks,
     lazy_regex,
     registry,
@@ -155,6 +155,94 @@
 STORE_GLOBAL = 4
 
 
+# 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(errors.BzrError):
+
+    _fmt = 'Loop involving %(refs)r while expanding "%(string)s".'
+
+    def __init__(self, string, refs):
+        self.string = string
+        self.refs = '->'.join(refs)
+
+
+class ExpandingUnknownOption(errors.BzrError):
+
+    _fmt = 'Option "%(name)s" is not defined while expanding "%(string)s".'
+
+    def __init__(self, name, string):
+        self.name = name
+        self.string = string
+
+
+class IllegalOptionName(errors.BzrError):
+
+    _fmt = 'Option "%(name)s" is not allowed.'
+
+    def __init__(self, name):
+        self.name = name
+
+
+class ConfigContentError(errors.BzrError):
+
+    _fmt = "Config file %(filename)s is not UTF-8 encoded\n"
+
+    def __init__(self, filename):
+        errors.BzrError.__init__(self)
+        self.filename = filename
+
+
+class ParseConfigError(errors.BzrError):
+
+    _fmt = "Error(s) parsing config file %(filename)s:\n%(errors)s"
+
+    def __init__(self, errors, filename):
+        errors.BzrError.__init__(self)
+        self.filename = filename
+        self.errors = '\n'.join(e.msg for e in errors)
+
+
+class ConfigOptionValueError(errors.BzrError):
+
+    _fmt = ('Bad value "%(value)s" for option "%(name)s".\n'
+            'See ``brz help %(name)s``')
+
+    def __init__(self, name, value):
+        errors.BzrError.__init__(self, name=name, value=value)
+
+
+class NoEmailInUsername(errors.BzrError):
+
+    _fmt = "%(username)r does not seem to contain a reasonable email address"
+
+    def __init__(self, username):
+        errors.BzrError.__init__(self)
+        self.username = username
+
+
+class NoSuchConfig(errors.BzrError):
+
+    _fmt = ('The "%(config_id)s" configuration does not exist.')
+
+    def __init__(self, config_id):
+        errors.BzrError.__init__(self, config_id=config_id)
+
+
+class NoSuchConfigOption(errors.BzrError):
+
+    _fmt = ('The "%(option_name)s" configuration option does not exist.')
+
+    def __init__(self, option_name):
+        errors.BzrError.__init__(self, option_name=option_name)
+
+
+class NoWhoami(errors.BzrError):
+
+    _fmt = ('Unable to determine your name.\n'
+        "Please, set your name with the 'whoami' command.\n"
+        'E.g. brz whoami "Your Name <[e-mail]>"')
+
+
 def signature_policy_from_unicode(signature_string):
     """Convert a string to a signing policy."""
     if signature_string.lower() == 'check-available':
@@ -342,11 +430,11 @@
                 else:
                     name = chunk[1:-1]
                     if name in _ref_stack:
-                        raise errors.OptionExpansionLoop(string, _ref_stack)
+                        raise 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)
+                        raise ExpandingUnknownOption(name, string)
                     if isinstance(value, list):
                         list_value = True
                         chunks.extend(value)
@@ -666,9 +754,9 @@
         try:
             self._parser = ConfigObj(co_input, encoding='utf-8')
         except configobj.ConfigObjError as e:
-            raise errors.ParseConfigError(e.errors, e.config.filename)
+            raise ParseConfigError(e.errors, e.config.filename)
         except UnicodeDecodeError:
-            raise errors.ConfigContentError(self.file_name)
+            raise ConfigContentError(self.file_name)
         # Make sure self.reload() will use the right file name
         self._parser.filename = self.file_name
         for hook in OldConfigHooks['load']:
@@ -840,7 +928,7 @@
         try:
             del section[option_name]
         except KeyError:
-            raise errors.NoSuchConfigOption(option_name)
+            raise NoSuchConfigOption(option_name)
         self._write_config_file()
         for hook in OldConfigHooks['remove']:
             hook(self, option_name)
@@ -1591,7 +1679,7 @@
     """
     name, email = parse_username(e)
     if not email:
-        raise errors.NoEmailInUsername(e)
+        raise NoEmailInUsername(e)
     return email
 
 
@@ -1669,9 +1757,9 @@
             # encoded, but the values in the ConfigObj are always Unicode.
             self._config = ConfigObj(self._input, encoding='utf-8')
         except configobj.ConfigObjError as e:
-            raise errors.ParseConfigError(e.errors, e.config.filename)
+            raise ParseConfigError(e.errors, e.config.filename)
         except UnicodeError:
-            raise errors.ConfigContentError(self._filename)
+            raise ConfigContentError(self._filename)
         return self._config
 
     def _check_permissions(self):
@@ -2202,9 +2290,9 @@
             try:
                 conf = ConfigObj(f, encoding='utf-8')
             except configobj.ConfigObjError as e:
-                raise errors.ParseConfigError(e.errors, self._external_url())
+                raise ParseConfigError(e.errors, self._external_url())
             except UnicodeDecodeError:
-                raise errors.ConfigContentError(self._external_url())
+                raise ConfigContentError(self._external_url())
         finally:
             f.close()
         return conf
@@ -2320,7 +2408,7 @@
                 trace.warning('Value "%s" is not valid for "%s"',
                               unicode_value, self.name)
             elif self.invalid == 'error':
-                raise errors.ConfigOptionValueError(self.name, unicode_value)
+                raise ConfigOptionValueError(self.name, unicode_value)
         return converted
 
     def get_override(self):
@@ -2520,7 +2608,7 @@
         :param option_name: The name to validate.
         """
         if _option_ref_re.match('{%s}' % option_name) is None:
-            raise errors.IllegalOptionName(option_name)
+            raise IllegalOptionName(option_name)
 
     def register(self, option):
         """Register a new option to its name.
@@ -3153,9 +3241,9 @@
                                          list_values=False)
         except configobj.ConfigObjError as e:
             self._config_obj = None
-            raise errors.ParseConfigError(e.errors, self.external_url())
+            raise ParseConfigError(e.errors, self.external_url())
         except UnicodeDecodeError:
-            raise errors.ConfigContentError(self.external_url())
+            raise ConfigContentError(self.external_url())
 
     def save_changes(self):
         if not self.is_loaded():
@@ -3717,11 +3805,11 @@
                     expanded = True
                     name = chunk[1:-1]
                     if name in _refs:
-                        raise errors.OptionExpansionLoop(string, _refs)
+                        raise OptionExpansionLoop(string, _refs)
                     _refs.append(name)
                     value = self._expand_option(name, env, _refs)
                     if value is None:
-                        raise errors.ExpandingUnknownOption(name, string)
+                        raise ExpandingUnknownOption(name, string)
                     chunks.append(value)
                     _refs.pop()
             result = ''.join(chunks)
@@ -4112,7 +4200,7 @@
                 if write_access:
                     self.add_cleanup(br.lock_write().unlock)
                 return br.get_config_stack()
-            raise errors.NoSuchConfig(scope)
+            raise NoSuchConfig(scope)
         else:
             try:
                 (_, br, _) = (
@@ -4137,7 +4225,7 @@
             value = self._quote_multiline(value)
             self.outf.write('%s\n' % (value,))
         else:
-            raise errors.NoSuchConfigOption(name)
+            raise NoSuchConfigOption(name)
 
     def _show_matching_options(self, name, directory, scope):
         name = lazy_regex.lazy_compile(name)
@@ -4182,7 +4270,7 @@
             # Explicitly save the changes
             conf.store.save_changes()
         except KeyError:
-            raise errors.NoSuchConfigOption(name)
+            raise NoSuchConfigOption(name)
 
 
 # Test registries
 
=== modified file 'breezy/errors.py'
--- breezy/errors.py	2017-07-15 15:59:40 +0000
+++ breezy/errors.py	2017-07-15 16:04:48 +0000
@@ -1639,43 +1639,6 @@
     _fmt = "Working tree has conflicts."
 
 
-class ConfigContentError(BzrError):
-
-    _fmt = "Config file %(filename)s is not UTF-8 encoded\n"
-
-    def __init__(self, filename):
-        BzrError.__init__(self)
-        self.filename = filename
-
-
-class ParseConfigError(BzrError):
-
-    _fmt = "Error(s) parsing config file %(filename)s:\n%(errors)s"
-
-    def __init__(self, errors, filename):
-        BzrError.__init__(self)
-        self.filename = filename
-        self.errors = '\n'.join(e.msg for e in errors)
-
-
-class ConfigOptionValueError(BzrError):
-
-    _fmt = ('Bad value "%(value)s" for option "%(name)s".\n'
-            'See ``brz help %(name)s``')
-
-    def __init__(self, name, value):
-        BzrError.__init__(self, name=name, value=value)
-
-
-class NoEmailInUsername(BzrError):
-
-    _fmt = "%(username)r does not seem to contain a reasonable email address"
-
-    def __init__(self, username):
-        BzrError.__init__(self)
-        self.username = username
-
-
 class DependencyNotPresent(BzrError):
 
     _fmt = 'Unable to import library "%(library)s": %(error)s'
@@ -2808,22 +2771,6 @@
         self.user_encoding = get_user_encoding()
 
 
-class NoSuchConfig(BzrError):
-
-    _fmt = ('The "%(config_id)s" configuration does not exist.')
-
-    def __init__(self, config_id):
-        BzrError.__init__(self, config_id=config_id)
-
-
-class NoSuchConfigOption(BzrError):
-
-    _fmt = ('The "%(option_name)s" configuration option does not exist.')
-
-    def __init__(self, option_name):
-        BzrError.__init__(self, option_name=option_name)
-
-
 class NoSuchAlias(BzrError):
 
     _fmt = ('The alias "%(alias_name)s" does not exist.')
@@ -3023,13 +2970,6 @@
         self.controldir = controldir
 
 
-class NoWhoami(BzrError):
-
-    _fmt = ('Unable to determine your name.\n'
-        "Please, set your name with the 'whoami' command.\n"
-        'E.g. brz whoami "Your Name <[e-mail]>"')
-
-
 class RecursiveBind(BzrError):
 
     _fmt = ('Branch "%(branch_url)s" appears to be bound to itself. '
@@ -3039,34 +2979,6 @@
         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
-
-
-class IllegalOptionName(BzrError):
-
-    _fmt = 'Option "%(name)s" is not allowed.'
-
-    def __init__(self, name):
-        self.name = name
-
-
 class HpssVfsRequestNotAllowed(BzrError):
 
     _fmt = ("VFS requests over the smart server are not allowed. Encountered: "
 
=== modified file 'breezy/tests/test_config.py'
--- breezy/tests/test_config.py	2017-07-04 20:03:11 +0000
+++ breezy/tests/test_config.py	2017-07-15 16:04:48 +0000
@@ -654,7 +654,7 @@
 
     def test_unknown_ref(self):
         c = self.get_config('')
-        self.assertRaises(errors.ExpandingUnknownOption,
+        self.assertRaises(config.ExpandingUnknownOption,
                           c.expand_options, '{foo}')
 
     def test_indirect_ref(self):
@@ -673,14 +673,15 @@
 
     def test_simple_loop(self):
         c = self.get_config('foo={foo}')
-        self.assertRaises(errors.OptionExpansionLoop, c.expand_options, '{foo}')
+        self.assertRaises(config.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,
+        e = self.assertRaises(config.OptionExpansionLoop,
                               c.expand_options, '{foo}')
         self.assertEqual('foo->bar->baz', e.refs)
         self.assertEqual('{foo}', e.string)
@@ -739,7 +740,7 @@
 [/another/branch/path]
 bar = {foo}/2
 ''')
-        self.assertRaises(errors.ExpandingUnknownOption,
+        self.assertRaises(config.ExpandingUnknownOption,
                           c.get_user_option, 'bar', expand=True)
 
     def test_cross_related_sections(self):
@@ -1534,7 +1535,7 @@
     def test_extract_email_address(self):
         self.assertEqual('[e-mail]',
                          config.extract_email_address('Jane <[e-mail]>'))
-        self.assertRaises(errors.NoEmailInUsername,
+        self.assertRaises(config.NoEmailInUsername,
                           config.extract_email_address, 'Jane Tester')
 
     def test_parse_username(self):
@@ -1595,14 +1596,14 @@
         t = self.get_transport()
         t.put_bytes('foo.conf', 'user=foo\n#\xff\n')
         conf = config.TransportConfig(t, 'foo.conf')
-        self.assertRaises(errors.ConfigContentError, conf._get_configobj)
+        self.assertRaises(config.ConfigContentError, conf._get_configobj)
 
     def test_load_erroneous_content(self):
         """Ensure we display a proper error on content that can't be parsed."""
         t = self.get_transport()
         t.put_bytes('foo.conf', '[open_section\n')
         conf = config.TransportConfig(t, 'foo.conf')
-        self.assertRaises(errors.ParseConfigError, conf._get_configobj)
+        self.assertRaises(config.ParseConfigError, conf._get_configobj)
 
     def test_load_permission_denied(self):
         """Ensure we get an empty config file if the file is inaccessible."""
@@ -2012,7 +2013,7 @@
             warnings[0])
 
     def assertCallsError(self, opt, value):
-        self.assertRaises(errors.ConfigOptionValueError,
+        self.assertRaises(config.ConfigOptionValueError,
                           opt.convert_from_unicode, None, value)
 
     def assertConvertInvalid(self, opt, invalid_value):
@@ -2179,9 +2180,9 @@
         self.assertEqual('A simple option', self.registry.get_help('foo'))
 
     def test_dont_register_illegal_name(self):
-        self.assertRaises(errors.IllegalOptionName,
+        self.assertRaises(config.IllegalOptionName,
                           self.registry.register, config.Option(' foo'))
-        self.assertRaises(errors.IllegalOptionName,
+        self.assertRaises(config.IllegalOptionName,
                           self.registry.register, config.Option('bar,'))
 
     lazy_option = config.Option('lazy_foo', help='Lazy help')
@@ -2202,10 +2203,10 @@
         # the option name which indirectly requires that the option name is a
         # valid python identifier. We violate that rule here (using a key that
         # doesn't match the option name) to test the option name checking.
-        self.assertRaises(errors.IllegalOptionName,
+        self.assertRaises(config.IllegalOptionName,
                           self.registry.register_lazy, ' foo', self.__module__,
                           'TestOptionRegistry.lazy_option')
-        self.assertRaises(errors.IllegalOptionName,
+        self.assertRaises(config.IllegalOptionName,
                           self.registry.register_lazy, '1,2', self.__module__,
                           'TestOptionRegistry.lazy_option')
 
@@ -2519,14 +2520,14 @@
         t = self.get_transport()
         t.put_bytes('foo.conf', 'user=foo\n#%s\n' % (self.invalid_utf8_char,))
         store = config.TransportIniFileStore(t, 'foo.conf')
-        self.assertRaises(errors.ConfigContentError, store.load)
+        self.assertRaises(config.ConfigContentError, store.load)
 
     def test_load_erroneous_content(self):
         """Ensure we display a proper error on content that can't be parsed."""
         t = self.get_transport()
         t.put_bytes('foo.conf', '[open_section\n')
         store = config.TransportIniFileStore(t, 'foo.conf')
-        self.assertRaises(errors.ParseConfigError, store.load)
+        self.assertRaises(config.ParseConfigError, store.load)
 
     def test_load_permission_denied(self):
         """Ensure we get warned when trying to load an inaccessible file."""
@@ -2576,14 +2577,14 @@
         with open('foo.conf', 'wb') as f:
             f.write('user=foo\n#%s\n' % (self.invalid_utf8_char,))
         conf = config.IniBasedConfig(file_name='foo.conf')
-        self.assertRaises(errors.ConfigContentError, conf._get_parser)
+        self.assertRaises(config.ConfigContentError, conf._get_parser)
 
     def test_load_erroneous_content(self):
         """Ensure we display a proper error on content that can't be parsed."""
         with open('foo.conf', 'wb') as f:
             f.write('[open_section\n')
         conf = config.IniBasedConfig(file_name='foo.conf')
-        self.assertRaises(errors.ParseConfigError, conf._get_parser)
+        self.assertRaises(config.ParseConfigError, conf._get_parser)
 
 
 class TestMutableStore(TestStore):
@@ -2868,7 +2869,7 @@
         store = config.TransportIniFileStore(self.get_transport(), 'foo.conf')
         self.assertEqual(False, store.is_loaded())
         exc = self.assertRaises(
-            errors.ParseConfigError, store._load_from_string,
+            config.ParseConfigError, store._load_from_string,
             'this is invalid !')
         self.assertEndsWith(exc.filename, 'foo.conf')
         # And the load failed
@@ -3668,7 +3669,7 @@
         self.assertExpansion('xxx', '{foo}')
 
     def test_unknown_ref(self):
-        self.assertRaises(errors.ExpandingUnknownOption,
+        self.assertRaises(config.ExpandingUnknownOption,
                           self.conf.expand_options, '{foo}')
 
     def test_illegal_def_is_ignored(self):
@@ -3692,7 +3693,7 @@
 
     def test_simple_loop(self):
         self.conf.store._load_from_string('foo={foo}')
-        self.assertRaises(errors.OptionExpansionLoop,
+        self.assertRaises(config.OptionExpansionLoop,
                           self.conf.expand_options, '{foo}')
 
     def test_indirect_loop(self):
@@ -3700,7 +3701,7 @@
 foo={bar}
 bar={baz}
 baz={foo}''')
-        e = self.assertRaises(errors.OptionExpansionLoop,
+        e = self.assertRaises(config.OptionExpansionLoop,
                               self.conf.expand_options, '{foo}')
         self.assertEqual('foo->bar->baz', e.refs)
         self.assertEqual('{foo}', e.string)
@@ -3771,7 +3772,7 @@
 [/another/branch/path]
 bar = {foo}/2
 ''')
-        self.assertRaises(errors.ExpandingUnknownOption,
+        self.assertRaises(config.ExpandingUnknownOption,
                           c.get, 'bar', expand=True)
 
     def test_cross_related_sections(self):
@@ -3859,7 +3860,7 @@
 ''')
         g_store.save()
         stack = config.LocationStack('/home/user/project/branch')
-        self.assertRaises(errors.ExpandingUnknownOption,
+        self.assertRaises(config.ExpandingUnknownOption,
                           stack.get, 'gfoo', expand=True)
 
     def test_expand_local_option_locally(self):
@@ -4166,7 +4167,7 @@
 
     def test_non_utf8_config(self):
         conf = config.AuthenticationConfig(_file=BytesIO(b'foo = bar\xff'))
-        self.assertRaises(errors.ConfigContentError, conf._get_config)
+        self.assertRaises(config.ConfigContentError, conf._get_config)
 
     def test_missing_auth_section_header(self):
         conf = config.AuthenticationConfig(_file=BytesIO(b'foo = bar'))
@@ -4174,7 +4175,7 @@
 
     def test_auth_section_header_not_closed(self):
         conf = config.AuthenticationConfig(_file=BytesIO(b'[DEF'))
-        self.assertRaises(errors.ParseConfigError, conf._get_config)
+        self.assertRaises(config.ParseConfigError, conf._get_config)
 
     def test_auth_value_not_boolean(self):
         conf = config.AuthenticationConfig(_file=BytesIO(b"""\
@@ -4765,5 +4766,5 @@
 
     def test_unknown(self):
         conf = config.MemoryStack('mail_client=firebird')
-        self.assertRaises(errors.ConfigOptionValueError, conf.get,
+        self.assertRaises(config.ConfigOptionValueError, conf.get,
                 'mail_client')