-
Notifications
You must be signed in to change notification settings - Fork 55
Load DuckDB native library by name first #421
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
base: main
Are you sure you want to change the base?
Conversation
When loading a native library, attempt to first load the library, `duckdb_java`, by name. Only when that fails, loading will revert to the old behaviour of searching via classpath/local file path. This allows for using DuckDB Java client in a setup where native library has been properly extracted (wrt OS/Arch), renamed to follow the specification and placed on JNI search path. For `System.loadLibrary` to work, the native library has to have the correct prefix (`lib` for Mac/Linux, no `lib` for Windows) and suffix (`.so`, `.dll` or `.dylib` for Linux/Windows/Mac, respectively). No architecture suffix is allowed.
|
Hi, thanks for the PR! It looks good to me. About the Graal metadata - yes, I think it is a good idea to include it. To fix the formatting errors (to pass the CI checks) you can run the following: python -m pip install --user clang_format==11.0.1
make format
make format-checkOr just apply the following patch: diff --git a/src/main/java/org/duckdb/DuckDBNative.java b/src/main/java/org/duckdb/DuckDBNative.java
index 599ba33d..78354799 100644
--- a/src/main/java/org/duckdb/DuckDBNative.java
+++ b/src/main/java/org/duckdb/DuckDBNative.java
@@ -24,7 +24,7 @@ final class DuckDBNative {
}
private static void loadLibraryFromFile(String libname) {
- String full_libname = "lib" + libname;
+ String full_libname = "lib" + libname;
String lib_suffix = ".so";
String os_name = "";
String os_arch;
@@ -53,8 +53,7 @@ final class DuckDBNative {
} else if (os_name_detect.startsWith("linux")) {
os_name = "linux";
}
- String lib_res_name = "/" + full_libname + lib_suffix
- + "_" + os_name + "_" + os_arch;
+ String lib_res_name = "/" + full_libname + lib_suffix + "_" + os_name + "_" + os_arch;
try {
Path lib_file = Files.createTempFile(full_libname, lib_suffix);
URL lib_res = DuckDBNative.class.getResource(lib_res_name);Just note that this area with unpacking/loading the shared lib is a subject to change. There are no exact plans right now, but in general the following changes are intended:
|
|
Thanks for updating the PR! Just FYI, this change is intended to be included into 1.4.2 update planned for Nov 10. |
This change makes the following arch-specific artifacts to be deployed
to Maven Central (in addition to the main `duckdb_jdbc-x.x.x.x.jar` that
remains unchanged), classifiers:
- `linux_amd64`
- `linux_amd64_musl`
- `linux_arm64`
- `linux_arm64_musl`
- `macos_universal`
- `windows_amd64`
- `windows_arm64`
Each arch-specific artifact contains a native library only for this
particular platform and can be specified in Maven dependencies with the
following syntax:
```xml
<dependency>
<groupId>org.duckdb</groupId>
<artifactId>duckdb_jdbc</artifactId>
<version>1.x.x.x</version>
<classifier>linux_amd64_musl</classifier>
</dependency>
```
Note that Windows and Linux-musl AArch64 artifacts are renamed from
`aarch64` to `arm64` to align with wider DuckDB arch naming.
Additionally an artifact without any native library is deployed with a
`nolib` classifier. It is intended to be used with an externally
provided native library, see duckdb#421 for details.
This change makes the following arch-specific artifacts to be deployed
to Maven Central (in addition to the main `duckdb_jdbc-x.x.x.x.jar` that
remains unchanged), classifiers:
- `linux_amd64`
- `linux_amd64_musl`
- `linux_arm64`
- `linux_arm64_musl`
- `macos_universal`
- `windows_amd64`
- `windows_arm64`
Each arch-specific artifact contains a native library only for this
particular platform and can be specified in Maven dependencies with the
following syntax:
```xml
<dependency>
<groupId>org.duckdb</groupId>
<artifactId>duckdb_jdbc</artifactId>
<version>1.x.x.x</version>
<classifier>linux_amd64_musl</classifier>
</dependency>
```
Note that Windows and Linux-musl AArch64 artifacts are renamed from
`aarch64` to `arm64` to align with wider DuckDB arch naming.
Additionally an artifact without any native library is deployed with a
`nolib` classifier. It is intended to be used with an externally
provided native library, see #421 for details.
Thanks, I will create a separate PR with Native Image properties. |
This PR is a continuation of duckdb#447 PR to allow using `duckdb_jdbc-x.x.x.x-nolib.jar` along with a JNI native library, that is loaded directly from file system. It extends the idea from duckdb#421 (and supersedes it) implementing the following logic: 1. if the driver JAR has a bundled native library (for current JVM os/arch), then this library will be unpacked to the temporary directory and loaded from there. If the library cannot be unpacked or loaded - there is no fallback to other methods (it is expected that `-nolib` JAR is used for other loading methods) 2. if the driver JAR does not hava a native library bundled inside it, then it will check whether a JNI native libary with the DuckDB internal naming (like `libduckdb_java.so_linux_amd64`) exists in file system next to the driver JAR (in the same directory). If library file is found there - then the driver will attempt to load it. If the file is found in file system, then it is expected that is can be loaded and there is no fallback to loading by name. 3. if the native lib is not found in the same directory, then, like in duckdb#421, the driver tries to load it using `duckdb_java` name (that will be translated by JVM to a platform-specific name like `libduckdb_java.so`). Testing: new test added that covers loading from the same dir and loading by name. Fixes: duckdb#444
When loading a native library, attempt to first load the library,
duckdb_java, by name. Only when that fails, loading will revert to the old behaviour of searching via classpath/local file path.The main motivation for this work was to allow creating a correct Native Image that will not have issues during initialization. The change does not provide the support out-of-the-box.
But it works in Native Image if someone (like us) extracts the native library (wrt OS/Arch), renames it to follow the specification and places it on JNI search path.
For
System.loadLibraryto work, the native library has to have the correct prefix (libfor Mac/Linux, nolibfor Windows) and suffix (.so,.dllor.dylibfor Linux/Windows/Mac, respectively). No architecture suffix is allowed.As it stands the current packaging logic does not create native library files with the right names but that seems like a deeper change in the build setup. Also, I have zero knowledge about how the artifacts are being used internally, in dev work, so I don't feel comfortable with changing it without guidance.
One could look at sqlite-jdbc for some inspiration on how to improve it (which is roughly what we had to do when repackaging DuckDB's Java client jar).
Also, if maintainers are fine with this change we can also include Native Image configuration in the final
duckdb_jdbc.jarwhich is a requirement for GraalVM's Native Image. More and more Java libraries provide similar Native Image configuration.That would mostly enable #180.