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

Proposed by Vincent Ladeuil on 2013-07-13
Status: Merged
Approved by: Vincent Ladeuil on 2013-07-14
Approved revision: 6580
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 2013-07-13 Approve on 2013-07-14
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.
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
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'.

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
1=== modified file 'bzrlib/tests/test_version.py'
2--- bzrlib/tests/test_version.py 2011-05-13 12:51:05 +0000
3+++ bzrlib/tests/test_version.py 2013-07-13 22:40:33 +0000
4@@ -17,12 +17,21 @@
5 """Tests for versioning of bzrlib."""
6
7 from cStringIO import StringIO
8+import platform
9 import re
10
11-from bzrlib import version, workingtree
12-from bzrlib.tests import TestCase, TestSkipped
13-
14-class TestBzrlibVersioning(TestCase):
15+from bzrlib import (
16+ tests,
17+ version,
18+ workingtree,
19+ )
20+from bzrlib.tests.scenarios import load_tests_apply_scenarios
21+
22+
23+load_tests = load_tests_apply_scenarios
24+
25+
26+class TestBzrlibVersioning(tests.TestCase):
27
28 def test_get_bzr_source_tree(self):
29 """Get tree for bzr source, if any."""
30@@ -33,7 +42,8 @@
31 # just assert that it must either return None or the tree.
32 src_tree = version._get_bzr_source_tree()
33 if src_tree is None:
34- raise TestSkipped("bzr tests aren't run from a bzr working tree")
35+ raise tests.TestSkipped(
36+ "bzr tests aren't run from a bzr working tree")
37 else:
38 # ensure that what we got was in fact a working tree instance.
39 self.assertIsInstance(src_tree, workingtree.WorkingTree)
40@@ -47,3 +57,20 @@
41 m = re.search(r"Python interpreter: (.*) [0-9]", out)
42 self.assertIsNot(m, None)
43 self.assertPathExists(m.group(1))
44+
45+class TestPlatformUse(tests.TestCase):
46+
47+ scenarios = [('ascii', dict(_platform='test-platform')),
48+ ('unicode', dict(_platform='Schr\xc3\xb6dinger'))]
49+
50+ def setUp(self):
51+ super(TestPlatformUse, self).setUp()
52+ self.permit_source_tree_branch_repo()
53+
54+ def test_platform(self):
55+ out = self.make_utf8_encoded_stringio()
56+ self.overrideAttr(platform, 'platform', lambda **kwargs: self._platform)
57+ version.show_version(show_config=False, show_copyright=False,
58+ to_file=out)
59+ self.assertContainsRe(out.getvalue(),
60+ r'(?m)^ Platform: %s' % self._platform)
61
62=== modified file 'bzrlib/version.py'
63--- bzrlib/version.py 2011-12-19 13:23:58 +0000
64+++ bzrlib/version.py 2013-07-13 22:40:33 +0000
65@@ -19,6 +19,7 @@
66 from __future__ import absolute_import
67
68 import os
69+import platform
70 import sys
71
72 import bzrlib
73@@ -32,8 +33,6 @@
74
75
76 def show_version(show_config=True, show_copyright=True, to_file=None):
77- import platform
78-
79 if to_file is None:
80 to_file = sys.stdout
81 to_file.write("Bazaar (bzr) %s\n" % bzrlib.__version__)
82@@ -66,7 +65,8 @@
83
84 to_file.write(" Python standard library:" + ' ')
85 to_file.write(os.path.dirname(os.__file__) + '\n')
86- to_file.write(" Platform: %s\n" % platform.platform(aliased=1))
87+ to_file.write(" Platform: %s\n"
88+ % platform.platform(aliased=1).decode('utf-8'))
89 to_file.write(" bzrlib: ")
90 if len(bzrlib.__path__) > 1:
91 # print repr, which is a good enough way of making it clear it's
92
93=== modified file 'doc/en/release-notes/bzr-2.6.txt'
94--- doc/en/release-notes/bzr-2.6.txt 2013-07-09 07:47:49 +0000
95+++ doc/en/release-notes/bzr-2.6.txt 2013-07-13 22:40:33 +0000
96@@ -72,6 +72,10 @@
97 causes breakage with docutils 0.9.1.
98 (Vincent Ladeuil, Jelmer Vernooij, #1066307)
99
100+* Support utf8 characters in platform names even without looking inside the
101+ box (Fedora's Schrödinger's Cat).
102+ (Toshio Kuratomi, Vincent Ladeuil, #1195783)
103+
104 * Warn when ``--show-base`` is used for ``pull`` in a treeless branch
105 instead of failing. It's useless but harmless. (Vincent Ladeuil, #1022160)
106