search

ament / ament_index

Apache License 2.0
13 stars 27 forks source link

ament_index/resource_index/packages files not packaged into debs #33

Closed stonier closed 5 years ago

stonier commented 5 years ago

This is very likely the wrong place for this ticket but I'm not rightly sure where it should be reported.

Problem:

Local workspace builds correctly install files into ament_index/resource_index/packages so that tools from ros2cli can discover packages correctly. However the same packages when built as debs do not install that file and subsequently require it to be forced via setup.py in the following manner (discovered on inspection of ros2cli/setup.py itself):

    data_files=[
        ('share/ament_index/resource_index/packages', [
            'resources/<name-of-my-package>']),
    ],

Expected:

A little surprised that ament index requires other packages to be aware of it.

dirk-thomas commented 5 years ago

The ros2cli package installes the empty marker file using data_files: https://github.com/ros2/ros2cli/blob/e5b0ceb73b5380a745a3dccba17c078ce42aa2c1/ros2cli/setup.py#L11-L14

However the same packages when built as debs do not install that file

The Debian does contain the file which can be checked with dpkg -L ros-dashing-ros2cli:

/opt/ros/dashing/share/ament_index/resource_index/packages/ros2cli

stonier commented 5 years ago

Worded badly, sorry.

dirk-thomas commented 5 years ago

Yes, ros2cli installs the empty marker file and does package it with the deb.

Please change the ticket title then.

Surprise is that python packages need to install an empty marker file into ament_index's share folder to be ros2cli compatible

In order to detect that a package is installed it needs to install that marker file. Similar to installing the packages manifest in ROS 1 - which needed to be crawled for and then parsed - just much faster at usage time. So if you need your package to be identified as a ROS package, e.g. by ros2 run <pkgname> ... this marker file is required.

stonier commented 5 years ago

Please change the ticket title then.

The other point I mentioned in the original description was that those markers do get installed in the development workspace (how?) without the aforementioned code to install them in setup.py. This does not survive it's way to the debian package, which then does need that code snippet in setup.py. The title is reflective of that.

dirk-thomas commented 5 years ago

In ROS 2 there is no devel space.

In a local build colcon will generate the marker file automatically in the install directory if not present: https://github.com/colcon/colcon-ros/blob/d938fcbe386f7efc9f0731b6ff20c885d72b476a/colcon_ros/task/ament_python/build.py#L38

dirk-thomas commented 5 years ago

The Debian package is not built using colcon. So it's custom logic doesn't apply.

stonier commented 5 years ago

Aye, the tools are all doing one thing or another with some supporting logic. It is possible to get it all to work (which I have now). Nonetheless, it's the integration of these things which creates an awkward experience with surprises.

Only once I'd put in what felt like hacks, I discovered this very nicely articulated doc to appreciate the hacks were part of a reasonably justified layer in the onion beyond package.xml.

The intent of this ticket is not to report a bug, but to ask, can something simple be done to make the entry into ros2-python packages a little less of a journey to shambhala? Possible ideas:

dirk-thomas commented 5 years ago

colcon-ros provides very large warnings (even errors) if a package.xml exists, but a package marker is not installed, also points to the official ament index documentation

What if a package really doesn't want to add itself to the index? Therefore I don't think this is an option.

colcon-ros does not provide any magic assistance, so problems are caught before the build farm

That would be possible. Currently it is intentionally providing this support to make it easier to users (which might never go towards a release of their packages). Not sure if breaking this use case is a good idea.

python packages have helpers to install package.xml, package markers like the cmake implementation does (more work involved, and I actually prefer explicit these days if human errors aren't going to cause an overdose of bugs because of the explicitness, even if it's a line or two of extra code)

It is pretty difficult if not impossible to provide a helper like that in the setup.py phase - simply because your dependencies might not be available / installed yet. That is why all pure Python packages use vanilla Python setuptools API only - no ament_python helper similar to ament_cmake.

could use a tutorial page on how to create python-ros packages to get people started (i.e. code + explanations)

:+1: :100: (as many other things not documented well enough at the moment). Any contribution towards more documentation would be greatly appreciated.

stonier commented 5 years ago

colcon-ros does not provide any magic assistance, so problems are caught before the build farm

That would be possible. Currently it is intentionally providing this support to make it easier to users (which might never go towards a release of their packages). Not sure if breaking this use case is a good idea.

If part of a working package is that marker, then I feel (rather strongly) that the build tool shouldn't be installing that marker. If someone tries to use my sources (regardless of whether it's the build farm or someone with an in-house build tool of their own) ... it's a broken package.

I create enough bugs of my own, don't need help from my build tool to unwittingly create more :)

stonier commented 5 years ago

It is pretty difficult if not impossible to provide a helper like that in the setup.py phase - simply because your dependencies might not be available / installed yet. That is why all pure Python packages use vanilla Python setuptools API only - no ament_python helper similar to ament_cmake.

I'm ok with explicitly having setup.py be as explicit as possible (less magic the better). The only concern might be for required destinations (e.g. share/ament_index/resources/packages) that might move, e.g.

    data_files=[
        (ament_index.packages_install_dir(), ['resources/my_package_marker']),
        ('share/' + package_name, ['package.xml']),
    ],
dirk-thomas commented 5 years ago

data_files=[(ament_index.packages_install_dir(), ['resources/my_package_marker']), ...`

You can't use ament_index in your setup.py file since you can't ensure that it is installed and available on your PYTHONPATH when the setup file is invoked.

stonier commented 5 years ago

You can't use ament_index in your setup.py file since you can't ensure that it is installed and available on your PYTHONPATH when the setup file is invoked.

What's the limitation? I've used this trick in the past - if I ensure it's in my build_depends list in package.xml, then it's available when the setup file is invoked (note: ament_index was just intended to be examplar...could be some other name).

dirk-thomas commented 5 years ago

What's the limitation?

You need to process the content of the setup.py file to determine the dependencies of the package. Therefore you can't expect that any of your declared dependencies are already available when the file is invoked.

stonier commented 5 years ago

Ah. I tried setting the xxx_requires variables for a ros-python package a while ago (mostly to see if anything would happen) and ran into some trouble so I've since been relying on package.xml to take care of that. That only works I think because ament & bloom pay no heed to these variables, so I've never re-engaged the experiment and made sure I don't have a redundant bit-rotting list of dependencies in setup.py.

Is/will there be a practical reason for the existence of populated xxx_requires variables?

dirk-thomas commented 5 years ago

Is/will there be a practical reason for the existence of populated xxx_requires variables?

Not if you have a package.xml file.

dirk-thomas commented 5 years ago

If part of a working package is that marker, then I feel (rather strongly) that the build tool shouldn't be installing that marker. If someone tries to use my sources (regardless of whether it's the build farm or someone with an in-house build tool of their own) ... it's a broken package.

We just discussed this idea and will go ahead and remove this helper from colcon-ros: see colcon/colcon-ros#74.

dirk-thomas commented 5 years ago

Assuming the referenced PR addresses your issue I will go ahead and close this ticket.

rahatchd commented 5 years ago

Since this is a breaking change, I think this should have been part of a major release and not a minor one.

I spent my entire work day chasing this down... not cool

kyrofa commented 5 years ago

I spent my entire work day chasing this down... not cool

Yep. Broke all sorts of CI around here and we had a flag day or two. Come on folks.

artivis commented 5 years ago

Hitting this issue too...