Fix CMake option for choosing MSVC runtime library (#1054)

**Issue:** Building with `-DSTATIC_CRT=ON` didn't actually do anything. **Diagnosis:** Our CMake logic relied on `${CMAKE_BUILD_TYPE}` being set, but for multi-config generators like Visual Studio `${CMAKE_BUILD_TYPE}` is ignored and most users (and IDEs) don't bother setting it. **Description of changes:** 1) Use `$<CONFIG>` generator expressions, instead of checking `${CMAKE_BUILD_TYPE}`, to support multi-config generators like Visual Studio. See: https://cmake.org/cmake/help/v3.22/manual/cmake-buildsystem.7.html#build-configurations. 2) Deprecate ~STATIC_CRT~ option, replace with `AWS_STATIC_MSVC_RUNTIME_LIBRARY=OFF|ON`. - The old option name was confusing, since we refer to our codebase as "the CRT", but this option referred to Microsoft's C runtime library.
awslabs · Sep 1, 2023 · 997380c · 997380c
1 parent e627dac
commit 997380c
Show file tree

Hide file tree

Showing 4 changed files with 33 additions and 40 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -2,7 +2,6 @@
 # SPDX-License-Identifier: Apache-2.0.
 
 cmake_minimum_required(VERSION 3.0)
-option(STATIC_CRT "Windows specific option that to specify static/dynamic run-time library" OFF)
 option(ALLOW_CROSS_COMPILED_TESTS "Allow tests to be compiled via cross compile, for use with qemu" OFF)
 
 project(aws-c-common LANGUAGES C VERSION 0.1.0)

diff --git a/README.md b/README.md
@@ -42,7 +42,7 @@ Or on windows,
 * -DENABLE_SANITIZERS=ON - Enables gcc/clang sanitizers, by default this adds -fsanitizer=address,undefined to the compile flags for projects that call aws_add_sanitizers.
 * -DENABLE_FUZZ_TESTS=ON - Includes fuzz tests in the unit test suite. Off by default, because fuzz tests can take a long time. Set -DFUZZ_TESTS_MAX_TIME=N to determine how long to run each fuzz test (default 60s).
 * -DCMAKE_INSTALL_PREFIX=/path/to/install - Standard way of installing to a user defined path. If specified when configuring aws-c-common, ensure the same prefix is specified when configuring other aws-c-* SDKs.
-* -DSTATIC_CRT=ON - On MSVC, use /MT(d) to link MSVCRT
+* -DAWS_STATIC_MSVC_RUNTIME_LIBRARY=ON - Windows-only. Turn ON to use the statically-linked MSVC runtime lib, instead of the DLL.
 
 ### API style and conventions
 Every API has a specific set of styles and conventions. We'll outline them here. These conventions are followed in every
@@ -197,7 +197,7 @@ Example:
     Not this:
 
         typedef int(*fn_name_fn)(void *);
-        
+
 * If a callback may be async, then always have it be async.
   Callbacks that are sometimes async and sometimes sync are hard to code around and lead to bugs
   (see [this blog post](https://blog.ometer.com/2011/07/24/callbacks-synchronous-and-asynchronous/)).
@@ -239,7 +239,7 @@ platform, you have more liberty on this.
 * When checking more than one error condition, check and log each condition separately with a unique message.
 
     Do this:
-    
+
         if (options->callback == NULL) {
             AWS_LOGF_ERROR(AWS_LS_SOME_SUBJECT, "Invalid options - callback is null");
             return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);
@@ -251,7 +251,7 @@ platform, you have more liberty on this.
         }
 
     Not this:
-    
+
         if (options->callback == NULL || options->allocator == NULL) {
             AWS_LOGF_ERROR(AWS_LS_SOME_SUBJECT, "Invalid options - something is null");
             return aws_raise_error(AWS_ERROR_INVALID_ARGUMENT);

diff --git a/cmake/AwsCFlags.cmake b/cmake/AwsCFlags.cmake
@@ -11,6 +11,8 @@ option(LEGACY_COMPILER_SUPPORT "This enables builds with compiler versions such
 option(AWS_SUPPORT_WIN7 "Restricts WINAPI calls to Win7 and older (This will have implications in downstream libraries that use TLS especially)" OFF)
 option(AWS_WARNINGS_ARE_ERRORS "Compiler warning is treated as an error. Try turning this off when observing errors on a new or uncommon compiler" OFF)
 option(AWS_ENABLE_TRACING "Enable tracing macros" OFF)
+option(AWS_STATIC_MSVC_RUNTIME_LIBRARY "Windows-only. Turn ON to use the statically-linked MSVC runtime lib, instead of the DLL" OFF)
+option(STATIC_CRT "Deprecated. Use AWS_STATIC_MSVC_RUNTIME_LIBRARY instead" OFF)
 
 # Check for Posix Large Files Support (LFS).
 # On most 64bit systems, LFS is enabled by default.
@@ -80,21 +82,19 @@ function(aws_set_common_properties target)
             list(APPEND AWS_C_FLAGS /DAWS_SUPPORT_WIN7=1)
         endif()
 
-        string(TOUPPER "${CMAKE_BUILD_TYPE}" _CMAKE_BUILD_TYPE)
-        if(STATIC_CRT)
-            string(REPLACE "/MD" "/MT" _FLAGS "${CMAKE_C_FLAGS_${_CMAKE_BUILD_TYPE}}")
+        # Set MSVC runtime libary.
+        # Note: there are other ways of doing this if we bump our CMake minimum to 3.14+
+        # See: https://cmake.org/cmake/help/latest/policy/CMP0091.html
+        if (AWS_STATIC_MSVC_RUNTIME_LIBRARY OR STATIC_CRT)
+            list(APPEND AWS_C_FLAGS "/MT$<$<CONFIG:Debug>:d>")
         else()
-            string(REPLACE "/MT" "/MD" _FLAGS "${CMAKE_C_FLAGS_${_CMAKE_BUILD_TYPE}}")
+            list(APPEND AWS_C_FLAGS "/MD$<$<CONFIG:Debug>:d>")
         endif()
-        string(REPLACE " " ";" _FLAGS "${_FLAGS}")
-        list(APPEND AWS_C_FLAGS "${_FLAGS}")
 
     else()
         list(APPEND AWS_C_FLAGS -Wall -Wstrict-prototypes)
 
-        if (NOT CMAKE_BUILD_TYPE STREQUAL Release)
-            list(APPEND AWS_C_FLAGS -fno-omit-frame-pointer)
-        endif()
+        list(APPEND AWS_C_FLAGS $<$<NOT:$<CONFIG:Release>>:-fno-omit-frame-pointer>)
 
         if(AWS_WARNINGS_ARE_ERRORS)
             list(APPEND AWS_C_FLAGS -Werror)
@@ -223,25 +223,28 @@ function(aws_set_common_properties target)
         list(APPEND AWS_C_DEFINES_PRIVATE -DHAVE_SYSCONF)
     endif()
 
-    if(CMAKE_BUILD_TYPE STREQUAL "" OR CMAKE_BUILD_TYPE MATCHES Debug)
-        list(APPEND AWS_C_DEFINES_PRIVATE -DDEBUG_BUILD)
-    else() # release build
+    list(APPEND AWS_C_DEFINES_PRIVATE $<$<CONFIG:Debug>:DEBUG_BUILD>)
+
+    if ((NOT SET_PROPERTIES_NO_LTO) AND AWS_ENABLE_LTO)
+        # enable except in Debug builds
+        set(_ENABLE_LTO_EXPR $<NOT:$<CONFIG:Debug>>)
+
+        # try to check whether compiler supports LTO/IPO
         if (POLICY CMP0069)
-            if ((NOT SET_PROPERTIES_NO_LTO) AND AWS_ENABLE_LTO)
-                cmake_policy(SET CMP0069 NEW) # Enable LTO/IPO if available in the compiler
-                include(CheckIPOSupported OPTIONAL RESULT_VARIABLE ipo_check_exists)
-                if (ipo_check_exists)
-                    check_ipo_supported(RESULT ipo_supported)
-                    if (ipo_supported)
-                        message(STATUS "Enabling IPO/LTO for Release builds")
-                        set(AWS_ENABLE_LTO ON)
-                    else()
-                        message(STATUS "AWS_ENABLE_LTO is enabled, but cmake/compiler does not support it, disabling")
-                        set(AWS_ENABLE_LTO OFF)
-                    endif()
+            cmake_policy(SET CMP0069 NEW)
+            include(CheckIPOSupported OPTIONAL RESULT_VARIABLE ipo_check_exists)
+            if (ipo_check_exists)
+                check_ipo_supported(RESULT ipo_supported)
+                if (ipo_supported)
+                    message(STATUS "Enabling IPO/LTO for Release builds")
+                else()
+                    message(STATUS "AWS_ENABLE_LTO is enabled, but cmake/compiler does not support it, disabling")
+                    set(_ENABLE_LTO_EXPR OFF)
                 endif()
             endif()
         endif()
+    else()
+        set(_ENABLE_LTO_EXPR OFF)
     endif()
 
     if(BUILD_SHARED_LIBS)
@@ -260,7 +263,5 @@ function(aws_set_common_properties target)
     target_compile_options(${target} PRIVATE ${AWS_C_FLAGS})
     target_compile_definitions(${target} PRIVATE ${AWS_C_DEFINES_PRIVATE} PUBLIC ${AWS_C_DEFINES_PUBLIC})
     set_target_properties(${target} PROPERTIES LINKER_LANGUAGE C C_STANDARD 99 C_STANDARD_REQUIRED ON)
-    if (AWS_ENABLE_LTO)
-        set_target_properties(${target} PROPERTIES INTERPROCEDURAL_OPTIMIZATION TRUE)
-    endif()
+    set_target_properties(${target} PROPERTIES INTERPROCEDURAL_OPTIMIZATION ${_ENABLE_LTO_EXPR}>)
 endfunction()
diff --git a/cmake/AwsTestHarness.cmake b/cmake/AwsTestHarness.cmake
@@ -54,13 +54,6 @@ function(generate_test_driver driver_exe_name)
     target_link_libraries(${driver_exe_name} PRIVATE ${PROJECT_NAME})
 
     set_target_properties(${driver_exe_name} PROPERTIES LINKER_LANGUAGE C C_STANDARD 99)
-    if (MSVC)
-        if(STATIC_CRT)
-            target_compile_options(${driver_exe_name} PRIVATE "/MT$<$<CONFIG:Debug>:d>")
-        else()
-            target_compile_options(${driver_exe_name} PRIVATE "/MD$<$<CONFIG:Debug>:d>")
-        endif()
-    endif()
     target_compile_definitions(${driver_exe_name} PRIVATE AWS_UNSTABLE_TESTING_API=1)
     target_include_directories(${driver_exe_name} PRIVATE ${CMAKE_CURRENT_LIST_DIR})
 
@@ -80,7 +73,7 @@ function(generate_cpp_test_driver driver_exe_name)
 
     set_target_properties(${driver_exe_name} PROPERTIES LINKER_LANGUAGE CXX)
     if (MSVC)
-        if(STATIC_CRT)
+        if(AWS_STATIC_MSVC_RUNTIME_LIBRARY OR STATIC_CRT)
             target_compile_options(${driver_exe_name} PRIVATE "/MT$<$<CONFIG:Debug>:d>")
         else()
             target_compile_options(${driver_exe_name} PRIVATE "/MD$<$<CONFIG:Debug>:d>")