search

ros2 / libyaml_vendor

CMake wrapper downloading and building libyaml
Apache License 2.0
2 stars 17 forks source link

Remove a warning message. #51

Closed clalancette closed 2 years ago

clalancette commented 2 years ago

None of our other vendor packages do this, so there is no reason that this one should.

Signed-off-by: Chris Lalancette clalancette@openrobotics.org

clalancette commented 2 years ago

CI:

traversaro commented 2 years ago

None of our other vendor packages do this, so there is no reason that this one should.

I is totally ok for me removing the WARNING, but I would just highlight a tricky difference between this and all the other ROS2 "vendor" packages that I known of: this is the only one that requires a specific exact version down to the patch number, and if that is not found builds the vendored version. All the other vendor packages I known of either do not check the version or just check a minimum version. That means that once libyaml 0.2.6 is released, in a distro that has libyaml 0.2.6 ros2 would silently install its own libyaml 0.2.5, with all the possible incompatiblity one may imagine. In fact, the warning was explicitly asked for by @hidmic in https://github.com/ros2/libyaml_vendor/issues/40#issuecomment-917169042 .

clalancette commented 2 years ago

That means that once libyaml 0.2.6 is released, in a distro that has libyaml 0.2.6 ros2 would silently install its own libyaml 0.2.5, with all the possible incompatiblity one may imagine. In fact, the warning was explicitly asked for by @hidmic in #40 (comment) .

Thanks for the context @traversaro . In short, I don't think that this vendor package should be special; we have plenty of other ones that we only conditionally build, and we also depend on in rclcpp. So I think what we should do here is to merge this PR, and also do another follow-up where we use the system version as long as it meets some minimum (0.2.2 is probably what I'll shoot for, as that is what is in Ubuntu 20.04 and Ubuntu 22.04).