cpp-best-practices / cmake_template

CMake for C++ Best Practices
The Unlicense
1.09k stars 118 forks source link

update msvc ASAN support and copy required install dlls #77

Open DewJunkie opened 8 months ago

DewJunkie commented 8 months ago

MSVC does not allow the use of ASAN on debug builds, so modified sanitizers.cmake to reflect that. MSVC requires the ASAN dlls to run, so I put an install for them into sanitizers.cmake.

The package updates are unrelated, but I was getting a lot of deprication warnings (promoted to errors) in msvc debug builds. Bumping the version to the latest resolved them.

codecov-commenter commented 8 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Comparison is base (d19d892) 24.70% compared to head (792ce81) 24.70%.

:exclamation: Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #77 +/- ## ======================================= Coverage 24.70% 24.70% ======================================= Files 6 6 Lines 251 251 Branches 142 143 +1 ======================================= Hits 62 62 - Misses 181 184 +3 + Partials 8 5 -3 ``` | [Flag](https://app.codecov.io/gh/cpp-best-practices/cmake_template/pull/77/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | Coverage Δ | | |---|---|---| | [Linux](https://app.codecov.io/gh/cpp-best-practices/cmake_template/pull/77/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | `14.89% <ø> (-1.54%)` | :arrow_down: | | [Windows](https://app.codecov.io/gh/cpp-best-practices/cmake_template/pull/77/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None) | `21.42% <ø> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=None#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

DewJunkie commented 8 months ago

I'm pretty certain that the build failures are related to cpp check now failing the constexpr unit tests, because it issues a warning that the tests will always be true.

cmake_template\test\constexpr_tests.cpp(10,41): warning GEB24262F: Condition 'factorial_constexpr(3)==6' is always true [knownConditionTrueFalse]
    STATIC_REQUIRE(factorial_constexpr(3) == 6);

Are you ok with a new option for cppcheck warnings as errors, currently it picks up the project warnings as errors. The other option would be to just not have it set in the constexpr tests.

It is also picking up some errors in spdlog, I'm guessing there must be a way to tell spdlog to not check 3rd party libs, but I guess for me I'd rather still see them.

cmake_template\out\build\windows-clang-release\_deps\spdlog-src\include\spdlog\details\null_mutex.h(19,5): warning GDD13A24C: Member variable 'null_atomic_int::value' is not initialized in the constructor. [uninitMemberVar]
      null_atomic_int() = default;

Not in any rush to merge this change, but it does make the windows builds clone->build->run, which I think is a plus.

lefticus commented 6 months ago

@DewJunkie The MSVC + ASAN + CMake has been no end of pain for me over the years. I dislike having to copy the DLL's around, but am up for it if it gets things working.

There's a couple of other CI fixes in the pipeline, I'm going to try and get those merged first, then sort this one out with you.

DewJunkie commented 6 months ago

@lefticus too true. I can put some time into seeing if I can just mark those files as runtime dependencies, rather than forcing them to get installed. install() is one of my weaker cmake areas.