Skip to content
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

Fix CMake option for choosing MSVC runtime library #1054

Merged
merged 8 commits into from
Sep 1, 2023
Merged

Conversation

graebm
Copy link
Contributor

@graebm graebm commented Aug 31, 2023

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.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@graebm graebm marked this pull request as ready for review August 31, 2023 17:17
@graebm graebm changed the title Fix STATIC_CRT option, for choosing static MSVC runtime library Fix STATIC_CRT option (chooses MSVC runtime library) Aug 31, 2023
…ead.

STATIC_CRT is confusingly named, since we refer to this codebase as "CRT", but this option refers to Microsoft's C-runtime library.
@graebm graebm changed the title Fix STATIC_CRT option (chooses MSVC runtime library) Fix CMake option for choosing MSVC runtime library Sep 1, 2023
@graebm graebm enabled auto-merge (squash) September 1, 2023 18:53
@graebm graebm merged commit 997380c into main Sep 1, 2023
@graebm graebm deleted the msvc-runtime branch September 1, 2023 19:07
graebm added a commit to awslabs/aws-crt-cpp that referenced this pull request Sep 6, 2023
**Issue:**
The `-DSTATIC_CRT=ON` CMake option for choosing the MSVC runtime library didn't affect submodules. Also it's a terribly named option, since we refer to this codebase as "CRT".

**Description of changes:**
1) Pick up this fix in the submodules: awslabs/aws-c-common#1054
2) ~STATIC_CRT~ is deprecated, use new `AWS_STATIC_MSVC_RUNTIME_LIBRARY` option instead.
3) Use `$<CONFIG>` generator expressions, instead of checking `${CMAKE_BUILD_TYPE}`, to better support multi-config generators like Visual Studio. See: https://cmake.org/cmake/help/v3.22/manual/cmake-buildsystem.7.html#build-configurations
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants