Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Native libraries are searched in lib/polyglot project dir #11874
Native libraries are searched in lib/polyglot project dir #11874
Changes from 38 commits
305db07
a67afda
c98bdaf
84be175
1649ddf
05851f5
c17b62d
83c43e7
c51a2cc
1d7c0bb
98c8a61
5f0ce0a
e2d7099
561bcdc
f0c930e
5f158b8
c6d5963
7edc3be
fc325f2
3cdad6e
eb555f4
51cc811
361ab14
218c19f
f9b71bc
9ed04b1
3d9a377
7bc8768
5a44328
158110c
a9c60a5
829c0a9
5d7d748
a1521cd
f4bec69
ceb3326
b4d3f98
05197f5
e6956db
7ea660e
7d89629
3ca596c
f1b233e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 don't think this is necessary neither good idea.
Please remember that NI build runs in build time, while the
.so
,.dll
files are about to be located in runtime. The paths are going to be different.Can you tell me: What was failing and why you think this change is helpful? We don't need anything like this to load
.so
forenso_parser
...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.
For
enso_parser
, we have full control over explicit search for the library in Parser$Worker$initializeLibraries - in that method, we try to explicitly load theenso_parser
native library first withSystem.loadLibrary
, and then withSystem.load
.For opencv, we don't have full control. Its native library
opencv_java470
is loaded in the static initializer ofnu.pattern.OpenCV
withSystem.loadLibrary("opencv_java470")
.OpenCV
class is used by classes in ourstd-bits/image
. In JVM mode, all classes fromstd-bits/image
(classes in packageorg.enso.image
) are loaded byHostClassLoader
, therefore, alsoOpenCV
class is loaded byHostClassLoader
, and so,System.loadLibrary("opencv_java470")
delegates to HostClassLoader.findLibrary. In native image, there is noHostClassLoader
, and we have to deal with theSystem.loadLibrary
call insideOpenCV
differently.The solution to include path to the native library inside
RuntimeSystemProperties
from NI build time was the only solution I could think of. It points to a path likedistribution/lib/Standard/Image/0.0.0-dev/polyglot/lib/...
where the native libraries are located. All the tests are currently passing, because we test NI only via cmdline, and we don't try to package the NI inside the AppImage. It is definitely not the most robust solution. I am opened to any alternatives.TL;DR; Is there any other way how to hook into
System.loadLibrary
calls during runtime in NI other than settingjava.library.path
viaRuntimeSystemProperties
during NI build?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.
Really? How comes there is no
HostClassLoader
? Of course it cannot load any new classes, but the classloader can/should still be there...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 an example of the use of HostClassLoader. Still some problems there, but this way we should be able to get
HostClassLoader
"into the game".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 have tried multiple times to somehow get
HostClassLoader
"into the game", but failed after many attempts. See #11874 (comment) . The alternative solution mentioned in #11874 (comment) is much easier and seems to work. It does not modify configuration of NI at all.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.
Copying the libraries seems like a reasonable and portable fix. Let's use it for now and keep the
findLibrary
approach for later (for example #7082).