Refactor library lookup #968
Conversation
* Try every prefix, not just the first one where the file exists. The first existing file might be for the wrong architecture or have other issues and cannot be loaded. * Show which directories were searched in error message. * Only consider /opt/homebrew/lib on darwin-aarch64. * Only rescue LoadError and RuntimeError, not Exception. * Refactor in smaller functions to improve readability. * Fixes ffi#880. * Example error message: Could not open library 'notexist': notexist: cannot open shared object file: No such file or directory. (LoadError) Could not open library 'libnotexist.so': libnotexist.so: cannot open shared object file: No such file or directory. Searched in <system library path>, /usr/lib, /usr/local/lib, /opt/local/lib
* The keys are not used, and the paths are typically already part of the error message. * Makes it possible to just use a recursive call instead of retry.
|
@larskanis Could you review this? |
|
@larskanis Could you take a look? |
| SEARCH_PATH = %w[/usr/lib /usr/local/lib /opt/local/lib] | ||
| if FFI::Platform::ARCH == 'aarch64' && FFI::Platform.mac? | ||
| SEARCH_PATH << '/opt/homebrew/lib' | ||
| end |
There was a problem hiding this comment.
Not sure if this is something you want to handle (and I wouldn't blame you if you don't):
Some Homebrew users install brew to a path that is neither /usr/local nor /opt/homebrew (e.g. they have no sudo access so use their $HOME). For these cases, one approach that works is to check if brew is in PATH and then check the output of brew --prefix.
There was a problem hiding this comment.
That seems an orthogonal issue to me.
Shelling out to brew --prefix seems quite expensive (e.g. while loading the ffi gem), maybe there should be a standard HOMEBREW_PREFIX env var exported by default or so, so we could read it without the cost of a subprocess.
Could you file a separate issue if you need that? (or maybe there is already one?)
There was a problem hiding this comment.
@eregon brew --prefix is the correct way to detect the homebrew location. or we need a configure flag for prepending an include dir to the initial build of the gem so that people can set their own configurations appropriately. /opt/homebrew and /usr/local are standard locations, but not the only options. However, this shouldn't be a blocker to get the standard fix out to the majority. people have been waiting for months and months. this is a serious breakage for rosetta and arm users.
Fix the easy standard way for the majority of users first, then we can come up with a way for the people who are in the outliers
There was a problem hiding this comment.
Yes I agree this is not a blocker. I think brew --prefix is too much of a cost on require, but maybe it's OK to compute it once lazily on ffi_lib on macOS.
What we need is @larskanis to review this PR and then make a release.
There was a problem hiding this comment.
Thinking more about it, it sounds really complicated to support Homebrew not in a default location.
For instance if Homebrew is installed both in amd64 and aarch64, how do we even call the "right" brew executable? It all sounds extremely messy and easy to mess up.
Anyway, if someone needs this they can make a PR.
|
@larskanis Gentle ping |
|
@larskanis sorry to disturb you, but could you take a look? this is break people especially for non Ruby developers |
The first existing file might be for the wrong architecture or have
other issues and cannot be loaded.
Could not open library 'notexist': notexist: cannot open shared object file: No such file or directory. (LoadError)Could not open library 'libnotexist.so': libnotexist.so: cannot open shared object file: No such file or directory.Searched in <system library path>, /usr/lib, /usr/local/lib, /opt/local/lib