#22189 closed task (fixed)
Document SSL dependency, recommend OpenSSL
Reported by: | charpent | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-7.6 |
Component: | documentation | Keywords: | |
Cc: | Merged in: | ||
Authors: | Emmanuel Charpentier | Reviewers: | Julian Rüth, André Apitzsch, Travis Scrimshaw |
Report Upstream: | N/A | Work issues: | |
Branch: | 8af2ca2 (Commits, GitHub, GitLab) | Commit: | |
Dependencies: | #22058 | Stopgaps: |
Description
The motivation has been discussed numerous times, and has become a somewhat urgent matter now that R stopped providing its own implementation.
This ticket aims at documenting this long known dependency, recommend using OpenSSL system-wide and document the possibility of compiling against other libraries with reduced functionality.
it depends on #22058, to allow for successful build against OpenSSL>1.1 (see this report) : we cannot decently recommend a library if its newer version breaks Sage building...
This ticket does not aim to modify any code, only the documentation (README.md
and the Sage Installation Guide).
Change History (32)
comment:1 Changed 5 years ago by
- Branch set to u/charpent/document_ssl_dependency__recommend_openssl
comment:2 follow-up: ↓ 3 Changed 5 years ago by
- Commit set to cd6b74e7d9016e029380ba70bd67bd208f6fa2de
comment:3 in reply to: ↑ 2 Changed 5 years ago by
- Status changed from new to needs_review
Forgot to add :
- The resulting Sage passes ptestlong with two failures :
---------------------------------------------------------------------- sage -t --long --warn-long 119.0 src/sage/homology/simplicial_complex.py # 1 doctest failed sage -t --long --warn-long 119.0 src/sage/rings/polynomial/multi_polynomial_ideal.py # 1 doctest failed ----------------------------------------------------------------------
The first one is transient (test passes fine when run standalone), the second one is an already-known problem.
- Forgot to set
needs_review
comment:4 follow-up: ↓ 6 Changed 5 years ago by
- Reviewers set to Julian Rüth
At the end it should now probably say "Sage 7.6". Feel free to set to positive review once this is fixed.
comment:5 Changed 5 years ago by
- Commit changed from cd6b74e7d9016e029380ba70bd67bd208f6fa2de to a37c08e12c28568985e1269ce25cfabbb3b26871
Branch pushed to git repo; I updated commit sha1. New commits:
a37c08e | Changed the target Sage version, as suggested by reviewer.
|
comment:6 in reply to: ↑ 4 Changed 5 years ago by
Replying to saraedum:
At the end it should now probably say "Sage 7.6".
Done.
Feel free to set to positive review once this is fixed.
I'd rather follow the basic requirement of no self-review : it's a basic safeguard against hubris, and, as all basic rules, it can have any value if and only if rigidly followed...
comment:7 Changed 5 years ago by
- Status changed from needs_review to positive_review
comment:8 follow-up: ↓ 9 Changed 5 years ago by
- Status changed from positive_review to needs_work
Author is missing...
comment:9 in reply to: ↑ 8 Changed 5 years ago by
- Status changed from needs_work to needs_review
comment:10 follow-up: ↓ 11 Changed 5 years ago by
Looks like you picked up a merge conflict with the next beta. (For future reference, missing author/review name can immediately be set back to positive review.)
comment:11 in reply to: ↑ 10 ; follow-up: ↓ 12 Changed 5 years ago by
- Status changed from needs_review to positive_review
Replying to tscrim:
Looks like you picked up a merge conflict with the next beta.
What do you mean ?
(For future reference, missing author/review name can immediately be set back to positive review.)
Done. Thanks.
comment:12 in reply to: ↑ 11 Changed 5 years ago by
- Status changed from positive_review to needs_work
comment:13 Changed 5 years ago by
- Work issues set to merge conflicts
comment:14 Changed 5 years ago by
- Commit changed from a37c08e12c28568985e1269ce25cfabbb3b26871 to c884e41ac51bb660074bf48cc6cb6577e8003eb1
Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:
c884e41 | From Sage 7.6, depend on system SSL, recommend OpenSSL.
|
comment:15 follow-up: ↓ 16 Changed 5 years ago by
I squashed the two commits (whose only the first was included in 7.6.beta1) and forced a push. This should fix the merge problem.
comment:16 in reply to: ↑ 15 Changed 5 years ago by
Replying to charpent:
I squashed the two commits and forced a push. This should fix the merge problem.
No, it doesn't : there is still a merge conflict, requiring to manually pick the "right" versin of the last line of src/doc/en/installation/source.rst
(i. e. what was fixed by the second commit). I do not understand why, and I'm stuck.
comment:17 Changed 5 years ago by
- Branch changed from u/charpent/document_ssl_dependency__recommend_openssl to u/tscrim/22189
- Commit changed from c884e41ac51bb660074bf48cc6cb6577e8003eb1 to 2ee7e18926b13d977e05981546706a275635a887
- Status changed from needs_work to positive_review
- Work issues merge conflicts deleted
I fixed the (trivial) merge conflict with 7.6.beta1.
New commits:
2ee7e18 | Merge branch 'u/charpent/document_ssl_dependency__recommend_openssl' of git://trac.sagemath.org/sage into u/tscrim/22189
|
comment:18 follow-up: ↓ 20 Changed 5 years ago by
- Status changed from positive_review to needs_work
There is a typo in
Sage's ``-pip`` facility (used to install some Sage packages) is disabled when Sage is compiled agaonst those libraries.
agaonst -> against
comment:19 Changed 5 years ago by
- Branch changed from u/tscrim/22189 to u/charpent/22189
comment:20 in reply to: ↑ 18 Changed 5 years ago by
- Commit changed from 2ee7e18926b13d977e05981546706a275635a887 to 8af2ca2ad19f764c8c46891b8b808b3cd0747b09
- Status changed from needs_work to needs_review
comment:21 Changed 5 years ago by
- Reviewers changed from Julian Rüth to Julian Rüth, André Apitzsch, Travis Scrimshaw
- Status changed from needs_review to positive_review
Latest changes LGTM.
comment:22 Changed 5 years ago by
- Branch changed from u/charpent/22189 to 8af2ca2ad19f764c8c46891b8b808b3cd0747b09
- Resolution set to fixed
- Status changed from positive_review to closed
comment:23 follow-up: ↓ 24 Changed 5 years ago by
- Commit 8af2ca2ad19f764c8c46891b8b808b3cd0747b09 deleted
What part of the Sage build will break without ssl
? Shouldn't we check for the presence of ssl
in the top-level configure
script and bail out right away if it's missing, rather than let Sage build for an hour or two before failing?
comment:24 in reply to: ↑ 23 Changed 5 years ago by
Replying to jhpalmieri:
What part of the Sage build will break without
ssl
?
- R will fail to build without an https-capable SSL
- Python won't fail overtly, but the resulting pip won't be able to install or list packages in https-accessed repositories : nowadays, that means all pip directories.
- ISTR thython license mention this and warns about "reduced functionalities".
The python license recommends openssl ; my tests showed that GnuTLS is able to create a functional R (but not a functional pip). Hence the dependency on "an SSL implementation) and the recommendation of OpenSSL. I didn't test the nss library...
Shouldn't we check for the presence of
ssl
in the top-levelconfigure
script and bail out right away if it's missing, rather than let Sage build for an hour or two before failing?
Indeed. But I don't know how (yet) how to do it cleanly (it is easy to test for OpenSSL, not so much for either OpenSSL or GnuTLS...). If I can find or devise a relevant minimal test program, I'll propose to add this test in the master configure file. If you have ideas...
Hope this helps,
comment:25 follow-up: ↓ 26 Changed 5 years ago by
I would suggest using python -c 'import ssl'
(using the system's Python), but this succeeds on my OS X box on which I just uninstalled Sage's version of openssl
, in order to test #22252. I don't know why it succeeds, but in any case, it's not a good test.
comment:26 in reply to: ↑ 25 ; follow-up: ↓ 27 Changed 5 years ago by
Replying to jhpalmieri:
I would suggest using
python -c 'import ssl'
(using the system's Python), but this succeeds on my OS X box on which I just uninstalled Sage's version ofopenssl
, in order to test #22252. I don't know why it succeeds,
Don't you have some SSL library installed systemwide ? OpenSSL is pretty ubiquitous nowadays (even on Debian base (not desktop !) systems, to the dismay of Jeroen Demeyer....)
but in any case, it's not a good test.
Indeed. What I need is a program testing a library common to (at least) OpenSSL and GnuTLS, and able to test the "https-worthiness" of a library. Not exactly a trivial request...
comment:27 in reply to: ↑ 26 Changed 5 years ago by
Replying to charpent:
Replying to jhpalmieri:
I would suggest using
python -c 'import ssl'
(using the system's Python), but this succeeds on my OS X box on which I just uninstalled Sage's version ofopenssl
, in order to test #22252. I don't know why it succeeds,Don't you have some SSL library installed systemwide ? OpenSSL is pretty ubiquitous nowadays (even on Debian base (not desktop !) systems, to the dismay of Jeroen Demeyer....)
I apparently do, since the system Python picks up something. I can't actually find it myself, nor does Sage's Python when it builds, since import ssl
fails for it.
comment:28 follow-up: ↓ 31 Changed 5 years ago by
I don't understand why this ticket got positive review. Sage does not depend on OpenSSL, so why document that it does? It might be recommended, but certainly not required. See #22620 for a follow-up.
comment:29 follow-up: ↓ 30 Changed 5 years ago by
interestingly, curl sources include vtls, (almost) an API to do (the client side of) TLS/SSL with different backends, e.g. DarwinSSL, OpenSSL (and a dozen others).
comment:30 in reply to: ↑ 29 ; follow-up: ↓ 32 Changed 5 years ago by
comment:31 in reply to: ↑ 28 Changed 5 years ago by
Replying to jdemeyer:
I don't understand why this ticket got positive review. Sage does not depend on OpenSSL, so why document that it does?
Because Python does. Because R needs curl, which does need (some) SSL to reach https repositories.
It might be recommended, but certainly not required.
Indeed. The current documentation requires some SSL library and recommends OpenSSL : not quite the same thing.
I also proved conclusively that :
- GnuTLS allowed for the successfil compilation of a https-capable R an
- that this GnuTLL library is insufficient for a functional pip.
See #22620 for a follow-up.
See #22620 for my judgemement on this edit war...
comment:32 in reply to: ↑ 30 Changed 5 years ago by
Replying to charpent:
Replying to dimpase:
interestingly, curl sources include vtls, (almost) an API to do (the client side of) TLS/SSL with different backends, e.g. DarwinSSL, OpenSSL (and a dozen others).
Do you think that this might ofar a solution to Jeroen's conudrum ? And our Mac OS/X woes ?
It is sure a fun project to allow python's _ssl
to use it, but it is a lot of work...
Initial proposition.
I noted a few inconsistencies/inecxactitudes while revising the text. I did not try to fix them : they require more information and belong to other tickets :
$SAGE_ROOT/src/doc/en/installation/source.rst
...$SAGE_ROOT/src/doc/en/installation/source.rst
seem to imply) or not ? If so, having agit
package is pointless...