Merge lp:~sil2100/ubuntu-archive-tools/plus-sign-review into lp:ubuntu-archive-tools

Proposed by Łukasz Zemczak
Status: Merged
Merged at revision: 1114
Proposed branch: lp:~sil2100/ubuntu-archive-tools/plus-sign-review
Merge into: lp:ubuntu-archive-tools
Diff against target: 33 lines (+3/-4)
1 file modified
sru-review (+3/-4)
To merge this branch: bzr merge lp:~sil2100/ubuntu-archive-tools/plus-sign-review
Reviewer Review Type Date Requested Status
Ubuntu Package Archive Administrators Pending
Review via email: mp+328449@code.launchpad.net

Commit message

Stop urlencoding source package name in sru-review when trying to access the debdiff as it makes it impossible to review packages with + in the name.

Description of the change

Do we really need to urlencode the source package name? In the current state it just makes the script not be able to access the debdiff, e.g.:
http://launchpadlibrarian.net/330880566/gtk+2.0_2.24.30-1ubuntu1.16.04.1_2.24.30-1ubuntu1.16.04.2.diff.gz

With quote() being used, the regexp cannot find the link in the queue webpage contents as the + sign is urlencoded. I wonder why this was added in the first place? Did something change in how links to the librarian are used?

To post a comment you must log in.
1115. By Łukasz Zemczak

Remove whitespace.

Revision history for this message
Colin Watson (cjwatson) wrote :

FWIW, the thing that changed was https://code.launchpad.net/~wgrant/launchpad/bug-677270/+merge/279974. Strictly speaking, to match that you ought to use quote(sourcepkg, safe='/~+'), but it makes no difference since there are no longer any characters that are legal in source package names and that are escaped in librarian URLs.

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'sru-review'
--- sru-review 2017-06-30 15:21:47 +0000
+++ sru-review 2017-08-02 14:40:44 +0000
@@ -37,10 +37,9 @@
37import sys37import sys
38import tempfile38import tempfile
39try:39try:
40 from urllib.parse import quote
41 from urllib.request import urlopen, urlretrieve40 from urllib.request import urlopen, urlretrieve
42except ImportError:41except ImportError:
43 from urllib import quote, urlopen, urlretrieve42 from urllib import urlopen, urlretrieve
44import webbrowser43import webbrowser
4544
46from launchpadlib.launchpad import Launchpad45from launchpadlib.launchpad import Launchpad
@@ -158,7 +157,7 @@
158 debdiff_re = re.compile(157 debdiff_re = re.compile(
159 'href="(http://launchpadlibrarian.net/'158 'href="(http://launchpadlibrarian.net/'
160 '\d+/%s_[^"_]+_[^_"]+\.diff\.gz)">\s*diff from' %159 '\d+/%s_[^"_]+_[^_"]+\.diff\.gz)">\s*diff from' %
161 re.escape(quote(sourcepkg)))160 re.escape(sourcepkg))
162161
163 queue_html = urlopen(queue_url).read()162 queue_html = urlopen(queue_url).read()
164163
@@ -186,7 +185,7 @@
186 '''185 '''
187 changes_re = re.compile(186 changes_re = re.compile(
188 'href="(https://launchpad.net/[^ "]+/%s_[^"]+_source.changes)"' %187 'href="(https://launchpad.net/[^ "]+/%s_[^"]+_source.changes)"' %
189 re.escape(quote(sourcepkg)))188 re.escape(sourcepkg))
190 sourcepub_re = re.compile(189 sourcepub_re = re.compile(
191 'href="(\+sourcepub/\d+/\+listing-archive-extra)"')190 'href="(\+sourcepub/\d+/\+listing-archive-extra)"')
192 #debdiff_re = re.compile(191 #debdiff_re = re.compile(

Subscribers

People subscribed via source and target branches