-
Notifications
You must be signed in to change notification settings - Fork 1.7k
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
Embedded native library in the JAR file #120
Conversation
extracted from it. The library is searched in the JAR in the ${os.name}-${os.arch} folder. The JAR is the LAST location where JNA will try to find the library. This feature is inhibited if the property jna.nounpack is true.
It looks interesting. If you want this in JNA, you need at least a bunch of tests. |
You're right, I've just added a unit test. |
} | ||
} | ||
finally { | ||
try { is.close(); } catch(IOException e) { } |
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.
I'll let @twall comment on this, but I think catching and ignoring exceptions here is a really bad practice. This is just not supposed to fail, and if it does, something has gone awfully wrong.
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.
This part is a small copy/paste of Native.LoadNativeLibraryFromJar().
In theory I agree that ignoring exceptions is a bad pratice, but such pattern is very common in finally blocks when a stream has to be closed (because there should be no error here, and even if there is one we do not really care).
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.
This is a commonly ignored exception in Java; there really is no recourse and in practice I've never actually seen the error.
yes you are right, but please notice in case a stream is closing sometimes a implicit call to flush is made and in that case you as developer are wondering why the stream doesn |
private static String getEmbeddedLibraryPathFromJar(String libraryName) | ||
{ | ||
// Do not extract the library from JAR if jna.nounpack=true | ||
if (Boolean.getBoolean("jna.nounpack")) { |
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.
This should probably be a different flag; jna.nounpack
specifically forbids unpacking JNA's native bits; it shouldn't be re-used to forbid unpacking additional native bits unless the two situations are identical (I don't think they are).
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.
I thought the idea was to forbid unpacking because, for example, we were not allowed to write in the temp folder. So I thought both situations were similar.
I like this feature, and had been intending on adding it for a while. |
if (Boolean.getBoolean("jna.nounpack")) { | ||
return null; | ||
} | ||
final String libraryPath = System.getProperty("os.name") + "-" + System.getProperty("os.arch") + "/"; |
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.
This prefix is problematic in that the two properties are not terribly consistent across OS versions and JVMs. JNA's more canonical representation would be better.
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.
I agree. However, the advantage of using these prefixes is that it makes it very easy to copy the native libs in the right folders from Maven or even Ant. But you're right, there are not very consistent (I don't understant why there are so many different names for Windows for exemple).
Applied in commit 3823533 |
Motivation: The manifest did contain stuff that is not used at all. Modifications: Remove unused entries Result: Cleanup
With this patch, a native library can now be embedded in a JAR and automatically
extracted from it.
The library is searched in the JAR in the ${os.name}-${os.arch} folder.
The JAR is the LAST location where JNA will try to find the library.
This feature is inhibited if the property jna.nounpack is true.