[VL] Add find and build glog/gflags #3638
Conversation
|
Thanks for opening a pull request! Could you open an issue for this pull request on Github Issues? https://github.com/oap-project/gluten/issues Then could you also rename commit message and pull request title in the following format? See also: |
|
@Yohahaha Could you help to review? Thanks! |
| set(GLUTEN_GFLAGS_BUILD_SHA256_CHECKSUM | ||
| 34af2f15cf7367513b352bdcd2493ab14ce43692d2dcd9dfc499492966c64dcf) | ||
| string(CONCAT GLUTEN_GFLAGS_SOURCE_URL | ||
| "https://github.com/gflags/gflags/archive/refs/tags/" | ||
| "v${GLUTEN_GFLAGS_VERSION}.tar.gz") |
There was a problem hiding this comment.
Could we use below pattern to allow user specify deps URL? same for glog.
if(DEFINED ENV{GLUTEN_GTEST_SOURCE_URL})
set(GLUTEN_GTEST_SOURCE_URL "$ENV{GLUTEN_GTEST_SOURCE_URL}")
endif()
There was a problem hiding this comment.
Let's merge this patch first, I will refine these deps download URL in follow up patch.
| set(GLUTEN_GLOG_MINIMUM_VERSION 0.4.0) | ||
| set(GLUTEN_GLOG_VERSION 0.6.0) | ||
|
|
||
| if(NOT BUILD_GLOG) |
There was a problem hiding this comment.
Curious could we just find first, then check glog_FOUND? seems BUILD_GLOG can be removed.
There was a problem hiding this comment.
It's a bit tricky here. We cannot make sure user has both glog and gflags installed. If glog is found but gflags is missing, it fails. The option is for ignoring system glog then directly build glog and gflags from source. https://github.com/oap-project/gluten/pull/3638/files#diff-9df362ebf18ceb320fa6423ff9140fd2bd75906d57d92ca018febfc3a93864a9R66
|
Thank you! LGTM, only some comments. |
|
===== Performance report for TPCH SF2000 with Velox backend, for reference only ====
|
Currently, the CMake/Findglog.cmake module only finds libglog itself but doesn't resolve its dependency gflags. When libglog is found as static library, linking it to shared library such as
libvelox.sowill lead to undefined symbol.