Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Re-enable ssldir creation target for openssl install #1973

Merged
merged 2 commits into from
May 11, 2022

Conversation

jpr5
Copy link
Contributor

@jpr5 jpr5 commented May 9, 2022

Prior commit attempts to shorten install by omitting documentation install,
however an additional target is needed to get the conf dirs for capturing the
PEMs et al.

Solves issue mentioned in discussion 1972.

Related to #1967

Prior commit attempts to shorten install by omitting documentation install,
however an additional target is needed to get the conf dirs for capturing the
PEMs et al.
@mislav mislav requested a review from eregon May 10, 2022 08:45
@mislav
Copy link
Member

mislav commented May 10, 2022

@jpr5 Thank you! The fix looks good to me.

@eregon
Copy link
Member

eregon commented May 10, 2022

In openssl 1.0.2 (the oldest version used in ruby-build), there appears to be no install_ssldirs target though:
https://github.com/raw/openssl/openssl/OpenSSL_1_0_2-stable/Makefile.org

So that seems an issue, as we'd break those (e.g., 2.3.8).
Could you try installing 2.3.8 with your change and see if that breaks (as expected)?
And then tweak your patch so it'd work both for 2.3.8 and newer versions?

@eregon
Copy link
Member

eregon commented May 10, 2022

I should have started with: Thank you for the fix and sorry for breaking that (at least it's not on a ruby-build release).

@mislav
Copy link
Member

mislav commented May 11, 2022

@eregon If we cannot reliably support all OpenSSL versions that we use, then we should revert #1967.

If compilation speed is an issue, the ultimate solution would be that ruby-build doesn't compile openssl in the first place #1974 (comment)

@eregon
Copy link
Member

eregon commented May 11, 2022

@mislav I'll make my own PR to try to fix this ASAP.

If compilation speed is an issue, the ultimate solution would be that ruby-build doesn't compile openssl in the first place

The cases I let ruby-build compile openssl is the cases I would need to somehow compile openssl anyway. But maybe on macOS we could default to use Homebrew's openssl, or the system openssl if it's usable nowadays.

@eregon
Copy link
Member

eregon commented May 11, 2022

@eregon eregon merged commit 1038c07 into rbenv:master May 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants