Before I forget, I've no idea what to do for Windows on line 600.

Regards,
iain

Library lookup by using LD_LIBRARY_PATH and co. is redundant to what the dynamic loader of the operating system does. Why should we repeat the search through these directories? Is there a particular issue you're trying to fix?

Yes, in that it doesn't find libraries that aren't in the usual place but are set in the env vars by the user, because I installed exempi yesterday using pkgsrc which installs into /opt/pkg and FFI didn't find the library. With these changes it works.

Rebate against master please.

@tduehr Okay, but because of this open pull request I'm not sure what to do about pushing it back up. Would you prefer/advise I use --force, --force-with-lease, or use a new branch?

Regards,
iain

The difference is moot unless you have multiple people working in the same branch. When I’m working on a PR, I usually just use —force because I know I’m the only person working on that branch in my copy of the repo.

@tduehr Thanks, I wasn't sure. The specs and tests pass on my machine so we'll see what happens with the CI.

iain

I still don't think that's the way to go. It's the responsibility of the dynamic loader of the operating system to resolve relative paths (by /etc/ld.conf, environment variables, etc.) not ffi's. I would rather argue to remove these hardcoded paths entirely, because they are simply wrong in a multiarch setup (and most dists are multiarch nowadays). I've seen a lot of issues by such hardcoded paths especially on Windows with MSYS2/MINGW. It often leads to linking to some library of a wrong arch.

If there's a particular loader issue with a library in /opt/pkg, it should be investigated why this happens before implementing new logic in a place where it doesn't belong to.

@larskanis Where's the call to the dynamic loader made?

@yb66 dlopen is called here.

@larskanis Thanks, I'm happy give that a look when I get a moment. In the meantime there is some hardcoded stuff with logic that doesn't always work as it should and these changes fix that, so I would say the changes should be integrated until the problem with the dlopen code is fixed. The tests/specs should definitely be looked at too but as I'm not very familiar with the codebase I'd prefer someone else do it.

I'd like to see something like this supported...

I'm on os x with ffi failing to load libsodium, even after setting LD_LIBRARY_PATH (worked on Linux) and DYLD_[FALLBACK_]LIBRARY_PATH. (seems same issue as #461)

It'd be great if there was an environment-variable-based way to configure how ffi does a lookup (rather than editing code or monkey-patching, etc.). Even if was something explicit and specific to ruby's ffi like RUBY_FFI_PATH_FOR_mylibrary=/the/absolute/path/lib/mylibrary.dylib

Adding an ffi specific ENV variable is a good idea. It'd be useful to override existing paths too. RUBY_FFI_LIBRARY_PATH

Also, yes, that's OSX clearing the normal library path variables. Use a ruby in /usr/local/ instead and it'll work.

My ruby is in /nix/store/... :(

@tduehr "Use a ruby in /usr/local/ instead and it'll work." Why is the suggestion to follow a not-always-helpful convention when there are better ways to handle this?

@pmahoney-raise You use Nix? Interesting. I keep mine in /opt/rubies because I'm using chruby.

It seems that we're outliers and hardcoding paths is the thing to do in 2018 (and beyond).

Any update on this?

@larskanis this is a fallback when the dynamic loader can't find the library. Though, we should probably check dyld for all possible library names before doing a search like this.

I think the issue we try to solve here and in the very first commit is purely MacOS specific. We already had a security incident on exactly this part of code, so that I think we should shrink the scope of this post-dyld processing to where it is really necessary.

The dynamic loader on both Linux and Windows respects the relevant environment variables properly, so that we should make the code MacOS-only. I don't have a Mac, so I can't reproduce the issues, but we should try to remove the absolute paths.

Good point, that would make it macOS only, and only for Apple unsupported configurations. This should probably change to only a user specified env variable.