Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

sensor_msgs dependency on boost #81

Closed
meyerj opened this issue Nov 19, 2015 · 3 comments
Closed

sensor_msgs dependency on boost #81

meyerj opened this issue Nov 19, 2015 · 3 comments

Comments

@meyerj
Copy link

meyerj commented Nov 19, 2015

The sensor_msgs package depends on boost because boost::str() and boost::format() are called to format an error message in point_cloud_conversion.h:120, but...

  • boost is not explicitly declared as a dependency (of type <run_depend> or <build_export_depend>?) in package.xml, only indirectly through message_runtime.

  • point_cloud_conversion.h is lacking an include directive for <boost/format.hpp> and includes this header only indirectly via ros/time.h:

    . /opt/ros/indigo/include/sensor_msgs/point_cloud_conversion.h
    .. /opt/ros/indigo/include/sensor_msgs/PointCloud.h
    ... /opt/ros/indigo/include/ros/serialization.h
    .... /opt/ros/indigo/include/ros/time.h
    ..... /usr/include/boost/math/special_functions/round.hpp
    ...... /usr/include/boost/math/policies/error_handling.hpp
    ....... /usr/include/boost/format.hpp
    
  • convertPointCloud2ToPointCloud() should return false after printing the message to stderr in line 120.

This is not directly related to those three issues, but we spent some time finding a problem while building sensor_msgs and dependents on a RedHat system with cmake 2.8.9, because sensor_msgs picked up and exported wrong Boost library paths in its generated cmake config if it was compiled without the -DBoost_NO_BOOST_CMAKE=ON argument (see http://stackoverflow.com/questions/9948375/cmake-find-package-succeeds-but-returns-wrong-path).

@jack-oquin
Copy link
Member

That looks like bug, alright.

Is that the only reference? That package should not even be writing to std::cerr, so the whole line should probably be replaced with a ROS_WARN_THROTTLE() command.

@meyerj
Copy link
Author

meyerj commented Nov 20, 2015

I have not really looked into what the convertPointCloud2ToPointCloud() function is doing nor am I using it. We only discovered these issues when we tried to fix build issues in RedHat.

But at least this line is the only result when searching for boost in the include folder:

$ grep -rn boost sensor_msgs                
sensor_msgs/include/sensor_msgs/point_cloud_conversion.h:120:      std::cerr << boost::str(boost::format("sensor_msgs::PointCloud accepts only float32 values, but field %d (%s) has field type %d!")%(int)d% input.fields[d].name%input.fields[d].datatype) << std::endl;
sensor_msgs/CHANGELOG.rst:179:* fix boost-finding stuff
sensor_msgs/CMakeLists.txt:5:# We want boost/format.hpp, which isn't in its own component.

tfoote added a commit that referenced this issue Dec 21, 2015
@tfoote
Copy link
Member

tfoote commented Dec 21, 2015

It does look like a heavy dependency for one line of display. Technically it's already a dependency since boost is part of the core but clearing it out is cleaner.

@jack-oquin it would be great to use the rosconsole macros but we don't have them as a dependency and adding them would almost double the number of packages required.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants