Bug 1231381 - python-urllib3: requirement of ssl_match_hostname is no longer needed since Python 2.7.9
Summary: python-urllib3: requirement of ssl_match_hostname is no longer needed since P...
Keywords:
Status: CLOSED NEXTRELEASE
Alias: None
Product: Fedora
Classification: Fedora
Component: python-urllib3
Version: 22
Hardware: Unspecified
OS: Unspecified
unspecified
unspecified
Target Milestone: ---
Assignee: Ralph Bean
QA Contact: Fedora Extras Quality Assurance
URL:
Whiteboard:
Depends On:
Blocks: 1229409
TreeView+ depends on / blocked
 
Reported: 2015-06-12 21:44 UTC by Carl George
Modified: 2015-09-21 10:50 UTC (History)
3 users (show)

Fixed In Version: 1.10.4-5.20150503gita91975b.fc23
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2015-09-18 20:51:03 UTC


Attachments (Terms of Use)
unbundle libraries from urllib3 (9.62 KB, patch)
2015-08-03 21:43 UTC, Carl George
no flags Details | Diff

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.


Note You need to log in before you can comment on or make changes to this bug.