Opened 2 years ago

Closed 15 months ago

#30674 closed enhancement (invalid)

Patch tornado so it can be imported in Python without an ssl module

Reported by: Matthias Köppe Owned by:
Priority: major Milestone: sage-duplicate/invalid/wontfix
Component: packages: standard Keywords:
Cc: Markus Wageringel, Samuel Lelièvre Merged in:
Authors: Reviewers: Markus Wageringel
Report Upstream: N/A Work issues:
Branch: u/gh-mwageringel/30674 (Commits, GitHub, GitLab) Commit: 30c5565e34d8ac4cb925a2e1f122c91577fb75a0
Dependencies: #29291 Stopgaps:

Status badges

Change History (17)

comment:1 Changed 2 years ago by Matthias Köppe

Branch: u/mkoeppe/tornado-no-ssl-required

comment:2 Changed 2 years ago by Matthias Köppe

Authors: Matthias Koeppe
Commit: dda2bd08f9ef296b7afde3005e4c9ad291337e47
Status: newneeds_review

New commits:

dda2bd0build/pkgs/tornado/patches: Patch tornado so it can be imported in Python without an ssl module

comment:3 Changed 2 years ago by Matthias Köppe

Description: modified (diff)

comment:4 Changed 2 years ago by Markus Wageringel

Status: needs_reviewneeds_work

The patch does not apply. Apparently the upstream sources have CRLF line endings.

[tornado-6.0.4.p0] Applying ../patches/0001-tornado-httpserver.py-import-ssl-only-when-TYPE_CHEC.patch
[tornado-6.0.4.p0] patching file tornado/httpserver.py
[tornado-6.0.4.p0] Hunk #1 FAILED at 26 (different line endings).
[tornado-6.0.4.p0] Hunk #2 FAILED at 41 (different line endings).
[tornado-6.0.4.p0] Hunk #3 FAILED at 157 (different line endings).

There is also at least one more use of ssl in tornado/httputil.py.

comment:5 Changed 2 years ago by git

Commit: dda2bd08f9ef296b7afde3005e4c9ad291337e47c5cf987a4bc2c4dce8e78bf3935de905254c106f

Branch pushed to git repo; I updated commit sha1. New commits:

c5cf987build/pkgs/tornado/patches: unix2dos

comment:6 Changed 2 years ago by git

Commit: c5cf987a4bc2c4dce8e78bf3935de905254c106fa98c7ecb867d4edd34864d6f45c4a064be9b6b07

Branch pushed to git repo; I updated commit sha1. New commits:

a98c7ecbuild/pkgs/tornado/patches: Make binary

comment:7 Changed 2 years ago by Matthias Köppe

Try this version please

comment:8 Changed 2 years ago by Markus Wageringel

Branch: u/mkoeppe/tornado-no-ssl-requiredu/gh-mwageringel/30674
Commit: a98c7ecb867d4edd34864d6f45c4a064be9b6b0730c5565e34d8ac4cb925a2e1f122c91577fb75a0

This version applies, but Jupyter does not start yet, as there are more uses of ssl. I have tried to patch tornado/httputil.py and tornado/iostream.py, but now the various uses of ssl in tornado/netutil.py need workarounds. I am not sure if this is a direction we want to dive into.

Traceback (most recent call last):
  File "/amd/compute/mwagerin/git/sage_compute/local/lib/python3.8/site-packages/sage/repl/ipython_kernel/install.py", line 307, in have_prerequisites
    from notebook.notebookapp import NotebookApp
  File "/amd/compute/mwagerin/git/sage_compute/local/lib/python3.8/site-packages/notebook/notebookapp.py", line 66, in <module>
    from tornado import httpserver
  File "/amd/compute/mwagerin/git/sage_compute/local/lib/python3.8/site-packages/tornado/httpserver.py", line 31, in <module>
    from tornado.http1connection import HTTP1ServerConnection, HTTP1ConnectionParameters
  File "/amd/compute/mwagerin/git/sage_compute/local/lib/python3.8/site-packages/tornado/http1connection.py", line 34, in <module>
    from tornado import iostream
  File "/amd/compute/mwagerin/git/sage_compute/local/lib/python3.8/site-packages/tornado/iostream.py", line 39, in <module>
    from tornado.netutil import ssl_wrap_socket, _client_ssl_defaults, _server_ssl_defaults
  File "/amd/compute/mwagerin/git/sage_compute/local/lib/python3.8/site-packages/tornado/netutil.py", line 23, in <module>
    import ssl
  File "/amd/compute/mwagerin/git/sage_compute/local/lib/python3.8/ssl.py", line 98, in <module>
    import _ssl             # if we can't import it, let the error propagate
ModuleNotFoundError: No module named '_ssl'

New commits:

30c556530674: patch tornado/httputil and tornado/iostream

comment:9 Changed 2 years ago by Markus Wageringel

Status: needs_workneeds_info

comment:10 Changed 2 years ago by Matthias Köppe

Milestone: sage-9.2sage-9.3

comment:11 Changed 21 months ago by Matthias Köppe

Milestone: sage-9.3sage-wishlist
Priority: criticalmajor

comment:12 Changed 16 months ago by Matthias Köppe

Authors: Matthias Koeppe
Dependencies: #29291
Milestone: sage-wishlistsage-duplicate/invalid/wontfix
Status: needs_infoneeds_review

Outdated; with #29291 we just require ssl.

comment:13 Changed 16 months ago by Samuel Lelièvre

Markus, if you agree, remove branch, add yourself as reviewer and set to positive review.

comment:14 Changed 16 months ago by Matthias Köppe

I don't think branches should or need to be removed when closing a ticket

comment:15 Changed 16 months ago by Samuel Lelièvre

Good point. I agree.

comment:16 Changed 15 months ago by Markus Wageringel

Reviewers: Markus Wageringel
Status: needs_reviewpositive_review

comment:17 Changed 15 months ago by Matthias Köppe

Resolution: invalid
Status: positive_reviewclosed
Note: See TracTickets for help on using tickets.