ros2 / rosidl_python

rosidl support for Python
Apache License 2.0
21 stars 45 forks source link

PYTHON_SOABI returns a host version of SOABI while cross-compiling #111

Open razr opened 4 years ago

razr commented 4 years ago

Bug report

Required Info:

After that all python ROS2 modules are built with the host extension and not loadable on the target system. build: _rclpy.cpython-38-x86_64-linux-gnu.so expected: _rclpy.cpython-38-vxworks.so

Expected behavior

It should give back a target SOABI, not the host one. Probably a bit more sophisticated check shall be implemented, e.g. run a target version of python3-config --extension-suffix

Actual behavior

always returns the host python SOABI version

Additional information

It is possible to define -DPYTHON_SOABI="my SOABI" to workaround it.

hidmic commented 4 years ago

@razr thanks for reporting ! Would you be willing to submit a fix for this issue?

razr commented 4 years ago

Yes, I will. However, I can provide it at the moment against the ROS2 dashing release and I can only test it for the VxWorks cross and Ubuntu 18.04 native builds. Please let me know if it is ok with you.

hidmic commented 4 years ago

Hmm, to patch release an LTS distro and hope we can fast-forward those changes to master at some point in the future isn't ideal. But I understand the situation. What do you think @nuclearsandwich ?

nuclearsandwich commented 4 years ago

Thanks for being willing to put in the work @razr! I definitely don't want to turn away your contributions but I also want to make sure that any work you do has a high chance of successfully being merged.

I'd much rather see a fix come in targeting the latest development branch with intent to backport. I'm also not sure if an explicit CMake define is the best way to fix the issue. Have you checked https://github.com/ros-tooling/cross_compile to see if they've solved this issue? If not it might be worth opening an issue there or transferring this one as the maintainers of that repository may have the resources to test on master before backporting any changes.

razr commented 4 years ago

No, I didn't look there but without fixing this part whatever they do most probably does not help. I do agree it is better to fix it in the master branch. I have just started with master and didn't hit this problem yet. We can just wait a bit :). I don't think it is a critical issue and a workaround exists.

Here https://index.ros.org/doc/ros2/Tutorials/Cross-compilation/ stated that PYTHON_SOABI shall be set. And here https://answers.ros.org/question/329161/rclpy-fails-to-run-on-arm64-after-cross-compilation/ someone complains that cross-compilation does not work due to exactly the reason I have described.

tobiasneumann commented 3 months ago

I think I had the same problem when compiling for openembedded. I reverted 0f43ffc490b7e4ec08022361bced5fa274246dfd to fix the problem.

The patch I'm using for humble (I think PR4) is the following.

From 5d39c2395f79c374c4cd942bde91e9fc5838397e Mon Sep 17 00:00:00 2001
From: Tobias Neumann <tobias.neumann@indurad.com>
Date: Wed, 27 Sep 2023 15:06:33 +0200
Subject: [PATCH] Revert "Fix rosidl_generator_py assuming incorect library
 names (#149)"

This reverts commit 0f43ffc490b7e4ec08022361bced5fa274246dfd.
---
 ...idl_generator_py_generate_interfaces.cmake | 24 +++++++++----------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/cmake/rosidl_generator_py_generate_interfaces.cmake b/cmake/rosidl_generator_py_generate_interfaces.cmake
index a6d8263..1f331f4 100644
--- a/cmake/rosidl_generator_py_generate_interfaces.cmake
+++ b/cmake/rosidl_generator_py_generate_interfaces.cmake
@@ -159,10 +159,7 @@ macro(set_lib_properties _build_type)
     RUNTIME_OUTPUT_DIRECTORY${_build_type} ${_output_path})
 endmacro()

-# Export target so downstream interface packages can link to it
-set(rosidl_generator_py_suffix "__rosidl_generator_py")
-
-set(_target_name_lib "${rosidl_generate_interfaces_TARGET}${rosidl_generator_py_suffix}")
+set(_target_name_lib "${rosidl_generate_interfaces_TARGET}__python")
 add_library(${_target_name_lib} SHARED ${_generated_c_files})
 target_link_libraries(${_target_name_lib}
   ${rosidl_generate_interfaces_TARGET}__rosidl_generator_c)
@@ -177,7 +174,7 @@ target_link_libraries(
   ${PythonExtra_LIBRARIES}
 )
 target_include_directories(${_target_name_lib}
-  PRIVATE
+  PUBLIC
   ${CMAKE_CURRENT_BINARY_DIR}/rosidl_generator_c
   ${CMAKE_CURRENT_BINARY_DIR}/rosidl_generator_py
   ${PythonExtra_INCLUDE_DIRS}
@@ -286,9 +283,17 @@ endforeach()

 set(PYTHON_EXECUTABLE ${_PYTHON_EXECUTABLE})

-# Depend on rosidl_generator_py generated targets from our dependencies
 foreach(_pkg_name ${rosidl_generate_interfaces_DEPENDENCY_PACKAGE_NAMES})
-  target_link_libraries(${_target_name_lib} ${${_pkg_name}_TARGETS${rosidl_generator_py_suffix}})
+  set(_pkg_install_base "${${_pkg_name}_DIR}/../../..")
+  set(_pkg_python_libname "${_pkg_name}__python")
+
+  if(WIN32)
+    target_link_libraries(${_target_name_lib} "${_pkg_install_base}/Lib/${_pkg_python_libname}.lib")
+  elseif(APPLE)
+    target_link_libraries(${_target_name_lib} "${_pkg_install_base}/lib/lib${_pkg_python_libname}.dylib")
+  else()
+    target_link_libraries(${_target_name_lib} "${_pkg_install_base}/lib/lib${_pkg_python_libname}.so")
+  endif()
 endforeach()

 set_lib_properties("")
@@ -300,14 +305,9 @@ if(WIN32)
 endif()
 if(NOT rosidl_generate_interfaces_SKIP_INSTALL)
   install(TARGETS ${_target_name_lib}
-    EXPORT export_${_target_name_lib}
     ARCHIVE DESTINATION lib
     LIBRARY DESTINATION lib
     RUNTIME DESTINATION bin)
-
-  # Export this target so downstream interface packages can depend on it
-  rosidl_export_typesupport_targets("${rosidl_generator_py_suffix}" "${_target_name_lib}")
-  ament_export_targets(export_${_target_name_lib})
 endif()

 if(BUILD_TESTING AND rosidl_generate_interfaces_ADD_LINTER_TESTS)
-- 
2.39.2