Merge lp://qastaging/~mvo/ubuntu-release-upgrader/lp1071388 into lp://qastaging/ubuntu-release-upgrader

Proposed by Michael Vogt
Status: Merged
Approved by: Barry Warsaw
Approved revision: 2587
Merged at revision: 2595
Proposed branch: lp://qastaging/~mvo/ubuntu-release-upgrader/lp1071388
Merge into: lp://qastaging/ubuntu-release-upgrader
Diff against target: 147 lines (+37/-10)
3 files modified
DistUpgrade/DistUpgradeView.py (+9/-2)
DistUpgrade/DistUpgradeViewText.py (+16/-8)
tests/test_view.py (+12/-0)
To merge this branch: bzr merge lp://qastaging/~mvo/ubuntu-release-upgrader/lp1071388
Reviewer Review Type Date Requested Status
Barry Warsaw Pending
Review via email: mp+132851@code.qastaging.launchpad.net

Description of the change

This should fix the encoding issue in LP: #1068389 when the users enters non-ascii answers in the text view.

To post a comment you must log in.
Revision history for this message
Michael Vogt (mvo) wrote :

This also needs backporting to the quantal version of the release upgrader.

Revision history for this message
Barry Warsaw (barry) wrote :

A couple of quick thoughts: you might want to use "utf-8" instead of "utf8". It doesn't matter in practice, but better matches what getpreferredencoding() returns. Very minor nit.

Also, in Python 3, we're generally calling subprocess.Popen(..., universal_newlines=True) now so that you can send unicode to the subprocess instead of bytes. That may avoid a call to .encode() below.

http://docs.python.org/3/library/subprocess.html#frequently-used-arguments

decoding the results of sys.stdin.readline() also looks a bit suspect to me, since that should already return str instead of bytes. Are you sure that works? See also `python3 -h` and the PYTHONIOENCODING envar.

Revision history for this message
Michael Vogt (mvo) wrote :

On Wed, Nov 07, 2012 at 08:47:18PM -0000, Barry Warsaw wrote:
> A couple of quick thoughts: you might want to use "utf-8" instead of "utf8". It doesn't matter in practice, but better matches what getpreferredencoding() returns. Very minor nit.

Thanks, I fixed that.

> Also, in Python 3, we're generally calling subprocess.Popen(..., universal_newlines=True) now so that you can send unicode to the subprocess instead of bytes. That may avoid a call to .encode() below.
>
> http://docs.python.org/3/library/subprocess.html#frequently-used-arguments

Thanks, I will have to look in more details, but afaict this is also
because we get the data as bytes sometimes.

> decoding the results of sys.stdin.readline() also looks a bit suspect to me, since that should already return str instead of bytes. Are you sure that works? See also `python3 -h` and the PYTHONIOENCODING envar.

This code runs under py2 most of the time as the upgrade needs to be
support back to precise with the default install there. I.e.:
$ python -c 'import sys; print sys.getdefaultencoding()'
ascii

$ python3 -c 'import sys; print(sys.getdefaultencoding())'
utf-8

And it seems like py2 does not honor PYTHONIOENCODING :/

Cheers,
 Michael

Revision history for this message
Barry Warsaw (barry) wrote :

Michael, have you pushed an update? The diff doesn't look any different.

2588. By Michael Vogt

DistUpgrade/DistUpgradeView.py: fix ENCODING

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.

Subscribers

People subscribed via source and target branches