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

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

Closed
stonier opened this issue Aug 17, 2019 · 20 comments
Closed

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

stonier opened this issue Aug 17, 2019 · 20 comments
Labels
question Further information is requested

Comments

@stonier
Copy link

stonier commented Aug 17, 2019

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:

  • Workspace installs / packaged deb installs shouldn't have different requirements
  • Processes for packaging debs know how to bring in this file automagically without it being explicitly dictated in setup.py (perhaps it should be explicit for both workspace and deb installs?)

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

@dirk-thomas dirk-thomas added more-information-needed Further information is required question Further information is requested labels Aug 21, 2019
@dirk-thomas
Copy link
Contributor

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
Copy link
Author

stonier commented Aug 25, 2019

Worded badly, sorry.

  • Yes, ros2cli installs the empty marker file and does package it with the deb.
  • Surprise is that python packages need to install an empty marker file into ament_index's share folder to be ros2cli compatible

@dirk-thomas
Copy link
Contributor

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.

@dirk-thomas dirk-thomas removed the more-information-needed Further information is required label Aug 25, 2019
@stonier
Copy link
Author

stonier commented Aug 25, 2019

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
Copy link
Contributor

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
Copy link
Contributor

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

@stonier
Copy link
Author

stonier commented Aug 25, 2019

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.

  • A working package developed with the reference build tools (colcon) did not guarantee that the debian package built via the build farm will work
  • Resolving took 2-3 hours - diving into ros2cli code to discover the ament index, confirming and understanding why c++ packages don't have a similar issue and finally identifying the code snippet required to install the markers

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:

  • 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
  • colcon-ros does not provide any magic assistance, so problems are caught before the build farm
  • build farm provides magic assistance in the same way (explicit is better)
  • 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)
  • could use a tutorial page on how to create python-ros packages to get people started (i.e. code + explanations)

@dirk-thomas
Copy link
Contributor

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)

👍 💯 (as many other things not documented well enough at the moment). Any contribution towards more documentation would be greatly appreciated.

@stonier
Copy link
Author

stonier commented Aug 27, 2019

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
Copy link
Author

stonier commented Aug 27, 2019

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
Copy link
Contributor

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
Copy link
Author

stonier commented Aug 27, 2019

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
Copy link
Contributor

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
Copy link
Author

stonier commented Aug 27, 2019

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
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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

@rahatchd
Copy link

rahatchd commented Sep 3, 2019

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
Copy link

kyrofa commented Sep 16, 2019

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
Copy link

artivis commented Sep 16, 2019

Hitting this issue too...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

5 participants