wrap CMP0091 in MSVC check #1580
Conversation
CMakeLists.txt
Outdated
| if(MSVC) | ||
| # Use the static runtime libraries when building statically for consistency with vcpkg's | ||
| # arch-windows-static triplets: | ||
| cmake_policy(SET CMP0091 NEW) # https://cmake.org/cmake/help/v3.15/variable/CMAKE_MSVC_RUNTIME_LIBRARY.html | ||
| if (NOT DEFINED CMAKE_MSVC_RUNTIME_LIBRARY) | ||
| set(CMAKE_MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>") | ||
| endif() |
There was a problem hiding this comment.
I am not sure why the windows CI legs are now failing with that linker warning on Windows:
https://dev.azure.com/azure-sdk/public/_build/results?buildId=705929&view=logs&j=1822ef0b-3ac4-586f-233b-6fdf166a140f&t=b129bef7-7ee0-56b0-68be-0867e926d36b&l=102
LINK : warning LNK4098: defaultlib 'LIBCMT' conflicts with use of other libs; use /NODEFAULTLIB:library [D:\vss\_work\1\s\build\sdk\tests\iot\provisioning\az_iot_provisioning_test.vcxproj]
LINK : error LNK1218: warning treated as error; no output file generated [D:\vss\_work\1\s\build\sdk\tests\iot\provisioning\az_iot_provisioning_test.vcxproj]
##[error]Cmd.exe exited with code '1'.
Presumable if (MSVC) condition is returning false, but I don't know why. I wonder if you need to do something like this instead: if(CMAKE_CXX_COMPILER_ID MATCHES "MSVC"). Try some tests @danewalton using message(...) to see if we actually enter those code blocks.
@BillyONeal maybe you have some ideas.
There was a problem hiding this comment.
I know! MSVC is not defined before the line saying project(az LANGUAGES C). @danewalton, try moving the check after that line, it should work.
|
MSVC is not defined before the |
|
@ahsonkhan @antkmsft thoughts here? Latest addition seems to be working |
just checked with Ubuntu 18.04 and looks to be working
|
| add_compile_options( | ||
| $<$<CONFIG:>:/MT> #---------| | ||
| $<$<CONFIG:Debug>:/MTd> #---|-- Statically link the runtime libraries | ||
| $<$<CONFIG:Release>:/MT> #--| | ||
| ) |
There was a problem hiding this comment.
@barcharcraz and @BillyONeal - can you please help review and sign-off on this approach.
There was a problem hiding this comment.
Just for reference I pulled it from here:
https://gitlab.kitware.com/cmake/cmake/-/issues/18390
CMakeLists.txt
Outdated
| if(MSVC OR WIN32) | ||
| # Use the static runtime libraries when building statically for consistency with vcpkg's | ||
| # arch-windows-static triplets: | ||
| cmake_policy(SET CMP0091 NEW) # https://cmake.org/cmake/help/v3.15/variable/CMAKE_MSVC_RUNTIME_LIBRARY.html | ||
| if (NOT DEFINED CMAKE_MSVC_RUNTIME_LIBRARY) | ||
| set(CMAKE_MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>") | ||
| endif() | ||
| endif() |
There was a problem hiding this comment.
If we are explicitly adding some compile options in global_compile_options, do we need to set cmake_policy anymore?
|
Merging for now but let me know if this needs to be changed |
| @@ -44,13 +44,6 @@ if(DEFINED ENV{VCPKG_DEFAULT_TRIPLET} AND NOT DEFINED VCPKG_TARGET_TRIPLET) | |||
| set(VCPKG_TARGET_TRIPLET "$ENV{VCPKG_DEFAULT_TRIPLET}" CACHE STRING "") | |||
| endif() | |||
|
|
|||
| # Use the static runtime libraries when building statically for consistency with vcpkg's | |||
There was a problem hiding this comment.
Consider including this fix in a changelog entry.
There was a problem hiding this comment.
Done in #1605
Thanks @danewalton
fixes #1514