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

Proposed by Steve Langasek on 2013-05-28
Status: Merged
Merge reported by: Steve Langasek
Merged at revision: not available
Proposed branch: lp:~vorlon/python-apt/lp.1069019
Merge into: lp:~ubuntu-core-dev/python-apt/ubuntu
Diff against target: 102 lines (+23/-16)
2 files modified
aptsources/sourceslist.py (+17/-16)
debian/changelog (+6/-0)
To merge this branch: bzr merge lp:~vorlon/python-apt/lp.1069019
Reviewer Review Type Date Requested Status
Brian Murray 2013-05-28 Approve on 2014-03-14
Barry Warsaw 2013-05-28 Pending
Review via email: mp+166148@code.launchpad.net

Description of the change

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?

To post a comment you must log in.
Barry Warsaw (barry) wrote :

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) wrote :

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) wrote :

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) wrote :

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) wrote :

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) wrote :

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.)

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'aptsources/sourceslist.py'
2--- aptsources/sourceslist.py 2012-11-19 13:22:42 +0000
3+++ aptsources/sourceslist.py 2013-05-28 22:27:26 +0000
4@@ -31,6 +31,7 @@
5 import re
6 import shutil
7 import time
8+import io
9
10 import apt_pkg
11 from .distinfo import DistInfo
12@@ -218,20 +219,20 @@
13 """ return the current line as string """
14 if self.invalid:
15 return self.line
16- line = ""
17+ line = u""
18 if self.disabled:
19- line = "# "
20+ line = u"# "
21
22 line += self.type
23
24 if self.architectures:
25- line += " [arch=%s]" % ",".join(self.architectures)
26- line += " %s %s" % (self.uri, self.dist)
27+ line += u" [arch=%s]" % u",".join(self.architectures)
28+ line += u" %s %s" % (self.uri, self.dist)
29 if len(self.comps) > 0:
30- line += " " + " ".join(self.comps)
31- if self.comment != "":
32- line += " #"+self.comment
33- line += "\n"
34+ line += u" " + u" ".join(self.comps)
35+ if self.comment != u"":
36+ line += u" #"+self.comment
37+ line += u"\n"
38 return line
39
40
41@@ -322,13 +323,13 @@
42 # there isn't any matching source, so create a new line and parse it
43 line = type
44 if architectures:
45- line += " [arch=%s]" % ",".join(architectures)
46- line += " %s %s" % (uri, dist)
47+ line += u" [arch=%s]" % u",".join(architectures)
48+ line += u" %s %s" % (uri, dist)
49 for c in comps:
50- line = line + " " + c
51+ line = line + u" " + c
52 if comment != "":
53- line = "%s #%s\n" % (line, comment)
54- line = line + "\n"
55+ line = u"%s #%s\n" % (line, comment)
56+ line = line + u"\n"
57 new_entry = SourceEntry(line)
58 if file is not None:
59 new_entry.file = file
60@@ -370,7 +371,7 @@
61 def load(self, file):
62 """ (re)load the current sources """
63 try:
64- with open(file, "r") as f:
65+ with io.open(file, "r", encoding="utf-8") as f:
66 for line in f:
67 source = SourceEntry(line, file)
68 self.list.append(source)
69@@ -388,14 +389,14 @@
70 "# Remember that you can only use http, ftp or file URIs\n"
71 "# CDROMs are managed through the apt-cdrom tool.\n")
72
73- with open(path, "w") as f:
74+ with io.open(path, "w", encoding="utf-8") as f:
75 f.write(header)
76 return
77
78 try:
79 for source in self.list:
80 if source.file not in files:
81- files[source.file] = open(source.file, "w")
82+ files[source.file] = io.open(source.file, "w", encoding="utf-8")
83 files[source.file].write(source.str())
84 finally:
85 for f in files:
86
87=== modified file 'debian/changelog'
88--- debian/changelog 2013-04-30 17:01:20 +0000
89+++ debian/changelog 2013-05-28 22:27:26 +0000
90@@ -4,6 +4,12 @@
91 - export codename in apt.package.Origin as well
92 (closes: #703401)
93
94+ [ Steve Langasek ]
95+ * aptsources/sourceslist.py: ensure that our sources are opened with UTF-8
96+ encoding, regardless of the current locale, and handle the sources lines
97+ as Unicode internally for consistency between python2 and python3.
98+ LP: #1069019.
99+
100 -- Michael Vogt <michael.vogt@ubuntu.com> Tue, 30 Apr 2013 19:00:44 +0200
101
102 python-apt (0.8.8.2ubuntu1) saucy; urgency=low

Subscribers

People subscribed via source and target branches

to all changes: