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 shared library build on macOS #5736

Closed
wants to merge 1 commit into from

Conversation

mkoeppe
Copy link

@mkoeppe mkoeppe commented Jun 8, 2024

macOS needs -install_name in the linker command for the dylib.

Text for release notes

see title

Further details

see sagemath/sage#38169

mkoeppe added a commit to mkoeppe/sage that referenced this pull request Jun 8, 2024
@fingolfin
Copy link
Member

fingolfin commented Jun 9, 2024

Thanks for the contribution @mkoeppe, I agree this goes in the right direction. Unfortunately this can't be merged as-is, as it breaks make testlibgap on macOS (and the fact that CI passes anyway highlights that we need to make sure at least one macOS runner also runs testlibgap). Indeed, the specified -install_name is only valid for an installed .dylib, but not for when GAP is run out of its source directory, as still is the norm for many (most?) GAP users. And make testlibgap tests exactly in that scenario. It shouldn't be too hard to resolve that, though -- we already build a separate GAP binary for installation (via the build/gap-install target) so we could do something similar for libgap.

mkoeppe added a commit to mkoeppe/sage that referenced this pull request Jun 17, 2024
orlitzky pushed a commit to orlitzky/sage that referenced this pull request Sep 17, 2024
Signed-off-by: Michael Orlitzky <michael@orlitzky.com>
orlitzky pushed a commit to orlitzky/sage that referenced this pull request Sep 17, 2024
Signed-off-by: Michael Orlitzky <michael@orlitzky.com>
orlitzky pushed a commit to orlitzky/sage that referenced this pull request Oct 12, 2024
orlitzky pushed a commit to orlitzky/sage that referenced this pull request Oct 12, 2024
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.

2 participants