Skip to content

Add pybind11 module as MODULE#497

Merged
scpeters merged 2 commits intogz-math7from
scpeters/py_module
Sep 7, 2022
Merged

Add pybind11 module as MODULE#497
scpeters merged 2 commits intogz-math7from
scpeters/py_module

Conversation

@scpeters
Copy link
Copy Markdown
Member

@scpeters scpeters commented Sep 6, 2022

🦟 Bug fix

This fixes an issue with linking to python libraries on macOS.

Summary

When trying to update the homebrew bottles in osrf/homebrew-simulation#2036 so that python bindings can be used without setting the PYTHONPATH, I ran into the following error:

osrf/simulation/gz-math7:
  * python modules have explicit framework links
    These python extension modules were linked directly to a Python
    framework binary. They should be linked with -undefined dynamic_lookup
    instead of -lpython or -framework Python.
      /usr/local/opt/gz-math7/lib/python3.10/site-packages/gz/math.cpython-310-darwin.so
      /usr/local/opt/gz-math7/lib/python3.10/site-packages/ignition/math.cpython-310-darwin.so

I found that using MODULE instead of SHARED in our pybind11_add_module call fixes this issue.

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

This fixes an issue with linking to python libraries
on macOS.

Signed-off-by: Steve Peters <scpeters@openrobotics.org>
@codecov
Copy link
Copy Markdown

codecov bot commented Sep 6, 2022

Codecov Report

Merging #497 (8769d58) into gz-math7 (61e8574) will not change coverage.
The diff coverage is n/a.

❗ Current head 8769d58 differs from pull request most recent head 72e726d. Consider uploading reports for the commit 72e726d to get more accurate results

@@            Coverage Diff            @@
##           gz-math7     #497   +/-   ##
=========================================
  Coverage     99.70%   99.70%           
=========================================
  Files            77       77           
  Lines          7007     7007           
=========================================
  Hits           6986     6986           
  Misses           21       21           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@mjcarroll mjcarroll added the bug Something isn't working label Sep 7, 2022
@scpeters scpeters merged commit b9d69e5 into gz-math7 Sep 7, 2022
@scpeters scpeters deleted the scpeters/py_module branch September 7, 2022 17:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working 🌱 garden Ignition Garden

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

5 participants