Change debug / optimized for generator expressions

[luca-della-vedova]

Using the optimized / debug directives in CMake is not allowed in modern versions, specifically when building a ROS2 package with ignition the following error occurs:

CMake Error at /usr/share/dart/cmake/DARTFindurdfdom.cmake:13 (set_target_properties):
  Property INTERFACE_LINK_LIBRARIES may not contain link-type keyword
  "optimized".  The INTERFACE_LINK_LIBRARIES property may contain
  configuration-sensitive generator-expressions which may be used to specify
  per-configuration rules.
Call Stack (most recent call first):
  /usr/share/dart/cmake/dart_utils-urdfComponent.cmake:6 (include)
  /usr/share/dart/cmake/DARTConfig.cmake:63 (include)
  /usr/share/dart/cmake/DARTConfig.cmake:123 (dart_traverse_components)
  /usr/share/dart/cmake/DARTConfig.cmake:171 (dart_package_init)
  /home/luca/ws_citadel/install/lib/cmake/ignition-physics2-dartsim/ignition-physics2-dartsim-config.cmake:106 (find_package)
  /usr/share/cmake-3.10/Modules/CMakeFindDependencyMacro.cmake:48 (find_package)
  /home/luca/ws_citadel/install/lib/cmake/ignition-physics2/ignition-physics2-config.cmake:194 (find_dependency)
  /home/luca/ws_citadel/install/lib/cmake/ignition-gazebo3/ignition-gazebo3-config.cmake:100 (find_package)
  /home/luca/ws_citadel/install/share/cmake/ignition-cmake2/cmake2/IgnUtils.cmake:188 (find_package)
  CMakeLists.txt:24 (ign_find_package)

The PR removed the debug / optimized directives in favor of generator expressions, as suggested in the CMake error.

[dirk-thomas]

Using the optimized / debug directives in CMake is not allowed in modern versions

Can you provide a link to CMake documentation for that change?

[luca-della-vedova]

So this is the source of the error in the CMake source code:
https://github.com/Kitware/CMake/blob/master/Source/cmTarget.cxx#L1547
which links to the following PR:
Kitware/CMake@d0a76ea
and finally to the policy:
https://cmake.org/cmake/help/v3.0/policy/CMP0022.html

[Karsten1987]

I triggered a CI job for this yesterday. I'll report back once the job is launched (it's still stuck in the build queue).

[Karsten1987]

CI output:

• Linux
• Linux-aarch64
• macOS
• Windows

Seems to me as if urdf can't find the appropriate urdfdom libs. How did you test this PR on your site?

[luca-della-vedova]

Apologies there was an unbalanced bracket that messed up the library names and the generator expression outputs. Should be fixed now.
I tested by building a colcon workspace with urdf and urdfdom and compiling both the version with my fix and the version without. I also added a line to the urdf cmake (which depends on urdfdom) to print to a file the value of the urdfdom_LIBRARIES variable exported:

file(GENERATE OUTPUT ~/urdf_ws/libs_exp CONTENT "${urdfdom_LIBRARIES}")

The result before the fix is:

optimized;/home/luca/urdf_ws/install/urdfdom/lib/liburdfdom_sensor.so;debug;/home/luca/urdf_ws/install/urdfdom/lib/liburdfdom_sensor.so;optimized;/home/luca/urdf_ws/install/urdfdom/lib/liburdfdom_model_state.so;debug;/home/luca/urdf_ws/install/urdfdom/lib/liburdfdom_model_state.so;optimized;/home/luca/urdf_ws/install/urdfdom/lib/liburdfdom_model.so;debug;/home/luca/urdf_ws/install/urdfdom/lib/liburdfdom_model.so;optimized;/home/luca/urdf_ws/install/urdfdom/lib/liburdfdom_world.so;debug;/home/luca/urdf_ws/install/urdfdom/lib/liburdfdom_world.so;console_bridge

While after the fix is:

/home/luca/urdf_ws/install/urdfdom/lib/liburdfdom_sensor.so;;/home/luca/urdf_ws/install/urdfdom/lib/liburdfdom_model_state.so;;/home/luca/urdf_ws/install/urdfdom/lib/liburdfdom_model.so;;/home/luca/urdf_ws/install/urdfdom/lib/liburdfdom_world.so;;console_bridge

So all looks in place now, again apologies for not spotting that earlier.

[Karsten1987]

kicking off new round of CI:

• Linux
• Linux-aarch64
• macOS
• Windows
• Windows-container

[Karsten1987]

For completeness, I started a Windows Debug build:

The warning is unrelated to this change.

[Karsten1987]

This change looks okay to me given that all CI builds come back green and it resolves the problem reported by @luca-della-vedova.

[ivanpauno]

@Karsten1987 Is this ready to be merged?

[Karsten1987]

Thanks for the reminder. Going ahead and merge.