AlenkaF commented on code in PR #14925:
URL: https://github.com/apache/arrow/pull/14925#discussion_r1050609889


##########
cpp/cmake_modules/BuildUtils.cmake:
##########
@@ -176,9 +176,12 @@ function(ADD_ARROW_LIB LIB_NAME)
       BUILD_SHARED
       BUILD_STATIC
       CMAKE_PACKAGE_NAME
+      INSTALL_ARCHIVE_DIR
+      INSTALL_LIBRARY_DIR
+      INSTALL_RUNTIME_DIR

Review Comment:
   If I understand correctly these variables are new and can be used to 
manipulate where a library is installed? And `libarrow_python` will be 
installed in `arrow/python` folder now, not `arrow/python/pyarrow` if I 
understand correctly?



##########
cpp/src/arrow/symbols.map:
##########
@@ -32,7 +32,6 @@
     };
     # Also export C-level helpers
     arrow_*;
-    pyarrow_*;

Review Comment:
   +1



##########
python/CMakeLists.txt:
##########
@@ -223,21 +234,205 @@ include(UseCython)
 
 include_directories(SYSTEM ${NUMPY_INCLUDE_DIRS} ${PYTHON_INCLUDE_DIRS} src)
 
+# PyArrow C++
+include(GNUInstallDirs)
+
+find_package(Arrow REQUIRED)
+
+set(PYARROW_CPP_ROOT_DIR pyarrow/src)
+set(PYARROW_CPP_SOURCE_DIR ${PYARROW_CPP_ROOT_DIR}/arrow/python)
+set(PYARROW_CPP_SRCS
+    ${PYARROW_CPP_SOURCE_DIR}/arrow_to_pandas.cc
+    ${PYARROW_CPP_SOURCE_DIR}/benchmark.cc
+    ${PYARROW_CPP_SOURCE_DIR}/common.cc
+    ${PYARROW_CPP_SOURCE_DIR}/datetime.cc
+    ${PYARROW_CPP_SOURCE_DIR}/decimal.cc
+    ${PYARROW_CPP_SOURCE_DIR}/deserialize.cc
+    ${PYARROW_CPP_SOURCE_DIR}/extension_type.cc
+    ${PYARROW_CPP_SOURCE_DIR}/gdb.cc
+    ${PYARROW_CPP_SOURCE_DIR}/helpers.cc
+    ${PYARROW_CPP_SOURCE_DIR}/inference.cc
+    ${PYARROW_CPP_SOURCE_DIR}/init.cc
+    ${PYARROW_CPP_SOURCE_DIR}/io.cc
+    ${PYARROW_CPP_SOURCE_DIR}/ipc.cc
+    ${PYARROW_CPP_SOURCE_DIR}/numpy_convert.cc
+    ${PYARROW_CPP_SOURCE_DIR}/numpy_to_arrow.cc
+    ${PYARROW_CPP_SOURCE_DIR}/python_test.cc
+    ${PYARROW_CPP_SOURCE_DIR}/python_to_arrow.cc
+    ${PYARROW_CPP_SOURCE_DIR}/pyarrow.cc
+    ${PYARROW_CPP_SOURCE_DIR}/serialize.cc
+    ${PYARROW_CPP_SOURCE_DIR}/udf.cc)
+set_source_files_properties(${PYARROW_CPP_SOURCE_DIR}/init.cc
+                            PROPERTIES SKIP_PRECOMPILE_HEADERS ON
+                                       SKIP_UNITY_BUILD_INCLUSION ON)
+
+set(PYARROW_CPP_SHARED_LINK_LIBS)
+set(PYARROW_CPP_SHARED_PRIVATE_LINK_LIBS)
+set(PYARROW_CPP_SHARED_INSTALL_INTERFACE_LIBS)
+set(PYARROW_CPP_STATIC_LINK_LIBS)
+set(PYARROW_CPP_STATIC_INSTALL_INTERFACE_LIBS)
+
 #
-# Dependencies
+# Arrow vs PyArrow C++ options
 #
 
+# Check all the options from Arrow and PyArrow C++ to be in line
+if(PYARROW_BUILD_DATASET)
+  find_package(ArrowDataset REQUIRED)
+  list(APPEND PYARROW_CPP_SHARED_LINK_LIBS ArrowDataset::arrow_dataset_shared)
+  list(APPEND PYARROW_CPP_SHARED_INSTALL_INTERFACE_LIBS
+       ArrowDataset::arrow_dataset_shared)
+  list(APPEND PYARROW_CPP_STATIC_LINK_LIBS ArrowDataset::arrow_dataset_static)
+  list(APPEND PYARROW_CPP_STATIC_INSTALL_INTERFACE_LIBS
+       ArrowDataset::arrow_dataset_static)
+endif()
+
+if(PYARROW_BUILD_PARQUET_ENCRYPTION)
+  if(PARQUET_REQUIRE_ENCRYPTION)
+    list(APPEND PYARROW_CPP_SRCS 
${PYARROW_CPP_SOURCE_DIR}/parquet_encryption.cc)
+    find_package(Parquet REQUIRED)
+    list(APPEND PYARROW_CPP_SHARED_LINK_LIBS Parquet::parquet_shared)
+    list(APPEND PYARROW_CPP_SHARED_INSTALL_INTERFACE_LIBS 
Parquet::parquet_shared)
+    list(APPEND PYARROW_CPP_STATIC_LINK_LIBS Parquet::parquet_static)
+    list(APPEND PYARROW_CPP_STATIC_INSTALL_INTERFACE_LIBS 
Parquet::parquet_static)
+  else()
+    message(FATAL_ERROR "You must build Arrow C++ with 
PARQUET_REQUIRE_ENCRYPTION=ON")
+  endif()
+endif()
+
+if(PYARROW_BUILD_HDFS)
+  if(NOT ARROW_HDFS)
+    message(FATAL_ERROR "You must build Arrow C++ with ARROW_HDFS=ON")
+  endif()
+endif()
+
+# Check for only Arrow C++ options
+if(ARROW_CSV)
+  list(APPEND PYARROW_CPP_SRCS ${PYARROW_CPP_SOURCE_DIR}/csv.cc)
+endif()
+
+if(ARROW_FILESYSTEM)
+  list(APPEND PYARROW_CPP_SRCS ${PYARROW_CPP_SOURCE_DIR}/filesystem.cc)
+endif()
+
+if(CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang" OR CMAKE_CXX_COMPILER_ID 
STREQUAL "Clang")
+  set_property(SOURCE ${PYARROW_CPP_SOURCE_DIR}/pyarrow.cc
+               APPEND_STRING
+               PROPERTY COMPILE_FLAGS " -Wno-cast-qual ")
+endif()
+
+if(NOT PYARROW_CPP_SHARED_LINK_LIBS)
+  list(APPEND PYARROW_CPP_SHARED_LINK_LIBS Arrow::arrow_shared)
+endif()
+if(NOT PYARROW_CPP_SHARED_INSTALL_INTERFACE_LIBS)
+  list(APPEND PYARROW_CPP_SHARED_INSTALL_INTERFACE_LIBS Arrow::arrow_shared)
+endif()
+if(NOT PYARROW_CPP_STATIC_LINK_LIBS)
+  list(APPEND PYARROW_CPP_STATIC_LINK_LIBS Arrow::arrow_static)
+endif()
+list(APPEND PYARROW_CPP_STATIC_LINK_LIBS ${PYTHON_OTHER_LIBS})
+if(NOT PYARROW_CPP_STATIC_INSTALL_INTERFACE_LIBS)
+  list(APPEND PYARROW_CPP_STATIC_INSTALL_INTERFACE_LIBS Arrow::arrow_static)
+endif()
+
+if(WIN32)
+  list(APPEND PYARROW_CPP_SHARED_LINK_LIBS ${PYTHON_LIBRARIES} 
${PYTHON_OTHER_LIBS})
+endif()
+
+add_arrow_lib(arrow_python
+              SOURCES
+              ${PYARROW_CPP_SRCS}
+              PRECOMPILED_HEADERS
+              "$<$<COMPILE_LANGUAGE:CXX>:${PYARROW_CPP_SOURCE_DIR}/pch.h>"
+              INSTALL_ARCHIVE_DIR
+              "."
+              INSTALL_LIBRARY_DIR
+              "."
+              INSTALL_RUNTIME_DIR
+              "."
+              OUTPUTS
+              PYARROW_CPP_LIBRARIES
+              SHARED_LINK_LIBS
+              ${PYARROW_CPP_SHARED_LINK_LIBS}
+              SHARED_PRIVATE_LINK_LIBS
+              ${PYARROW_CPP_SHARED_PRIVATE_LINK_LIBS}
+              SHARED_INSTALL_INTERFACE_LIBS
+              ${PYARROW_CPP_SHARED_INSTALL_INTERFACE_LIBS}
+              STATIC_LINK_LIBS
+              ${PYARROW_CPP_STATIC_LINK_LIBS}
+              STATIC_INSTALL_INTERFACE_LIBS
+              ${PYARROW_CPP_STATIC_INSTALL_INTERFACE_LIBS}
+              EXTRA_INCLUDES
+              ${PYARROW_CPP_ROOT_DIR})
+
+foreach(LIB_TARGET ${PYARROW_CPP_LIBRARIES})
+  target_compile_definitions(${LIB_TARGET} PRIVATE ARROW_PYTHON_EXPORTING)
+endforeach()
+
+if(ARROW_BUILD_STATIC AND MSVC)
+  target_compile_definitions(arrow_python_static PUBLIC ARROW_PYTHON_STATIC)
+endif()
+
+set(PYARROW_CPP_FLIGHT_SRCS ${PYARROW_CPP_SOURCE_DIR}/flight.cc)
 if(PYARROW_BUILD_FLIGHT)
-  set(ARROW_FLIGHT TRUE)
+  if(NOT ARROW_FLIGHT)
+    message(FATAL_ERROR "You must build Arrow C++ with ARROW_FLIGHT=ON")
+  endif()
+  # Must link to shared libarrow_flight: we don't want to link more than one
+  # copy of gRPC into the eventual Cython shared object, otherwise gRPC calls
+  # fail with weird errors due to multiple copies of global static state (The
+  # other solution is to link gRPC shared everywhere instead of statically only
+  # in Flight)
+  if(NOT ARROW_BUILD_SHARED)
+    message(FATAL_ERROR "You must build Arrow C++ with ARROW_BUILD_SHARED=ON")

Review Comment:
   +1



##########
python/CMakeLists.txt:
##########
@@ -223,21 +234,205 @@ include(UseCython)
 
 include_directories(SYSTEM ${NUMPY_INCLUDE_DIRS} ${PYTHON_INCLUDE_DIRS} src)
 
+# PyArrow C++
+include(GNUInstallDirs)
+
+find_package(Arrow REQUIRED)
+
+set(PYARROW_CPP_ROOT_DIR pyarrow/src)
+set(PYARROW_CPP_SOURCE_DIR ${PYARROW_CPP_ROOT_DIR}/arrow/python)
+set(PYARROW_CPP_SRCS
+    ${PYARROW_CPP_SOURCE_DIR}/arrow_to_pandas.cc
+    ${PYARROW_CPP_SOURCE_DIR}/benchmark.cc
+    ${PYARROW_CPP_SOURCE_DIR}/common.cc
+    ${PYARROW_CPP_SOURCE_DIR}/datetime.cc
+    ${PYARROW_CPP_SOURCE_DIR}/decimal.cc
+    ${PYARROW_CPP_SOURCE_DIR}/deserialize.cc
+    ${PYARROW_CPP_SOURCE_DIR}/extension_type.cc
+    ${PYARROW_CPP_SOURCE_DIR}/gdb.cc
+    ${PYARROW_CPP_SOURCE_DIR}/helpers.cc
+    ${PYARROW_CPP_SOURCE_DIR}/inference.cc
+    ${PYARROW_CPP_SOURCE_DIR}/init.cc
+    ${PYARROW_CPP_SOURCE_DIR}/io.cc
+    ${PYARROW_CPP_SOURCE_DIR}/ipc.cc
+    ${PYARROW_CPP_SOURCE_DIR}/numpy_convert.cc
+    ${PYARROW_CPP_SOURCE_DIR}/numpy_to_arrow.cc
+    ${PYARROW_CPP_SOURCE_DIR}/python_test.cc
+    ${PYARROW_CPP_SOURCE_DIR}/python_to_arrow.cc
+    ${PYARROW_CPP_SOURCE_DIR}/pyarrow.cc
+    ${PYARROW_CPP_SOURCE_DIR}/serialize.cc
+    ${PYARROW_CPP_SOURCE_DIR}/udf.cc)
+set_source_files_properties(${PYARROW_CPP_SOURCE_DIR}/init.cc
+                            PROPERTIES SKIP_PRECOMPILE_HEADERS ON
+                                       SKIP_UNITY_BUILD_INCLUSION ON)
+
+set(PYARROW_CPP_SHARED_LINK_LIBS)
+set(PYARROW_CPP_SHARED_PRIVATE_LINK_LIBS)
+set(PYARROW_CPP_SHARED_INSTALL_INTERFACE_LIBS)
+set(PYARROW_CPP_STATIC_LINK_LIBS)
+set(PYARROW_CPP_STATIC_INSTALL_INTERFACE_LIBS)
+
 #
-# Dependencies
+# Arrow vs PyArrow C++ options
 #
 
+# Check all the options from Arrow and PyArrow C++ to be in line
+if(PYARROW_BUILD_DATASET)
+  find_package(ArrowDataset REQUIRED)
+  list(APPEND PYARROW_CPP_SHARED_LINK_LIBS ArrowDataset::arrow_dataset_shared)
+  list(APPEND PYARROW_CPP_SHARED_INSTALL_INTERFACE_LIBS
+       ArrowDataset::arrow_dataset_shared)
+  list(APPEND PYARROW_CPP_STATIC_LINK_LIBS ArrowDataset::arrow_dataset_static)
+  list(APPEND PYARROW_CPP_STATIC_INSTALL_INTERFACE_LIBS
+       ArrowDataset::arrow_dataset_static)
+endif()
+
+if(PYARROW_BUILD_PARQUET_ENCRYPTION)
+  if(PARQUET_REQUIRE_ENCRYPTION)
+    list(APPEND PYARROW_CPP_SRCS 
${PYARROW_CPP_SOURCE_DIR}/parquet_encryption.cc)
+    find_package(Parquet REQUIRED)
+    list(APPEND PYARROW_CPP_SHARED_LINK_LIBS Parquet::parquet_shared)
+    list(APPEND PYARROW_CPP_SHARED_INSTALL_INTERFACE_LIBS 
Parquet::parquet_shared)
+    list(APPEND PYARROW_CPP_STATIC_LINK_LIBS Parquet::parquet_static)
+    list(APPEND PYARROW_CPP_STATIC_INSTALL_INTERFACE_LIBS 
Parquet::parquet_static)
+  else()
+    message(FATAL_ERROR "You must build Arrow C++ with 
PARQUET_REQUIRE_ENCRYPTION=ON")
+  endif()
+endif()
+
+if(PYARROW_BUILD_HDFS)
+  if(NOT ARROW_HDFS)
+    message(FATAL_ERROR "You must build Arrow C++ with ARROW_HDFS=ON")
+  endif()
+endif()
+
+# Check for only Arrow C++ options
+if(ARROW_CSV)
+  list(APPEND PYARROW_CPP_SRCS ${PYARROW_CPP_SOURCE_DIR}/csv.cc)
+endif()
+
+if(ARROW_FILESYSTEM)
+  list(APPEND PYARROW_CPP_SRCS ${PYARROW_CPP_SOURCE_DIR}/filesystem.cc)
+endif()
+
+if(CMAKE_CXX_COMPILER_ID STREQUAL "AppleClang" OR CMAKE_CXX_COMPILER_ID 
STREQUAL "Clang")
+  set_property(SOURCE ${PYARROW_CPP_SOURCE_DIR}/pyarrow.cc
+               APPEND_STRING
+               PROPERTY COMPILE_FLAGS " -Wno-cast-qual ")
+endif()
+
+if(NOT PYARROW_CPP_SHARED_LINK_LIBS)
+  list(APPEND PYARROW_CPP_SHARED_LINK_LIBS Arrow::arrow_shared)
+endif()
+if(NOT PYARROW_CPP_SHARED_INSTALL_INTERFACE_LIBS)
+  list(APPEND PYARROW_CPP_SHARED_INSTALL_INTERFACE_LIBS Arrow::arrow_shared)
+endif()
+if(NOT PYARROW_CPP_STATIC_LINK_LIBS)
+  list(APPEND PYARROW_CPP_STATIC_LINK_LIBS Arrow::arrow_static)
+endif()
+list(APPEND PYARROW_CPP_STATIC_LINK_LIBS ${PYTHON_OTHER_LIBS})
+if(NOT PYARROW_CPP_STATIC_INSTALL_INTERFACE_LIBS)
+  list(APPEND PYARROW_CPP_STATIC_INSTALL_INTERFACE_LIBS Arrow::arrow_static)
+endif()
+
+if(WIN32)
+  list(APPEND PYARROW_CPP_SHARED_LINK_LIBS ${PYTHON_LIBRARIES} 
${PYTHON_OTHER_LIBS})
+endif()
+
+add_arrow_lib(arrow_python
+              SOURCES
+              ${PYARROW_CPP_SRCS}
+              PRECOMPILED_HEADERS
+              "$<$<COMPILE_LANGUAGE:CXX>:${PYARROW_CPP_SOURCE_DIR}/pch.h>"
+              INSTALL_ARCHIVE_DIR
+              "."
+              INSTALL_LIBRARY_DIR
+              "."
+              INSTALL_RUNTIME_DIR
+              "."
+              OUTPUTS
+              PYARROW_CPP_LIBRARIES
+              SHARED_LINK_LIBS
+              ${PYARROW_CPP_SHARED_LINK_LIBS}
+              SHARED_PRIVATE_LINK_LIBS
+              ${PYARROW_CPP_SHARED_PRIVATE_LINK_LIBS}
+              SHARED_INSTALL_INTERFACE_LIBS
+              ${PYARROW_CPP_SHARED_INSTALL_INTERFACE_LIBS}
+              STATIC_LINK_LIBS
+              ${PYARROW_CPP_STATIC_LINK_LIBS}
+              STATIC_INSTALL_INTERFACE_LIBS
+              ${PYARROW_CPP_STATIC_INSTALL_INTERFACE_LIBS}
+              EXTRA_INCLUDES
+              ${PYARROW_CPP_ROOT_DIR})
+
+foreach(LIB_TARGET ${PYARROW_CPP_LIBRARIES})
+  target_compile_definitions(${LIB_TARGET} PRIVATE ARROW_PYTHON_EXPORTING)
+endforeach()
+
+if(ARROW_BUILD_STATIC AND MSVC)
+  target_compile_definitions(arrow_python_static PUBLIC ARROW_PYTHON_STATIC)
+endif()
+
+set(PYARROW_CPP_FLIGHT_SRCS ${PYARROW_CPP_SOURCE_DIR}/flight.cc)
 if(PYARROW_BUILD_FLIGHT)
-  set(ARROW_FLIGHT TRUE)
+  if(NOT ARROW_FLIGHT)
+    message(FATAL_ERROR "You must build Arrow C++ with ARROW_FLIGHT=ON")

Review Comment:
   +1



-- 
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: github-unsubscr...@arrow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to