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

Install generated Python interfaces in a Python package #131

Merged
merged 1 commit into from May 28, 2021

Conversation

hidmic
Copy link
Contributor

@hidmic hidmic commented May 26, 2021

This makes Python interfaces discoverable through pkg_resources and importlib.metadata.

Full CI to be safe:

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

Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
@hidmic hidmic merged commit d68c385 into master May 28, 2021
@delete-merged-branch delete-merged-branch bot deleted the hidmic/install-python-package branch May 28, 2021 11:14
@jacobperron
Copy link
Member

This change prevents a package from building if it both generates interfaces and installs a Python package. For example, I have a package that is not longer building since it does something like this:

rosidl_generate_interfaces(${PROJECT_NAME}_msgs
  ...
)

ament_python_install_package(${PROJECT_NAME})

I see an error like:

CMake Error at /home/jacob/ros2-linux/share/ament_cmake_python/cmake/ament_python_install_package.cmake:108 (add_custom_target):
  add_custom_target cannot create target
  "ament_cmake_python_symlink_my_package" because another target with the
  same name already exists.  The existing target is a custom target created
  in source directory
  ...

@hidmic
Copy link
Contributor Author

hidmic commented Jun 7, 2021

I can't say I didn't see this one coming.

A user-defined ament_python_install_package(${PROJECT_NAME}) contends with rosidl_generate_interfaces() to install the ${PROJECT_NAME} Python package. That was not a problem before because there was no Python package (with metadata, discoverable). Python sources were just incrementally pushed to the install space.

Maybe there is a way for an ament_python_install_package(${PROJECT_NAME}) call to extend or augment previous ones, somehow. We would not be able to tell if that was on purpose or not though.
WDYT @ros2/team ?

@jacobperron
Copy link
Member

Maybe there is a way for an ament_python_install_package(${PROJECT_NAME}) call to extend or augment previous ones, somehow. We would not be able to tell if that was on purpose or not though.

If it's possible, we could make it an option to extend a previous call to ament_python_install_package. This way we would know if someone is doing this on purpose. Then the error message could also be improved to suggest using the "extend" option if it detects multiple calls to ament_python_install_package with the same target name.

@jtbandes
Copy link
Member

I'm running into this error in rosbridge_suite as well, when trying to build for Rolling: RobotWebTools/rosbridge_suite#581

Is there any recommendation for how to fix or work around this issue?

@jtbandes
Copy link
Member

Filed a new issue to track rather than discussing on an old merged PR: #141

jtbandes added a commit to RobotWebTools/rosbridge_suite that referenced this pull request Oct 15, 2021
…; enable Rolling in CI (#665)

**Public API Changes**
The msg and srv interfaces under `rosapi` are moving to a new package `rosapi_msgs`. The ones from `rosbridge_library` were used only for testing and are moving to a new package `rosbridge_test_msgs`.


**Description**
Fixes #581. Closes #602.
Due to a [change](ros2/rosidl_python#131) in rosidl_python, the generated python packages containing msg classes were conflicting with the python package these libraries export (ros2/rosidl_python#141). The solution recommended in that thread was to split these definitions into separate packages.
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.

None yet

4 participants