ignore_once for launch files?

[gavanderhoorn]

A quick test made it seem that's not supported, but I didn't really try very hard:

  <!-- catkin_lint: ignore_once launch_depend -->
  <node name="rviz" pkg="rviz" type="rviz" args="..." />

This is a package where the .launch file is basically "extra", and adding a strict dependency on rviz would complicate installations on headless systems.

[gavanderhoorn]

I'm guessing something around here needs to change, but I'm not yet sure what:

catkin_lint/src/catkin_lint/checks/manifest.py

Lines 97 to 109 in 6a3c610

	if node.tag is not ET.Comment:
	relevant_deps = info.exec_dep
	dep_type = exec_dep_type
	if node.tag == "test" or is_test_launch_file:
	relevant_deps |= info.test_dep
	dep_type = "test"
	pkg = node.get("pkg")
	if pkg is not None and pkg != info.manifest.name and info.env.get_package_type(pkg) == PackageType.CATKIN and pkg not in relevant_deps and pkg not in essential_packages:
	info.report(WARNING, "LAUNCH_DEPEND", type=dep_type, pkg=pkg, file_location=(src_filename, node.sourceline or 0))
	for mo in re.finditer(r"\$\(find\s+([^<>)]+)\)", "<>".join(node.values() + [node.text or "", node.tail or ""])):
	pkg = mo.group(1)
	if pkg is not None and pkg != info.manifest.name and info.env.get_package_type(pkg) == PackageType.CATKIN and pkg not in relevant_deps and pkg not in essential_packages:
	info.report(WARNING, "LAUNCH_DEPEND", type=dep_type, pkg=pkg, file_location=(src_filename, node.sourceline or 0))

[Timple]

We typically put these launch files in the test folder. And add a test_depend on rviz.
But that still feels a bit like a workaround since it doesn't really test anything. So this could be nice.

[gavanderhoorn]

We typically put these launch files in the test folder. And add a test_depend on rviz.

yeah, that's not really what we're going for.

it'd be nice if the ROS manifest format supported something like optional dependencies, which would be ideal for this.

Another option would be using a different package to host such optional .launch files, but that's not doable in this case.

[roehling]

Would it be feasible for you to add example to the file names or put them in a folder where the name contains example? Because there is an exception for binaries with test or example in their names that catkin_lint will not complain if they are not installed, and it would be nicely consistent to have the same convention for launch files, too.

[gavanderhoorn]

They're not actually either tests or examples. They're just regular .launch files, which happen to have dependencies we can't really hard-code into the manifest (the headless example in the OP is a good one actually).

It's OK if you feel this should not support exceptions in catkin_lint. We'll just have to live with it then.

[roehling]

Well, as it turns out, that was easier to implement than I thought initially.

[gavanderhoorn]

Awesome, thanks a lot 👍

[gavanderhoorn]

And I almost feel bad for asking, but will you be cutting a new release as well with this included?

---

Edit: just noticed that catkin_lint does not include ignored issues in .launch files in the summary it prints after it completes a scan:

catkin_lint: checked 8 packages and found 0 problems
catkin_lint: 4 messages have been ignored. Use --show-ignored to see them

There should be 5, 4 in the CMakeLists.txt and one in the .launch file.

[roehling]

I was going to wait for another day or two, as experience tells me issues come in clusters. Then again, there are plenty of natural numbers available, I'm not going to run out for patch releases. 😉