Opened 4 years ago
Closed 3 years ago
#21944 closed defect (fixed)
OpenSSL for Sierra
Reported by: | Kosta | Owned by: | |
---|---|---|---|
Priority: | major | Milestone: | sage-8.1 |
Component: | build: configure | Keywords: | days79, osx |
Cc: | jdemeyer, Kosta, jhpalmieri | Merged in: | |
Authors: | Konstantin Kliakhandler | Reviewers: | Marcelo Forets |
Report Upstream: | N/A | Work issues: | |
Branch: | 31aca55 (Commits, GitHub, GitLab) | Commit: | 31aca55368e50a68b60901c199fe6b287b275fc3 |
Dependencies: | Stopgaps: |
Description (last modified by )
Sage 7.4 and above (at the very least) builds without the OpenSSL bindings for python. The path from #19626 has changed and needs to be updated on newer versions of OS X to:
"${xcode}/Toolchains/XcodeDefault.xctoolchain/usr/lib/swift-migrator/sdks/MacOSX.sdk/usr/include/"
(/sdk/ ->/sdks/)
Change History (32)
comment:1 Changed 4 years ago by
- Description modified (diff)
comment:2 Changed 4 years ago by
- Branch set to u/Kosta/openssl_for_sierra
comment:3 Changed 4 years ago by
- Commit set to 9e2978e7f4f0a1594ffc9439963bb218d311fbe7
comment:4 Changed 4 years ago by
- Status changed from new to needs_review
comment:5 Changed 4 years ago by
comment:6 Changed 4 years ago by
New commits:
9e2978e | Link python with OpenSSL on Sierra. Fixes #21944
|
comment:7 Changed 4 years ago by
comment:8 Changed 4 years ago by
- Cc jdemeyer added
This hack is getting a little out of hand. It might be reasonable to start including openssl manually in OS X.
comment:9 follow-up: ↓ 11 Changed 4 years ago by
- Status changed from needs_review to needs_work
- The indentation in the script is inconsistent.
- Why check for
.../usr/include
and not.../usr/include/openssl
?
comment:10 Changed 4 years ago by
- Commit changed from 9e2978e7f4f0a1594ffc9439963bb218d311fbe7 to 2500ade5919cb644582ccde9f21cb2c3e29c7e4a
Branch pushed to git repo; I updated commit sha1. New commits:
2500ade | Improve indentation and test for correct dir.
|
comment:11 in reply to: ↑ 9 Changed 4 years ago by
Thanks for the comment!
Please see the latest commit - my editor indented now the whole file (and I reverted the couple of places where it wanted to indent differently than how it was indented by previous people). Hopefully now will be better.
Regarding the checks, I fixed the tested dirs.
Replying to jdemeyer:
- The indentation in the script is inconsistent.
- Why check for
.../usr/include
and not.../usr/include/openssl
?
comment:12 Changed 4 years ago by
- Cc Kosta added
comment:13 follow-up: ↓ 15 Changed 4 years ago by
The elif
/else
is still indented inconsistently.
comment:14 Changed 4 years ago by
- Commit changed from 2500ade5919cb644582ccde9f21cb2c3e29c7e4a to 1857d1f41eebd51f82c56abb8e488a3a6cf8bc14
Branch pushed to git repo; I updated commit sha1. New commits:
1857d1f | Fix indentation in if/else.
|
comment:15 in reply to: ↑ 13 Changed 4 years ago by
Thanks! I completely forgot you have elif in bash.
Replying to jdemeyer:
The
elif
/else
is still indented inconsistently.
comment:16 Changed 4 years ago by
- Status changed from needs_work to needs_review
comment:17 Changed 4 years ago by
- Keywords osx added
It looks like beyond Sierra this will not be enough (I am running a beta version on one of my computers and here the new path does not exist. I will add an additional check for where homebrew installs openssl.
comment:18 Changed 4 years ago by
- Status changed from needs_review to needs_work
comment:19 Changed 4 years ago by
On my Sierra machine, the directory
/Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/lib/swift-migrator/sdks/MacOSX.sdk/usr/include/
does not include an openssl
subdirectory. /usr/lib
contains libssl.dylib
, but I'm not sure about the header files. What would they be called? (I also can't find them on an El Capitan machine.)
comment:20 Changed 4 years ago by
Having updated to 10.12.2 I do not have this directory either, anymore. Therefore, I think the most reasonable way of proceeding is either to require users to roll their own openssl (or perhaps some compatible alternative) via e.g. homebrew, or to bundle it in the install (since even gcc is bundled, I think adding openssl would make a small difference).
What do you think?
Here is how the headers look on my end, with a homebrew openssl (note that the text scrolls to the right):
% ls /usr/local/opt/openssl/include/openssl aes.h bio.h camellia.h comp.h des.h dso.h ec.h err.h krb5_asn.h md5.h objects.h ossl_typ.h pkcs7.h rc4.h seed.h ssl.h stack.h txt_db.h x509.h asn1.h blowfish.h cast.h conf.h des_old.h dtls1.h ecdh.h evp.h kssl.h mdc2.h ocsp.h pem.h pqueue.h ripemd.h sha.h ssl2.h symhacks.h ui.h x509_vfy.h asn1_mac.h bn.h cmac.h conf_api.h dh.h e_os2.h ecdsa.h hmac.h lhash.h modes.h opensslconf.h pem2.h rand.h rsa.h srp.h ssl23.h tls1.h ui_compat.h x509v3.h asn1t.h buffer.h cms.h crypto.h dsa.h ebcdic.h engine.h idea.h md4.h obj_mac.h opensslv.h pkcs12.h rc2.h safestack.h srtp.h ssl3.h ts.h whrlpool.h
comment:21 Changed 4 years ago by
- Branch changed from u/Kosta/openssl_for_sierra to u/Kosta/21944/osx_openssl
comment:22 Changed 4 years ago by
- Cc jhpalmieri added
- Commit changed from 1857d1f41eebd51f82c56abb8e488a3a6cf8bc14 to d8515390a163cd238e48808cacb25ebbcaee2ccc
I removed the code that takes the openssl libraries from the swift framework as apparently this is a very short-lived solution (already disappeared on 10.12.1).
Instead, I am now allowing the user to specify directly via OPENSSL_INCLUDE environment variable, and if it is not defined, I test the directory homebrew installs openssl to. If someone knows where fink/macports install the headers, I can add those as well. Finally, where would be a good place to put the documentation regarding the new environment variable?
New commits:
d851539 | Let user specify OPENSSL or search homebrew on OSX
|
comment:23 Changed 4 years ago by
According to the top level configure file, Sage is not likely to build if either fink or MacPorts? is installed:
########################################################################### # (OS X only) # Sage will probably not build at all if either Fink or MacPorts can be # found, and the error messages can be extremely confusing. Even if it does # build, the product will probably be wrong. This runs a basic check to # find them. Once the Sage build process is perfected, this won't be necessary. # dphilp 15/9/2008 ########################################################################### PORTS_PATH=`which port` if test -f "$PORTS_PATH"; then as_fn_error $? "\"found MacPorts in $PORTS_PATH. Either: (1) rename /opt/local and /sw, or (2) change PATH and DYLD_LIBRARY_PATH (Once Sage is built, you can restore them.)" "$LINENO" 5 fi FINK_PATH=`which fink` if test -f "$FINK_PATH"; then as_fn_error $? "\"found Fink in $FINK_PATH. Either: (1) rename /opt/local and /sw, or (2) change PATH and DYLD_LIBRARY_PATH (Once Sage is built, you can restore them.)" "$LINENO" 5 fi fi
So we shouldn't use them for headers. I don't know if the same is true of homebrew.
comment:24 Changed 4 years ago by
Ah, good to know!
Well, I successfully built it while having homebrew installed and it worked fine AFAICT, but still, I can't exclude "the product will probably be wrong". If you don't like it, I can just remove the test for the homebrew headers and just rely on the user to input the appropriate environment variable.
comment:25 Changed 4 years ago by
- Commit changed from d8515390a163cd238e48808cacb25ebbcaee2ccc to 0c9279e524046b4774a29b2f3ce4ec84ddcebce6
comment:26 Changed 4 years ago by
- Commit changed from 0c9279e524046b4774a29b2f3ce4ec84ddcebce6 to c180d701ed715a1c55d04ccd6af04c1e8954f13b
Branch pushed to git repo; I updated commit sha1. New commits:
c180d70 | Integrate better with previous solutions and with homebrew.
|
comment:27 Changed 4 years ago by
- Status changed from needs_work to needs_review
comment:28 Changed 4 years ago by
- Commit changed from c180d701ed715a1c55d04ccd6af04c1e8954f13b to 31aca55368e50a68b60901c199fe6b287b275fc3
comment:29 Changed 3 years ago by
- Milestone changed from sage-7.5 to sage-8.1
- Reviewers set to Marcelo Forets
i run this on my macbook with "El Capitan", and it solves the TLS/SSL issue successfully. i think this is very helpful for mac users.
unless some of you disagree, i'd suggest to accept it (in a week or so). thanks.
comment:30 Changed 3 years ago by
- Status changed from needs_review to positive_review
comment:31 Changed 3 years ago by
The branch may be obtained here: https://github.com/dimpase/sagetrac-mirror/tree/u/Kosta/21944/osx_openssl
comment:32 Changed 3 years ago by
- Branch changed from u/Kosta/21944/osx_openssl to 31aca55368e50a68b60901c199fe6b287b275fc3
- Resolution set to fixed
- Status changed from positive_review to closed
New commits:
Link python with OpenSSL on Sierra. Fixes #21944