Skip to content

Suppress protobuf related Windows warnings#334

Merged
mjcarroll merged 12 commits intomainfrom
mjcarroll/fix_windows_warnings
Jul 27, 2022
Merged

Suppress protobuf related Windows warnings#334
mjcarroll merged 12 commits intomainfrom
mjcarroll/fix_windows_warnings

Conversation

@mjcarroll
Copy link
Contributor

🦟 Bug fix

Summary

Followup to gz-msgs#256: gazebosim/gz-msgs#256 (comment)

There were a few more warnings present, which come from "unprotected" protobuf includes. This takes every protobuf (library, not generated) include statement and wraps them with the correct pragmas.

Checklist

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: Michael Carroll <michael@openrobotics.org>
@mjcarroll mjcarroll requested a review from caguero as a code owner July 21, 2022 21:23
@github-actions github-actions bot added the 🌱 garden Ignition Garden label Jul 21, 2022
@mjcarroll
Copy link
Contributor Author

CC: @Blast545

@chapulina chapulina added the Windows Windows support label Jul 21, 2022
@chapulina
Copy link
Contributor

The UNIT_Discovery_TEST failure looks new

@mjcarroll
Copy link
Contributor Author

The UNIT_Discovery_TEST failure looks new

It reliably fails on main for me as well.

@Blast545
Copy link
Contributor

😢 The PR job seems to have more warnings that the reference buildfarm failure: (36 -> 55)
https://build.osrfoundation.org/job/ign_transport-pr-win/289/

Signed-off-by: Michael Carroll <michael@openrobotics.org>
@chapulina chapulina added the bug Something isn't working label Jul 23, 2022
Signed-off-by: Michael Carroll <michael@openrobotics.org>
Signed-off-by: Michael Carroll <michael@openrobotics.org>
@mjcarroll
Copy link
Contributor Author

Using the SYSTEM include directive should cause MSVC to use the /external directive to mark those directories/includes to not show warnings.

In MSVC < 16.10, this requires an additional /experimental:external flag to be passed to the compiler: https://docs.microsoft.com/en-us/cpp/build/reference/external-external-headers-diagnostics?view=msvc-170

It looks like we are using something newer than that, so I'm not sure where the issue is at this point.

Signed-off-by: Michael Carroll <michael@openrobotics.org>
Signed-off-by: Michael Carroll <michael@openrobotics.org>
Signed-off-by: Michael Carroll <michael@openrobotics.org>
Signed-off-by: Michael Carroll <michael@openrobotics.org>
Signed-off-by: Michael Carroll <michael@openrobotics.org>
@codecov
Copy link

codecov bot commented Jul 27, 2022

Codecov Report

Merging #334 (93bf268) into main (80f71eb) will not change coverage.
The diff coverage is 100.00%.

❗ Current head 93bf268 differs from pull request most recent head 9efd4f1. Consider uploading reports for the commit 9efd4f1 to get more accurate results

@@           Coverage Diff           @@
##             main     #334   +/-   ##
=======================================
  Coverage   88.11%   88.11%           
=======================================
  Files          52       52           
  Lines        4848     4848           
=======================================
  Hits         4272     4272           
  Misses        576      576           
Impacted Files Coverage Δ
include/gz/transport/RepHandler.hh 85.36% <ø> (ø)
include/gz/transport/ReqHandler.hh 80.58% <ø> (ø)
include/gz/transport/SubscriptionHandler.hh 84.90% <ø> (ø)
src/NetUtils.cc 71.96% <ø> (ø)
src/Node.cc 92.30% <ø> (ø)
src/NodeShared.cc 77.76% <ø> (ø)
src/NodeSharedPrivate.hh 100.00% <ø> (ø)
src/cmd/gz.cc 95.31% <ø> (ø)
src/Discovery.cc 75.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 80f71eb...9efd4f1. Read the comment docs.

Signed-off-by: Michael Carroll <michael@openrobotics.org>
@mjcarroll mjcarroll merged commit a8be887 into main Jul 27, 2022
@mjcarroll mjcarroll deleted the mjcarroll/fix_windows_warnings branch July 27, 2022 20:06
mjcarroll added a commit that referenced this pull request Jul 27, 2022
Backport #334 to citadel

Signed-off-by: Michael Carroll <michael@openrobotics.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working 🌱 garden Ignition Garden Windows Windows support

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

4 participants