Skip to content

Commit

Permalink
PR suggestions
Browse files Browse the repository at this point in the history
Remove explicit setting of ARROW_JAVA_JNI_ARCH_DIR in build system.
Clarify that this should get auto-detected.
  • Loading branch information
jduo committed Jan 19, 2024
1 parent 3607ed8 commit c5723d1
Show file tree
Hide file tree
Showing 6 changed files with 8 additions and 20 deletions.
2 changes: 1 addition & 1 deletion ci/scripts/java_jni_build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ cmake \
-DBUILD_TESTING=${ARROW_JAVA_BUILD_TESTS} \
-DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE} \
-DCMAKE_PREFIX_PATH=${arrow_install_dir} \
\ -DCMAKE_INSTALL_PREFIX=${prefix_dir} \
-DCMAKE_INSTALL_PREFIX=${prefix_dir} \
-DCMAKE_UNITY_BUILD=${CMAKE_UNITY_BUILD:-OFF} \
-DProtobuf_USE_STATIC_LIBS=ON \
-GNinja \
Expand Down
1 change: 0 additions & 1 deletion ci/scripts/java_jni_manylinux_build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,6 @@ cmake \
-DARROW_S3=${ARROW_S3} \
-DARROW_USE_CCACHE=${ARROW_USE_CCACHE} \
-DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE} \
-DARROW_JAVA_JNI_ARCH_DIR=${normalized_arch} \
-DCMAKE_INSTALL_PREFIX=${ARROW_HOME} \
-DCMAKE_UNITY_BUILD=${CMAKE_UNITY_BUILD} \
-DGTest_SOURCE=BUNDLED \
Expand Down
1 change: 0 additions & 1 deletion ci/scripts/java_jni_windows_build.sh
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ cmake \
-DARROW_WITH_SNAPPY=ON \
-DARROW_WITH_ZSTD=ON \
-DCMAKE_BUILD_TYPE=${CMAKE_BUILD_TYPE} \
-DARROW_JAVA_JNI_ARCH_DIR=x86_64 \
-DCMAKE_INSTALL_PREFIX=${install_dir} \
-DCMAKE_UNITY_BUILD=${CMAKE_UNITY_BUILD} \
-GNinja \
Expand Down
6 changes: 0 additions & 6 deletions docs/source/developers/java/building.rst
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,6 @@ CMake
-DARROW_JAVA_JNI_ENABLE_DEFAULT=OFF \
-DBUILD_TESTING=OFF \
-DCMAKE_BUILD_TYPE=Release \
-DARROW_JAVA_JNI_ARCH_DIR=<your system's architecture> \
-DCMAKE_INSTALL_PREFIX=java-dist
$ cmake --build java-cdata --target install --config Release
$ ls -latr java-dist/lib
Expand All @@ -184,7 +183,6 @@ CMake
-DARROW_JAVA_JNI_ENABLE_DEFAULT=OFF ^
-DBUILD_TESTING=OFF ^
-DCMAKE_BUILD_TYPE=Release ^
-DARROW_JAVA_JNI_ARCH_DIR=x86_64 ^
-DCMAKE_INSTALL_PREFIX=java-dist
$ cmake --build java-cdata --target install --config Release
$ dir "java-dist/bin"
Expand Down Expand Up @@ -220,7 +218,6 @@ CMake
-DARROW_SUBSTRAIT=ON \
-DARROW_USE_CCACHE=ON \
-DCMAKE_BUILD_TYPE=Release \
-DARROW_JAVA_JNI_ARCH_DIR=<your system's architecture> \
-DCMAKE_INSTALL_PREFIX=java-dist \
-DCMAKE_UNITY_BUILD=ON
$ cmake --build cpp-jni --target install --config Release
Expand All @@ -231,7 +228,6 @@ CMake
-DARROW_JAVA_JNI_ENABLE_DEFAULT=ON \
-DBUILD_TESTING=OFF \
-DCMAKE_BUILD_TYPE=Release \
-DARROW_JAVA_JNI_ARCH_DIR=<your system's architecture> \
-DCMAKE_INSTALL_PREFIX=java-dist \
-DCMAKE_PREFIX_PATH=$PWD/java-dist \
-DProtobuf_ROOT=$PWD/../cpp-jni/protobuf_ep-install \
Expand Down Expand Up @@ -269,7 +265,6 @@ CMake
-DARROW_WITH_ZLIB=ON ^
-DARROW_WITH_ZSTD=ON ^
-DCMAKE_BUILD_TYPE=Release ^
-DARROW_JAVA_JNI_ARCH_DIR=x86_64 ^
-DCMAKE_INSTALL_PREFIX=java-dist ^
-DCMAKE_UNITY_BUILD=ON ^
-GNinja
Expand All @@ -286,7 +281,6 @@ CMake
-DARROW_JAVA_JNI_ENABLE_ORC=ON ^
-DBUILD_TESTING=OFF ^
-DCMAKE_BUILD_TYPE=Release ^
-DARROW_JAVA_JNI_ARCH_DIR=x86_64 ^
-DCMAKE_INSTALL_PREFIX=java-dist ^
-DCMAKE_PREFIX_PATH=$PWD/java-dist
$ cmake --build java-jni --target install --config Release
Expand Down
13 changes: 7 additions & 6 deletions java/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -74,17 +74,18 @@ if(BUILD_TESTING)
GTest::gtest_main)
endif()

# If the user hasn't set ARROW_JAVA_JNI_ARCH_DIR, derive the normalized
# operating system from the host processor.
# The ARROW_JAVA_JNI_ARCH_DIR will automatically be derived the normalized
# operating system from system processor. The user can override this variable
# if auto-detection fails.
if("${ARROW_JAVA_JNI_ARCH_DIR}" STREQUAL "")
if("${CMAKE_HOST_SYSTEM_PROCESSOR}" STREQUAL "aarch64")
if("${CMAKE_SYSTEM_PROCESSOR}" STREQUAL "aarch64")
set(ARROW_JAVA_JNI_ARCH_DIR "aarch_64")
elseif("${CMAKE_HOST_SYSTEM_PROCESSOR}" STREQUAL "i386")
elseif("${CMAKE_SYSTEM_PROCESSOR}" STREQUAL "i386")
set(ARROW_JAVA_JNI_ARCH_DIR "x86_64")
elseif("${CMAKE_HOST_SYSTEM_PROCESSOR}" STREQUAL "arm64")
elseif("${CMAKE_SYSTEM_PROCESSOR}" STREQUAL "arm64")
set(ARROW_JAVA_JNI_ARCH_DIR "aarch_64")
else()
set(ARROW_JAVA_JNI_ARCH_DIR "${CMAKE_HOST_SYSTEM_PROCESSOR}")
set(ARROW_JAVA_JNI_ARCH_DIR "${CMAKE_SYSTEM_PROCESSOR}")
endif()
endif()

Expand Down
5 changes: 0 additions & 5 deletions java/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -1055,7 +1055,6 @@
-DARROW_JAVA_JNI_ENABLE_DEFAULT=OFF
-DBUILD_TESTING=OFF
-DCMAKE_BUILD_TYPE=Release
-DARROW_JAVA_JNI_ARCH_DIR=${os.detected.arch}
-DCMAKE_INSTALL_PREFIX=${arrow.c.jni.dist.dir}
</commandlineArgs>
<workingDirectory>../</workingDirectory>
Expand Down Expand Up @@ -1128,7 +1127,6 @@
-DARROW_SUBSTRAIT=${ARROW_DATASET}
-DARROW_USE_CCACHE=ON
-DCMAKE_BUILD_TYPE=Release
-DARROW_JAVA_JNI_ARCH_DIR=${os.detected.arch}
-DCMAKE_INSTALL_PREFIX=java-dist
-DCMAKE_UNITY_BUILD=ON
</commandlineArgs>
Expand Down Expand Up @@ -1169,7 +1167,6 @@
-DARROW_JAVA_JNI_ENABLE_DEFAULT=ON
-DBUILD_TESTING=OFF
-DCMAKE_BUILD_TYPE=Release
-DARROW_JAVA_JNI_ARCH_DIR=${os.detected.arch}
-DCMAKE_INSTALL_PREFIX=${arrow.dataset.jni.dist.dir}
-DCMAKE_PREFIX_PATH=${project.basedir}/../java-dist/lib/${os.detected.arch}/cmake
-DProtobuf_USE_STATIC_LIBS=ON
Expand Down Expand Up @@ -1248,7 +1245,6 @@
-DARROW_WITH_ZLIB=ON
-DARROW_WITH_ZSTD=ON
-DCMAKE_BUILD_TYPE=Release
-DARROW_JAVA_JNI_ARCH_DIR=${os.detected.arch}
-DCMAKE_INSTALL_PREFIX=java-dist
-DCMAKE_UNITY_BUILD=ON
-GNinja
Expand Down Expand Up @@ -1290,7 +1286,6 @@
-DARROW_JAVA_JNI_ENABLE_DEFAULT=ON
-DBUILD_TESTING=OFF
-DCMAKE_BUILD_TYPE=Release
-DARROW_JAVA_JNI_ARCH_DIR=${os.detected.arch}
-DCMAKE_INSTALL_PREFIX=${arrow.dataset.jni.dist.dir}
-DCMAKE_PREFIX_PATH=${project.basedir}/../java-dist/lib/${os.detected.arch}/cmake
</commandlineArgs>
Expand Down

0 comments on commit c5723d1

Please sign in to comment.