Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Supress msvc dll exported interface warning #263

Merged
merged 6 commits into from
May 29, 2024

Conversation

Blast545
Copy link
Contributor

Summary

Suppresses a warning appearing in the CI job for ign-launch5. Reference build: https://build.osrfoundation.org/job/gz_launch-ign-launch5-win/49/

This fix was applied to gz-launch here: https://github.com/gazebosim/gz-launch/pull/199/files#diff-c3f9cb0a10e4084bda0abece3428e80d90754073fc083b78887bfbcf6276bb07

cc: @Crola1702

Checklist

  • Signed all commits for DCO
  • Added tests
  • Updated documentation (as needed)
  • Updated migration guide (as needed)
  • Consider updating Python bindings (if the library has them)
  • codecheck passed (See contributing)
  • All tests passed (See test coverage)
  • While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

Signed-off-by: Jorge J. Perez <jjperez@ekumenlabs.com>
@Blast545 Blast545 requested a review from nkoenig as a code owner May 29, 2024 14:37
@Blast545 Blast545 requested a review from azeey May 29, 2024 14:37
@github-actions github-actions bot added the 🏯 fortress Ignition Fortress label May 29, 2024
@@ -20,7 +20,7 @@
#include <memory>
#include <string>

// #include <ignition/common/SuppressWarning.hh>
#include <ignition/utils/SuppressWarning.hh>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You need to link against ignition-utils${IGN_UTILS_VER}::core for this to work. You'd add that in

${BACKWARD_LIBRARIES}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just added it with 7ba196f. But I think the cmd code copies back the Manager.hh source into the cmd folder and then the library is not linked there.

Should I link it there:

target_link_libraries(ign PUBLIC
?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, we'll need to link ignition-utils there as well then.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Manager.hh seems to be used in various places, I had to add a few extra link includes, CI seems to be happy now.

Blast545 added 5 commits May 29, 2024 12:03
Signed-off-by: Jorge J. Perez <jjperez@ekumenlabs.com>
Signed-off-by: Jorge J. Perez <jjperez@ekumenlabs.com>
Signed-off-by: Jorge J. Perez <jjperez@ekumenlabs.com>
Signed-off-by: Jorge J. Perez <jjperez@ekumenlabs.com>
Signed-off-by: Jorge J. Perez <jjperez@ekumenlabs.com>
@Blast545 Blast545 merged commit bdaacf5 into ign-launch5 May 29, 2024
9 checks passed
@Blast545 Blast545 deleted the blast545/fix_win_warnings_dll_launch5 branch May 29, 2024 19:33
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
🏯 fortress Ignition Fortress
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants