CRITICAL BUG: proper escaping for ROS_PACKAGE_NAME definition

[jbohren]

Without this, sometimes this ends up as (for a packange named foo_pkg):
-DROS_PACKAGE_NAME="foo_pkg"

Which breaks everything, since on the command line this needs to be the following so that the quotes end up in the variable:
-DROS_PACKAGE_NAME=\"foo_pkg\"

See this cmake thread here: http://www.cmake.org/pipermail/cmake/2007-June/014611.html

It looks like the old rosbuild method used to use single quotes:
ROS_PACKAGE_NAME='"foo"'

[dirk-thomas]

I could not reproduce any problem - it seems to work fine for me for several existing packages. You mentioned that it "sometimes" ends up breaking everything. Can you please provide a reproducible scenario where that happens?

[jbohren]

It was happening on multiple machines, and the build commands were definitely not escaping quotes for some packages. Does my patch not properly escape the quotes?

[dirk-thomas]

You patch looks absolutely fine. But before merging anything I usually try to reproduce the issue before, apply the patch and reproduce that it fixes the issue.

[jbohren]

What's the easiest way to send you a 5.3MB tarball of my workspace?

[dirk-thomas]

If the workspace contains only checkout repos without modifications than a .rosinstall file or and export with vcstool would be the easiest. You could also just paste a list of command sequence (like mkdir ws, git clone ..., catkin_make, fails with ...) or something similar.

[jbohren]

The workspace contains (currently) closed-source code, I'm trying to remove all the information now.

[jbohren]

Requires ros groovy:

- git: {local-name: control_toolbox, uri: '[email protected]:ros-controls/control_toolbox.git',
    version: 605efb4}
- git: {local-name: intuitive_research_kit, uri: '[email protected]:jbohren/rosconsole_broken_minimal_example.git',
    version: 02e0c43}
- git: {local-name: realtime_tools, uri: '[email protected]:ros-controls/realtime_tools.git',
    version: 19b5e34}
- git: {local-name: ros_control, uri: '[email protected]:ros-controls/ros_control.git',
    version: db0f479}
- git: {local-name: ros_controllers, uri: '[email protected]:ros-controls/ros_controllers.git',
    version: bde2c57}
- git: {local-name: gazebo_ros_pkgs, uri: '[email protected]:ros-simulation/gazebo_ros_pkgs.git',
    version: c508961}

What happens on my system is that when it gets to irk_gazebo_plugins, you see these errors (and more like them):

[100%] Building CXX object intuitive_research_kit/irk_gazebo_plugins/CMakeFiles/irk_gazebo_plugins.dir/src/robot_sim_mtm.o
In file included from /opt/ros/groovy/include/ros/node_handle.h:32:0,
                 from /opt/ros/groovy/include/ros/ros.h:45,
                 from /home/jbohren/ws/broken_rosconsole/src/intuitive_research_kit/irk_gazebo_plugins/src/robot_sim_mtm.cpp:2:
/opt/ros/groovy/include/ros/publisher.h: In member function ‘void ros::Publisher::publish(const boost::shared_ptr<T>&) const’:
/opt/ros/groovy/include/ros/publisher.h:69:11: error: expected ‘)’ before ‘irk_gazebo_plugins’
/opt/ros/groovy/include/ros/publisher.h:69:11: error: expected ‘)’ before ‘irk_gazebo_plugins’
/opt/ros/groovy/include/ros/publisher.h:69:11: error: expected ‘)’ before ‘irk_gazebo_plugins’
/opt/ros/groovy/include/ros/publisher.h:75:11: error: expected ‘)’ before ‘irk_gazebo_plugins’
/opt/ros/groovy/include/ros/publisher.h:75:11: error: expected ‘)’ before ‘irk_gazebo_plugins’
/opt/ros/groovy/include/ros/publisher.h:75:11: error: expected ‘)’ before ‘irk_gazebo_plugins’
/opt/ros/groovy/include/ros/publisher.h:79:7: error: expected ‘)’ before ‘irk_gazebo_plugins’
/opt/ros/groovy/include/ros/publisher.h:79:7: error: expected ‘)’ before ‘irk_gazebo_plugins’
/opt/ros/groovy/include/ros/publisher.h:79:7: error: expected ‘)’ before ‘irk_gazebo_plugins’
In file included from /opt/ros/groovy/include/ros/node_handle.h:32:0,
                 from /opt/ros/groovy/include/ros/ros.h:45,
                 from /home/jbohren/ws/broken_rosconsole/src/intuitive_research_kit/irk_gazebo_plugins/src/robot_sim_mtm.cpp:2:
/opt/ros/groovy/include/ros/publisher.h: In member function ‘void ros::Publisher::publish(const M&) const’:
/opt/ros/groovy/include/ros/publisher.h:102:11: error: expected ‘)’ before ‘irk_gazebo_plugins’
/opt/ros/groovy/include/ros/publisher.h:102:11: error: expected ‘)’ before ‘irk_gazebo_plugins’
/opt/ros/groovy/include/ros/publisher.h:102:11: error: expected ‘)’ before ‘irk_gazebo_plugins’
/opt/ros/groovy/include/ros/publisher.h:108:11: error: expected ‘)’ before ‘irk_gazebo_plugins’
/opt/ros/groovy/include/ros/publisher.h:108:11: error: expected ‘)’ before ‘irk_gazebo_plugins’
/opt/ros/groovy/include/ros/publisher.h:108:11: error: expected ‘)’ before ‘irk_gazebo_plugins’
/opt/ros/groovy/include/ros/publisher.h:112:7: error: expected ‘)’ before ‘irk_gazebo_plugins’
/opt/ros/groovy/include/ros/publisher.h:112:7: error: expected ‘)’ before ‘irk_gazebo_plugins’
/opt/ros/groovy/include/ros/publisher.h:112:7: error: expected ‘)’ before ‘irk_gazebo_plugins’

If I run with VERBOSE=1, I see this as the command:

[100%] Building CXX object intuitive_research_kit/irk_gazebo_plugins/CMakeFiles/irk_gazebo_plugins.dir/src/robot_sim_mtm.o
cd /home/jbohren/ws/broken_rosconsole/build/intuitive_research_kit/irk_gazebo_plugins && /usr/bin/c++   -Dirk_gazebo_plugins_EXPORTS -DROS_BUILD_SHARED_LIBS=1 -fPIC -I/home/jbohren/ws/broken_rosconsole/build/devel/include -I/home/jbohren/ws/broken_rosconsole/src/gazebo_ros_pkgs/gazebo_ros_control/include -I/home/jbohren/ws/broken_rosconsole/src/gazebo_ros_pkgs/gazebo_plugins/include -I/home/jbohren/ws/broken_rosconsole/src/ros_control/controller_manager/include -I/home/jbohren/ws/broken_rosconsole/src/ros_control/controller_interface/include -I/home/jbohren/ws/broken_rosconsole/src/ros_control/hardware_interface/include -I/home/jbohren/ws/broken_rosconsole/src/realtime_tools/include -I/opt/ros/groovy/include -I/opt/ros/groovy/include/pcl-1.6 -I/usr/include/eigen3 -I/usr/include/vtk-5.8 -I/usr/include/qhull -I/home/jbohren/ws/groovy/isolated_underlay/devel_isolated/gazebo/include/gazebo-1.8 -I/home/jbohren/ws/groovy/isolated_underlay/devel_isolated/gazebo/include/gazebo-1.8/gazebo    -DROS_PACKAGE_NAME="irk_gazebo_plugins" -o CMakeFiles/irk_gazebo_plugins.dir/src/robot_sim_mtm.o -c /home/jbohren/ws/broken_rosconsole/src/intuitive_research_kit/irk_gazebo_plugins/src/robot_sim_mtm.cpp

As you see above, the build command incorrectly passes the command-line argument -DROS_PACKAGE_NAME="irk_gazebo_plugins" without escaped quotes.

[dirk-thomas]

I guess you workspace depends on a non-ROS gazebo installation. Which of version of the gazebo project have you installed?

[jbohren]

Gazebo 1.8.61~precise from http://gazebosim.org/wiki/1.6/install#Ubuntu_Debians

[jbohren]

So here's something strange, when building with my patch, I see this cmake warning:

CMake Warning (dev) at /opt/ros/groovy/share/rosconsole/cmake/rosconsole-extras.cmake:4 (add_definitions):
  Policy CMP0005 is not set: Preprocessor definition values are now escaped
  automatically.  Run "cmake --help-policy CMP0005" for policy details.  Use
  the cmake_policy command to set the policy and suppress this warning.
Call Stack (most recent call first):
  /opt/ros/groovy/share/rosconsole/cmake/rosconsoleConfig.cmake:173 (include)
  /opt/ros/groovy/share/roscpp/cmake/roscppConfig.cmake:151 (find_package)
  /opt/ros/groovy/share/catkin/cmake/catkinConfig.cmake:72 (find_package)
  intuitive_research_kit/irk_gazebo_plugins/CMakeLists.txt:4 (find_package)
This warning is for project developers.  Use -Wno-dev to suppress it.

I didn't see this before because every package is also showing the warning referenced here: ros/catkin#452

[jbohren]

[ yavin: ~/ws/broken_rosconsole/build ] cmake --help-policy CMP0005
cmake version 2.8.7
  CMP0005
       Preprocessor definition values are now escaped automatically.

       This policy determines whether or not CMake should generate escaped
       preprocessor definition values added via add_definitions.  CMake
       versions 2.4 and below assumed that only trivial values would be given
       for macros in add_definitions calls.  It did not attempt to escape
       non-trivial values such as string literals in generated build rules.
       CMake versions 2.6 and above support escaping of most values, but
       cannot assume the user has not added escapes already in an attempt to
       work around limitations in earlier versions.

       The OLD behavior for this policy is to place definition values given
       to add_definitions directly in the generated build rules without
       attempting to escape anything.  The NEW behavior for this policy is to
       generate correct escapes for all native build tools automatically.
       See documentation of the COMPILE_DEFINITIONS target property for
       limitations of the escaping implementation.

       This policy was introduced in CMake version 2.6.0.  CMake version
       2.8.7 warns when the policy is not set and uses OLD behavior.  Use the
       cmake_policy command to set it to OLD or NEW explicitly.

[dirk-thomas]

This looks like a duplicate of ros/catkin#452 than. Your package might specify a too old required version.

[jbohren]

Yeah, that's what it looks like. So CMake's minimum required version isn't just a minimum required version, but changes the behavior of CMake.

[dirk-thomas]

Indeed - another non-intuitive detail of CMake.

[jbohren]

Indeed - another non-intuitive detail of CMake.

Seriously.