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

[Michael Vogt]

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

[Michael Vogt (mvo)]

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

[Barry Warsaw (barry)]

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.

[Michael Vogt (mvo)]

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

[Barry Warsaw (barry)]

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

[lp:~mvo/ubuntu-release-upgrader/lp1071388]

2588. By Michael Vogt on 2012-11-30

DistUpgrade/DistUpgradeView.py: fix ENCODING

=== modified file 'DistUpgrade/DistUpgradeView.py'
--- DistUpgrade/DistUpgradeView.py	2012-10-02 16:53:12 +0000
+++ DistUpgrade/DistUpgradeView.py	2012-11-30 17:18:23 +0000
@@ -36,6 +36,14 @@
 from .DistUpgradeApport import apport_pkgfailure
 
 
+try:
+    locale.setlocale(locale.LC_ALL, "")
+    (code, ENCODING) = locale.getdefaultlocale()
+except:
+    logging.exception("getting the encoding failed")
+    ENCODING = "utf-8"   #pyflakes
+
+
 def FuzzyTimeToStr(sec):
   " return the time a bit fuzzy (no seconds if time > 60 secs "
   #print("FuzzyTimeToStr: ", sec)
@@ -373,8 +381,7 @@
         if downloadSize > 0:
           downloadSizeStr = apt_pkg.size_to_str(downloadSize)
           if isinstance(downloadSizeStr, bytes):
-              downloadSizeStr = downloadSizeStr.decode(
-                  locale.getpreferredencoding())
+              downloadSizeStr = downloadSizeStr.decode(ENCODING)
           msg += _("\n\nYou have to download a total of %s. ") % (
               downloadSizeStr)
           msg += self.getAcquireProgress().estimatedDownloadTime(downloadSize)

=== modified file 'DistUpgrade/DistUpgradeViewText.py'
--- DistUpgrade/DistUpgradeViewText.py	2012-10-19 07:17:35 +0000
+++ DistUpgrade/DistUpgradeViewText.py	2012-11-30 17:18:23 +0000
@@ -28,13 +28,19 @@
 import apt
 import os
 
-from .DistUpgradeView import DistUpgradeView, InstallProgress, AcquireProgress
+from .DistUpgradeView import (
+    AcquireProgress,
+    DistUpgradeView,
+    ENCODING,
+    InstallProgress,
+    )
 import apt.progress
 
 import gettext
 from .DistUpgradeGettext import gettext as _
 from .utils import twrap
 
+
 class TextAcquireProgress(AcquireProgress, apt.progress.text.AcquireProgress):
     def __init__(self):
         apt.progress.text.AcquireProgress.__init__(self)
@@ -44,6 +50,7 @@
         AcquireProgress.pulse(self, owner)
         return True
 
+
 class TextCdromProgressAdapter(apt.progress.base.CdromProgress):
     """ Report the cdrom add progress  """
     def update(self, text, step):
@@ -57,7 +64,8 @@
 
 
 class DistUpgradeViewText(DistUpgradeView):
-    " text frontend of the distUpgrade tool "
+    """ text frontend of the distUpgrade tool """
+
     def __init__(self, datadir=None, logdir=None):
         # indicate that we benefit from using gnu screen
         self.needs_screen = True
@@ -127,7 +135,7 @@
       if extended_msg:
         print(twrap(extended_msg))
       print(_("To continue please press [ENTER]"))
-      sys.stdin.readline()
+      sys.stdin.readline().decode(ENCODING, "backslashreplace")
     def error(self, summary, msg, extended_msg=None):
       print()
       print(twrap(summary))
@@ -137,10 +145,10 @@
       return False
     def showInPager(self, output):
       """ helper to show output in a pager """
-      # we need to send a utf8 encode str (bytes in py3) to the pipe
+      # we need to send a encoded str (bytes in py3) to the pipe
       # LP: #1068389
       if not isinstance(output, bytes):
-          output = output.encode("utf-8")
+          output = output.encode(ENCODING)
       for pager in ["/usr/bin/sensible-pager", "/bin/more"]:
           if os.path.exists(pager):
               p = subprocess.Popen([pager,"-"],stdin=subprocess.PIPE)
@@ -161,7 +169,7 @@
       print(" %s %s" % (_("Continue [yN] "), _("Details [d]")), end="")
       sys.stdout.flush()
       while True:
-        res = sys.stdin.readline()
+        res = sys.stdin.readline().decode(ENCODING, "backslashreplace")
         # TRANSLATORS: the "y" is "yes"
         if res.strip().lower().startswith(_("y")):
           return True
@@ -205,14 +213,14 @@
       print(twrap(msg))
       if default == 'No':
           print(_("Continue [yN] "), end="")
-          res = sys.stdin.readline()
+          res = sys.stdin.readline().decode(ENCODING, "backslashreplace")
           # TRANSLATORS: first letter of a positive (yes) answer
           if res.strip().lower().startswith(_("y")):
               return True
           return False
       else:
           print(_("Continue [Yn] "), end="")
-          res = sys.stdin.readline()
+          res = sys.stdin.readline().decode(ENCODING, "backslashreplace")
           # TRANSLATORS: first letter of a negative (no) answer
           if res.strip().lower().startswith(_("n")):
               return False

=== modified file 'tests/test_view.py'
--- tests/test_view.py	2012-11-09 12:50:59 +0000
+++ tests/test_view.py	2012-11-30 17:18:23 +0000
@@ -7,11 +7,23 @@
 import tempfile
 import unittest
 
+from mock import patch
+
 from DistUpgrade.DistUpgradeViewText import DistUpgradeViewText
 
 
 class TestDistUpradeView(unittest.TestCase):
 
+    def test_prompt_with_unicode_lp1071388(self):
+        with tempfile.TemporaryFile() as f:
+            f.write("some unicode: ä".encode("utf-8"))
+            f.flush()
+            f.seek(0)
+            with patch("sys.stdin", f):
+                v = DistUpgradeViewText()
+                res = v.askYesNoQuestion("Some text", "some more")
+                self.assertEqual(res, False)
+
     def test_show_in_pager_lp1068389(self):
         """Regression test for LP: #1068389"""
         output = tempfile.NamedTemporaryFile()