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: use macOS Frameworks #1456

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

dgramop
Copy link
Contributor

@dgramop dgramop commented Apr 4, 2024

We want solvespace to dynamically link with only one OpenGL library at a time, in particular the one that ships with MacOS. For this we use macOS frameworks, and pass the appropriate flag to the linker.

We should not use default cmake logic for finding openGL, since it seems to find the wrong version of libGL, that may have been installed by some other package at some point.

Closes issue #1454

We want solvespace to dynamically link with only one OpenGL library at a
time, in particular the one that ships with MacOS. For this we use macOS
frameworks, and pass the appropriate flag to the linker.

We should not use default cmake logic for finding openGL, since it seems
to find the wrong version of libGL, that may have been installed by some
other package at some point.
@dgramop
Copy link
Contributor Author

dgramop commented Apr 4, 2024

We can close that issue regardless of whether this PR is accepted :)

I think I figured out what's broken and it's unclear if Solvespace should change, find_package in cmake should change, or if whatever package in homebrew or elsewhere that provides /opt/local/lib/libGL.dylib should change.

The case for this PR and making the change in solvespace or CMake's FindOpenGL module is that it makes solvespace more resilient to other weird non-system installs of openGL in MacOS

@ruevs ruevs requested review from ruevs, vespakoen and rpavlik and removed request for ruevs April 5, 2024 04:22
@ruevs
Copy link
Member

ruevs commented Apr 5, 2024

It's interesting why did CMake find the "wrong" OpenGL on your system.
According to the documentation it should have used the framework. Perhaps (as you mentioned) it is a problem with CMake, or is it the way we are using it? My experience with CMake is very limited - @rpavlik and @vespakoen - do you have some time to review this?

@dgramop does the new artefact (built by the CI in this pull request) work for you? (I hope you can see it).

@dgramop
Copy link
Contributor Author

dgramop commented Apr 5, 2024

I agree with the scare quotes around "wrong", I wonder if some other software that uses cmake relies on its behavior on macOS to find whatever it found on my system instead of just using the framework.

I can't see the MacOS artifact for the PR. I can see artifacts for commits pushed to master, this one just has "solvespace-x86_64".

@ruevs
Copy link
Member

ruevs commented Apr 5, 2024

I can't see the MacOS artifact for the PR.

Sorry I forgot - the CI for pull requests does not keep the artefacts.

@dgramop
Copy link
Contributor Author

dgramop commented Apr 17, 2024

Hey! Just checking in, did anyone get a chance to try this out on a mac? Maybe there was something I mistinterpreted and was supposed to follow up on?

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

Successfully merging this pull request may close these issues.

None yet

2 participants