Bug 1231381

Summary: python-urllib3: requirement of ssl_match_hostname is no longer needed since Python 2.7.9
Product: [Fedora] Fedora Reporter: Carl George <carl>
Component: python-urllib3Assignee: Ralph Bean <rbean>
Status: CLOSED NEXTRELEASE QA Contact: Fedora Extras Quality Assurance <extras-qa>
Severity: unspecified Docs Contact:
Priority: unspecified    
Version: 22CC: carl, rbean, sagarun
Target Milestone: ---   
Target Release: ---   
Hardware: Unspecified   
OS: Unspecified   
Whiteboard:
Fixed In Version: 1.10.4-5.20150503gita91975b.fc23 Doc Type: Bug Fix
Doc Text:
Story Points: ---
Clone Of: Environment:
Last Closed: 2015-09-18 20:51:03 UTC Type: Bug
Regression: --- Mount Type: ---
Documentation: --- CRM:
Verified Versions: Category: ---
oVirt Team: --- RHEL 7.3 requirements from Atomic Host:
Cloudforms Team: --- Target Upstream Version:
Bug Depends On:    
Bug Blocks: 1229409    
Attachments:
Description Flags
unbundle libraries from urllib3 none

Description Carl George 2015-06-12 21:44:39 UTC
===== Description of problem =====

Python 3.4's entire ssl module was backported to 2.7.9 [1].  Therefore, this package doesn't technically need to depend on python-backports-ssl_match_hostname.  However, there is some clean up that must be done before removing the dependency in the spec file.

Upstream urllib3 has a shim file (packages/ssl_match_hostname/__init__.py) that attempts to import CertificateError and match_hostname from the following locations, in this order:

* the system standard library ssl module
* the system backports.ssl_match_hostname module
* the bundled packages/ssl_match_hostname/_implementation.py module

Fedora currently deletes the entire packages directory (including the shim) [3] and sets up symlinks to the system backports.ssl_match_hostname module [4].  I think a better solution would be to fix the imports throughout the rest of the code to just simply point to the system standard library ssl module.  Here are the places I know would need to be patched.

test/test_connectionpool.py:
-from urllib3.packages.ssl_match_hostname import CertificateError
+from ssl import CertificateError

urllib3/connection.py:
-from .packages.ssl_match_hostname import match_hostname
+from ssl import match_hostname

urllib3/connectionpool.py:
-from .packages.ssl_match_hostname import CertificateError
+from ssl import CertificateError

Then no symlinks would be needed, and we could remove the Requires and BuildRequires on python-backports-ssl_match_hostname in the spec file.

===== Version-Release number of selected component (if applicable) =====

python-urllib3-1.10.4-2.20150503gita91975b.fc22

===== Actual results =====

The RPM has an unnecessary dependency.

===== Expected results =====

The RPM only depends on things it actually needs.

===== Additional info =====

[1] https://www.python.org/downloads/release/python-279/
[2] https://github.com/shazow/urllib3/blob/1.10.4/urllib3/packages/ssl_match_hostname/__init__.py#L1-L10
[3] http://pkgs.fedoraproject.org/cgit/python-urllib3.git/tree/python-urllib3.spec#n124
[4] http://pkgs.fedoraproject.org/cgit/python-urllib3.git/tree/python-urllib3.spec#n130

Comment 1 Carl George 2015-08-03 21:43:05 UTC
Created attachment 1058893 [details]
unbundle libraries from urllib3

Here is my attempt at an unbundling patch.  It does require some changes to the spec file.

* apply the patch, of course
* add a line to delete urllib3/packages during %prep
* remove all the manipulation of the packages directory and files during %install and %check (deleting individual files, symlinks, copying, etc.)

I have a working spec file that updates it to 1.11 as well.  The only other change I had to make was to always apply Patch100, because otherwise I get the error:

No handlers could be found for logger "nose.plugins.cover"

Comment 2 Carl George 2015-09-02 14:12:35 UTC
Can I get feedback on this proposed change?  I would like to see this change made in time for F23.  For convenience, here is the completed spec file and new patches.  It builds successfully in mock.

https://carlgeorge.fedorapeople.org/python-urllib3/python-urllib3.spec
https://carlgeorge.fedorapeople.org/python-urllib3/python-urllib3-unbundle-import-fixes.patch
https://carlgeorge.fedorapeople.org/python-urllib3/python-urllib3-skip-test_no_ssl.patch

Comment 3 Ralph Bean 2015-09-02 15:11:09 UTC
(In reply to Carl George from comment #2)
> Can I get feedback on this proposed change?  I would like to see this change
> made in time for F23.  For convenience, here is the completed spec file and
> new patches.  It builds successfully in mock.

Carl, thank you for working on this.  I'm so sorry I let this languish for so long without responding.  It's been a busy summer.

(In reply to Carl George from comment #0)
> Fedora currently deletes the entire packages directory (including the shim)
> [3] and sets up symlinks to the system backports.ssl_match_hostname module
> [4].  I think a better solution would be to fix the imports throughout the
> rest of the code to just simply point to the system standard library ssl
> module.

That's the way we originally did it with both python-requests and python-urllib3, but upstream was quite unfriendly to the idea of us applying patches and changing their imports.  Although I can't see how it would be the case with python-six, some people are using other code that is manipulating the bundled packages at runtime (monkey-patching), and when we swapped out the imports, their monkey-patches stopped working.

We finally came to an agreement with upstream that symlinking would be best for them.. and it in turn ends up being nice for the package maintainer as well.  Everytime they do a new release, we don't have to rebase our patches of their import statements -- the symlinks just do the work.

https://twitter.com/sigmavirus24/status/529816751651819520

I'm friendly to making the ssl swap on F22 and greater, but if we could do it with symlinks alone, that would be preferred.  Perhaps we can replace the urllib3/packages/ssl_match_hostname.py module with a symlink to the stdlib ssl module.  While a little dishonest (it's more than just the match_hostname module), it seems like everything else should still work.

I'll ping upstream on this to see if they have an opinion.

Comment 4 Ralph Bean 2015-09-02 15:16:04 UTC
Notified upstream:  https://github.com/shazow/urllib3/issues/702

Comment 5 Fedora Update System 2015-09-08 17:37:37 UTC
python-urllib3-1.10.4-5.20150503gita91975b.fc23 has been submitted as an update to Fedora 23. https://bodhi.fedoraproject.org/updates/FEDORA-2015-15377

Comment 6 Fedora Update System 2015-09-08 17:39:25 UTC
python-urllib3-1.10.4-5.20150503gita91975b.fc22 has been submitted as an update to Fedora 22. https://bodhi.fedoraproject.org/updates/FEDORA-2015-15378

Comment 7 Fedora Update System 2015-09-08 21:25:25 UTC
python-urllib3-1.10.4-5.20150503gita91975b.fc22 has been pushed to the Fedora 22 testing repository. If problems still persist, please make note of it in this bug report.\nIf you want to test the update, you can install it with \n su -c 'yum --enablerepo=updates-testing update python-urllib3'. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2015-15378

Comment 8 Fedora Update System 2015-09-10 05:52:15 UTC
python-urllib3-1.10.4-5.20150503gita91975b.fc23 has been pushed to the Fedora 23 testing repository. If problems still persist, please make note of it in this bug report.\nIf you want to test the update, you can install it with \n su -c 'yum --enablerepo=updates-testing update python-urllib3'. You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2015-15377

Comment 9 Fedora Update System 2015-09-18 20:51:01 UTC
python-urllib3-1.10.4-5.20150503gita91975b.fc22 has been pushed to the Fedora 22 stable repository. If problems still persist, please make note of it in this bug report.

Comment 10 Fedora Update System 2015-09-21 10:50:24 UTC
python-urllib3-1.10.4-5.20150503gita91975b.fc23 has been pushed to the Fedora 23 stable repository. If problems still persist, please make note of it in this bug report.