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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
