ament_cmake_python: Update ament_python_install_package() to allow installing more files into an existing package.

[rolker]

It seems like ros2/rosidl_python#141, errors with generating interfaces and using ament_python_install_package(), is blocking the release of some packages. The one I happen to encounter is audio_common: ros-drivers/audio_common#227

Following the discussions, I found this suggestion as a better alternative to a proposed fix in rosidl_python: ros2/rosidl_python#187 (review)

"I think the best option would be to update ament_python_install_package() to allow installing more files into an existing package.

For example,"

rosidl_generate_interfaces(...)

ament_python_install_package(... EXTEND_EXISTING)

Can someone implement this?

[brta-jc]

This has been an issue since the initial release of Humble and there seems to be little will to fix it despite people putting in proposed MR's. Currently we still maintain a fork of rosidl_generator_py and as such cannot build our packages on the upstream ROS industrial CI.
I really hope it gets fixed, or at least the workarounds merged, but so far despite the noise no luck.

[clalancette]

I'm replicating the comment from ros2/rosidl_python#141 (comment) here:

So we discussed this bug a bit.

The most ament_cmake way to do this is to change up the way that ament_python_install_package works. For most things that are part of ament_cmake, the way that they work is that the call (to e.g. ament_export_libraries) doesn't do much work, and instead sets up a bunch of cmake variables, along with a hook. When ament_package is eventually called, it calls all of the hooks, and at that point the hook does the work based on the cmake variables. This is done this way so that ament_package has a full view of the environment, and avoids problems exactly like this.

For whatever reason, ament_python_install_package does not work like this at the moment. We should change ament_python_install_package to work in the more ament_cmake way, which will resolve this bug.

[clalancette]

Just to follow up and be clear; the core team will probably not have time to look into this at the moment. So contributions to fix things as stated in #514 (comment) are very much appreciated.

[fujitatomoya]

@clalancette can you tag this with help-wanted? (i do not have power for that...)

[knorth55]

@clalancette
First of all, thank you for your contribution to ROS community.
But I have several question on this issue.

1. The reason of closing Add EXTEND_EXISTING argument to ament_python_install_package #517

I made a hotfix on rosidl_python to solve this issue, which was closed.
I also think this PR is temporary, so it is understandable to be closed.
ros2/rosidl_python#187

Our contributor also made a fix, which was again closed.
I thought this PR looks good (better than mine).
Can you give us more detailed information about how we should change it?
#517

I also think that we can merge the PR and solve perfectly later.
I can understand your policy, but we are trying solving the issue for 1 and a half year...

3. The discussion

#514 (comment)

So we discussed this bug a bit.

I'm also curious about the discussion.
The discussion is open on github? I want to know how the discussion was and who involved in the discussion.

Again, I really thank your work.

cc. @rolker

[sillkjc]

Tagging along to say this is still preventing us from working effectively with upstream ROS build farm. We maintain a codebase of 100+ packages and splitting them into msgs and srvs is inappropriate for some, and far too much work overall.

[torydebra]

We occured in the same issue. We momentarly solved using another name for the ament_python_install_package, like ament_python_install_package(${PROJECT_NAME}_py), hence renaming the internal repo folder containing the python modules and the __init__.py, and renaming the name of the python modules when calling the import.

It seems to work (for now), how bad is this approach?

[JTShuai]

We occured in the same issue. We momentarly solved using another name for the ament_python_install_package, like ament_python_install_package(${PROJECT_NAME}_py), hence renaming the internal repo folder containing the python modules and the __init__.py, and renaming the name of the python modules when calling the import.

It seems to work (for now), how bad is this approach?

This sounds like a nice solution.
So, I should change the Python module folder name to, e.g., my_pkg_py? But what should I do for the __init__.pyfile and for my scripts/py_node.py file?

Can you please give me some details about it? Thx!

[torydebra]

We occured in the same issue. We momentarly solved using another name for the ament_python_install_package, like ament_python_install_package(${PROJECT_NAME}_py), hence renaming the internal repo folder containing the python modules and the __init__.py, and renaming the name of the python modules when calling the import.
It seems to work (for now), how bad is this approach?

This sounds like a nice solution. So, I should change the Python module folder name to, e.g., my_pkg_py? But what should I do for the __init__.pyfile and for my scripts/py_node.py file?

Can you please give me some details about it? Thx!

Yes, folder name as my_pkg_py, with inside a __init__.py which imports as from my_pkg_py.fileName import className. The nodes (executable) in scripts folder import modules as from my_pkg_py import fileName

You can see it here https://github.com/ADVRHumanoids/nicla_vision_ros2/