kou commented on code in PR #39221:
URL: https://github.com/apache/arrow/pull/39221#discussion_r1456751270


##########
java/gandiva/CMakeLists.txt:
##########
@@ -84,6 +84,11 @@ if(CXX_LINKER_SUPPORTS_VERSION_SCRIPT)
   )
 endif()
 
+set(CMAKE_INSTALL_LIBDIR
+    "${CMAKE_INSTALL_PREFIX}/lib/gandiva_jni/${ARROW_JAVA_JNI_ARCH_DIR}")
+set(CMAKE_INSTALL_BINDIR
+    "${CMAKE_INSTALL_PREFIX}/bin/gandiva_jni/${ARROW_JAVA_JNI_ARCH_DIR}")

Review Comment:
   Please don't override the standard CMake variables.
   Could you use our CMake variables instead such as 
`ARROW_JAVA_JNI_GANDIVA_LIBDIR`?



##########
ci/scripts/java_jni_build.sh:
##########
@@ -24,6 +24,18 @@ arrow_install_dir=${2}
 build_dir=${3}/java_jni
 # The directory where the final binaries will be stored when scripts finish
 dist_dir=${4}
+normalized_arch=$(arch)
+case ${normalized_arch} in
+  aarch64)
+    normalized_arch=aarch_64
+    ;;
+  i386)
+    normalized_arch=x86_64
+    ;;
+  arm64)
+    normalized_arch=aarch_64
+    ;;
+  esac

Review Comment:
   Can we do this in `java/CMakeLists.txt` instead of specifying this by a user?
   For example, can we use `CMAKE_HOST_SYSTEM_PROCESSOR` 
https://cmake.org/cmake/help/latest/variable/CMAKE_HOST_SYSTEM_PROCESSOR.html 
for this?



-- 
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]

Reply via email to