Revert "Export YAML_CPP_DLL define on Windows (#30)"

[jacobperron]

This reverts commit efe5371.

The change is causing MSBuild warnings, so I'm hesitant to release this change into Foxy. If we want to consider it, we should at the very least resolve the warnings coming from core packages (see #31 (comment)).

FYI @Ace314159

[Ace314159]

Thanks for mentioning me!

Based on #31 (comment), if we merge the 2 unmerged PRs into the foxy branch, the warnings will not appear. The PRs cannot be merged into rolling because #30 is not in rolling.

I am hesitant on the revert because it will reintroduce #10, causing some packages that currently build to not build.

[jacobperron]

I am hesitant on the revert because it will reintroduce #10, causing some packages that currently build to not build.

True, though this bug has been in Foxy for a long time and the fix has not been released yet. So, any packages that currently build against Foxy probably already had to workaround the issue, unless they are building Foxy from source.

I guess we can leave the patch in Foxy, I think the only downside is that it will cause a build warning on Windows for any packages that currently have the workaround (including some of the package in our ros2.repos file).

Pinging @ros2/team for thoughts.

[Ace314159]

Just to provide more information, not all packages have this workaround, such as robot_localization. I'm sure there's also other packages without this workaround since Windows support is usually not a main concern.

[jacobperron]

I suppose we need to determine why removing the workaround on Rolling results in failure to build.

[Ace314159]

#25 was supposed to fix #10 for Rolling, but it appears it didn't. We would be able to remove the workaround if something like #30 was also put into Rolling.

[clalancette]

#25 was supposed to fix #10 for Rolling, but it appears it didn't. We would be able to remove the workaround if something like #30 was also put into Rolling.

Yes, exactly. We should port that over to Rolling, which I think should fix this.

[clalancette]

Yes, exactly. We should port that over to Rolling, which I think should fix this.

And further, we should probably just close this PR. This code that this is removing is what is actually making things work on Foxy :).

[jacobperron]

Make sense to me.

Here's ports of #30 to Rolling and Humble:

• [rolling] Export YAML_CPP_DLL define on Windows (#30) #38
• [humble] Export YAML_CPP_DLL define on Windows (#30) #39