Skip to content
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.

Sign up for GitHub

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

Jump to bottom

ARROW-6858: [C++] Simplify transitive build option dependencies #14224

Merged
merged 4 commits into from
Sep 27, 2022
Merged

ARROW-6858: [C++] Simplify transitive build option dependencies #14224

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
de04fca
ARROW-6858: [C++] Simplify transitive build option dependencies
kou Sep 23, 2022
b8ccd84
Fix wording
kou Sep 26, 2022
fdfa168
Fix if position
kou Sep 26, 2022
2cd791d
Fix end mark
kou Sep 27, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions ci/docker/conda-integration.dockerfile
View file
Open in desktop
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
View file
Open in desktop
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()
Comment on lines -412 to -419
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW this broke my local development because of no longer including those (although I should probably start using presets ..)

Now, it's probably fine to remove this now Python C++ has moved, but we do assume that some C++ modules are built on the pyarrow side (eg we assume that CSV is always built, while with the above change you need to ensure manually that this is done in your cmake call).
In any case we should update the documentation at https://arrow.apache.org/docs/dev/developers/python.html#build-and-test to indicate that there are a few components required to be able to build pyarrow.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, thanks for the info.

OK. I add ARROW_PYTHON again but I make it a deprecated option. We'll remove ARROW_PYTHON eventually.

See also: ARROW-17868


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
View file
Open in desktop
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