Patchwork [05,of,15] hgweb: avoid using `sslutil.modernssl`

login
register
mail settings
Submitter Manuel Jacob
Date May 30, 2020, 5:52 a.m.
Message ID <f8ae379a8dcfebbec6cf.1590817937@tmp>
Download mbox | patch
Permalink /patch/46400/
State New
Headers show

Comments

Manuel Jacob - May 30, 2020, 5:52 a.m.
# HG changeset patch
# User Manuel Jacob <me@manueljacob.de>
# Date 1590807553 -7200
#      Sat May 30 04:59:13 2020 +0200
# Node ID f8ae379a8dcfebbec6cf6570d8d38fa1e3b6bcea
# Parent  fa4460229a8d1a392564d0cbe78216760154822c
# EXP-Topic require_modern_ssl
hgweb: avoid using `sslutil.modernssl`

`sslutil.modernssl` is going to be removed. Since the point of using this
attribute was to check the importability of the `sslutil`, a different
attribute can be used. `sslutil.wrapserversocket` is used because it’s anyway
used a few lines below.
Yuya Nishihara - May 30, 2020, 12:58 p.m.
On Sat, 30 May 2020 07:52:17 +0200, Manuel Jacob wrote:
> # HG changeset patch
> # User Manuel Jacob <me@manueljacob.de>
> # Date 1590807553 -7200
> #      Sat May 30 04:59:13 2020 +0200
> # Node ID f8ae379a8dcfebbec6cf6570d8d38fa1e3b6bcea
> # Parent  fa4460229a8d1a392564d0cbe78216760154822c
> # EXP-Topic require_modern_ssl
> hgweb: avoid using `sslutil.modernssl`
> 
> `sslutil.modernssl` is going to be removed. Since the point of using this
> attribute was to check the importability of the `sslutil`, a different
> attribute can be used. `sslutil.wrapserversocket` is used because it’s anyway
> used a few lines below.
> 
> diff --git a/mercurial/hgweb/server.py b/mercurial/hgweb/server.py
> --- a/mercurial/hgweb/server.py
> +++ b/mercurial/hgweb/server.py
> @@ -313,7 +313,7 @@ class _httprequesthandlerssl(_httpreques
>          try:
>              from .. import sslutil
>  
> -            sslutil.modernssl
> +            sslutil.wrapserversocket
>          except ImportError:

I wonder if we should still support Python builds of ssl disabled. If we
should, setup.py condition needs to be relaxed.
Gregory Szorc - May 30, 2020, 3:49 p.m.
On Sat, May 30, 2020 at 6:25 AM Yuya Nishihara <yuya@tcha.org> wrote:

> On Sat, 30 May 2020 07:52:17 +0200, Manuel Jacob wrote:
> > # HG changeset patch
> > # User Manuel Jacob <me@manueljacob.de>
> > # Date 1590807553 -7200
> > #      Sat May 30 04:59:13 2020 +0200
> > # Node ID f8ae379a8dcfebbec6cf6570d8d38fa1e3b6bcea
> > # Parent  fa4460229a8d1a392564d0cbe78216760154822c
> > # EXP-Topic require_modern_ssl
> > hgweb: avoid using `sslutil.modernssl`
> >
> > `sslutil.modernssl` is going to be removed. Since the point of using this
> > attribute was to check the importability of the `sslutil`, a different
> > attribute can be used. `sslutil.wrapserversocket` is used because it’s
> anyway
> > used a few lines below.
> >
> > diff --git a/mercurial/hgweb/server.py b/mercurial/hgweb/server.py
> > --- a/mercurial/hgweb/server.py
> > +++ b/mercurial/hgweb/server.py
> > @@ -313,7 +313,7 @@ class _httprequesthandlerssl(_httpreques
> >          try:
> >              from .. import sslutil
> >
> > -            sslutil.modernssl
> > +            sslutil.wrapserversocket
> >          except ImportError:
>
> I wonder if we should still support Python builds of ssl disabled. If we
> should, setup.py condition needs to be relaxed.
>

What environments might not have ssl support?

Strictly speaking, it is possible to produce a Python distribution without
ssl support. But it isn't something I've seen in years.
Manuel Jacob - May 30, 2020, 4:04 p.m.
On 2020-05-30 17:49, Gregory Szorc wrote:
> On Sat, May 30, 2020 at 6:25 AM Yuya Nishihara <yuya@tcha.org> wrote:
> 
>> On Sat, 30 May 2020 07:52:17 +0200, Manuel Jacob wrote:
>> > # HG changeset patch
>> > # User Manuel Jacob <me@manueljacob.de>
>> > # Date 1590807553 -7200
>> > #      Sat May 30 04:59:13 2020 +0200
>> > # Node ID f8ae379a8dcfebbec6cf6570d8d38fa1e3b6bcea
>> > # Parent  fa4460229a8d1a392564d0cbe78216760154822c
>> > # EXP-Topic require_modern_ssl
>> > hgweb: avoid using `sslutil.modernssl`
>> >
>> > `sslutil.modernssl` is going to be removed. Since the point of using this
>> > attribute was to check the importability of the `sslutil`, a different
>> > attribute can be used. `sslutil.wrapserversocket` is used because it’s
>> anyway
>> > used a few lines below.
>> >
>> > diff --git a/mercurial/hgweb/server.py b/mercurial/hgweb/server.py
>> > --- a/mercurial/hgweb/server.py
>> > +++ b/mercurial/hgweb/server.py
>> > @@ -313,7 +313,7 @@ class _httprequesthandlerssl(_httpreques
>> >          try:
>> >              from .. import sslutil
>> >
>> > -            sslutil.modernssl
>> > +            sslutil.wrapserversocket
>> >          except ImportError:
>> 
>> I wonder if we should still support Python builds of ssl disabled. If 
>> we
>> should, setup.py condition needs to be relaxed.
>> 
> 
> What environments might not have ssl support?
> 
> Strictly speaking, it is possible to produce a Python distribution 
> without
> ssl support. But it isn't something I've seen in years.

I sometimes compile Python interpreters (usually PyPy) with a limited 
set of extension modules. However, I very rarely run Mercurial on top of 
these and in these cases I can comment out that check manually.

I would be fine with leaving it as is or sending a patch relaxing it.
Gregory Szorc - May 30, 2020, 4:14 p.m.
On Sat, May 30, 2020 at 9:04 AM Manuel Jacob <me@manueljacob.de> wrote:

> On 2020-05-30 17:49, Gregory Szorc wrote:
> > On Sat, May 30, 2020 at 6:25 AM Yuya Nishihara <yuya@tcha.org> wrote:
> >
> >> On Sat, 30 May 2020 07:52:17 +0200, Manuel Jacob wrote:
> >> > # HG changeset patch
> >> > # User Manuel Jacob <me@manueljacob.de>
> >> > # Date 1590807553 -7200
> >> > #      Sat May 30 04:59:13 2020 +0200
> >> > # Node ID f8ae379a8dcfebbec6cf6570d8d38fa1e3b6bcea
> >> > # Parent  fa4460229a8d1a392564d0cbe78216760154822c
> >> > # EXP-Topic require_modern_ssl
> >> > hgweb: avoid using `sslutil.modernssl`
> >> >
> >> > `sslutil.modernssl` is going to be removed. Since the point of using
> this
> >> > attribute was to check the importability of the `sslutil`, a different
> >> > attribute can be used. `sslutil.wrapserversocket` is used because it’s
> >> anyway
> >> > used a few lines below.
> >> >
> >> > diff --git a/mercurial/hgweb/server.py b/mercurial/hgweb/server.py
> >> > --- a/mercurial/hgweb/server.py
> >> > +++ b/mercurial/hgweb/server.py
> >> > @@ -313,7 +313,7 @@ class _httprequesthandlerssl(_httpreques
> >> >          try:
> >> >              from .. import sslutil
> >> >
> >> > -            sslutil.modernssl
> >> > +            sslutil.wrapserversocket
> >> >          except ImportError:
> >>
> >> I wonder if we should still support Python builds of ssl disabled. If
> >> we
> >> should, setup.py condition needs to be relaxed.
> >>
> >
> > What environments might not have ssl support?
> >
> > Strictly speaking, it is possible to produce a Python distribution
> > without
> > ssl support. But it isn't something I've seen in years.
>
> I sometimes compile Python interpreters (usually PyPy) with a limited
> set of extension modules. However, I very rarely run Mercurial on top of
> these and in these cases I can comment out that check manually.
>
> I would be fine with leaving it as is or sending a patch relaxing it.
>

I would prefer to leave it as is and avoid the complexity until someone
actually complains about it.

Patch

diff --git a/mercurial/hgweb/server.py b/mercurial/hgweb/server.py
--- a/mercurial/hgweb/server.py
+++ b/mercurial/hgweb/server.py
@@ -313,7 +313,7 @@  class _httprequesthandlerssl(_httpreques
         try:
             from .. import sslutil
 
-            sslutil.modernssl
+            sslutil.wrapserversocket
         except ImportError:
             raise error.Abort(_(b"SSL support is unavailable"))