Skip to content

Commit

Permalink
ARROW-6858: [C++] Simplify transitive build option dependencies (#14224)
Browse files Browse the repository at this point in the history
This approach adds "depends" information to each (bool) build options and use it to resolve transitive build option dependencies automatically. This approach implements topological sort in CMake to resolve transitive dependencies.

Another approach proposed in the associated Jira issue: It creates a Python script that generates a CMake code (.cmake file) that handles transitive dependencies. Dependencies information are written in the Python script.

I think that this approach is better than the another approach because:

* We can put option definitions and their dependencies into the same place. (We don't need to put them into .cmake and .py.)
* We don't need to regenerate a .cmake file when we update option dependencies.
* We can specify dependencies information with a simple way. (We can just add "DEPENDS ARROW_XXX ARROW_YYY ..." to an option definition.)

Here are downsides of this approach:

* We need to maintain topological sort implementation in CMake. Because CMake doesn't provide a topological sort feature that is used in CMake internally. But topological sort algorithm is well-known (Tarjan's algorithm was published at 1976) and its implementation in this approach has only 20+ CMake lines. I think that we can maintain it.
* This can't support complex conditions such as "ARROW_X AND NOT ARROW_Y". But we don't have any complex condition for now.

Lead-authored-by: Sutou Kouhei <kou@clear-code.com>
Co-authored-by: Sutou Kouhei <kou@cozmixng.org>
Signed-off-by: Yibo Cai <yibo.cai@arm.com>
  • Loading branch information
kou and kou committed Sep 27, 2022
1 parent 44bbf9c commit 53ac2a0
Show file tree
Hide file tree
Showing 4 changed files with 155 additions and 135 deletions.
1 change: 1 addition & 0 deletions ci/docker/conda-integration.dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ ENV ARROW_BUILD_INTEGRATION=ON \
ARROW_DATASET=OFF \
ARROW_FILESYSTEM=OFF \
ARROW_FLIGHT=ON \
ARROW_FLIGHT_SQL=ON \
ARROW_GANDIVA=OFF \
ARROW_HDFS=OFF \
ARROW_JEMALLOC=OFF \
Expand Down
102 changes: 10 additions & 92 deletions cpp/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -167,16 +167,6 @@ if("$ENV{CMAKE_EXPORT_COMPILE_COMMANDS}" STREQUAL "1"
set(CMAKE_EXPORT_COMPILE_COMMANDS 1)
endif()

# ----------------------------------------------------------------------
# cmake options
include(DefineOptions)

if(ARROW_BUILD_SHARED AND NOT ARROW_POSITION_INDEPENDENT_CODE)
message(WARNING "Can't disable position-independent code to build shared libraries, enabling"
)
set(ARROW_POSITION_INDEPENDENT_CODE ON)
endif()

# Needed for linting targets, etc.
if(${CMAKE_VERSION} VERSION_LESS "3.12.0")
find_package(PythonInterp)
Expand All @@ -192,6 +182,16 @@ else()
set(PYTHON_EXECUTABLE ${Python3_EXECUTABLE})
endif()

# ----------------------------------------------------------------------
# cmake options
include(DefineOptions)

if(ARROW_BUILD_SHARED AND NOT ARROW_POSITION_INDEPENDENT_CODE)
message(WARNING "Can't disable position-independent code to build shared libraries, enabling"
)
set(ARROW_POSITION_INDEPENDENT_CODE ON)
endif()

if(ARROW_USE_SCCACHE
AND NOT CMAKE_C_COMPILER_LAUNCHER
AND NOT CMAKE_CXX_COMPILER_LAUNCHER)
Expand Down Expand Up @@ -350,88 +350,6 @@ if(UNIX)
add_custom_target(iwyu-all ${BUILD_SUPPORT_DIR}/iwyu/iwyu.sh all)
endif(UNIX)

#
# Set up various options
#

if(ARROW_BUILD_BENCHMARKS
OR ARROW_BUILD_TESTS
OR ARROW_BUILD_INTEGRATION
OR ARROW_FUZZING)
set(ARROW_TESTING ON)
endif()

if(ARROW_GANDIVA)
set(ARROW_WITH_RE2 ON)
set(ARROW_WITH_UTF8PROC ON)
endif()

if(ARROW_BUILD_INTEGRATION AND ARROW_FLIGHT)
set(ARROW_FLIGHT_SQL ON)
endif()

if(ARROW_FLIGHT_SQL)
set(ARROW_FLIGHT ON)
endif()

if(ARROW_CUDA
OR ARROW_FLIGHT
OR ARROW_PARQUET
OR ARROW_BUILD_TESTS
OR ARROW_BUILD_BENCHMARKS)
set(ARROW_IPC ON)
endif()

if(ARROW_SUBSTRAIT)
set(ARROW_PARQUET ON)
set(ARROW_IPC ON)
set(ARROW_COMPUTE ON)
set(ARROW_DATASET ON)
endif()

if(ARROW_SKYHOOK)
set(ARROW_DATASET ON)
set(ARROW_PARQUET ON)
set(ARROW_WITH_LZ4 ON)
set(ARROW_WITH_SNAPPY ON)
endif()

if(ARROW_TESTING)
set(ARROW_JSON ON)
endif()

if(ARROW_DATASET)
set(ARROW_COMPUTE ON)
set(ARROW_FILESYSTEM ON)
endif()

if(ARROW_PARQUET)
set(ARROW_COMPUTE ON)
endif()

if(ARROW_PYTHON)
set(ARROW_COMPUTE ON)
set(ARROW_CSV ON)
set(ARROW_DATASET ON)
set(ARROW_FILESYSTEM ON)
set(ARROW_HDFS ON)
set(ARROW_JSON ON)
endif()

if(MSVC_TOOLCHAIN)
# ORC doesn't build on windows
set(ARROW_ORC OFF)
# Plasma using glog is not fully tested on windows.
set(ARROW_USE_GLOG OFF)
endif()

if(ARROW_ORC)
set(ARROW_WITH_LZ4 ON)
set(ARROW_WITH_SNAPPY ON)
set(ARROW_WITH_ZLIB ON)
set(ARROW_WITH_ZSTD ON)
endif()

# datetime code used by iOS requires zlib support
if(IOS)
set(ARROW_WITH_ZLIB ON)
Expand Down
23 changes: 1 addition & 22 deletions cpp/CMakePresets.json
Original file line number Diff line number Diff line change
Expand Up @@ -117,23 +117,14 @@
"ARROW_GANDIVA": "ON"
}
},
{
"name": "features-python",
"inherits": "features-main",
"hidden": true,
"cacheVariables": {
"ARROW_PYTHON": "ON"
}
},
{
"name": "features-maximal",
"inherits": [
"features-main",
"features-cuda",
"features-filesystems",
"features-flight",
"features-gandiva",
"features-python"
"features-gandiva"
],
"hidden": true,
"displayName": "Debug build with everything enabled (except benchmarks and CUDA)",
Expand Down Expand Up @@ -194,12 +185,6 @@
"displayName": "Debug build with tests and Gandiva",
"cacheVariables": {}
},
{
"name": "ninja-debug-python",
"inherits": ["base-debug", "features-python"],
"displayName": "Debug build with tests and Python support",
"cacheVariables": {}
},
{
"name": "ninja-debug-maximal",
"inherits": ["base-debug", "features-maximal"],
Expand Down Expand Up @@ -243,12 +228,6 @@
"displayName": "Release build with Gandiva",
"cacheVariables": {}
},
{
"name": "ninja-release-python",
"inherits": ["base-release", "features-python"],
"displayName": "Release build with Python support",
"cacheVariables": {}
},
{
"name": "ninja-release-maximal",
"inherits": ["base-release", "features-maximal"],
Expand Down
Loading

0 comments on commit 53ac2a0

Please sign in to comment.