[Windows] It is nice YAML_CPP_DLL is exported by default.

[seanyen]

On Windows, the yaml-cpp is built as shared libraries. In such case, YAML_CPP_DLL is required to define to correctly consume the yaml-cpp (otherwise, visibility decoration won't be defined and it results in linkage time error).

And currently every project consuming yaml-cpp will need to define it in their CMake project to enable Windows build. It would be nice to see this exported directly from this package and all the downstream project can get the definition for free.

UPDATED:
For example,

• https://github.com/ros-perception/image_common/blob/ros2/camera_calibration_parsers/src/parse_yml.cpp#L49
• https://github.com/ros2/rosbag2/blob/master/rosbag2_transport/src/rosbag2_transport/recorder.cpp#L36
• https://github.com/ros2/rviz/blob/ros2/rviz_common/src/rviz_common/yaml_config_reader.cpp#L42

And I discovered this issue when I try to enable navigation2 on Windows.

[dirk-thomas]

@seanyen Please provide a pull request to add the necessary definition to the ExternalProject_Add call.

[dirk-thomas]

Please also describe your exact steps when you run into problem since afaik we are currently able to use this package as is on Windows without problems.

[seanyen]

Thanks for the pointer. I will be looking how to export it from ExternalProject_Add and I added some examples how does it affect the downstream projects.

[dirk-thomas]

So everything works fine atm but you propose a change in order to be able to remove these workarounds in other packages. That wasn't clear from the original description. Sounds good to me.

Please also make pull requests for the referenced packages so that it can be checked that the to-be-proposed change makes today workarounds obsolete.

[hidmic]

Are you still picking this up @seanyen ?

[dirk-thomas]

@seanyen Friendly ping.

[jacobperron]

Bumping. I also ran into this issue; I had to add #define YAML_CPP_DLL to resolve.

[clalancette]

This should now be solved on the latest with #25 being merged. It should also be solved in Galactic and Foxy with PRs #30 and #31, so I'm going to go ahead and close this out.

[Ace314159]

@clalancette I see #30 and #31 haven't been merged yet. Are there plans to merge them or is something holding them up?

[jacobperron]

I've approved and triggered Windows CI for #30 and #31.

[jacobperron]

Circling back here, it's not clear that #25 actually resolved this issue since removing the YAML_CPP_DLL patches in connected PRs results in a build failure.

Connected PRs:

• Remove YAML_CPP_DLL define rviz#831
• Remove YAML_CPP_DLL define rosbag2#964

Maybe there's something else missing in the downstream packages.