Opened 3 years ago

Closed 2 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) Commit: 31aca55368e50a68b60901c199fe6b287b275fc3
Dependencies: Stopgaps:

Description (last modified by Kosta)

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 3 years ago by Kosta

  • Description modified (diff)

comment:2 Changed 3 years ago by Kosta

  • Branch set to u/Kosta/openssl_for_sierra

comment:3 Changed 3 years ago by Kosta

  • Commit set to 9e2978e7f4f0a1594ffc9439963bb218d311fbe7

comment:4 Changed 3 years ago by Kosta

  • Status changed from new to needs_review

comment:5 Changed 3 years ago by Kosta

New commits:

9e2978eLink python with OpenSSL on Sierra. Fixes #21944

comment:6 Changed 3 years ago by Kosta

New commits:

9e2978eLink python with OpenSSL on Sierra. Fixes #21944

comment:7 Changed 3 years ago by Kosta

  • Authors set to Konstantin Kliakhandler

comment:8 Changed 3 years ago by Kosta

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

  • Status changed from needs_review to needs_work
  1. The indentation in the script is inconsistent.
  1. Why check for .../usr/include and not .../usr/include/openssl?

comment:10 Changed 3 years ago by git

  • Commit changed from 9e2978e7f4f0a1594ffc9439963bb218d311fbe7 to 2500ade5919cb644582ccde9f21cb2c3e29c7e4a

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

2500adeImprove indentation and test for correct dir.

comment:11 in reply to: ↑ 9 Changed 3 years ago by Kosta

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:

  1. The indentation in the script is inconsistent.
  1. Why check for .../usr/include and not .../usr/include/openssl?
Last edited 3 years ago by Kosta (previous) (diff)

comment:12 Changed 3 years ago by Kosta

  • Cc Kosta added

comment:13 follow-up: Changed 3 years ago by jdemeyer

The elif/else is still indented inconsistently.

comment:14 Changed 3 years ago by git

  • Commit changed from 2500ade5919cb644582ccde9f21cb2c3e29c7e4a to 1857d1f41eebd51f82c56abb8e488a3a6cf8bc14

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

1857d1fFix indentation in if/else.

comment:15 in reply to: ↑ 13 Changed 3 years ago by Kosta

Thanks! I completely forgot you have elif in bash.

Replying to jdemeyer:

The elif/else is still indented inconsistently.

comment:16 Changed 3 years ago by Kosta

  • Status changed from needs_work to needs_review

comment:17 Changed 3 years ago by Kosta

  • 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 3 years ago by Kosta

  • Status changed from needs_review to needs_work

comment:19 Changed 3 years ago by jhpalmieri

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 3 years ago by Kosta

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 3 years ago by Kosta

  • Branch changed from u/Kosta/openssl_for_sierra to u/Kosta/21944/osx_openssl

comment:22 Changed 3 years ago by Kosta

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

d851539Let user specify OPENSSL or search homebrew on OSX

comment:23 Changed 3 years ago by jhpalmieri

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 3 years ago by Kosta

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 3 years ago by git

  • Commit changed from d8515390a163cd238e48808cacb25ebbcaee2ccc to 0c9279e524046b4774a29b2f3ce4ec84ddcebce6

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

0b0b7d5Let user specify OPENSSL or search homebrew on OSX
0c9279eFixed missing fi statement

comment:26 Changed 3 years ago by git

  • Commit changed from 0c9279e524046b4774a29b2f3ce4ec84ddcebce6 to c180d701ed715a1c55d04ccd6af04c1e8954f13b

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

c180d70Integrate better with previous solutions and with homebrew.

comment:27 Changed 3 years ago by Kosta

  • Status changed from needs_work to needs_review

comment:28 Changed 3 years ago by git

  • Commit changed from c180d701ed715a1c55d04ccd6af04c1e8954f13b to 31aca55368e50a68b60901c199fe6b287b275fc3

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

2bb9e93Let user specify OPENSSL or search homebrew on OSX
8ef439fFixed missing fi statement
31aca55Integrate better with previous solutions and with homebrew.

comment:29 Changed 2 years ago by mforets

  • 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 2 years ago by mforets

  • Status changed from needs_review to positive_review

comment:32 Changed 2 years ago by vbraun

  • Branch changed from u/Kosta/21944/osx_openssl to 31aca55368e50a68b60901c199fe6b287b275fc3
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.