#27705 closed defect (fixed)
Check for more strictly required extension modules in Python build: zlib sqlite3
Reported by: | embray | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-9.1 |
Component: | packages: standard | Keywords: | |
Cc: | dimpase, mjo, jdemeyer, jhpalmieri, vbraun, slelievre, vklein, tscrim, kcrisman, saraedum, vdelecroix | Merged in: | |
Authors: | Matthias Koeppe | Reviewers: | Dima Pasechnik |
Report Upstream: | N/A | Work issues: | |
Branch: | c3542d5 (Commits, GitHub, GitLab) | Commit: | |
Dependencies: | Stopgaps: |
Description (last modified by )
When building Python 2/3 the Python build system attempts to build several extension modules that are part of the stdlib, but does not fail the build if building some of those modules fails, as many of them are optional.
There are some such modules that are mandatory for Sage, so we check after the Python build that they can be imported, and force the build to fail if not:
echo "Testing importing of various modules..." import_errors=false test_modules="ctypes math hashlib crypt readline socket" if [ "$UNAME" = "Darwin" ]; then test_modules="$test_modules _scproxy" fi for module in $test_modules; do if $PYTHON -c "import $module"; then echo "$module module imported OK" else echo >&2 "$module module failed to import" import_errors=true fi done if $import_errors; then echo >&2 "Error: One or more modules failed to import." exit 1 fi
This came up recently in light of #26899, where the zlib
module was silently failing to build on some macOS versions. Hence, we should have also added zlib
to this list.
It turns out also that elliptic_curves has a build-time dependency on the sqlite3
module being built properly, so we should add it as well.
Also ssl
should be added, but we reserve this for a follow-up ticket that addresses the whole openssl mess on macOS.
Change History (32)
comment:1 Changed 3 years ago by
- Description modified (diff)
comment:2 Changed 3 years ago by
- Milestone sage-8.8 deleted
As the Sage-8.8 release milestone is pending, we should delete the sage-8.8 milestone for tickets that are not actively being worked on or that still require significant work to move forward. If you feel that this ticket should be included in the next Sage release at the soonest please set its milestone to the next release milestone (sage-8.9).
comment:3 follow-up: ↓ 6 Changed 3 years ago by
I think we should also add ssl
to this list, unless a flag is passed to Sage's configure
explicitly requesting an SSL-less build.
comment:5 Changed 3 years ago by
- Description modified (diff)
comment:6 in reply to: ↑ 3 Changed 3 years ago by
Replying to embray:
I think we should also add
ssl
to this list, unless a flag is passed to Sage'sconfigure
explicitly requesting an SSL-less build.
That's a good idea, I think, because it avoids nasty surprises later when people try to use pip.
comment:7 Changed 3 years ago by
- Milestone set to sage-9.1
comment:8 Changed 3 years ago by
Actually, is crypt
used anywhere in sage? I'm having trouble building it in #29012.
comment:9 Changed 3 years ago by
- Branch set to u/mkoeppe/check_for_more_strictly_required_extension_modules_in_python_build
comment:10 Changed 3 years ago by
- Cc jdemeyer jhpalmieri vbraun slelievre vklein tscrim kcrisman saraedum added
- Commit set to da5e0fcee826ad1c667c52341220c90306f0e222
- Status changed from new to needs_review
- Summary changed from Check for more strictly required extension modules in Python build to Check for more strictly required extension modules in Python build: zlib sqlite3 ssl
New commits:
da5e0fc | build/pkgs/python3/spkg-build: Check that Python build has modules zlib sqlite3 ssl
|
comment:11 Changed 3 years ago by
- Reviewers set to Dima Pasechnik
- Status changed from needs_review to positive_review
lgtm
I expect some noise about ssl, but well...
comment:12 Changed 3 years ago by
Thank you!
comment:13 Changed 3 years ago by
- Branch changed from u/mkoeppe/check_for_more_strictly_required_extension_modules_in_python_build to da5e0fcee826ad1c667c52341220c90306f0e222
- Resolution set to fixed
- Status changed from positive_review to closed
comment:14 Changed 3 years ago by
- Commit da5e0fcee826ad1c667c52341220c90306f0e222 deleted
- Resolution fixed deleted
- Status changed from closed to new
Testing importing of various modules... ctypes module imported OK math module imported OK hashlib module imported OK crypt module imported OK readline module imported OK socket module imported OK zlib module imported OK sqlite3 module imported OK Traceback (most recent call last): File "<string>", line 1, in <module> File "/home/buildbot/slave/sage_git/build/local/var/tmp/sage/build/python3-3.7.3.p1/src/Lib/ssl.py", line 98, in <module> import _ssl # if we can't import it, let the error propagate ModuleNotFoundError: No module named '_ssl' ssl module failed to import Error: One or more modules failed to import.
comment:15 Changed 3 years ago by
hmm, isn't this buildbot broken?
comment:16 Changed 3 years ago by
I get this same error on OS X.
comment:17 Changed 3 years ago by
Does moving these tests to spkg-install from spkg-build fixes this?
comment:18 Changed 3 years ago by
No. From earlier in the log file:
Could not build the ssl module! Python requires an OpenSSL 1.0.2 or 1.1 compatible libssl with X509_VERIFY_PARAM_set1_host(). LibreSSL 2.6.4 and earlier do not provide the necessary APIs, https://github.com/libressl-portable/portable/issues/381
It works if I first install openssl
via homebrew.
comment:19 Changed 3 years ago by
that is, is actually works as it should, i.e. you cannot build a broken (ssh-less) Python any more?
comment:20 follow-up: ↓ 21 Changed 3 years ago by
If you want to force that change, then at the very least, you have to change the documentation from saying that openssl
is recommended to saying that it is required. You will also be adding another obstacle to installing Sage on OS X.
comment:21 in reply to: ↑ 20 ; follow-up: ↓ 23 Changed 3 years ago by
Replying to jhpalmieri:
If you want to force that change, then at the very least, you have to change the documentation from saying that
openssl
is recommended to saying that it is required. You will also be adding another obstacle to installing Sage on OS X.
Sage without a functioning openssl (and thus not fully functioning pip, probably broken jupyter too) is pretty much broken, that is, it's probably more about saving potential users time.
I ventured into this mess on MacOS last year https://bugs.python.org/issue36344 and the "logic" of what's going on with openssl support made me question my own sanity :-)
But perhaps Python 3.8 is an improvement.
IMHO we'd ease the suffering of everyone wanting to build/run Sage on MacOS by telling them to use Conda or Homebrew.
comment:22 Changed 3 years ago by
For this ticket, I'll change the error for a missing openssl
module into a warning.
comment:23 in reply to: ↑ 21 Changed 3 years ago by
Replying to dimpase:
Replying to jhpalmieri:
If you want to force that change, then at the very least, you have to change the documentation from saying that
openssl
is recommended to saying that it is required. You will also be adding another obstacle to installing Sage on OS X.Sage without a functioning openssl (and thus not fully functioning pip, probably broken jupyter too) is pretty much broken, that is, it's probably more about saving potential users time.
On the same machine, vanilla Sage 9.1.beta1 runs the Jupyter notebook just fine, despite Python's failure to build the ssl
module. pip
is indeed broken, but it apparently hasn't bothered me before, so I am at least one user who can survive without a functioning ssl
Python module.
comment:24 Changed 3 years ago by
dup: #22776
comment:25 Changed 3 years ago by
- Branch changed from da5e0fcee826ad1c667c52341220c90306f0e222 to u/mkoeppe/da5e0fcee826ad1c667c52341220c90306f0e222
comment:26 Changed 3 years ago by
- Commit set to 8a294fbc9876c9892cf4e9557b857ca20a8a375e
- Description modified (diff)
- Status changed from new to needs_review
- Summary changed from Check for more strictly required extension modules in Python build: zlib sqlite3 ssl to Check for more strictly required extension modules in Python build: zlib sqlite3
comment:27 Changed 2 years ago by
- Commit changed from 8a294fbc9876c9892cf4e9557b857ca20a8a375e to c3542d532b537f10e6dd2f89c8f69401b91a185f
Branch pushed to git repo; I updated commit sha1. New commits:
c3542d5 | Merge tag '9.1.beta4' into t/27705/da5e0fcee826ad1c667c52341220c90306f0e222
|
comment:28 Changed 2 years ago by
- Cc vdelecroix added
comment:29 Changed 2 years ago by
Needs review
comment:31 Changed 2 years ago by
- Branch changed from u/mkoeppe/da5e0fcee826ad1c667c52341220c90306f0e222 to c3542d532b537f10e6dd2f89c8f69401b91a185f
- Resolution set to fixed
- Status changed from positive_review to closed
comment:32 Changed 2 years ago by
- Commit c3542d532b537f10e6dd2f89c8f69401b91a185f deleted
Follow-up for ssl: #29291
Correction: It's elliptic_curves that has the sqlite3 dependency.