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.

Sign up for GitHub

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

Jump to bottom

Fix linking against libraries from Meson project on macOS #260

Merged
merged 4 commits into from
Feb 1, 2023
Merged

Fix linking against libraries from Meson project on macOS #260

merged 4 commits into from
Feb 1, 2023

Conversation

pastewka
Copy link
Contributor

This PR fixes linking against libraries created during the Meson build on macOS. It uses otool and install_name_tool to update the LC_RPATH load command in the dylib. (This is identical to how the library is patched on Linux.)

@rgommers rgommers added the enhancement New feature or request label Jan 14, 2023
@pastewka pastewka marked this pull request as draft January 14, 2023 19:43
@rgommers rgommers mentioned this pull request Jan 14, 2023
Closed
@rgommers
Copy link
Contributor

This looks promising, thanks for tackling this topic @pastewka!

The fix-pip step seems on the Cirrus CI Debian jobs seems to be failing consistently, probably because pypa/pip#11623 was merged a few days ago. @dnicolodi would you mind having a look?

@dnicolodi
Copy link
Member

@rgommers Yes, upgrading pip to a version that fixes a known issue we are hitting on Debian fails because the merge branch has been deleted. Fixing that is on my todo list. I'll do that as soon as a find a little bit of time.

@pastewka pastewka marked this pull request as ready for review January 14, 2023 20:17
@pastewka
Copy link
Contributor Author

@rgommers @dnicolodi Happy to contribute, meson-python is a fantastic project that has made my life a whole lot easier. It looks like all tests pass now, except the Debian tests which as discussed seem unrelated to this PR.

@dnicolodi
Copy link
Member

Please squash the commits

@pastewka
Copy link
Contributor Author

Done

HDembinski
HDembinski reviewed Jan 16, 2023
View reviewed changes
mesonpy/__init__.py Outdated Show resolved Hide resolved
@pastewka
Copy link
Contributor Author

I've also updated the README to reflect status on macOS.

@pastewka pastewka force-pushed the 23_fix_macos_libs branch 3 times, most recently from fdd46f4 to d124297 Compare January 18, 2023 16:44
@pastewka
Copy link
Contributor Author

pastewka commented Feb 1, 2023

Is there anything else here that I should change? What is the process for getting this into a release?

pastewka and others added 2 commits February 1, 2023 21:02
@pastewka @FFY00
Fix linking against libraries from Meson project on macOS
d764723 
Signed-off-by: Filipe Laíns <lains@riseup.net>
@FFY00
update docs regarding this change
8a427f1 
Signed-off-by: Filipe Laíns <lains@riseup.net>
FFY00
FFY00 approved these changes Feb 1, 2023
View reviewed changes
Copy link
Member

@FFY00 FFY00 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pastewka sorry for the delay! I rebased this PR on main, we should now be able to merge as soon as the CI runs. Thank you so much for the contribution!

@FFY00
fix typing handling to match our new approach
7004bb7 
Signed-off-by: Filipe Laíns <lains@riseup.net>
@FFY00
fix isort
f4aeb94 
Signed-off-by: Filipe Laíns <lains@riseup.net>
@FFY00 FFY00 merged commit 4a3558c into mesonbuild:main Feb 1, 2023
FFY00 pushed a commit that referenced this pull request Feb 1, 2023
@pastewka @FFY00
ENH: fix linking against libraries from Meson project on macOS
d75f4cd 
PR #260

Signed-off-by: Filipe Laíns <lains@riseup.net>
@rgommers rgommers added this to the v0.13.0 milestone Feb 2, 2023
@rgommers
Copy link
Contributor

rgommers commented Feb 2, 2023

Awesome, great to see this land - thanks @pastewka and @FFY00!

@pastewka
Copy link
Contributor Author

pastewka commented Feb 2, 2023

Thanks for merging this, happy to contribute!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rgommers rgommers rgommers left review comments

@HDembinski HDembinski HDembinski left review comments

@FFY00 FFY00 FFY00 approved these changes

Assignees
No one assigned
Labels
enhancement New feature or request
Projects
None yet
Milestone
v0.13.0
Development

Successfully merging this pull request may close these issues.
5 participants
@pastewka @rgommers @dnicolodi @HDembinski @FFY00