Merge lp:~vorlon/python-apt/lp.1069019 into lp:~ubuntu-core-dev/python-apt/ubuntu

[Steve Langasek]

Possible fix for bug #1069019, using io.open() and enforcing use of Unicode
internally for the handling of the sources.list entries. Does this look
sane?

[Barry Warsaw (barry)]

I think I still prefer using codecs.open() instead of io.open(), but mostly because the former is more common usage. It probably ultimately doesn't matter, and when Python 2 is dropped, either module prefix can be dropped (but neither hurt in either case).

Any chance you can come up with a test case for this bug?

[Brian Murray (brian-murray)]

I tried recreating the bug by creating a sources.list entry with unicode in the comment (e.g. # disabled‽). Using the version of python-apt in saucy I was unable to disable this line or actually any other line in sources.list.

I then built an update version of the saucy package using Steve's patch in this branch and I was still not able to disable any line in my sources.list file. So I don't think this patch is sufficient.

[Brian Murray (brian-murray)]

I tested this again following the steps in the above comment, but this time I rebooted after installing python-apt, python3-apt and python-apt-common. After the reboot I am then able to enable and disable sources in software-properties.

[Brian Murray (brian-murray)]

I don't think we should keep holding out for a test case, especially given bug 1278280 regarding the release-upgrader (which includes sourceslist.py from python-apt) crashing. With the errors associated with that bug we should have positive confirmation that fix is working rather quickly.

I'll go ahead and take care of merging and uploading this.

review: Approve

[Brian Murray (brian-murray)]

I've uploaded this to trusty, but the python-apt branch is out of date so I'm not sure what to do with the mp.

[Steve Langasek (vorlon)]

It looks like in the time since this MP was raised, the python-apt upstream branch moved from Launchpad to git.debian.org, and since then the package was in sync with Debian. So let's close this out as "merged" and not worry about the ~ubuntu-core-dev branch. (Ideally we would not list Vcs-Git for this package as pointing to debian since there's no Ubuntu branch there, but that's secondary.)

=== modified file 'aptsources/sourceslist.py'
--- aptsources/sourceslist.py	2012-11-19 13:22:42 +0000
+++ aptsources/sourceslist.py	2013-05-28 22:27:26 +0000
@@ -31,6 +31,7 @@
 import re
 import shutil
 import time
+import io
 
 import apt_pkg
 from .distinfo import DistInfo
@@ -218,20 +219,20 @@
         """ return the current line as string """
         if self.invalid:
             return self.line
-        line = ""
+        line = u""
         if self.disabled:
-            line = "# "
+            line = u"# "
 
         line += self.type
 
         if self.architectures:
-            line += " [arch=%s]" % ",".join(self.architectures)
-        line += " %s %s" % (self.uri, self.dist)
+            line += u" [arch=%s]" % u",".join(self.architectures)
+        line += u" %s %s" % (self.uri, self.dist)
         if len(self.comps) > 0:
-            line += " " + " ".join(self.comps)
-        if self.comment != "":
-            line += " #"+self.comment
-        line += "\n"
+            line += u" " + u" ".join(self.comps)
+        if self.comment != u"":
+            line += u" #"+self.comment
+        line += u"\n"
         return line
 
 
@@ -322,13 +323,13 @@
         # there isn't any matching source, so create a new line and parse it
         line = type
         if architectures:
-            line += " [arch=%s]" % ",".join(architectures)
-        line += " %s %s" % (uri, dist)
+            line += u" [arch=%s]" % u",".join(architectures)
+        line += u" %s %s" % (uri, dist)
         for c in comps:
-            line = line + " " + c
+            line = line + u" " + c
         if comment != "":
-            line = "%s #%s\n" % (line, comment)
-        line = line + "\n"
+            line = u"%s #%s\n" % (line, comment)
+        line = line + u"\n"
         new_entry = SourceEntry(line)
         if file is not None:
             new_entry.file = file
@@ -370,7 +371,7 @@
     def load(self, file):
         """ (re)load the current sources """
         try:
-            with open(file, "r") as f:
+            with io.open(file, "r", encoding="utf-8") as f:
                 for line in f:
                     source = SourceEntry(line, file)
                     self.list.append(source)
@@ -388,14 +389,14 @@
                 "# Remember that you can only use http, ftp or file URIs\n"
                 "# CDROMs are managed through the apt-cdrom tool.\n")
 
-            with open(path, "w") as f:
+            with io.open(path, "w", encoding="utf-8") as f:
                 f.write(header)
             return
 
         try:
             for source in self.list:
                 if source.file not in files:
-                    files[source.file] = open(source.file, "w")
+                    files[source.file] = io.open(source.file, "w", encoding="utf-8")
                 files[source.file].write(source.str())
         finally:
             for f in files:

=== modified file 'debian/changelog'
--- debian/changelog	2013-04-30 17:01:20 +0000
+++ debian/changelog	2013-05-28 22:27:26 +0000
@@ -4,6 +4,12 @@
     - export codename in apt.package.Origin as well
       (closes: #703401)
 
+  [ Steve Langasek ]
+  * aptsources/sourceslist.py: ensure that our sources are opened with UTF-8
+    encoding, regardless of the current locale, and handle the sources lines
+    as Unicode internally for consistency between python2 and python3.
+    LP: #1069019.
+
  -- Michael Vogt <[e-mail]>  Tue, 30 Apr 2013 19:00:44 +0200
 
 python-apt (0.8.8.2ubuntu1) saucy; urgency=low