kou commented on code in PR #41494:
URL: https://github.com/apache/arrow/pull/41494#discussion_r1588416238
##########
python/CMakeLists.txt:
##########
@@ -265,11 +246,44 @@ message(STATUS "NumPy include dir: ${NUMPY_INCLUDE_DIRS}")
include(UseCython)
-# PyArrow C++
+# Arrow C++ and set default PyArrow build options
include(GNUInstallDirs)
-
find_package(Arrow REQUIRED)
+# Top level cmake dir
+if("${CMAKE_SOURCE_DIR}" STREQUAL "${CMAKE_CURRENT_SOURCE_DIR}")
Review Comment:
It's for avoiding defining our options when this is used via
`add_subdirectory()` (or
[`FetchContent`](https://cmake.org/cmake/help/latest/module/FetchContent.html)).
We don't need this because:
* This is never used via `add_subdirectory()`
* Our option definitions are harmless with `add_subdirectory()` use
##########
python/setup.py:
##########
@@ -270,23 +235,28 @@ def append_cmake_bool(value, varname):
cmake_options.append('-D{0}={1}'.format(
varname, 'on' if value else 'off'))
+ def maybe_append_cmake_bool(varname):
+ env_var = varname.replace("PYARROW_BUILD", "PYARROW_WITH")
+ if env_var in os.environ:
+ value = strtobool(os.environ.get(env_var))
+ append_cmake_bool(value, varname)
Review Comment:
How about doing this in `CMakeLists.txt` too?
We can refer an environment variable by `"$ENV{PYARROW_WITH_CUDA}` in
`CMakeLists.txt`.
See also: https://cmake.org/cmake/help/latest/variable/ENV.html
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
