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 Python linking on OSX #806

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

Conversation

Tobias-Fischer
Copy link

See e.g. ros-perception/vision_opencv#331 for reasoning

Fixes #805

@Tobias-Fischer
Copy link
Author

/cc @raghavauppuluri13 @wolfv

Signed-off-by: Tobias Fischer <[email protected]>
@raghavauppuluri13
Copy link

I added the missing flags, but didn't know about removing the python linking. Nice!

Signed-off-by: Tobias Fischer <[email protected]>
@sloretz
Copy link
Contributor

sloretz commented Jul 21, 2021

CI (repos: https://gist.githubusercontent.com/sloretz/5a5bb6c456cbbe4a1a13c9c48df25939/raw/9a8fe89ca80e3062354ee6f87211dfac8edd8dd4/ros2.repos build: --packages-up-to rclpy test: --packages-select rclpy)

  • Linux Build Status
  • Linux-aarch64 Build Status
  • macOS Build Status
  • Windows Build Status

Copy link
Contributor

@sloretz sloretz left a comment

Choose a reason for hiding this comment

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

LGTM with green CI

@Tobias-Fischer
Copy link
Author

Note that we still face issues in ros2 doctor. Do you have any idea where else PYTHON_LIBRARIES are linked directly @sloretz by any chance?

@sloretz
Copy link
Contributor

sloretz commented Jul 21, 2021

Do you have any idea where else PYTHON_LIBRARIES are linked directly @sloretz by any chance?

maybe here? https://github.com/ros2/rosidl_python/blob/17de859d09fdddc611f003757a9df1acab865cdf/rosidl_generator_py/cmake/rosidl_generator_py_generate_interfaces.cmake#L174

@sloretz
Copy link
Contributor

sloretz commented Jul 23, 2021

@Tobias-Fischer Any idea why OSX CI for this PR had a segfault in one of the rclpy tests? I don't see the same issue on the nightly OSX job.

projectroot.test_c_handle

-- run_test.py: invoking following command in '/Users/osrf/jenkins-agent/workspace/ci_osx/ws/build/rclpy':
 - /Users/osrf/jenkins-agent/workspace/ci_osx/ws/build/rclpy/test_c_handle --gtest_output=xml:/Users/osrf/jenkins-agent/workspace/ci_osx/ws/build/rclpy/test_results/rclpy/test_c_handle.gtest.xml
dyld: Symbol not found: _PyExc_MemoryError
  Referenced from: /Users/osrf/jenkins-agent/workspace/ci_osx/ws/install/rclpy/lib/librclpy_common.dylib
  Expected in: flat namespace
 in /Users/osrf/jenkins-agent/workspace/ci_osx/ws/install/rclpy/lib/librclpy_common.dylib
-- run_test.py: return code -6

@Tobias-Fischer
Copy link
Author

Unfortunately I have no idea re this exception.

Any clues @traversaro @wolfv?

@traversaro
Copy link

It may be worth to check if the Python executable used for the tests is the same Python whose libraries (and headers) have been used to compile the extension.

@traversaro
Copy link

It may be worth to check if the Python executable used for the tests is the same Python whose libraries (and headers) have been used to compile the extension.

The headers are particularly tricky, as even if CMake appears to have found the correct Python, the incorrect Python headers could be accidentally be used by compilation unit if the directory of the wrong Python is in the include path as it is installed in the prefix of some other dependency. This is however is just an hypothesis.

By inspecting the CI log, it seems that both Python 3.8 (the one that is then used to pip install dependencies and to create a venv) and Python 3.7 (that instead should be never used) are installed:

python 3.7.7
[email protected] 3.8.2

I am not sure if this is playing a role, perhaps it could make sense to try to uninstall the Python 3.7 if possible?

@traversaro
Copy link

However, I quickly checked and _PyExc_MemoryError is defined in any Python > 3.6 (see https://github.com/python/cpython/blob/3.6/Objects/exceptions.c#L2332), so it may be also worth to print the symbols exported (https://stackoverflow.com/questions/4506121/how-to-print-a-list-of-symbols-exported-from-a-dynamic-library) by the used Python library to understand if _PyExc_MemoryError is indeed provided or not.

@wolfv
Copy link

wolfv commented Jul 25, 2021

Maybe the difference is that we remove the "DYLD_LIBRARY_PATH" and in "vanilla ROS" it's set to some value? Just guessing

@audrow audrow changed the base branch from master to rolling June 28, 2022 14:21
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.

Fatal Python error: PyMUTEX_LOCK(gil->mutex) failed on MacOS
5 participants