search

norlab-ulaval / libnabo

A fast K Nearest Neighbor library for low-dimensional spaces
http://norlab-ulaval.github.io/libnabo/
BSD 3-Clause "New" or "Revised" License
431 stars 142 forks source link

catkin not required for pure cmake packages #116

Closed Timple closed 2 years ago

Timple commented 2 years ago

This prevens rosdep resolving in ROS2

ethzasl-jenkins commented 2 years ago

Can one of the admins verify this patch?

pomerlef commented 2 years ago

ok to test

Timple commented 2 years ago

If it speeds-up the review (or reduces risk), I can make the flag:

<run_depend condition="$ROS_VERSION == 1">catkin</run_depend>
pomerlef commented 2 years ago

yes, could you add that line (in libpointmatcher too). I'll try to process the PRs next week.

patriksc commented 2 years ago

Hi! The change in the package.xml from this merge is causing issues when building our library COVINS: https://github.com/VIS4ROB-lab/covins

As soon as we call catkin build <xzy> (e.g. catkin build eigen_catkin), catkin throws Error(s): The "run_depend" tag must not have the following attributes: condition, caused by libnabo/package.xml.

We found a workaround for now and removed the libnabo dependency, but maybe you want to consider whether this change is technically really necessary, since it seem like there might be some compatibility issues (we experienced this on 3 different machines with both Ubuntu 18 and Ubuntu 20).

Timple commented 2 years ago

Ow dear, that is a regression from my side. I forgot to add the new package format:

<package format="3">

Which supports the condition flag. PR #119

Strange though, I added this condition exactly for the reason not to break backward compatibility. Apologies for that. I would expect it to break CI though.

patriksc commented 2 years ago

Indeed, would as well expect CI catches this... Anyway, no problem, thanks for the quick reply and for taking care of this.