Options for loading native libraries #294
Conversation
| end | ||
|
|
||
| $LIBS += " -l libssl.#{RbConfig::CONFIG['NATIVE_DLEXT']}" |
There was a problem hiding this comment.
It's unfortunate we have two mechanisms: OPENSSL_PREFIX for the headers and remap for the libs.
There was a problem hiding this comment.
It's also easy to make the mistake to pick up the system header but use another library with -Xcexts.remap
There was a problem hiding this comment.
Would it be possible to use the remap system to find the headers, at least for OpenSSL?
There was a problem hiding this comment.
FWIW MRI uses --with-openssl-dir=PREFIX. rbenv/ruby-build#377 (comment)
There was a problem hiding this comment.
That's an autoconf option - we don't use autoconf in our system.
I'm not sure about combining finding the includes with finding the library. What about making the first thing more precise - make it OPENSSL_INCLUDE_DIR?
There was a problem hiding this comment.
Plus that autoconf option hard codes the library path into the executable - that's what we don't want to do.
There was a problem hiding this comment.
I think it's good to combine both and only give a prefix-like option, as then it's impossible to pick the headers from one prefix and the lib from another.
There was a problem hiding this comment.
e.g.: on Darwin, when installing a gem that directly links against libssl (Puma does for instance), it will by default pick the Homebrew libssl, but use the system openssl.h header, which not going to work.
BTW, installing a gem linking to a native lib also supports this kind of dir option (via dir_config in the extconf.rb):
gem install mysql2 -- --with-mysql-dir=/usr/local/mysql
But indeed, this is build-time config and here we are rather talking about link-time config, which usually doesn't need headers. It might be good to unify them though, for simplicity.
| try { | ||
| ForeignAccess.sendExecute(executeSulongLoadLibraryNode, sulongLoadLibraryFunction, libraryRubyString); | ||
| } catch (UnsupportedTypeException | ArityException | UnsupportedMessageException e) { | ||
| throw new JavaException(e); | ||
| } | ||
| } | ||
|
|
||
| private String remapNativeLibrary(String library) { |
There was a problem hiding this comment.
Needs to be synchronized as well.
There was a problem hiding this comment.
Ah, actually not since getNativeLibraryMap() is the only mutator.
There was a problem hiding this comment.
It would be good to use Collections.unmodifiableMap to clarify this property.
* commit 'abb98ed83d76306ecf14af4676a625b240f59156': Remove repeated property read
No description provided.