Re: [PR] build: Make the build system work out of box [arrow-datafusion-comet]

["advancedxy (via GitHub)" <gi...@apache.org>]

advancedxy commented on code in PR #136:
URL: https://github.com/apache/arrow-datafusion-comet/pull/136#discussion_r1508366162


##########
.github/actions/rust-test/action.yaml:
##########
@@ -64,9 +60,6 @@ runs:
         # This is required to run some JNI related tests on the Rust side
         JAVA_LD_LIBRARY_PATH=$JAVA_HOME/lib/server:$JAVA_HOME/lib:$JAVA_HOME/lib/jli
         # special handing for java 1.8 for both linux and mac distributions
-        if [ $JAVA_VERSION == "8" ]; then

Review Comment:
   Did some search. It looks like that upgrading to jni-rs 0.21 removes the need for explicit setting (DY)LD_LIBRARY_PATHs. See https://github.com/jni-rs/jni-rs/pull/293 for related code change.
   
   First thing first, this test condition is wrong as we have changed the `JAVA_VERSION` to 1.8 during the iteration of MR refinements. So the next lines are never executed.
   
   This line was added when I'm adding support for JDK 1.8 support.  At the time of make all the tests running, the jni-rs was upgraded from 0.19 to 0.21. So my PR with these wrong and unnecessary lines passes. 



-- 
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: github-unsubscribe@arrow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org