Skip to content

Commit

Permalink
CMake: Move resolve dependency macros under velox_ prefix (#11392)
Browse files Browse the repository at this point in the history
Summary:
It is best practice in CMake to use a project prefix for project specific macros, functions and variables. This makes it easier to compose projects in larger builds as well as differentiate general functions from project specific functions.

This moves several macros and it will require some simple migration of any CMake code reusing the resolve dependency macros:

 * build_dependency -> velox_build_dependency
 * list_subdirs -> removed after reviewer suggestion
 * resolve_dependency -> velox_resolve_dependency
 * resolve_dependency_url -> velox_resolve_dependency_url
 * set_source -> velox_set_source
 * set_with_default -> velox_set_with_default

This should be a simple change as the other changes are just to adapt to the changed macro names.

Pull Request resolved: #11392

Reviewed By: xiaoxmeng

Differential Revision: D65763456

Pulled By: kgpai

fbshipit-source-id: f748e5431e246dc8c095c4838ef2f4b96de6acf2
  • Loading branch information
cryos authored and facebook-github-bot committed Nov 11, 2024
1 parent cc48ac9 commit dfc737d
Show file tree
Hide file tree
Showing 24 changed files with 121 additions and 125 deletions.
59 changes: 19 additions & 40 deletions CMake/ResolveDependency.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

# MODULE: ThirdpartyToolchain
#
# PROVIDES: resolve_dependency( dependency_name dependencyName [...] )
# PROVIDES: velox_resolve_dependency( dependency_name dependencyName [...] )
#
# Provides the ability to resolve third party dependencies. If the dependency is
# already available in the system it will be used.
Expand All @@ -24,7 +24,7 @@
# should match find_package() standards.
#
# EXAMPLE USAGE: # Download and setup or use already installed dependency.
# include(ThirdpartyToolchain) resolve_dependency(folly)
# include(ThirdpartyToolchain) velox_resolve_dependency(folly)
#
# ==============================================================================

Expand All @@ -38,19 +38,19 @@ list(APPEND CMAKE_MODULE_PATH
# Enable SSL certificate verification for file downloads
set(CMAKE_TLS_VERIFY true)

macro(build_dependency dependency_name)
macro(velox_build_dependency dependency_name)
string(TOLOWER ${dependency_name} dependency_name_lower)
include(${dependency_name_lower})
endmacro()

# * Macro to resolve third-party dependencies.
#
# Provides the macro resolve_dependency(). This macro will allow us to find the
# dependency via the usage of find_package or use the custom
# build_dependency(dependency_name) macro to download and build the third party
# dependency.
# Provides the macro velox_resolve_dependency(). This macro will allow us to
# find the dependency via the usage of find_package or use the custom
# velox_build_dependency(dependency_name) macro to download and build the third
# party dependency.
#
# resolve_dependency(dependency_name [...] )
# velox_resolve_dependency(dependency_name [...] )
#
# [...]: the macro will pass all arguments after DEPENDENCY_NAME on to
# find_package. ${dependency_name}_SOURCE is expected to be set to either AUTO,
Expand All @@ -63,7 +63,7 @@ endmacro()
# In the case of setting ${dependency_name}_SOURCE to SYSTEM if the dependency
# is not found the build will fail and will not fall back to download and build
# from source.
macro(resolve_dependency dependency_name)
macro(velox_resolve_dependency dependency_name)
set(find_package_args ${dependency_name} ${ARGN})
list(REMOVE_ITEM find_package_args REQUIRED QUIET)
if(${dependency_name}_SOURCE STREQUAL "AUTO")
Expand All @@ -72,13 +72,13 @@ macro(resolve_dependency dependency_name)
set(${dependency_name}_SOURCE "SYSTEM")
else()
set(${dependency_name}_SOURCE "BUNDLED")
build_dependency(${dependency_name})
velox_build_dependency(${dependency_name})
endif()
message(STATUS "Using ${${dependency_name}_SOURCE} ${dependency_name}")
elseif(${dependency_name}_SOURCE STREQUAL "SYSTEM")
find_package(${find_package_args} REQUIRED)
elseif(${dependency_name}_SOURCE STREQUAL "BUNDLED")
build_dependency(${dependency_name})
velox_build_dependency(${dependency_name})
else()
message(
FATAL_ERROR
Expand All @@ -87,9 +87,9 @@ macro(resolve_dependency dependency_name)
endmacro()

# By using a macro we don't need to propagate the value into the parent scope.
macro(set_source dependency_name)
set_with_default(${dependency_name}_SOURCE ${dependency_name}_SOURCE
${VELOX_DEPENDENCY_SOURCE})
macro(velox_set_source dependency_name)
velox_set_with_default(${dependency_name}_SOURCE ${dependency_name}_SOURCE
${VELOX_DEPENDENCY_SOURCE})
message(
STATUS "Setting ${dependency_name} source to ${${dependency_name}_SOURCE}")
endmacro()
Expand All @@ -98,7 +98,7 @@ endmacro()
# ENV or var_name is defined then set var_name to ${DEFAULT}. If called from
# within a nested scope the variable will not propagate into outer scopes
# automatically! Use PARENT_SCOPE.
function(set_with_default var_name envvar_name default)
function(velox_set_with_default var_name envvar_name default)
if(DEFINED ENV{${envvar_name}})
set(${var_name}
$ENV{${envvar_name}}
Expand All @@ -110,40 +110,19 @@ function(set_with_default var_name envvar_name default)
endif()
endfunction()

# List subdirectories of ${dir}
function(list_subdirs var dir)
if(NOT IS_DIRECTORY ${dir})
message(FATAL_ERROR "${dir} is not a directory!")
endif()

# finds files & dirs
file(GLOB children ${dir}/*)
set(dirs "")

foreach(child ${children})
if(IS_DIRECTORY ${child})
list(APPEND dirs ${child})
endif()
endforeach()

set(${var}
${dirs}
PARENT_SCOPE)
endfunction()

# Set custom source url with a optional sha256 checksum.
macro(resolve_dependency_url dependency_name)
macro(velox_resolve_dependency_url dependency_name)
# Prepend prefix for default checksum.
string(PREPEND VELOX_${dependency_name}_BUILD_SHA256_CHECKSUM "SHA256=")

set_with_default(
velox_set_with_default(
VELOX_${dependency_name}_SOURCE_URL VELOX_${dependency_name}_URL
${VELOX_${dependency_name}_SOURCE_URL})
message(VERBOSE "Set VELOX_${dependency_name}_SOURCE_URL to "
"${VELOX_${dependency_name}_SOURCE_URL}")
if(DEFINED ENV{VELOX_${dependency_name}_URL})
set_with_default(VELOX_${dependency_name}_BUILD_SHA256_CHECKSUM
VELOX_${dependency_name}_SHA256 "")
velox_set_with_default(VELOX_${dependency_name}_BUILD_SHA256_CHECKSUM
VELOX_${dependency_name}_SHA256 "")
if(DEFINED ENV{VELOX_${dependency_name}_SHA256})
string(PREPEND VELOX_${dependency_name}_BUILD_SHA256_CHECKSUM "SHA256=")
endif()
Expand Down
2 changes: 1 addition & 1 deletion CMake/resolve_dependency_modules/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ Ideally all patches should be upstream when possible and removed once merged.
## Adding new dependencies

- Copy `template.cmake` and rename it to the name used in `find_package` but all lower-case.
- Switch `find_package` vs `set_source('package')` `resolve_dependency('package' 'optional args for find_package')` in `CMakeLists.txt`
- Switch `find_package` vs `velox_set_source('package')` `velox_resolve_dependency('package' 'optional args for find_package')` in `CMakeLists.txt`
- Update the template with the correct package name and download url/repo etc., set any necessary package options
- Try to build and make necessary changes
- Repeat until success :D (Feel free to raise and issue for review & support)
Expand Down
9 changes: 5 additions & 4 deletions CMake/resolve_dependency_modules/absl.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,12 @@ include_guard(GLOBAL)
set(VELOX_ABSL_BUILD_VERSION 20240116.2)
set(VELOX_ABSL_BUILD_SHA256_CHECKSUM
733726b8c3a6d39a4120d7e45ea8b41a434cdacde401cba500f14236c49b39dc)
string(CONCAT VELOX_ABSL_SOURCE_URL
"https://github.com/abseil/abseil-cpp/archive/refs/tags/"
"${VELOX_ABSL_BUILD_VERSION}.tar.gz")
string(
CONCAT VELOX_ABSL_SOURCE_URL
"https://github.com/abseil/abseil-cpp/archive/refs/tags/"
"${VELOX_ABSL_BUILD_VERSION}.tar.gz")

resolve_dependency_url(ABSL)
velox_resolve_dependency_url(ABSL)

message(STATUS "Building Abseil from source")

Expand Down
2 changes: 1 addition & 1 deletion CMake/resolve_dependency_modules/arrow/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ if(VELOX_ENABLE_ARROW)
"https://archive.apache.org/dist/arrow/arrow-${VELOX_ARROW_BUILD_VERSION}/apache-arrow-${VELOX_ARROW_BUILD_VERSION}.tar.gz"
)

resolve_dependency_url(ARROW)
velox_resolve_dependency_url(ARROW)

ExternalProject_Add(
arrow_ep
Expand Down
6 changes: 4 additions & 2 deletions CMake/resolve_dependency_modules/boost/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ string(
set(VELOX_BOOST_BUILD_SHA256_CHECKSUM
4d27e9efed0f6f152dc28db6430b9d3dfb40c0345da7342eaa5a987dde57bd95)

resolve_dependency_url(BOOST)
velox_resolve_dependency_url(BOOST)
message(STATUS "Building boost from source")

# required for Boost::thread
Expand Down Expand Up @@ -66,5 +66,7 @@ set(BUILD_SHARED_LIBS OFF)
FetchContent_MakeAvailable(Boost)

list(TRANSFORM BOOST_HEADER_ONLY PREPEND Boost::)
target_link_libraries(boost_headers INTERFACE ${BOOST_HEADER_ONLY})
target_link_libraries(
boost_headers
INTERFACE ${BOOST_HEADER_ONLY})
add_library(Boost::headers ALIAS boost_headers)
2 changes: 1 addition & 1 deletion CMake/resolve_dependency_modules/c-ares.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ string(
"https://github.com/c-ares/c-ares/archive/refs/tags/"
"${VELOX_CARES_BUILD_VERSION}.tar.gz")

resolve_dependency_url(CARES)
velox_resolve_dependency_url(CARES)

message(STATUS "Building C-ARES from source")

Expand Down
4 changes: 2 additions & 2 deletions CMake/resolve_dependency_modules/cpr.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@ set(VELOX_CPR_SOURCE_URL
# Add the dependency for curl, so that we can define the source URL for curl in
# curl.cmake. This will override the curl version declared by cpr.
set(curl_SOURCE BUNDLED)
resolve_dependency(curl)
velox_resolve_dependency(curl)

resolve_dependency_url(CPR)
velox_resolve_dependency_url(CPR)

message(STATUS "Building cpr from source")
FetchContent_Declare(
Expand Down
2 changes: 1 addition & 1 deletion CMake/resolve_dependency_modules/curl.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ string(
VELOX_CURL_SOURCE_URL "https://github.com/curl/curl/releases/download/"
"curl-${VELOX_CURL_VERSION_UNDERSCORES}/curl-${VELOX_CURL_VERSION}.tar.xz")

resolve_dependency_url(CURL)
velox_resolve_dependency_url(CURL)

FetchContent_Declare(
curl
Expand Down
2 changes: 1 addition & 1 deletion CMake/resolve_dependency_modules/duckdb.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ set(VELOX_DUCKDB_SOURCE_URL
"https://github.com/duckdb/duckdb/archive/refs/tags/v${VELOX_DUCKDB_VERSION}.tar.gz"
)

resolve_dependency_url(DUCKDB)
velox_resolve_dependency_url(DUCKDB)

message(STATUS "Building DuckDB from source")
# We need remove-ccache.patch to remove adding ccache to the build command
Expand Down
2 changes: 1 addition & 1 deletion CMake/resolve_dependency_modules/fmt.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ set(VELOX_FMT_BUILD_SHA256_CHECKSUM
set(VELOX_FMT_SOURCE_URL
"https://github.com/fmtlib/fmt/archive/${VELOX_FMT_VERSION}.tar.gz")

resolve_dependency_url(FMT)
velox_resolve_dependency_url(FMT)

message(STATUS "Building fmt from source")
FetchContent_Declare(
Expand Down
2 changes: 1 addition & 1 deletion CMake/resolve_dependency_modules/folly/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ set(VELOX_FOLLY_SOURCE_URL
"https://github.com/facebook/folly/releases/download/${VELOX_FOLLY_BUILD_VERSION}/folly-${VELOX_FOLLY_BUILD_VERSION}.tar.gz"
)

resolve_dependency_url(FOLLY)
velox_resolve_dependency_url(FOLLY)

message(STATUS "Building Folly from source")

Expand Down
9 changes: 5 additions & 4 deletions CMake/resolve_dependency_modules/gflags.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,12 @@ include_guard(GLOBAL)
set(VELOX_GFLAGS_VERSION 2.2.2)
set(VELOX_GFLAGS_BUILD_SHA256_CHECKSUM
34af2f15cf7367513b352bdcd2493ab14ce43692d2dcd9dfc499492966c64dcf)
string(CONCAT VELOX_GFLAGS_SOURCE_URL
"https://github.com/gflags/gflags/archive/refs/tags/"
"v${VELOX_GFLAGS_VERSION}.tar.gz")
string(
CONCAT VELOX_GFLAGS_SOURCE_URL
"https://github.com/gflags/gflags/archive/refs/tags/"
"v${VELOX_GFLAGS_VERSION}.tar.gz")

resolve_dependency_url(GFLAGS)
velox_resolve_dependency_url(GFLAGS)

message(STATUS "Building gflags from source")
FetchContent_Declare(
Expand Down
2 changes: 1 addition & 1 deletion CMake/resolve_dependency_modules/glog.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ set(VELOX_GLOG_SOURCE_URL
"https://github.com/google/glog/archive/refs/tags/v${VELOX_GLOG_VERSION}.tar.gz"
)

resolve_dependency_url(GLOG)
velox_resolve_dependency_url(GLOG)

message(STATUS "Building glog from source")
FetchContent_Declare(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
# limitations under the License.
include_guard(GLOBAL)

set_source(gRPC)
resolve_dependency(gRPC CONFIG 1.48.1 REQUIRED)
velox_set_source(gRPC)
velox_resolve_dependency(gRPC CONFIG 1.48.1 REQUIRED)

set(VELOX_GOOGLE_CLOUD_CPP_BUILD_VERSION 2.22.0)
set(VELOX_GOOGLE_CLOUD_CPP_BUILD_SHA256_CHECKSUM
Expand All @@ -24,7 +24,7 @@ string(
"https://github.com/googleapis/google-cloud-cpp/archive/refs/tags/"
"v${VELOX_GOOGLE_CLOUD_CPP_BUILD_VERSION}.tar.gz")

resolve_dependency_url(GOOGLE_CLOUD_CPP)
velox_resolve_dependency_url(GOOGLE_CLOUD_CPP)

message(STATUS "Building Google Cloud CPP storage from source")

Expand Down
6 changes: 3 additions & 3 deletions CMake/resolve_dependency_modules/grpc.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@
# limitations under the License.
include_guard(GLOBAL)

set_source(absl)
resolve_dependency(absl CONFIG REQUIRED)
velox_set_source(absl)
velox_resolve_dependency(absl CONFIG REQUIRED)

set(VELOX_GRPC_BUILD_VERSION 1.48.1)
set(VELOX_GRPC_BUILD_SHA256_CHECKSUM
Expand All @@ -24,7 +24,7 @@ string(
"https://github.com/grpc/grpc/archive/refs/tags/"
"v${VELOX_GRPC_BUILD_VERSION}.tar.gz")

resolve_dependency_url(GRPC)
velox_resolve_dependency_url(GRPC)

message(STATUS "Building gRPC from source")

Expand Down
2 changes: 1 addition & 1 deletion CMake/resolve_dependency_modules/gtest.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ set(VELOX_GTEST_SOURCE_URL
"https://github.com/google/googletest/archive/refs/tags/v${VELOX_GTEST_VERSION}.tar.gz"
)

resolve_dependency_url(GTEST)
velox_resolve_dependency_url(GTEST)

message(STATUS "Building gtest from source")
FetchContent_Declare(
Expand Down
20 changes: 15 additions & 5 deletions CMake/resolve_dependency_modules/icu.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@ string(
"release-${VELOX_ICU4C_BUILD_VERSION}-1/"
"icu4c-${VELOX_ICU4C_BUILD_VERSION}_1-src.tgz")

resolve_dependency_url(ICU4C)
velox_resolve_dependency_url(ICU4C)

message(STATUS "Building ICU4C from source")

ProcessorCount(NUM_JOBS)
set_with_default(NUM_JOBS NUM_THREADS ${NUM_JOBS})
velox_set_with_default(NUM_JOBS NUM_THREADS ${NUM_JOBS})
find_program(MAKE_PROGRAM make REQUIRED)

set(ICU_CFG --disable-tests --disable-samples)
Expand Down Expand Up @@ -65,12 +65,22 @@ file(MAKE_DIRECTORY ${ICU_INCLUDE_DIRS})
file(MAKE_DIRECTORY ${ICU_LIBRARIES})

# Create a target for each component
set(icu_components data i18n io uc tu test)
set(icu_components
data
i18n
io
uc
tu
test)

foreach(component ${icu_components})
add_library(ICU::${component} SHARED IMPORTED)
string(CONCAT ICU_${component}_LIBRARY ${ICU_LIBRARIES} "/libicu"
${component} ".so")
string(
CONCAT ICU_${component}_LIBRARY
${ICU_LIBRARIES}
"/libicu"
${component}
".so")
file(TOUCH ${ICU_${component}_LIBRARY})
set_target_properties(
ICU::${component}
Expand Down
13 changes: 7 additions & 6 deletions CMake/resolve_dependency_modules/protobuf.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,15 @@ if(${VELOX_PROTOBUF_BUILD_VERSION} LESS 22.0)
"v${VELOX_PROTOBUF_BUILD_VERSION}/protobuf-all-${VELOX_PROTOBUF_BUILD_VERSION}.tar.gz"
)
else()
set_source(absl)
resolve_dependency(absl CONFIG REQUIRED)
string(CONCAT VELOX_PROTOBUF_SOURCE_URL
"https://github.com/protocolbuffers/protobuf/archive/"
"v${VELOX_PROTOBUF_BUILD_VERSION}.tar.gz")
velox_set_source(absl)
velox_resolve_dependency(absl CONFIG REQUIRED)
string(
CONCAT VELOX_PROTOBUF_SOURCE_URL
"https://github.com/protocolbuffers/protobuf/archive/"
"v${VELOX_PROTOBUF_BUILD_VERSION}.tar.gz")
endif()

resolve_dependency_url(PROTOBUF)
velox_resolve_dependency_url(PROTOBUF)

message(STATUS "Building Protobuf from source")

Expand Down
Loading

0 comments on commit dfc737d

Please sign in to comment.