CMAKE_PREFIX_PATH broken if package does not export build type ament_cmake

[StefanFabian]

I've encountered some build issues where my package used headers from the underlay instead of from my source workspace, and traced it back to colcon-ros.
The issue is happening in this function where the AMENT_PREFIX_PATH is added to the CMAKE_PREFIX_PATH.

colcon-ros/colcon_ros/task/__init__.py

Lines 8 to 13 in 65ac2ec

	def add_app_to_cpp(env):
	"""
	Add AMENT_PREFIX_PATH to CMAKE_PREFIX_PATH.
	Each ament prefix path is inserted before the first catkin prefix path or
	at the end if none of the CMake prefix paths has a '.catkin' marker file.

The problem is that if you forget to include <export><build_type>ament_cmake</build_type></export> in the package XML of a package.
The build will add a .catkin marker file to the install location.
This causes the linked function to insert the AMENT_PREFIX_PATH with the underlying binary workspace in front of this source package, which may be very early in the list of source packages.
Subsequently, when building, it will now prioritize the underlay over the source workspace, which I don't think is intended behavior.

I don't really understand what the rationale was behind adding AMENT_PREFIX_PATH before the first .catkin marked directory, or why a .catkin marker file is placed when the build_type is not properly exported.
So, I can't propose a solution, but the combination creates some very hard-to-find weird behavior.

[peci1]

I also got into this problem which is actually quite hard to debug.

For me it was caused by https://github.com/manifoldsdk/odin_ros_driver which has some pretty unorthodox (I'd say vomittable) build approach (they have ROS 1 and ROS 2 package.xmls in the repo and cmake decides which one should be symlinked to package.xml based on a whitelist of ROS_DISTRO, which of course doesn't have Kilted in it; probably never heard of package format 3). To build their driver on Kilted, you have to set a CMake variable in advance, which I keep forgetting, so the package sometimes tries to be built as ROS 1. That might probably be how it happened that something has created the .catkin marker in install folder. Fortunately, removing it manually helps as it doesn't get recreated.

A question to the developers of this package: do we still need to care about catkin packages in colcon these days? Couldn't we just drop this silly mechanism?