Merge lp:~gz/brz/mutter_unicode into lp:brz

[Martin Packman]

Make mutter format with unicode

Previously making everything utf-8 encoded bytes before interpolating was most robust, but better particularly with Python 3 to leave as unicode till all formatting has happened.

[Jelmer Vernooij (jelmer)]

Merci!

review: Approve

[The Breezy Bot (the-breezy-bot)]

Running landing tests failed
https://ci.breezy-vcs.org/job/land-brz/217/

[The Breezy Bot (the-breezy-bot)]

Merging failed
https://ci.breezy-vcs.org/job/land-brz/220/

[The Breezy Bot (the-breezy-bot)]

Running landing tests failed
https://ci.breezy-vcs.org/job/land-brz/221/

[The Breezy Bot (the-breezy-bot)]

Running landing tests failed
https://ci.breezy-vcs.org/job/land-brz/226/

=== modified file 'breezy/bzr/remote.py'
--- breezy/bzr/remote.py	2018-06-30 22:12:58 +0000
+++ breezy/bzr/remote.py	2018-06-30 23:03:12 +0000
@@ -4262,8 +4262,8 @@
     def find(name):
         try:
             return context[name]
-        except KeyError as key_err:
-            mutter('Missing key %r in context %r', key_err.args[0], context)
+        except KeyError:
+            mutter('Missing key \'%s\' in context %r', name, context)
             raise err
     def get_path():
         """Get the path from the context if present, otherwise use first error
@@ -4271,12 +4271,11 @@
         """
         try:
             return context['path'].decode('utf-8')
-        except KeyError as key_err:
+        except KeyError:
             try:
                 return err.error_args[0].decode('utf-8')
-            except IndexError as idx_err:
-                mutter(
-                    'Missing key %r in context %r', key_err.args[0], context)
+            except IndexError:
+                mutter('Missing key \'path\' in context %r', context)
                 raise err
     if not isinstance(err.error_verb, bytes):
         raise TypeError(err.error_verb)

=== modified file 'breezy/tests/test_export_pot.py'
--- breezy/tests/test_export_pot.py	2018-06-19 19:44:00 +0000
+++ breezy/tests/test_export_pot.py	2018-06-30 23:03:12 +0000
@@ -221,7 +221,7 @@
         self.check_context(context2A, path, 4)
         context2B = context1.from_string("not there")
         self.check_context(context2B, path, 21)
-        self.assertContainsRe(self.get_log(), "String 'not there' not found")
+        self.assertContainsRe(self.get_log(), "String b?'not there' not found")
 
 
 class TestWriteOption(tests.TestCase):

=== modified file 'breezy/tests/test_trace.py'
--- breezy/tests/test_trace.py	2017-10-27 00:18:42 +0000
+++ breezy/tests/test_trace.py	2018-06-30 23:03:12 +0000
@@ -197,6 +197,12 @@
         log = self.get_log()
         self.assertContainsRe(log, 'the unicode character')
 
+    def test_trace_argument_exception(self):
+        err = Exception('an error')
+        mutter(u'can format stringable classes %s', err)
+        log = self.get_log()
+        self.assertContainsRe(log, 'can format stringable classes an error')
+
     def test_report_broken_pipe(self):
         try:
             raise IOError(errno.EPIPE, 'broken pipe foofofo')
@@ -237,21 +243,21 @@
         self.assertEndsWith(log, ' "a string")\n')
 
     def test_mutter_never_fails(self):
-        # Even if the decode/encode stage fails, mutter should not
-        # raise an exception
-        # This test checks that mutter doesn't fail; the current behaviour
-        # is that it doesn't fail *and writes non-utf8*.
-        mutter(u'Writing a greek mu (\xb5) works in a unicode string')
-        mutter(b'But fails in an ascii string \xb5')
-        mutter(b'and in an ascii argument: %s', b'\xb5')
+        """Even with unencodable input mutter should not raise errors."""
+        mutter(u'can write unicode \xa7')
+        mutter('can interpolate unicode %s', u'\xa7')
+        mutter(b'can write bytes \xa7')
+        mutter('can repr bytes %r', b'\xa7')
+        mutter('can interpolate bytes %s', b'\xa7')
+        # Log will always be written as utf-8
         log = self.get_log()
-        self.assertContainsRe(log, 'Writing a greek mu')
-        self.assertContainsRe(log, "But fails in an ascii string")
-        # However, the log content object does unicode replacement on reading
-        # to let it get unicode back where good data has been written. So we
-        # have to do a replaceent here as well.
-        self.assertContainsRe(log, b"ascii argument: \xb5".decode('utf8',
-            'replace'))
+        self.assertContainsRe(
+            log,
+            u'.* +can write unicode \xa7\n'
+            u'.* +can interpolate unicode \xa7\n'
+            u'.* +can write bytes \ufffd\n'
+            u'.* +can repr bytes b\'\\\\xa7\'\n'
+            u'.* +can interpolate bytes (?:\ufffd|b\'\\\\xa7\')\n')
 
     def test_show_error(self):
         show_error('error1')

=== modified file 'breezy/trace.py'
--- breezy/trace.py	2018-06-17 11:15:04 +0000
+++ breezy/trace.py	2018-06-30 23:03:12 +0000
@@ -138,6 +138,16 @@
     _brz_logger.error(*args, **kwargs)
 
 
+class _Bytes(str):
+    """Compat class for displaying bytes on Python 2."""
+
+    def __repr__(self):
+        return 'b' + str.__repr__(self)
+
+    def __unicode__(self):
+        return self.decode('ascii', 'replace')
+
+
 def mutter(fmt, *args):
     if _trace_file is None:
         return
@@ -146,24 +156,20 @@
     if (getattr(_trace_file, 'closed', None) is not None) and _trace_file.closed:
         return
 
-    if isinstance(fmt, text_type):
-        fmt = fmt.encode('utf8')
+    # Let format strings be specified as ascii bytes to help Python 2
+    if isinstance(fmt, bytes):
+        fmt = fmt.decode('ascii', 'replace')
 
-    if len(args) > 0:
-        # It seems that if we do ascii % (unicode, ascii) we can
-        # get a unicode cannot encode ascii error, so make sure that "fmt"
-        # is a unicode string
-        real_args = []
-        for arg in args:
-            if isinstance(arg, text_type):
-                arg = arg.encode('utf8')
-            real_args.append(arg)
-        out = fmt % tuple(real_args)
+    if args:
+        if not PY3:
+            args = tuple(
+                _Bytes(arg) if isinstance(arg, bytes) else arg for arg in args)
+        out = fmt % args
     else:
         out = fmt
     now = time.time()
-    out = b'%0.3f  %s\n' % (now - _brz_log_start_time, out)
-    _trace_file.write(out)
+    out = '%0.3f  %s\n' % (now - _brz_log_start_time, out)
+    _trace_file.write(out.encode('utf-8'))
     # there's no explicit flushing; the file is typically line buffered.
 
 
@@ -612,8 +618,8 @@
             # Try saving the details that would have been logged in some form
             msg = args = "<Unformattable>"
             try:
-                msg = repr(record.msg).encode("ascii", "backslashescape")
-                args = repr(record.args).encode("ascii", "backslashescape")
+                msg = repr(record.msg)
+                args = repr(record.args)
             except Exception:
                 pass
             # Using mutter() bypasses the logging module and writes directly

=== modified file 'python3.passing'
--- python3.passing	2018-06-30 22:12:58 +0000
+++ python3.passing	2018-06-30 23:03:12 +0000
@@ -12699,6 +12699,7 @@
 breezy.tests.test_chk_map.TestLeafNode.test_current_size_width_changed
 breezy.tests.test_chk_map.TestLeafNode.test_key_new
 breezy.tests.test_chk_map.TestLeafNode.test_unique_serialised_prefix_empty_new
+breezy.tests.test_chk_map.TestLeafNode.test_unmap_missing
 breezy.tests.test_chk_map.TestMapSearchKeys.test_search_key_is_passed_to_root_node
 breezy.tests.test_chk_map.TestMapSearchKeys.test_search_key_passed_via__ensure_root
 breezy.tests.test_chk_map.TestMap.test_init_and_save_new
@@ -13826,6 +13827,7 @@
 breezy.tests.test_export_pot.TestModuleContext.test_from_class
 breezy.tests.test_export_pot.TestModuleContext.test_from_class_missing
 breezy.tests.test_export_pot.TestModuleContext.test_from_string
+breezy.tests.test_export_pot.TestModuleContext.test_from_string_missing
 breezy.tests.test_export_pot.TestModuleContext.test___init__
 breezy.tests.test_export_pot.TestNormalize.test_multi_lines
 breezy.tests.test_export_pot.TestNormalize.test_single_line
@@ -16162,6 +16164,8 @@
 breezy.tests.test_remote.Test_ClientMedium_remote_is_at_least.test__remember_remote_is_before
 breezy.tests.test_remote.Test_ClientMedium_remote_path_from_transport.test_remote_path_from_transport
 breezy.tests.test_remote.Test_ClientMedium_remote_path_from_transport.test_remote_path_from_transport_http
+breezy.tests.test_remote.TestErrorTranslationRobustness.test_context_missing_a_key
+breezy.tests.test_remote.TestErrorTranslationRobustness.test_path_missing
 breezy.tests.test_remote.TestErrorTranslationRobustness.test_unrecognised_server_error
 breezy.tests.test_remote.TestErrorTranslationSuccess.test_Diverged
 breezy.tests.test_remote.TestErrorTranslationSuccess.test_generic_IndexError_no_classname
@@ -16882,6 +16886,7 @@
 breezy.tests.test_smart.TestSmartServerRepositoryGetRevisionGraph.test_specific_revision_argument
 breezy.tests.test_smart.TestSmartServerRepositoryGetRevisionSignatureText.test_get_signature
 breezy.tests.test_smart.TestSmartServerRepositoryGetSerializerFormat.test_get_serializer_format
+breezy.tests.test_smart.TestSmartServerRepositoryGetStreamForMissingKeys.test_unknown_format
 breezy.tests.test_smart.TestSmartServerRepositoryGetStream.test_ancestry_of
 breezy.tests.test_smart.TestSmartServerRepositoryGetStream.test_search
 breezy.tests.test_smart.TestSmartServerRepositoryGetStream.test_search_everything
@@ -17450,6 +17455,7 @@
 breezy.tests.test_trace.TestTrace.test_report_external_import_error
 breezy.tests.test_trace.TestTrace.test_report_import_syntax_error
 breezy.tests.test_trace.TestTrace.test_show_error
+breezy.tests.test_trace.TestTrace.test_trace_argument_exception
 breezy.tests.test_trace.TestTrace.test_trace_argument_unicode
 breezy.tests.test_trace.TestTrace.test_trace_argument_utf8
 breezy.tests.test_trace.TestTrace.test_trace_unicode