Bundled folly installation needs to link with [email protected]

[majetideepak]

Problem description

Folly needs OpenSSL 1.1 version.
In the presence of other OpenSSL versions, it is possible that folly in bundled mode can pick up other incompatible versions.
In the setup-macos.sh scripts, we set OPENSSL_ROOT_DIR=$(brew --prefix [email protected]).
We need to set something similar for bundled folly install as well.

Build error seen
CMake Error at /usr/local/Cellar/cmake/3.26.4/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:230 (message):
Could NOT find OpenSSL, try to set the path to OpenSSL root folder in the
system variable OPENSSL_ROOT_DIR: Found unsuitable version "1.0.2d", but
required is at least "1.1.1" (found
/opt/homebrew/Cellar/openssl/1.0.2d_1/lib/libcrypto.dylib, )
Call Stack (most recent call first):
/usr/local/Cellar/cmake/3.26.4/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:598 (_FPHSA_FAILURE_MESSAGE)
/usr/local/Cellar/cmake/3.26.4/share/cmake/Modules/FindOpenSSL.cmake:670 (find_package_handle_standard_args)
_build/release/_deps/folly-src/CMake/folly-deps.cmake:81 (find_package)
_build/release/_deps/folly-src/CMakeLists.txt:132 (include)

System information

Mac M1 with OpenSSL 1.0.2d present.

CMake log

No response

[majetideepak]

This error is reported here #5222 (comment)
and here #1446

[rishitc]

Hi @majetideepak,
I want to share a quick update on my PR for this issue:

• I set the environment variable OPENSSL_ROOT_DIR in the main CMake file so that it is available to folly when building Velox.
  • This step is done only when I detect MacOS via CMake.
• Locally building the project with CPU_TARGET="arm64" make works fine.
  • I don't need to run the command export OPENSSL_ROOT_DIR=/opt/homebrew/opt/[email protected] as a prerequisite anymore

I should have my PR up for review soon 👍

[majetideepak]

@rishitc thanks for the update. Can you check if just make works without the need to add CPU_TARGET="arm64".

[majetideepak]

The bundled folly CMake files are in velox/CMake/resolve_dependency_modules. Considering adding the variable there.

[rishitc]

@rishitc thanks for the update. Can you check if just make works without the need to add CPU_TARGET="arm64".

Hi @majetideepak,
Yes, the build works fine without adding CPU_TARGET="arm64" as well 👍

[rishitc]

Hi @majetideepak,
Here is an update on my pull request. It's now up for review (link: #5361), and I made sure to pass all the CI tests.

Please let me know if you need any further information or have any questions.
Also, would you like me to make further changes to the patch? Just let me know, and I'll gladly make any necessary adjustments.

Thanks

[rishitc]

Hi @majetideepak,
I noticed that no one has been assigned this issue yet. Can you assign this issue to me since I've already raised the PR for resolving it?

Thanks

[majetideepak]

@rishitc I missed your updates. Looks like we are seeing issues with openssl version on CI as well. We will merge your fix.

[rishitc]

Hi @majetideepak,
Thanks for the update 👍

I was also following the discussion in #5426, from which I gathered that my fix is not solving the CI issue, though it appears to fix the problem when building locally.

There was one observation from your PR (#5428) that the macos-build-macos-intel CI build failed for your PR while it ran successfully in my PR (#5361). I'm still trying to understand why the same code change gave inconsistent results.
Could the changes made in some of the commits between my CI run, and yours have caused the variation in behaviour for the same code?

I looked at the output for the failed build for your PR (#5428). Looking at the CI output (link) the reason the build failure is that I believe the CI used OpenSSL version 3 (the giveaway was the 'HMAC_CTX_free' deprecated errors) even though the environment variable OPENSSL_ROOT_DIR was set via CMake, so it should have used version 1.1.

Locally, even on my system (MacOS M1), the default OpenSSL is version 3; however, setting the OPENSSL_ROOT_DIR environment variable causes folly to use the correct version of OpenSSL 1.1.

[majetideepak]

Could the changes made in some of the commits between my CI run, and yours have caused the variation in behaviour for the same code?

@rishitc I believe the CI MacOS environment changed between your run and my run which caused this.