-
Notifications
You must be signed in to change notification settings - Fork 38
fix(deps): some warnings when building for Windows on Linux #904
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
danielschenk
commented
Jul 7, 2025
- protobuf: don't pass unsupported /MP flag to MSVC frontend for clang
- protobuf: fix incorrectly disabled invalid-offsetof warning on clang
- include all external library headers as system headers, so we don't have to disable warnings for our own compilation units
Dependency ReviewThe following issues were found:
Snapshot WarningsConsider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice. License Issuesosal/freertos/CMakeLists.txt
external/crypto/micro-ecc/CMakeLists.txt
external/segger_rtt/CMakeLists.txt
external/protobuf/CMakeLists.txt
external/crypto/tiny-aes128/CMakeLists.txt
cmake/emil_test_helpers.cmake
infra/syntax/CMakeLists.txt
external/args/CMakeLists.txt
lwip/lwip/CMakeLists.txt
osal/threadx/CMakeLists.txt
external/crypto/mbedtls/CMakeLists.txt
OpenSSF ScorecardScorecard details
Scanned Files
|
🦙 MegaLinter status:
|
| Descriptor | Linter | Files | Fixed | Errors | Warnings | Elapsed time |
|---|---|---|---|---|---|---|
| ✅ ACTION | actionlint | 12 | 0 | 0 | 0.25s | |
| ✅ CPP | clang-format | 1027 | 8 | 0 | 0 | 7.3s |
| ✅ DOCKERFILE | hadolint | 2 | 0 | 0 | 0.36s | |
| ✅ JSON | jsonlint | 7 | 0 | 0 | 0.16s | |
| ✅ JSON | prettier | 7 | 0 | 0 | 0 | 0.61s |
| markdownlint | 6 | 0 | 4 | 0 | 1.11s | |
| markdown-link-check | 6 | 1 | 0 | 145.73s | ||
| ✅ MARKDOWN | markdown-table-formatter | 6 | 0 | 0 | 0 | 0.22s |
| ✅ REPOSITORY | checkov | yes | no | no | 20.77s | |
| ✅ REPOSITORY | git_diff | yes | no | no | 0.05s | |
| ✅ REPOSITORY | grype | yes | no | no | 23.22s | |
| ✅ REPOSITORY | ls-lint | yes | no | no | 0.06s | |
| ✅ REPOSITORY | secretlint | yes | no | no | 5.96s | |
| ✅ REPOSITORY | syft | yes | no | no | 1.41s | |
| ✅ REPOSITORY | trivy | yes | no | no | 5.9s | |
| ✅ REPOSITORY | trivy-sbom | yes | no | no | 0.17s | |
| ✅ REPOSITORY | trufflehog | yes | no | no | 3.2s | |
| lychee | 138 | 1 | 0 | 5.3s | ||
| prettier | 22 | 1 | 1 | 0 | 0.84s | |
| ✅ YAML | v8r | 22 | 0 | 0 | 4.93s | |
| ✅ YAML | yamllint | 22 | 0 | 0 | 0.52s |
See detailed report in MegaLinter reports
|
| # /MP is added by protobuf in case of MSVC, but that check is a (partial) false positive since we use the MSVC _frontend_ for clang | ||
| # which does not support this flag | ||
| remove_mp(libprotobuf libprotobuf-lite libprotoc libupb) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But there is also a configuration with plain old MSVC, so /MP should not be removed unconditionally
| endforeach() | ||
| endfunction() | ||
|
|
||
| function(remove_mp) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about merging remove_mp into add_protobuf_target_properties, and renaming that to modify_protobuf_target_properties?



