-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
ARROW-17418: [Doc][Java] Dataset library able to compile with mvn command #13889
Conversation
java/pom.xml
Outdated
<configuration> | ||
<executable>cmake</executable> | ||
<commandlineArgs> | ||
-DARROW_BOOST_USE_SHARED=OFF |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can use ARROW_DEPENDENCY_USE_SHARED=OFF
instead of multiple ARROW_*_USE_SHARED=OFF
s.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed
java/pom.xml
Outdated
-DCMAKE_INSTALL_LIBDIR=lib | ||
-DCMAKE_INSTALL_PREFIX=../java-dist | ||
-DCMAKE_UNITY_BUILD=ON | ||
-Dre2_SOURCE=BUNDLED |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We can use ARROW_DEPENDENCY_SOURCE=BUNDLED
instead of multiple *_SOURCE=BUNDLED
s.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changed
java/pom.xml
Outdated
-DARROW_ORC=${ARROW_ORC} | ||
-DCMAKE_BUILD_TYPE=Release | ||
-DCMAKE_INSTALL_LIBDIR=lib | ||
-DCMAKE_INSTALL_PREFIX=../java-dist |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we use the arrow.c.jni.dist.dir
property instead of ../java-dist
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added
java/pom.xml
Outdated
<configuration> | ||
<executable>cmake</executable> | ||
<commandlineArgs> | ||
--build . --target install |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
--config Release
may be missed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added
LGTM. Is there any reason this can't be merged? |
java/pom.xml
Outdated
-DCMAKE_INSTALL_LIBDIR=lib | ||
-DCMAKE_INSTALL_PREFIX=${arrow.cpp.build.dir} | ||
-DCMAKE_UNITY_BUILD=ON | ||
-DARROW_DEPENDENCY_SOURCE=BUNDLED |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you sort this list in alphabetical order?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that it's not done. Could you check again? Or should I push a commit for it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorted
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
+1
We can merge this after we sort lists.
java/pom.xml
Outdated
-DARROW_DEPENDENCY_USE_SHARED=OFF | ||
-DARROW_CSV=ON | ||
-DARROW_DATASET=ON | ||
-DARROW_FILESYSTEM=ON | ||
-DARROW_GANDIVA=ON | ||
-DARROW_GANDIVA_JAVA=ON | ||
-DARROW_GANDIVA_STATIC_LIBSTDCPP=ON | ||
-DARROW_JNI=ON | ||
-DARROW_ORC=ON | ||
-DARROW_PARQUET=ON | ||
-DARROW_PLASMA=ON | ||
-DARROW_PLASMA_JAVA_CLIENT=ON | ||
-DARROW_S3=ON | ||
-DARROW_USE_CCACHE=ON | ||
-DCMAKE_BUILD_TYPE=Release | ||
-DCMAKE_INSTALL_LIBDIR=lib | ||
-DCMAKE_INSTALL_PREFIX=java-dist | ||
-DCMAKE_UNITY_BUILD=ON | ||
-DARROW_DEPENDENCY_SOURCE=BUNDLED |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you sort this list in alphabetical order?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorted
java/pom.xml
Outdated
-DCMAKE_BUILD_TYPE=Release | ||
-DCMAKE_INSTALL_PREFIX=${arrow.c.jni.dist.dir} | ||
-DCMAKE_PREFIX_PATH=${project.basedir}/../java-dist | ||
-DARROW_JAVA_JNI_ENABLE_DEFAULT=OFF | ||
-DARROW_JAVA_JNI_ENABLE_DATASET=ON | ||
-DBUILD_TESTING=OFF |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you sort this list in alphabetical order?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorted
java/pom.xml
Outdated
-DCMAKE_INSTALL_LIBDIR=lib | ||
-DCMAKE_INSTALL_PREFIX=${arrow.cpp.build.dir} | ||
-DCMAKE_UNITY_BUILD=ON | ||
-DARROW_DEPENDENCY_SOURCE=BUNDLED |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems that it's not done. Could you check again? Or should I push a commit for it?
Could you rebase on the master? |
Rebase + Update documentation to use new maven profiles, please your review, thanks |
Benchmark runs are scheduled for baseline = f57e1ba and contender = f4e79b1. f4e79b1 is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
…mand (apache#13889) Dataset/ORC/Gandiva library (.so / .dylib) able to compile with mvn command $ mvn clean generate-resources -Pgenerate-jnicpp-dylib_so -DARROW_DATASET=ON -DARROW_ORC=ON -DARROW_GANDIVA=ON -N Authored-by: david dali susanibar arce <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]>
…mand (apache#13889) Dataset/ORC/Gandiva library (.so / .dylib) able to compile with mvn command $ mvn clean generate-resources -Pgenerate-jnicpp-dylib_so -DARROW_DATASET=ON -DARROW_ORC=ON -DARROW_GANDIVA=ON -N Authored-by: david dali susanibar arce <[email protected]> Signed-off-by: Sutou Kouhei <[email protected]>
Dataset/ORC/Gandiva library (.so / .dylib) able to compile with mvn command
$ mvn clean generate-resources -Pgenerate-jnicpp-dylib_so -DARROW_DATASET=ON -DARROW_ORC=ON -DARROW_GANDIVA=ON -N