This is an archive of the discontinued Mercurial Phabricator instance.

mail: modernize check for Python-with-TLS
ClosedPublic

Authored by durin42 on Jul 16 2018, 7:16 PM.

Details

Summary

We used to be going indirectly through the socket module, but now we
just check for the ssl module.

Diff Detail

Repository
rHG Mercurial
Lint
Lint Skipped
Unit
Unit Tests Skipped

Event Timeline

durin42 created this revision.Jul 16 2018, 7:16 PM
yuja added a subscriber: yuja.Jul 17 2018, 9:25 AM

+def _pyhastls():
+ """Returns true iff Python has TLS support, false otherwise."""
+ try:
+ import ssl
+ getattr(ssl, 'HAS_TLS', False)
+ return True
+ except ImportError:
+ return False

Maybe we can simply remove the check since we've dropped support for
Python 2.5.

Maybe we can simply remove the check since we've dropped support for
Python 2.5.

No, we're looking to see if the ssl module is importable - if it's not, that means Python was compiled without TLS support (which is an option, even on 3.x and 2.7). I'm just sniffing for this particular attribute to force the import. Maybe I should add a comment?

yuja added a comment.Jul 18 2018, 7:48 AM
>   Maybe we can simply remove the check since we've dropped support for
>   Python 2.5.
No, we're looking to see if the ssl module is importable - if it's not, that means Python was compiled without TLS support (which is an option, even on 3.x and 2.7). I'm just sniffing for this particular attribute to force the import.

That's true, but I don't think we've ever had support for Python 2.6+ without
OpenSSL. Some commands would work thanks to our demandimport, but it also means
some stdlib modules (e.g. httlib) would be broken because import ssl doesn't
fail.

durin42 updated this revision to Diff 10191.Aug 9 2018, 4:43 PM
durin42 updated this revision to Diff 10195.Aug 9 2018, 4:48 PM
indygreg accepted this revision.Aug 9 2018, 11:00 PM
indygreg added a subscriber: indygreg.

I think this utility function should live in sslutil.py.

mercurial/mail.py
85–92

We may want to consider moving this to sslutil.py and refactoring sslutil.py to not crash and burn if the ssl module doesn't work (which I'm pretty sure it will do today).

This revision is now accepted and ready to land.Aug 9 2018, 11:00 PM
This revision was automatically updated to reflect the committed changes.