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

Add fix for CMake OpenSSL root parameter #4229

Closed
wants to merge 1 commit into from

Conversation

Mwni
Copy link

@Mwni Mwni commented Jul 11, 2022

The documentation incorrectly instructs to use OPENSSL_ROOT instead of OPENSSL_ROOT_DIR when creating build files with CMake.

This change copies the variable OPENSSL_ROOT to OPENSSL_ROOT_DIR whenever only the former is defined. This is to

  • statisfy the fixed requirement by CMake of calling the path OPENSSL_ROOT_DIR
  • fix the documentation
  • stay in conformity with the naming convention of BOOST_ROOT.

The documentation incorrectly instructs to use
OPENSSL_ROOT instead of OPENSSL_ROOT_DIR when
creating build files with CMake.

This change copies OPENSSL_ROOT to OPENSSL_ROOT_DIR
whenever only the former is defined. This is to
statisfy the fixed requirement by CMake of calling
the path OPENSSL_ROOT_DIR, fix the documentation,
and stay in conformity with the naming convention
of BOOST_ROOT.
@thejohnfreeman
Copy link
Collaborator

What documentation instructs the use of OPENSSL_ROOT? Can you link it, please?

@Mwni
Copy link
Author

Mwni commented Mar 4, 2023

https://github.com/XRPLF/rippled/tree/develop/Builds/VisualStudio2019#build-using-visual-studio-integrated-cmake
The VS 2017 documentation has moved and changed since when this PR was filed.

@thejohnfreeman
Copy link
Collaborator

I see. That documentation is slated to be removed in #4381. Soon thereafter I plan to come through this CMake code and remove the variable shuffling entirely. Our CMake should call find_package and leave the discovery up to whatever Find Module / Package Configuration File the builder decides to use, whether generated by Conan, supplied by CMake, or written by themselves.

@Mwni
Copy link
Author

Mwni commented Mar 4, 2023

Very well. I'm closing this.

@Mwni Mwni closed this Mar 4, 2023
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.

2 participants