fkie/catkin_lint

[Suggestion] Launch exec depends and missing install directories

StefanFabian opened this issue · 7 comments

I'm currently working on some checks for our workspace and this package already provides a lot of what I need (thank you btw) but there's some stuff I'm missing.

  1. It would be cool if the linter could detect missing exec_depends from launch files.
    E.g., if a launch file uses a node with pkg="PACKAGE" or $(find PACKAGE), the package should exec_depend on PACKAGE.
  2. If a package contains directories other than include and src they should usually be installed.
    Maybe all directories is too broad but common directories like config, launch etc. should produce a warning. If you want to go full-on elaborate, the linter could also check for files that are referred to from other catkin packages and make sure they are installed.

re 1. I agree that would be a useful feature. I'll give it some thought and try to come up with a nice implementation for it.

re 2. All directories would definitely be too broad, and I've seen enough packages where the install layout differs from the source layout (i.e. files get installed, but not in the same directory structure as in the package). Usually, I try to check consistency, meaning if some file gets installed that depends on another file, I emit a warning if the second file is not installed, too.

re 1. I agree that would be a useful feature. I'll give it some thought and try to come up with a nice implementation for it.

Could we then also get checks for packages mentioned in #includes but not in package.xml?

Could we then also get checks for packages mentioned in #includes but not in package.xml?

That's difficult to get robust. All I could do is assume the include/PACKAGENAME/header.h file system layout and warn if something looks like it's missing a dependency. It might be useful, but I'm not sure.

I was not completely serious when I wrote that "feature request".

It's of course up to you, but the request seems like feature-creep to me. Checking .launch files does not seem like something a tool called catkin_lint should do.

And it would probably also not be easy to get it robust; similar to #includes (where we can have all sorts of conditional logic and other macros), things like substitution args an other roslaunch functionality may make it difficult to successfully parse the files (although you could perhaps lean on the roslaunch Python libraries for some of it).

Could we then also get checks for packages mentioned in #includes but not in package.xml?

That's difficult to get robust. All I could do is assume the include/PACKAGENAME/header.h file system layout and warn if something looks like it's missing a dependency. It might be useful, but I'm not sure.

No, that would not work. It's actually notoriously difficult to figure out which files get #included and from where in C/C++. There are a few tools that do this ("include what you use" and others), and some actually use on entire compiler toolchains to get at the required information.

As I wrote: it was not a serious request.

Well, the launch files have a well-defined and easy to parse format, and I just happen to return from a remote site demo where I ended up having exactly this problem (unable to launch nodes because of missing exec_depends), so this feature request struck a nerve ;-)

Looking for pkg attributes and $(find) strings seems to be doable, at least.

It's of course up to you, but the request seems like feature-creep to me. Checking .launch files does not seem like something a tool called catkin_lint should do.

Well, since "Catkin is a collection of CMake macros and associated code used to build packages used in ROS.", I disagree that linting launch files which are an integral part of many ROS packages would be something that does not fit a tool called catkin_lint.
Especially, since a catkin package with launch files and missing exec_depends for the packages used in the launch files is faulty.

Nevertheless, it's just a suggestion and i will write a quick and dirty python script for the meantime anyway.

Well, since "Catkin is a collection of CMake macros and associated code used to build packages used in ROS.", I disagree that linting launch files which are an integral part of many ROS packages would be something that does not fit a tool called catkin_lint.
Especially, since a catkin package with launch files and missing exec_depends for the packages used in the launch files is faulty.

We'll agree to disagree then.

But it doesn't matter: I'm not a maintainer here. Just a user.

Only @roehling decides.