Skip to content

ARROW-16941: [Java][Dataset] Update more jni_util.h paths - #13503

Merged
kou merged 3 commits into
apache:masterfrom
kou:java-jni-util
Jul 5, 2022
Merged

ARROW-16941: [Java][Dataset] Update more jni_util.h paths#13503
kou merged 3 commits into
apache:masterfrom
kou:java-jni-util

Conversation

@kou

@koukou commented Jul 4, 2022

Copy link
Copy Markdown
Member

This is a follow-up of #13481.

We also need to update jni_util.h path in
java/dataset/src/main/cpp/jni_util_test.cc.

Removing "./" is needless but it's redundant.

This is a follow-up of apache#13481.
We also need to update jni_util.h path in
java/dataset/src/main/cpp/jni_util_test.cc.
Removing "./" is needless but it's redundant.
@kou
kou requested a review from lidavidmJuly 4, 2022 02:02
@kou

kou commented Jul 4, 2022

Copy link
Copy Markdown
MemberAuthor

Cc: @lwhite1

@github-actions

Copy link
Copy Markdown

@lidavidmlidavidm left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for catching this, Kou (would have thought CI tests this, does it not?)

@kou

kou commented Jul 4, 2022

Copy link
Copy Markdown
MemberAuthor

It seems that we don't have a job that uses ARROW_JNI=ON and ARROW_BUILD_TESTS=ON.

@kou

kou commented Jul 4, 2022

Copy link
Copy Markdown
MemberAuthor

Should we also add the following change to this pull request?

diff --git a/ci/docker/linux-apt-jni.dockerfile b/ci/docker/linux-apt-jni.dockerfile
index 6673b001d7..be6562eae6 100644
--- a/ci/docker/linux-apt-jni.dockerfile+++ b/ci/docker/linux-apt-jni.dockerfile@@ -69,7 +69,7 @@ ARG cmake=3.11.4
RUN wget -nv -O - https://github.com/Kitware/CMake/releases/download/v${cmake}/cmake-${cmake}-Linux-x86_64.tar.gz | tar -xzf - -C /opt
ENV PATH=/opt/cmake-${cmake}-Linux-x86_64/bin:$PATH
-ENV ARROW_BUILD_TESTS=OFF \+ENV ARROW_BUILD_TESTS=ON \
ARROW_DATASET=ON \
ARROW_FLIGHT=OFF \
ARROW_GANDIVA_JAVA=ON \

@lidavidm

Copy link
Copy Markdown
Member

Yes, let's make sure at least one CI job covers this.

@kou

kou commented Jul 5, 2022

Copy link
Copy Markdown
MemberAuthor

OK. I enable it.

@kou

kou commented Jul 5, 2022

Copy link
Copy Markdown
MemberAuthor

+1

@kou
kou merged commit e44f79a into apache:masterJul 5, 2022
@kou
kou deleted the java-jni-util branch July 5, 2022 06:23
drin pushed a commit to drin/arrow that referenced this pull request Jul 5, 2022
This is a follow-up of apache#13481.
We also need to update jni_util.h path in
java/dataset/src/main/cpp/jni_util_test.cc.
Removing "./" is needless but it's redundant.
Authored-by: Sutou Kouhei <kou@clear-code.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
pribor pushed a commit to GlobalWebIndex/arrow that referenced this pull request Oct 24, 2025
This is a follow-up of apache#13481.
We also need to update jni_util.h path in
java/dataset/src/main/cpp/jni_util_test.cc.
Removing "./" is needless but it's redundant.
Authored-by: Sutou Kouhei <kou@clear-code.com>
Signed-off-by: Sutou Kouhei <kou@clear-code.com>
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants

@kou@lidavidm