Merge lp:~vila/bzr/1195783-platform-utf8 into lp:bzr

Proposed by Vincent Ladeuil
Status: Merged
Approved by: Vincent Ladeuil
Approved revision: no longer in the source branch.
Merged at revision: 6581
Proposed branch: lp:~vila/bzr/1195783-platform-utf8
Merge into: lp:bzr
Diff against target: 105 lines (+39/-8)
3 files modified
bzrlib/tests/test_version.py (+32/-5)
bzrlib/version.py (+3/-3)
doc/en/release-notes/bzr-2.6.txt (+4/-0)
To merge this branch: bzr merge lp:~vila/bzr/1195783-platform-utf8
Reviewer Review Type Date Requested Status
John A Meinel Approve
Review via email: mp+174591@code.launchpad.net

Commit message

Make 'bzr version' support utf8 platform names.

Description of the change

This fixes http://pad.lv/1195783 by considering that platform names are utf8
encoded (see discussion on the bug report).

To post a comment you must log in.
Revision history for this message
John A Meinel (jameinel) wrote :

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 2013-07-14 2:41, Vincent Ladeuil wrote:
> Vincent Ladeuil has proposed merging
> lp:~vila/bzr/1195783-platform-utf8 into lp:bzr.
>
> Requested reviews: bzr-core (bzr-core) Related bugs: Bug #1195783
> in Bazaar: "UnicodeDecodeError from bzr version if platform
> contains non-ASCII" https://bugs.launchpad.net/bzr/+bug/1195783
>
> For more details, see:
> https://code.launchpad.net/~vila/bzr/1195783-platform-utf8/+merge/174591
>
> This fixes http://pad.lv/1195783 by considering that platform
> names are utf8 encoded (see discussion on the bug report).
>

Do we want to add a test that we do a best-effort on names that aren't
actually utf-8 but aren't ascii either? (The normal we did was utf8,
iso-8859-1, but I would be fine with just decode(, 'replace') here).

 review: approve

John
=:->
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.13 (Cygwin)
Comment: Using GnuPG with Thunderbird - http://www.enigmail.net/

iEYEARECAAYFAlHiY44ACgkQJdeBCYSNAANRmQCgxZnGbdMhdSGYSFD0ZH3PkHHi
gnkAniIcFz8JPGbsRhD8gDgP5nblMASG
=q2dl
-----END PGP SIGNATURE-----

review: Approve
Revision history for this message
Vincent Ladeuil (vila) wrote :

> Do we want to add a test that we do a best-effort on names that aren't
> actually utf-8 but aren't ascii either? (The normal we did was utf8,
> iso-8859-1, but I would be fine with just decode(, 'replace') here).

Fedora had some fun adding such a distribution name but it's still a distribution name, not some random file anybody can put garbage into. Time will tell but I doubt we'll encounter a case where an invalid encoding requires 'replace'.

Revision history for this message
Vincent Ladeuil (vila) wrote :

sent to pqm by email

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'bzrlib/tests/test_version.py'
--- bzrlib/tests/test_version.py 2011-05-13 12:51:05 +0000
+++ bzrlib/tests/test_version.py 2013-07-13 22:40:33 +0000
@@ -17,12 +17,21 @@
17"""Tests for versioning of bzrlib."""17"""Tests for versioning of bzrlib."""
1818
19from cStringIO import StringIO19from cStringIO import StringIO
20import platform
20import re21import re
2122
22from bzrlib import version, workingtree23from bzrlib import (
23from bzrlib.tests import TestCase, TestSkipped24 tests,
2425 version,
25class TestBzrlibVersioning(TestCase):26 workingtree,
27 )
28from bzrlib.tests.scenarios import load_tests_apply_scenarios
29
30
31load_tests = load_tests_apply_scenarios
32
33
34class TestBzrlibVersioning(tests.TestCase):
2635
27 def test_get_bzr_source_tree(self):36 def test_get_bzr_source_tree(self):
28 """Get tree for bzr source, if any."""37 """Get tree for bzr source, if any."""
@@ -33,7 +42,8 @@
33 # just assert that it must either return None or the tree.42 # just assert that it must either return None or the tree.
34 src_tree = version._get_bzr_source_tree()43 src_tree = version._get_bzr_source_tree()
35 if src_tree is None:44 if src_tree is None:
36 raise TestSkipped("bzr tests aren't run from a bzr working tree")45 raise tests.TestSkipped(
46 "bzr tests aren't run from a bzr working tree")
37 else:47 else:
38 # ensure that what we got was in fact a working tree instance.48 # ensure that what we got was in fact a working tree instance.
39 self.assertIsInstance(src_tree, workingtree.WorkingTree)49 self.assertIsInstance(src_tree, workingtree.WorkingTree)
@@ -47,3 +57,20 @@
47 m = re.search(r"Python interpreter: (.*) [0-9]", out)57 m = re.search(r"Python interpreter: (.*) [0-9]", out)
48 self.assertIsNot(m, None)58 self.assertIsNot(m, None)
49 self.assertPathExists(m.group(1))59 self.assertPathExists(m.group(1))
60
61class TestPlatformUse(tests.TestCase):
62
63 scenarios = [('ascii', dict(_platform='test-platform')),
64 ('unicode', dict(_platform='Schr\xc3\xb6dinger'))]
65
66 def setUp(self):
67 super(TestPlatformUse, self).setUp()
68 self.permit_source_tree_branch_repo()
69
70 def test_platform(self):
71 out = self.make_utf8_encoded_stringio()
72 self.overrideAttr(platform, 'platform', lambda **kwargs: self._platform)
73 version.show_version(show_config=False, show_copyright=False,
74 to_file=out)
75 self.assertContainsRe(out.getvalue(),
76 r'(?m)^ Platform: %s' % self._platform)
5077
=== modified file 'bzrlib/version.py'
--- bzrlib/version.py 2011-12-19 13:23:58 +0000
+++ bzrlib/version.py 2013-07-13 22:40:33 +0000
@@ -19,6 +19,7 @@
19from __future__ import absolute_import19from __future__ import absolute_import
2020
21import os21import os
22import platform
22import sys23import sys
2324
24import bzrlib25import bzrlib
@@ -32,8 +33,6 @@
3233
3334
34def show_version(show_config=True, show_copyright=True, to_file=None):35def show_version(show_config=True, show_copyright=True, to_file=None):
35 import platform
36
37 if to_file is None:36 if to_file is None:
38 to_file = sys.stdout37 to_file = sys.stdout
39 to_file.write("Bazaar (bzr) %s\n" % bzrlib.__version__)38 to_file.write("Bazaar (bzr) %s\n" % bzrlib.__version__)
@@ -66,7 +65,8 @@
6665
67 to_file.write(" Python standard library:" + ' ')66 to_file.write(" Python standard library:" + ' ')
68 to_file.write(os.path.dirname(os.__file__) + '\n')67 to_file.write(os.path.dirname(os.__file__) + '\n')
69 to_file.write(" Platform: %s\n" % platform.platform(aliased=1))68 to_file.write(" Platform: %s\n"
69 % platform.platform(aliased=1).decode('utf-8'))
70 to_file.write(" bzrlib: ")70 to_file.write(" bzrlib: ")
71 if len(bzrlib.__path__) > 1:71 if len(bzrlib.__path__) > 1:
72 # print repr, which is a good enough way of making it clear it's72 # print repr, which is a good enough way of making it clear it's
7373
=== modified file 'doc/en/release-notes/bzr-2.6.txt'
--- doc/en/release-notes/bzr-2.6.txt 2013-07-09 07:47:49 +0000
+++ doc/en/release-notes/bzr-2.6.txt 2013-07-13 22:40:33 +0000
@@ -72,6 +72,10 @@
72 causes breakage with docutils 0.9.1.72 causes breakage with docutils 0.9.1.
73 (Vincent Ladeuil, Jelmer Vernooij, #1066307)73 (Vincent Ladeuil, Jelmer Vernooij, #1066307)
7474
75* Support utf8 characters in platform names even without looking inside the
76 box (Fedora's Schrödinger's Cat).
77 (Toshio Kuratomi, Vincent Ladeuil, #1195783)
78
75* Warn when ``--show-base`` is used for ``pull`` in a treeless branch79* Warn when ``--show-base`` is used for ``pull`` in a treeless branch
76 instead of failing. It's useless but harmless. (Vincent Ladeuil, #1022160)80 instead of failing. It's useless but harmless. (Vincent Ladeuil, #1022160)
7781