Fix Windows compilation

[Tobias-Fischer]

The recent commit dd1779e broke compilation on Windows. This PR fixes this issue.

Compiler error without this patch:

%SRC_DIR%\ros-noetic-rviz\src\work\src\rviz\default_plugin\screw_display.cpp(87): error C2491: 'rviz::ScrewDisplay<MessageType>::ScrewDisplay': definition of dllimport function not allowed
%SRC_DIR%\ros-noetic-rviz\src\work\src\rviz\default_plugin\screw_display.cpp(95): error C2491: 'rviz::ScrewDisplay<MessageType>::onInitialize': definition of dllimport function not allowed
%SRC_DIR%\ros-noetic-rviz\src\work\src\rviz\default_plugin\screw_display.cpp(103): error C2491: 'rviz::ScrewDisplay<MessageType>::reset': definition of dllimport function not allowed
%SRC_DIR%\ros-noetic-rviz\src\work\src\rviz\default_plugin\screw_display.cpp(125): error C2491: 'rviz::ScrewDisplay<MessageType>::updateProperties': definition of dllimport function not allowed
%SRC_DIR%\ros-noetic-rviz\src\work\src\rviz\default_plugin\screw_display.cpp(132): error C2491: 'rviz::ScrewDisplay<MessageType>::updateHistoryLength': definition of dllimport function not allowed
%SRC_DIR%\ros-noetic-rviz\src\work\src\rviz\default_plugin\screw_display.cpp(191): error C2491: 'rviz::ScrewDisplay<MessageType>::processMessagePrivate': definition of dllimport function not allowed

With patch compilation succeeds.

[Tobias-Fischer]

Many thanks @traversaro for the hint

/cc @wolfv

[rhaschke]

Thanks for reporting this issue. However, I think it should be fine to remove the EXPORT declaration completely. All other symbols going into the plugin lib are declared without any such EXPORT as well. The only symbol using it is InteractiveMarker, which is also used by the main binary directly.
Could you please try and adapt the PR if so? I cannot build on Windows myself.

@seanyen: Would it be possible to add a Windows CI build to avoid such issues in future?
Any help is highly welcome.

[rhaschke]

Here as well.

[Tobias-Fischer]

Hi, thanks for getting back to me so quickly. I tried removing the export first and it didn't work for some reason. Unfortunately I won't have time to look at this again. Many other files probably have unnecessary exports, too - I suggest to merge this PR to get a working compilation and open an issue to clean the exports up. What do you think?

[Tobias-Fischer]

PS: on https://github.com/RoboStack/ros-noetic we'll have regular builds on Win, OSX and Linux.

[rhaschke]

I tried removing the export first and it didn't work for some reason.

Maybe, you forgot the one export in declaration of WrenchVisual?
I pushed another commit removing all exports. Please, give that a try on Windows.

Many other files probably have unnecessary exports, too.

No, that's the only plugin source declaring a superfluent export. To keep the code clean, I would prefer to fix it immediately.

on https://github.com/RoboStack/ros-noetic we'll have regular builds on Win, OSX and Linux.

Would be nice to have CI integrated here with github...

[wolfv]

If you're interested I can push a GitHub action that uses micromamba to install the RViz dependencies from our robostack channel!

[rhaschke]

Hi @wolfv, I'm not used to github actions yet. Don't you need to pay for that service? If not, a full stack of github actions to build rviz on Windows would be highly welcome.

[wolfv]

nope, github actions for open source is free. Azure Pipelines is also free & open source and both are pretty similar.

Travis CI is the one which, unfortunately, recently changed their business model to disallow "unlimited" use for opensource projects.

[rhaschke]

github actions for open source is free.

Good to know. Need to learn about its configuration now...

Travis CI is the one which, unfortunately, recently changed their business model to disallow "unlimited" use for opensource projects.

Yeah, they did. But they already reverted that decision for open-source: https://blog.travis-ci.com/oss-announcement

[Tobias-Fischer]

This is good to be merged.

[rhaschke]

Thanks for testing the latest commit on Windows.