Merge lp:~vila/bzr/2.2-693880-ssl-readline into lp:bzr/2.2

Proposed by Vincent Ladeuil
Status: Merged
Approved by: John A Meinel
Approved revision: no longer in the source branch.
Merge reported by: Vincent Ladeuil
Merged at revision: not available
Proposed branch: lp:~vila/bzr/2.2-693880-ssl-readline
Merge into: lp:bzr/2.2
Diff against target: 46 lines (+16/-9)
2 files modified
NEWS (+3/-0)
bzrlib/transport/http/_urllib2_wrappers.py (+13/-9)
To merge this branch: bzr merge lp:~vila/bzr/2.2-693880-ssl-readline
Reviewer Review Type Date Requested Status
John A Meinel Approve
Jelmer Vernooij (community) code Approve
Review via email: mp+44671@code.launchpad.net

Commit message

Fix https compatibility with python2.7

Description of the change

Bug #693880 broke the workaround describe in the patch (I won't copy it here :).

This came with the dev version that recently landed in natty.

Long story short, this is somewhat critical as it breaks https access, so the sooner we land it the better.

Since the patch is small, I targeted 2.2 so we won't have to backport in a hurry if people upgrade to python2.7 while using a stable version, but I think we may have released 2.3 when the corresponding change is released in python.

The most likely to encounter this problem are early natty adopters, so I'll make sure this get included in 2.3b5 asap and may release it on 2011-01-06 as previously planned.

I've tested this fix on babune again python 2.4, 2.5 and 2.7 and locally on 2.6.

To post a comment you must log in.
Revision history for this message
Jelmer Vernooij (jelmer) wrote :

Thanks for fixing this!

An inline comment explaining why this hack is necessary (and mentioning SSLFile) would be nice.

review: Approve (code)
Revision history for this message
Vincent Ladeuil (vila) wrote :

sent to pqm by email

Revision history for this message
Martin Packman (gz) wrote :

Test? I didn't get any complaint about this code locally, not sure if that means it's untested or if it's not exercised without paramiko or something.

Revision history for this message
John A Meinel (jameinel) wrote :

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 12/24/2010 11:22 AM, Vincent Ladeuil wrote:
> Vincent Ladeuil has proposed merging lp:~vila/bzr/2.2-693880-ssl-readline into lp:bzr/2.2.
>
> Requested reviews:
> bzr-core (bzr-core)
>
> For more details, see:
> https://code.launchpad.net/~vila/bzr/2.2-693880-ssl-readline/+merge/44671
>
> Bug #693880 broke the workaround describe in the patch (I won't copy it here :).
>
> This came with the dev version that recently landed in natty.
>
> Long story short, this is somewhat critical as it breaks https access, so the sooner we land it the better.
>
> Since the patch is small, I targeted 2.2 so we won't have to backport in a hurry if people upgrade to python2.7 while using a stable version, but I think we may have released 2.3 when the corresponding change is released in python.
>
> The most likely to encounter this problem are early natty adopters, so I'll make sure this get included in 2.3b5 asap and may release it on 2011-01-06 as previously planned.
>
> I've tested this fix on babune again python 2.4, 2.5 and 2.7 and locally on 2.6.

It is only missing a comment on why the version check has to exist. (on
version X size is not accepted, in version Y size is being passed by
default.)

Other than that:

 merge: approve

John
=:->

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (Cygwin)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk0iMVoACgkQJdeBCYSNAAMlCACg0J1xjHAittp9QhjqiZTkTfoE
5KwAoKfyTKWeTzSrzaaj2O1cFNkIU193
=rrZm
-----END PGP SIGNATURE-----

review: Approve
Revision history for this message
Vincent Ladeuil (vila) wrote :

sent to pqm by email

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
1=== modified file 'NEWS'
2--- NEWS 2010-12-16 16:03:26 +0000
3+++ NEWS 2010-12-24 17:39:11 +0000
4@@ -32,6 +32,9 @@
5 because this can cause knock-on errors at awkward times.
6 (Andrew Bennetts, #687653)
7
8+* ``https`` access works again with recent versions of python2.7.
9+ (Vincent Ladeuil, #693880)
10+
11 * RevisionTree.is_executable no longer returns None for directories and
12 symlinks. Instead, it returns False, like other Trees and methods.
13 (Aaron Bentley, #681885)
14
15=== modified file 'bzrlib/transport/http/_urllib2_wrappers.py'
16--- bzrlib/transport/http/_urllib2_wrappers.py 2010-10-08 10:09:37 +0000
17+++ bzrlib/transport/http/_urllib2_wrappers.py 2010-12-24 17:39:11 +0000
18@@ -90,15 +90,19 @@
19 self.report_activity(len(s), 'read')
20 return s
21
22- def readline(self):
23- # This should be readline(self, size=-1), but httplib in python 2.4 and
24- # 2.5 defines a SSLFile wrapper whose readline method lacks the size
25- # parameter. So until we drop support for 2.4 and 2.5 and since we
26- # don't *need* the size parameter we'll stay with readline(self)
27- # -- vila 20090209
28- s = self.filesock.readline()
29- self.report_activity(len(s), 'read')
30- return s
31+ # httplib in python 2.4 and 2.5 defines a SSLFile wrapper whose readline
32+ # method lacks the size parameter. python2.6 provides a proper ssl socket
33+ # and added it. python2.7 uses it, forcing us to provide it.
34+ if sys.version_info < (2, 6):
35+ def readline(self):
36+ s = self.filesock.readline()
37+ self.report_activity(len(s), 'read')
38+ return s
39+ else:
40+ def readline(self, size=-1):
41+ s = self.filesock.readline(size)
42+ self.report_activity(len(s), 'read')
43+ return s
44
45 def __getattr__(self, name):
46 return getattr(self.filesock, name)

Subscribers

People subscribed via source and target branches