Page MenuHomePhabricator

[CMAKE] Fix wrong Openssl include directory variable name
ClosedPublic

Authored by Fabien on Dec 16 2019, 20:11.

Details

Summary

While there are crypto libraries, there is no crypto include, only a
global include.
This avoid having OSX to fail finding the symbol when openssl is
installed via Homebrew.

Test Plan

On OSX Catalina, with openssl installed via Homebrew as per the build-osx.md documentation:

mkdir buildcmake && cd buildcmake
cmake -GNinja ..

Check in the output that the EVP_MD_CTX_new symbol is found.

Diff Detail

Repository
rABC Bitcoin ABC
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

jasonbcox added a subscriber: jasonbcox.

IMO the test plan is lacking. Some thoughts:

  • I happen to know that the CI was ported over to CMake recently, so we know the current build isn't broken, but this should be explicitly tested regardless.
  • A runnable test plan would be nice for those that have OSX.
This revision is now accepted and ready to land.Dec 16 2019, 22:17

@jasonbcox I updated the test plan to use runnable commands.

Since the openssl library is in the search path for all the other builds, the issue only happen to cause trouble on OSX.
Furthermore if the symbol is not found (under normal circumstances, because it is really not part of the header), it is handled in the code and won't cause any trouble.
This exact behavior is not easily testable on CI, as it would require to have an OSX agent.

What can be possibly done is to compare the Cmake cache values with the expected values for our agents, but this makes the test be hardware + OS + installation dependent.