GH-36927: [Java][Docs] Enable Gandiva build as part of Java maven commands #36929

davisusanibar · 2023-07-28T12:34:35Z

Rationale for this change

To close: #36927

What changes are included in this PR?

Enable MacOS Gandiva build as part of Java maven commands
Enable Windows ORC build as part of Java maven commands

Are these changes tested?

Yes:

Gandiva: mvn generate-resources -Pgenerate-libs-jni-macos-linux -N
ORC: mvn generate-resources -Pgenerate-libs-jni-windows -N

Are there any user-facing changes?

No

Closes: [Java][Docs] Enable Gandiva build as part of Java maven commands #36927

github-actions · 2023-07-28T12:35:35Z

⚠️ GitHub issue #36927 has been automatically assigned in GitHub to PR creator.

davisusanibar · 2023-07-28T12:39:20Z

Hi @danepitkin please could you help me to validate if this changes works properly on your OS environment?

Steps:

Make sure the local folder used by Java mvn commands is clean::
$ cd arrow
$ rm -rf cpp-jni java-dist java-jni
Build MacOS artifacts:
$ mvn generate-resources -Pgenerate-libs-jni-macos-linux -N

Thank you in advance.

danepitkin · 2023-07-28T14:35:50Z

java/pom.xml

-        <ARROW_CSV>ON</ARROW_CSV>
-        <ARROW_ORC>OFF</ARROW_ORC>
+        <ARROW_DATASET>ON</ARROW_DATASET>
+        <ARROW_GANDIVA>OFF</ARROW_GANDIVA>


I think ARROW_GANDIVA is accidentally set to OFF here

Or is it not supported on Windows?

Doesn't have JNI yet because we need to install LLVM to build Gandiva (Could NOT find LLVM (missing: LLVM_DIR)).

danepitkin · 2023-07-28T14:51:46Z

Hi @danepitkin please could you help me to validate if this changes works properly on your OS environment?

Steps:

Make sure the local folder used by Java mvn commands is clean::
$ cd arrow
$ rm -rf cpp-jni java-dist java-jni

Build MacOS artifacts:
$ mvn generate-resources -Pgenerate-libs-jni-macos-linux -N

Thank you in advance.

Works for me! I checked out your branch, deleted local artifacts, and can confirm it successfully built dataset/gandiva/orc modules.

danepitkin

Overall LGTM. Thanks @davisusanibar !

danepitkin · 2023-07-28T19:48:59Z

java/pom.xml

+                    -DProtobuf_USE_STATIC_LIBS=ON
+                    -DProtobuf_ROOT=${project.basedir}/../cpp-jni/protobuf_ep-install


Just out of curiosity, why are we making this change?

Ah, is this for https://github.com/apache/arrow/blob/main/java/gandiva/CMakeLists.txt#L41 ?

We can use this solution for now but we should find a better solution that doesn't require additional CMake options such as Protobuf_ROOT and Protobuf_USE_STATIC_LIBS. Could you open an issue for it?
(We may add support for installing bundled protoc for this case.)

kou

+1

kou · 2023-07-28T20:34:57Z

java/pom.xml

+                    -DPARQUET_BUILD_EXAMPLES=OFF
+                    -DPARQUET_BUILD_EXECUTABLES=OFF
+                    -DPARQUET_REQUIRE_ENCRYPTION=OFF


FYI: They are not harmful but they are redundant because they are OFF by default.

kou · 2023-07-28T20:41:19Z

docs/source/developers/java/building.rst

+          -DPARQUET_BUILD_EXAMPLES=OFF ^
+          -DPARQUET_BUILD_EXECUTABLES=OFF ^
+          -DPARQUET_REQUIRE_ENCRYPTION=OFF ^


FYI: They are not harmful but they are redundant because they are OFF by default.

kou · 2023-07-28T20:43:27Z

docs/source/developers/java/building.rst

@@ -227,6 +223,7 @@ CMake
          -DCMAKE_INSTALL_PREFIX=java-dist \
          -DCMAKE_UNITY_BUILD=ON
      $ cmake --build cpp-jni --target install --config Release
+      $ export JAVA_JNI_CMAKE_ARGS="-DProtobuf_ROOT=$PWD/cpp-jni/protobuf_ep-install"


We need to specify this in the following cmake ... command line because JAVA_JNI_CMAKE_ARGS is only for ci/scripts/java_*_build.sh.

kou · 2023-07-28T20:47:27Z

java/pom.xml

+                    -DProtobuf_USE_STATIC_LIBS=ON
+                    -DProtobuf_ROOT=${project.basedir}/../cpp-jni/protobuf_ep-install


Ah, is this for https://github.com/apache/arrow/blob/main/java/gandiva/CMakeLists.txt#L41 ?

We can use this solution for now but we should find a better solution that doesn't require additional CMake options such as Protobuf_ROOT and Protobuf_USE_STATIC_LIBS. Could you open an issue for it?
(We may add support for installing bundled protoc for this case.)

davisusanibar · 2023-07-31T21:25:31Z

Issues open for:

Enable Gandiva support on Windows OS - [Java][JNI][Windows]: Enable support for Gandiva build artifacts on Windows OS #36958
Improve current cmake java libraries dependencies that needs Protobuf_ROOT and Protobuf_USE_STATIC_LIBS variables - [Java][JNI] Improve current cmake java libraries dependencies that needs Protobuf_ROOT and Protobuf_USE_STATIC_LIBS variables #36957

docs/source/developers/java/building.rst

kou

+1

conbench-apache-arrow · 2023-08-09T15:45:54Z

After merging your PR, Conbench analyzed the 5 benchmarking runs that have been run so far on merge-commit 8273f7a.

There were no benchmark performance regressions. 🎉

The full Conbench report has more details.

…en commands (apache#36929) ### Rationale for this change To close: apache#36927 ### What changes are included in this PR? - Enable MacOS Gandiva build as part of Java maven commands - Enable Windows ORC build as part of Java maven commands ### Are these changes tested? Yes: - Gandiva: mvn generate-resources -Pgenerate-libs-jni-macos-linux -N - ORC: mvn generate-resources -Pgenerate-libs-jni-windows -N ### Are there any user-facing changes? No * Closes: apache#36927 Lead-authored-by: david dali susanibar arce <davi.sarces@gmail.com> Co-authored-by: Sutou Kouhei <kou@cozmixng.org> Signed-off-by: Sutou Kouhei <kou@clear-code.com>

fix: Enable Gandiva build as part of Java maven commands

f207567

davisusanibar requested a review from lidavidm as a code owner July 28, 2023 12:34

github-actions bot added the awaiting review Awaiting review label Jul 28, 2023

github-actions bot added Component: Java Component: Documentation labels Jul 28, 2023

danepitkin reviewed Jul 28, 2023

View reviewed changes

github-actions bot added awaiting committer review Awaiting committer review and removed awaiting review Awaiting review labels Jul 28, 2023

danepitkin reviewed Jul 28, 2023

View reviewed changes

kou approved these changes Jul 28, 2023

View reviewed changes

github-actions bot added awaiting merge Awaiting merge awaiting review Awaiting review and removed awaiting committer review Awaiting committer review labels Jul 28, 2023

fix: clean documentation

a5b2143

github-actions bot added awaiting review Awaiting review and removed awaiting review Awaiting review awaiting merge Awaiting merge labels Jul 31, 2023

kou reviewed Aug 1, 2023

View reviewed changes

docs/source/developers/java/building.rst Outdated Show resolved Hide resolved

Sort

7d57065

kou approved these changes Aug 1, 2023

View reviewed changes

github-actions bot added awaiting changes Awaiting changes awaiting change review Awaiting change review and removed awaiting review Awaiting review awaiting changes Awaiting changes labels Aug 1, 2023

kou merged commit 8273f7a into apache:main Aug 1, 2023
16 checks passed

kou removed the awaiting change review Awaiting change review label Aug 1, 2023

github-actions bot added the awaiting merge Awaiting merge label Aug 1, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GH-36927: [Java][Docs] Enable Gandiva build as part of Java maven commands #36929

GH-36927: [Java][Docs] Enable Gandiva build as part of Java maven commands #36929

davisusanibar commented Jul 28, 2023 •

edited by github-actions bot

Loading

github-actions bot commented Jul 28, 2023

davisusanibar commented Jul 28, 2023

danepitkin Jul 28, 2023

danepitkin Jul 28, 2023

davisusanibar Jul 28, 2023

danepitkin commented Jul 28, 2023 •

edited

Loading

danepitkin left a comment

danepitkin Jul 28, 2023

kou Jul 28, 2023

kou left a comment

kou Jul 28, 2023

kou Jul 28, 2023

kou Jul 28, 2023

kou Jul 28, 2023

davisusanibar commented Jul 31, 2023

kou left a comment

conbench-apache-arrow bot commented Aug 9, 2023

		-DProtobuf_USE_STATIC_LIBS=ON
		-DProtobuf_ROOT=${project.basedir}/../cpp-jni/protobuf_ep-install

GH-36927: [Java][Docs] Enable Gandiva build as part of Java maven commands #36929

GH-36927: [Java][Docs] Enable Gandiva build as part of Java maven commands #36929

Conversation

davisusanibar commented Jul 28, 2023 • edited by github-actions bot Loading

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

github-actions bot commented Jul 28, 2023

davisusanibar commented Jul 28, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danepitkin commented Jul 28, 2023 • edited Loading

danepitkin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kou left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

davisusanibar commented Jul 31, 2023

kou left a comment

Choose a reason for hiding this comment

conbench-apache-arrow bot commented Aug 9, 2023

davisusanibar commented Jul 28, 2023 •

edited by github-actions bot

Loading

danepitkin commented Jul 28, 2023 •

edited

Loading