Fix arrow install with custom INSTALL_PREFIX

[czentgr]

If the recently added INSTALL_PREFIX does not point to /usr/local in Linux then the bundled Thrift installation from the Arrow dependency fails because it cannot locate Boost.

Boost was previously installed into the INSTALL_PREFIX but the Thrift CMake in Arrow does not pass on any PREFIX_PATH settings to Thrift. The PREFIX_PATH is set to the INSTALL_PREFIX and works with Arrow itself just fine.

The error would be

Could NOT find Boost (missing: Boost_INCLUDE_DIR) (Required is at least
version "1.56")

[netlify]

✅ Deploy Preview for meta-velox canceled.

Name	Link
🔨 Latest commit	13b9727
🔍 Latest deploy log	https://app.netlify.com/sites/meta-velox/deploys/67044997a479050008892368

[czentgr]

@majetideepak FYI

[zuyu]

@czentgr I think #11059 also fixes the Arrow build. So, we might not need the fix in this PR.

[majetideepak]

@czentgr I see that thrift needs boost only for testing. https://thrift.apache.org/lib/cpp.html
Can you check if we can disable the thrift tests?

[czentgr]

@czentgr I think #11059 also fixes the Arrow build. So, we might not need the fix in this PR.

@majetideepak ^^ please see.
@zuyu this proposal would change how the INSTALL_PREFIX works. I do not believe this is the correct solution.

[czentgr]

@czentgr I see that thrift needs boost only for testing. https://thrift.apache.org/lib/cpp.html Can you check if we can disable the thrift tests?

Yes, we can turn them off. However, they are explicitly turned on in the install function, e.g.

velox/scripts/setup-centos9.sh

Line 194 in 4e45bc5

-DARROW_TESTING=ON \

We can turn it off and see what happens. We don't use the tests but maybe someone else is relying on them to be present?

EDIT: the thrift tests. They come in from the thrift bundling. Checking.

[majetideepak]

Can we turn off the Thrift tests?

[czentgr]

While it says BOOST is needed for thrift tests only, in the arrow expernal projects the BUILD_TESTING flag is turned off by default. We can see this being passed to the thrift configure

[13/218] Performing configure step for 'thrift_ep'
   '/usr/local/lib/python3.10/dist-packages/cmake/data/bin/cmake' '-DCMAKE_C_COMPILER=/usr/bin/cc' '-DCMAKE_CXX_COMPILER=/usr/bin/c++' '-DCMAKE_AR=/usr/bin/ar' '-DCMAKE_RANLIB=/usr/bin/ranlib' '-DBUILD_SHARED_LIBS=OFF' '-DBUILD_STATIC_LIBS=ON' '-DBUILD_TESTING=OFF' '-DCMAKE_BUILD_TYPE=RELEASE' '-DCMAKE_CXX_FLAGS=-mavx2 -mfma -mavx -mf16c -mlzcnt -std=c++17 -mbmi2 -fdiagnostics-color=always -fPIC' '-DCMAKE_CXX_FLAGS_DEBUG=-g -O0 -ggdb   -Wno-error' '-DCMAKE_CXX_FLAGS_MISIZEREL=-Os -DNDEBUG ' '-DCMAKE_CXX_FLAGS_RELEASE=-O3 -DNDEBUG -O2 -ftree-vectorize  ' '-DCMAKE_CXX_FLAGS_RELWITHDEBINFO=-O2 -g -DNDEBUG -ftree-vectorize -ggdb  ' '-DCMAKE_CXX_STANDARD=17' '-DCMAKE_C_FLAGS= -fPIC' '-DCMAKE_C_FLAGS_DEBUG=-g -O0 -ggdb   -Wno-error' '-DCMAKE_C_FLAGS_MISIZEREL=-Os -DNDEBUG ' '-DCMAKE_C_FLAGS_RELEASE=-O3 -DNDEBUG -O2 -ftree-vectorize  ' '-DCMAKE_C_FLAGS_RELWITHDEBINFO=-O2 -g -DNDEBUG -ftree-vectorize -ggdb  ' '-DCMAKE_EXPORT_NO_PACKAGE_REGISTRY=' '-DCMAKE_FIND_PACKAGE_NO_PACKAGE_REGISTRY=' '-DCMAKE_INSTALL_LIBDIR=lib' '-DCMAKE_OSX_SYSROOT=' '-DCMAKE_VERBOSE_MAKEFILE=FALSE' '-DCMAKE_C_COMPILER_LAUNCHER=/usr/bin/ccache' '-DCMAKE_CXX_COMPILER_LAUNCHER=/usr/bin/ccache' '-DCMAKE_INSTALL_PREFIX=/root/velox/deps-download/arrow/cpp/_build/thrift_ep-install' '-DCMAKE_INSTALL_RPATH=/root/velox/deps-download/arrow/cpp/_build/thrift_ep-install/lib' '-DBoost_NO_BOOST_CMAKE=ON' '-DBUILD_COMPILER=OFF' '-DBUILD_EXAMPLES=OFF' '-DBUILD_TUTORIALS=OFF' '-DCMAKE_DEBUG_POSTFIX=' '-DWITH_AS3=OFF' '-DWITH_CPP=ON' '-DWITH_C_GLIB=OFF' '-DWITH_JAVA=OFF' '-DWITH_JAVASCRIPT=OFF' '-DWITH_LIBEVENT=OFF' '-DWITH_NODEJS=OFF' '-DWITH_PYTHON=OFF' '-DWITH_QT5=OFF' '-DWITH_ZLIB=OFF' '-GNinja' '-S' '/root/velox/deps-download/arrow/cpp/_build/thrift_ep-prefix/src/thrift_ep' '-B' '/root/velox/deps-download/arrow/cpp/_build/thrift_ep-prefix/src/thrift_ep-build'

'-DBUILD_TESTING=OFF' is set.

The issue is that this line comes before checking the BUILD_TESTING value in the thrift CMakeLists.txt

# Dependencies
include(BoostMacros)
find_package(Threads)
include(CTest)

if(BUILD_TESTING)
  message(STATUS "Building with unittests")

  enable_testing()
  # Define "make check" as alias for "make test"
  add_custom_target(check COMMAND ctest)
else ()
  message(STATUS "Building without tests")
endif ()

but the error occurs due to BoostMacros not being found:

  build/cmake/BoostMacros.cmake:23 (find_package)
  lib/cpp/CMakeLists.txt:22 (REQUIRE_BOOST_HEADERS)

In summary, the documentation is not quite correct. Boost is required regardless of whether or not testing is enabled. Or when it says boost is only used for testing, it doesn't mean that BUILD_TESTING set to false avoids the Boost dependency.

[facebook-github-bot]

@xiaoxmeng has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

[facebook-github-bot]

@xiaoxmeng merged this pull request in 63c848d.

[conbench-facebook]

Conbench analyzed the 1 benchmark run on commit 63c848d6.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.