-
Notifications
You must be signed in to change notification settings - Fork 469
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
ORC-265: [C++] Add documentation for C++ build support #191
Conversation
site/_docs/building.md
Outdated
LZ4_HOME=<PATH to LZ4 library> \ | ||
PROTOBUF_HOME=<PATH to Protobuf library> \ | ||
cmake .. -DBUILD_JAVA=OFF | ||
% make package test-out |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd suggest using:
cmake .. -DSNAPPY_HOME=<PATH> -DZLIB_HOME=<PATH> ...
to be more consistent.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi Owen, the current setup depends on <LIBNAME>_HOME
being environmental variables and not CMake variables.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need to explicitly set OPENSSL_ROOT_DIR on my mac, otherwise I will end up with the following error message:
CMake Error at /Applications/CMake.app/Contents/share/cmake-3.7/Modules/FindPackageHandleStandardArgs.cmake:138 (message):
Could NOT find OpenSSL, try to set the path to OpenSSL root folder in the
system variable OPENSSL_ROOT_DIR (missing: OPENSSL_INCLUDE_DIR)
Call Stack (most recent call first):
/Applications/CMake.app/Contents/share/cmake-3.7/Modules/FindPackageHandleStandardArgs.cmake:378 (_FPHSA_FAILURE_MESSAGE)
/Applications/CMake.app/Contents/share/cmake-3.7/Modules/FindOpenSSL.cmake:385 (find_package_handle_standard_args)
CMakeLists.txt:41 (find_package)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wgtmac That is correct. We manually set it in travis-ci testing as well.
cmake -DOPENSSL_ROOT_DIR=`brew --prefix openssl` ..
I will check with Anatoli Shein if we can fix this / or add documentation here. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm using cmake 3.5.1, but it works for me. Why would you need environment variables instead of cmake variables?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, looking at the ThirdpartyToolchain.cmake, it goes the other way. It is looking at the environment variable and setting the cmake variable. Setting the cmake variable directly works fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is correct. The recent changes made did put the dependency on CMake variables. Fixed the doc.
f13da19
to
ef3ae96
Compare
I verified the modified page by building locally.