search

ros-infrastructure / catkin_pkg

Standalone Python library for the catkin build system.
https://github.com/ros/catkin
Other
46 stars 89 forks source link

Porting catkin_* scripts to ament #330

Closed ijnek closed 2 years ago

ijnek commented 2 years ago

Scripts like catkin_generate_changelog and catkin_prepare_release have either just worked, or have been adapted to work for ament packages in ROS2.

While porting across bloom documentation to ros2 docs (https://github.com/ros2/ros2_documentation/pull/2375 - WIP), I thought it's quite confusing that catkin_* scripts were being used to prepare ament packages in ROS2. If you're new and learn ROS2 straight up, the word catkin doesn't mean anything.

Are there are any plans on renaming these scripts to ament_* on https://github.com/ament , or some alternative?

nuclearsandwich commented 2 years ago

I don't think that the tool names are likely to change since I don't think the catkin_pkg module name is likely to change. I think it's fine to celebrate ROS 2's "lineage" and carry forward the tools which continue to work for the community.

I thought it's quite confusing that catkin_* scripts were being used to prepare ament packages in ROS2. If you're new and learn ROS2 straight up, the word catkin doesn't mean anything.

It's really difficult in some ways to separate the package manifest format used in ROS from the catkin build system since they were introduced together (well before my time), with catkin replacing the previous rosbuild build system and package.xml replacing the previous "stacks and packages" stack.xml and manifest.xml files. However, the package.xml format used by ROS is not inherently specific to catkin or ament even if both build systems are designed to make use of them. Maybe the correct thing to do is abandon any attempt at a distinction and just refer to it as the "catkin package.xml format", and ideally these tools would work with any packages that use the package.xml format regardless of build type. I wouldn't want to be unilateral here but if you're looking for a glossary entry for "catkin" that is relevant to ROS 2 users that's probably close enough to correct to be getting on with.

catkin_generate_changelog and catkin_prepare_release have either just worked, or have been adapted to work for ament packages in ROS2.

Since both systems use the same package.xml format and release tag conventions, both tools work fine for cmake-based build types (catkin, cmake, ament_cmake). When updating ament_python packages catkin_prepare_release does not update the version present in the setup.py file. I feel a little embarrassed that this has been a "known issue" within our team for a while and we've never seen fit to actually create an Issue for it. I'll do that next.

In the earliest days of ROS 2 (again before my time), ament was being built completely outside of catkin_pkg in order to avoid adversely affecting the library being used in ROS (1) while developing ROS 2. I think this also gave the ROS 2 team at the time the freedom to potentially be heretical with respect to some aspects of ROS. The ament_package Python module, which now just houses the base templates for workspace scripts used in ament workspaces is all that remains of this "reimplementation" since everything ROS 2 specific was included in the Package Manifest Format 3 specification (REP-0149).

ijnek commented 2 years ago

@nuclearsandwich

Thank you for the background, its helping me understand why things are designed the way they are.

Maybe the correct thing to do is abandon any attempt at a distinction and just refer to it as the "catkin package.xml format"

Referring to the package.xml as the "catkin package.xml format" is confusing because having a package.xml seesm to currently be a requirement for all ros packages (catkin, ament, third-party, *_vendor).

May I suggest an alternative, to prefix the scripts that apply across all packages with pkg_* (or package_), like pkg_generate_changelog and pkg_prepare_release?

nuclearsandwich commented 2 years ago

Referring to the package.xml as the "catkin package.xml format" is confusing because having a package.xml seesm to currently be a requirement for all ros packages (catkin, ament, third-party, *_vendor).

I won't discount that this is a possible source for confusion. You are correct that having a package.xml is essentially necessary for any package in ROS but ROS is not necessary for any other ecosystem that wants to use the package.xml format. The same is true for catkin and ament themselves, as well as other aspects of ROS infrastructure. They are primary tools for use with ROS but they are not theoretically limited to ROS. (This is quite a digression but it is theoretically possible to build an entire Linux distribution using ROS infrastructure tools and it's been an April Fools' project idea of mine for a while) My reasoning for the suggested nomenclature is that the format originated with catkin, not that it somehow implies the use of catkin. But your points are valid so maybe it's even better to refer to it as the "REP-149 package.xml" in reference to the ROS enhancement proposal which defines the current version of the format. This grooves with the Python community which describes things like PEP518-compatible build systems. What do you think of that suggestion? My least favorite part of that is when we revise the package.xml format we typically create a new REP so we previously would have referred to it as a REP-140 package.xml and will someday refer to it with a new REP number as we move to version 4 and beyond.

May I suggest an alternative, to prefix the scripts that apply across all packages with pkg_* (or package_), like pkg_generate_changelog and pkg_prepare_release?

I'm not a fan of this change either. pkg as a prefix is already so overloaded: pkgsrc, pkg-config, and FreeBSD's pkg(1) are just a few examples that come to mind. As long as this library is called catkin_pkg, I don't anticipate a proposal to rename the included executables that I would approve. Where the existing names create some confusion about the breadth of their utility we'll have to use documentation and help text to communicate their full utility.

ijnek commented 2 years ago

@nuclearsandwich thanks for your response, I had enough time to think about this one. Although I do think there is confusion, I can't think of a solution apart from rebranding currently so-called "catkin packages" to something else. I belive the effort in changing the REPs outweigh the benefits (at least for now).

I'll be interested in any discussions that happen in the future, but such renamings are major changes that should be discussed thoroughly before committing.

I'm going to close this issue off, as the answer to the original issue is - no, we shouldn't rename all the scripts.

nuclearsandwich commented 2 years ago

@ijnek thank you for raising the question and having the discussion with us. It was, in my esteem, a question asked in good faith with the project's accessibility and approachability in mind and I encourage you to open similar issues where you feel the need.