Opened 3 years ago

Closed 2 years ago

Last modified 2 years ago

#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:

Status badges

Description (last modified by mkoeppe)

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 embray

  • Description modified (diff)

Correction: It's elliptic_curves that has the sqlite3 dependency.

comment:2 Changed 3 years ago by embray

  • 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: Changed 3 years ago by embray

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:4 Changed 3 years ago by mkoeppe

  • Cc dimpase mjo added

See also #29002, #27824

comment:5 Changed 3 years ago by dimpase

  • Description modified (diff)

comment:6 in reply to: ↑ 3 Changed 3 years ago by mkoeppe

Replying to embray:

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.

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 mkoeppe

  • Milestone set to sage-9.1

comment:8 Changed 3 years ago by mkoeppe

Actually, is crypt used anywhere in sage? I'm having trouble building it in #29012.

comment:9 Changed 3 years ago by mkoeppe

  • Branch set to u/mkoeppe/check_for_more_strictly_required_extension_modules_in_python_build

comment:10 Changed 3 years ago by mkoeppe

  • Authors set to Matthias Koeppe
  • 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:

da5e0fcbuild/pkgs/python3/spkg-build: Check that Python build has modules zlib sqlite3 ssl

comment:11 Changed 3 years ago by dimpase

  • 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 mkoeppe

Thank you!

comment:13 Changed 3 years ago by vbraun

  • 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 vbraun

  • 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 dimpase

hmm, isn't this buildbot broken?

comment:16 Changed 3 years ago by jhpalmieri

I get this same error on OS X.

comment:17 Changed 3 years ago by dimpase

Does moving these tests to spkg-install from spkg-build fixes this?

comment:18 Changed 3 years ago by jhpalmieri

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 dimpase

that is, is actually works as it should, i.e. you cannot build a broken (ssh-less) Python any more?

comment:20 follow-up: Changed 3 years ago by 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.

comment:21 in reply to: ↑ 20 ; follow-up: Changed 3 years ago by 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.

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 mkoeppe

For this ticket, I'll change the error for a missing openssl module into a warning.

Version 0, edited 3 years ago by mkoeppe (next)

comment:23 in reply to: ↑ 21 Changed 3 years ago by jhpalmieri

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 mkoeppe

dup: #22776

comment:25 Changed 3 years ago by mkoeppe

  • Branch changed from da5e0fcee826ad1c667c52341220c90306f0e222 to u/mkoeppe/da5e0fcee826ad1c667c52341220c90306f0e222

comment:26 Changed 3 years ago by mkoeppe

  • 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

New commits:

dade952build/pkgs/python3/spkg-build: Check that Python build has modules zlib sqlite3 ssl
8a294fbRemove ssl from the list of required modules

comment:27 Changed 2 years ago by git

  • Commit changed from 8a294fbc9876c9892cf4e9557b857ca20a8a375e to c3542d532b537f10e6dd2f89c8f69401b91a185f

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

c3542d5Merge tag '9.1.beta4' into t/27705/da5e0fcee826ad1c667c52341220c90306f0e222

comment:28 Changed 2 years ago by mkoeppe

  • Cc vdelecroix added

comment:29 Changed 2 years ago by mkoeppe

Needs review

comment:30 Changed 2 years ago by dimpase

  • Status changed from needs_review to positive_review

ok

comment:31 Changed 2 years ago by vbraun

  • 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 mkoeppe

  • Commit c3542d532b537f10e6dd2f89c8f69401b91a185f deleted

Follow-up for ssl: #29291

Note: See TracTickets for help on using tickets.