Skip to content
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

Fix MacOs 14 Sonoma dlopen cache lookup #2549

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

takacsmark
Copy link

Applied a fix to f8b237e to the find_library patch based on the original Stackoverflow hint https://stackoverflow.com/questions/63475461/unable-to-import-opengl-gl-in-python-on-macos.

Closes #2547

Updated the find_library patch for backward compatibility
Closes vispy#2547
@djhoese
Copy link
Member

djhoese commented Nov 15, 2023

@brisvag what napari folks are your typical mac testers? Anyone want to sanity check this PR? Otherwise the fact that CI is happy (which is running on an intel mac runner iirc) seems to be a pretty good test to me.

@djhoese djhoese changed the title MacOs 14 Sonoma dlopen cache fix Fix MacOs 14 Sonoma dlopen cache lookup Nov 15, 2023
@djhoese djhoese self-assigned this Nov 15, 2023
@brisvag
Copy link
Collaborator

brisvag commented Nov 16, 2023

@psobolewskiPhD is our resident mac bug hunter :)

@psobolewskiPhD
Copy link
Contributor

I can test this on Ventura (13.6) to ensure no regression. I'll try to find time to update my personal laptop to Sonoma, but probably not till next weekend at the soonest.

@ataffanel
Copy link

I am maintaining a program that broke on Mac sonoma because of this bug and I can confirm that this PR fixes the problem on my test Mac M1 running Sonoma.

Is there anything more that needs to be checked before this can be merged?

@djhoese
Copy link
Member

djhoese commented Mar 19, 2024

@ataffanel Do you have pyopengl installed in your environment? I'm wondering if this PR isn't needed if pyopengl is installed.

@psobolewskiPhD
Copy link
Contributor

psobolewskiPhD commented Mar 20, 2024

IMO pyopengl is not a vispy requirement, so this should get fixed.
I think I'm ok with this solution, but worried it may break things on older than macOS 11--only when pyopengl is not installed--because of changing the path?
Also I'm still very puzzled by the lowercase quartz as noted #2547 (comment)

I don't get why I don't see issues locally with pyopengl despite CDLL reporting None:

>>> from vispy.ext.cocoapy import quartz
>>> quartz
<CDLL 'None', handle fffffffffffffffe at 0x1053f3050>

By using a Q it loads properly:

In [1]: from vispy.ext.cocoapy import quartz

In [2]: quartz
Out[2]: <CDLL '/System/Library/Frameworks/Quartz.framework/Quartz', handle 3ad9bf508 at 0x108251d30>

I suspect this PR and the patch might just be masking an issue the with cdll loading of quartz, but my use case (napari mostly) just doesn't touch quartz I guess?

Looking more closely at the monkey patch:

try:
import OpenGL.GL # noqa
except ImportError:
# print('Drat, patching for Big Sur')
orig_util_find_library = util.find_library
def new_util_find_library(name):
res = orig_util_find_library(name)
if res:
return res
lut = {
'objc': 'libobjc.dylib',
'quartz': 'Quartz.framework/Quartz'
}
return lut.get(name, name+'.framework/'+name)
util.find_library = new_util_find_library

We see that if the original find_library works, then it just uses it.
The lut from the monkey patch only triggers if the name isn't found
And I checked, this is the case only for lowercase quartz. When it triggers, instead of None it returns the wrong path and then leads to the issue. Now with this PR, without pyopengl, when quartz fails this PR provides the correct path -- note it uses Q already!
So it seems the simple solution is to directly use Quartz here:

quartz = cdll.LoadLibrary(util.find_library('quartz'))

Which should also solve the issue by preventing the monkeypatch from firing.
I made a branch with this:
https://github.com/psobolewskiPhD/vispy/tree/fix_quartz_cdll_simple
it seems to work fine, but maybe there is an ancient macOS where Quartz was quartz? But it was Quartz when first available in 10.4 if I'm reading these docs right
https://developer.apple.com/library/archive/documentation/MacOSX/Conceptual/OSX_Technology_Overview/SystemFrameworks/SystemFrameworks.html
(the document itself is from 2015, so ~macos 10.11, vs Big Sur from 2020.

Frankly, it looks like the fix in ctypes for cdlls in bigsur is out in python >3.8
https://bugs.python.org/issue41100
vispy doesn't support 3.7 anymore.
So one could drop the monkey patch entirely and just fix the Q
I tried that, but get a cibuildwheel test failure.... with cdll loading -- AppKit as far as I can tell.
Looks like it's a python 3.8 issue...
#1885 (comment)
Anyhow, here's the branch
https://github.com/psobolewskiPhD/vispy/tree/fix_quartz_cdll

@ataffanel
Copy link

@djhoese No, I do not have pyopengl. Installing pyopengl does indeed fixes the crash even when using the current vispy release! So adding pyopengl as a dependency to Vispy might also be a solution? Though, if it can be avoided it feels that avoiding to add a new dependency would be a good thing. At least this does give me a quick way forward to make my software work on mac, I can add the dependency on my side.

@psobolewskiPhD You branch works fine on a clean Sonoma (M1) using Apple's Python 3.9 and yes, it does look much simpler! I am trying to test on older version on an old X86 mac, but I am not sure at how long back to try. I tried 10.13 and it would not even install the command line tools, 10.15 does not want to install vispy with apple's python and installing python with brew is currently recompiling everything. I have a couple of mac setup right now to I can help testing but the question is: what is the oldest version of MacOS/Python supported by Vispy?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cannot find Quartz library on MacOS Sonoma
5 participants