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

Make rosidl packages use FindPython3 instead of FindPythonInterp #612

Merged
merged 1 commit into from
Aug 11, 2021

Conversation

sloretz
Copy link
Contributor

@sloretz sloretz commented Aug 6, 2021

This makes some rosidl packages use FindPython3 instead of FindPythonInterp.

Marking as Draft because I'm not sure if the removal of FindPythonInterp in rosidl_adapter was dependend upon by any
downstream packages - I'll make it not a draft if CI is green.

Related to ros2/python_cmake_module#6

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
@sloretz sloretz self-assigned this Aug 6, 2021
@sloretz
Copy link
Contributor Author

sloretz commented Aug 6, 2021

CI (build: --packages-above-and-dependencies rosidl_adapter rosidl_generator_c rosidl_generator_cpp rosidl_typesupport_introspection_c rosidl_typesupport_introspection_cpp test: --packages-select rosidl_adapter rosidl_generator_c rosidl_generator_cpp rosidl_typesupport_introspection_c rosidl_typesupport_introspection_cpp sensor_msgs)

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

@sloretz sloretz marked this pull request as ready for review August 6, 2021 21:56
@nuclearsandwich
Copy link
Member

The Rpr job will fail until ament/ament_cmake#352 is released.

@sloretz sloretz merged commit 36ed120 into master Aug 11, 2021
@delete-merged-branch delete-merged-branch bot deleted the use_FindPython3_rosidl branch August 11, 2021 18:56
@pablogs9
Copy link
Member

Hi @sloretz and @clalancette, this PR has broken the micro-ROS build tools for micro-ROS Rolling, just the previous one works but this makes building (or generating) interfaces fail with weird behavior:

/__w/micro_ros_setup/micro_ros_setup/firmware/mcu_ws/install/lib/rosidl_typesupport_microxrcedds_c/rosidl_typesupport_microxrcedds_c: 17: import: not found
/__w/micro_ros_setup/micro_ros_setup/firmware/mcu_ws/install/lib/rosidl_typesupport_microxrcedds_c/rosidl_typesupport_microxrcedds_c: 18: import: not found
/__w/micro_ros_setup/micro_ros_setup/firmware/mcu_ws/install/lib/rosidl_typesupport_microxrcedds_c/rosidl_typesupport_microxrcedds_c: 19: import: not found
/__w/micro_ros_setup/micro_ros_setup/firmware/mcu_ws/install/lib/rosidl_typesupport_microxrcedds_c/rosidl_typesupport_microxrcedds_c: 21: from: not found
/__w/micro_ros_setup/micro_ros_setup/firmware/mcu_ws/install/lib/rosidl_typesupport_microxrcedds_c/rosidl_typesupport_microxrcedds_c: 23: Syntax error: "(" unexpected
make[2]: *** [CMakeFiles/builtin_interfaces__rosidl_typesupport_microxrcedds_c.dir/build.make:86: rosidl_typesupport_microxrcedds_c/builtin_interfaces/msg/detail/microxrcedds/duration__type_support_c.c] Error 2
make[1]: *** [CMakeFiles/Makefile2:150: CMakeFiles/builtin_interfaces__rosidl_typesupport_microxrcedds_c.dir/all] Error 2

As far as I have understood, this means that the rosidl_typesupport_microxrcedds_c is being executed as Bash script and that's why those syntax errors appear and the import command is not installed.

Later, I have found that the interpreter line is not in the first line of my file but in line 15, moving it to line 1 seems to solve the problem.

But my point now is, why using "${PYTHON_EXECUTABLE}" this file is treated as a Python script regardless of this first interpreter line (#!/usr/bin/env python3) and with the new ${python_interpreter} it behaves differently?

@clalancette
Copy link
Contributor

In short, this is a bug either in this PR or one of the related ones. We are looking into it.

mjcarroll added a commit that referenced this pull request Aug 12, 2021
mjcarroll added a commit that referenced this pull request Aug 12, 2021
…erp (#612)"

This reverts commit 36ed120.

Signed-off-by: Michael Carroll <michael@openrobotics.org>
@mjcarroll
Copy link
Member

It looks like this will need to be reverted: #615

@sloretz
Copy link
Contributor Author

sloretz commented Aug 12, 2021

Hi @pablogs9 I opened micro-ROS/rosidl_typesupport_microxrcedds#34 to make those packages in micro-ROS use the FindPython3 CMake module. That PR can be merged regardless of whether this one is reverted or not.

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.

5 participants