ros2/rosidl_python

PYTHON_SOABI returns a host version of SOABI while cross-compiling

razr opened this issue · 6 comments

razr commented

Bug report

Required Info:

  • Operating System:
    • host: Ubuntu 18.04
    • target: VxWorks 7
  • Installation type:
    • source compilation
  • Version or commit hash:
    • dashing release
  • DDS implementation:
    • N/A
  • Client library (if applicable):
    • N/A

Steps to reproduce issue

While cross-compiling the ROS2 dashing release for VxWorks and encounter the following problem
FindPythonExtra.cmake gives back a host PYTHON_SOABI instead of a target one.
It runs get_config_var('SOABI') using native python
"from sysconfig import get_config_var"
"print(get_config_var('SOABI'))"

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.

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

razr commented

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.

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 ?

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

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.

I think I had the same problem when compiling for openembedded.
I reverted 0f43ffc 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