search

floooh / fips

High-level build system for distributed, multi-platform C/C++ projects.
MIT License
468 stars 82 forks source link

Fix /MT /MD flags for Cmake 3.15+ #278

Closed mattiasljungstrom closed 2 years ago

mattiasljungstrom commented 2 years ago

The /MT /MD flags are not set correctly in CMake 3.15+. Maybe you want to do this in a different place, but this works.

See: https://cmake.org/cmake/help/latest/variable/CMAKE_MSVC_RUNTIME_LIBRARY.html#variable:CMAKE_MSVC_RUNTIME_LIBRARY

floooh commented 2 years ago

Thanks! It would probably make more sense to let projects configure this directly, but I don't know yet how much this would break :)

floooh commented 2 years ago

Oooops, something weird in GH Actions after the update. Investigating...

Screenshot 2022-02-08 at 15 37 16
floooh commented 2 years ago

...also happens on my Windows box with VS2019 and cmake 3.22.2

There's a weird "additional option":

image

floooh commented 2 years ago

Reverting to the last commit fixes the issue, so I'll do that. From looking at your changes I have no idea how that would happen though... hmm.

floooh commented 2 years ago

Ok I have undone the merge commit for now.

Any idea what the problem could be?

floooh commented 2 years ago

Ah, found the problem, note the little "d" here at the end:

https://github.com/floooh/fips/blob/85b7904bd15f2bdfaf53cb6ffe3b1b893848b08b/cmake-toolchains/windows.cmake#L60

mattiasljungstrom commented 2 years ago

ok, right, missed that, assumed that it would be empty if not setup.

although I see the %(AdditionalOptions) even without these changes.

mattiasljungstrom commented 2 years ago

ok, now I understand, the 'd' is the problem, sorry about that!

mattiasljungstrom commented 2 years ago

should I do a new PR or you want to fix it?

floooh commented 2 years ago

Would be nice if you could provide a new PR :)

I think all uses of the FIPS_VS_CRT_FLAGS variable must be moved up into the "cmake version < 3.15" block.