Fix case where MongoDB and MariaDB extensions fail in native #8788
Conversation
|
If you think I should add a new test containing these two extensions, I can obviously do that. |
| } | ||
|
|
||
| @TargetClass(com.sun.jna.Native.class) | ||
| final class JnaNativeSubstitutions { |
There was a problem hiding this comment.
Since this substitution targets a class which isn't MongoDB specific, does it belong here?
My concern is that if such subtitutions are kept within an extension, then they will eventually conflict with other extensions.
I guess it could be acceptable as a temporary solution, if there's an issue tracking a further improvement.
There was a problem hiding this comment.
Yeah, the ideal situation would be to have a jna extension that does these substitutions just like we have with Netty.
That's probably a good thing to follow up with - an easy thing for someone to contribute as their first contribution.
There was a problem hiding this comment.
We usually put that ones in core considering they are targetting JDK classes.
There was a problem hiding this comment.
This isn't a JDK class however. It's coming from this dependency:
Line 1786 in 6a612a8
| public class SimpleSocketHandlerFunction implements SocketHandlerFunction { | ||
| @Override | ||
| public Socket apply(Options options, String host) throws IOException { | ||
| return Utils.standardSocket(options, host); |
There was a problem hiding this comment.
should this perhaps log a warning or throw an exception if a MariaDB feature is being disabled?
We shouldn't silentily ignore configuration.
There was a problem hiding this comment.
Good idea yeah! Throwing an exception I think it the proper way to go. WDYT?
|
funny, was checking the changes in the MariaDB driver and it seems this odd JNA comment in the MariaDB driver was actually introduced precisely to improve GraalVM AOT.. :) Looks good! |
|
ok well was going to merge but @gsmet preceded me. One really can't spend an extra minute checking details |
|
Thanks folks |
3 participants
Fixes: #8785