GH-882: Add support for loading native library from a user specified location - #883
GH-882: Add support for loading native library from a user specified location#883lidavidm merged 1 commit into
Conversation
Sorry, something went wrong.
This comment has been minimized.
|
Is there precedent for this kind of flag in other JNI libraries? |
Sorry, something went wrong.
See for instance https://github.com/xerial/sqlite-jdbc/blob/755cf1d0223a83b97909fca43c8027e3e02bc021/src/main/java/org/sqlite/SQLiteJDBCLoader.java#L300 and https://github.com/jnr/jffi/blob/master/src/main/java/com/kenai/jffi/internal/StubLoader.java#L302 A possible alternative could be to check java.library.path indirectly by calling System.load first in the try/catch block. I did not go for this option since it might make existing systems pick up a library from somewhere unintentionally. The custom system property makes this explicitly opt-in. As described in the issue, loading executable code from /tmp is problematic since it's sometimes forbidden by security settings of the host OS. This is an actual deployment problem for the system I work on since we have no control over those settings. |
Sorry, something went wrong.
|
Thanks. I'll give this a fuller review but if there's precedent then the idea is good by me. |
Sorry, something went wrong.
| return; | ||
| } | ||
| } catch (UnsatisfiedLinkError e) { | ||
| // Ignore this error and fall back to extracting from the JAR file |
There was a problem hiding this comment.
Actually, would it be better to error if the property is provided but the file doesn't exist? That way you don't unexpectedly/silently fall back when you intended to load from somewhere else.
Sorry, something went wrong.
There was a problem hiding this comment.
In my specific application the native library will be have been stripped from the jar and loading will fail so the end result will be the same either way.
I modeled this on examples like the ones I referenced where possible alternatives are tried in a specified order. Happy to change this to fail early though. Let me know what you would prefer.
Sorry, something went wrong.
There was a problem hiding this comment.
Hmm, we can leave it as is then.
Sorry, something went wrong.
|
Ah, the JNI CI is still broken for now...we can queue this for after #865 |
Sorry, something went wrong.
|
Ah but in the meantime do you mind helping the linter out? Error: Failed to execute goal com.diffplug.spotless:spotless-maven-plugin:2.44.4:check (spotless-check) on project arrow-c-data: The following files had format violations:
Error: src/main/java/org/apache/arrow/c/jni/JniLoader.java
Error: @@ -91,8 +91,7 @@
Error: ······}
Error: ····}
Error:
Error: -····final·String·libraryToLoad·=
Error: -········name·+·"/"·+·getNormalizedArch()·+·"/"·+·libraryName;
Error: +····final·String·libraryToLoad·=·name·+·"/"·+·getNormalizedArch()·+·"/"·+·libraryName;
Error: ····try·{
Error: ······File·temp·=
Error: ··········File.createTempFile("jnilib-",·".tmp",·new·File(System.getProperty("java.io.tmpdir")));
Error: Run 'mvn spotless:apply' to fix these violations.
Error: -> [Help 1]
|
Sorry, something went wrong.
|
Ok, please rebase and CI should be fixed! |
Sorry, something went wrong.
What's Changed
Add an opt-in code path that attempts to load the native library relative to the path specified by the arrow.cdata.library.path system property.
Closes #882.