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

cmake: support MSVC_RUNTIME_LIBRARY property #8851

Merged
merged 1 commit into from
Aug 28, 2021

Conversation

compnerd
Copy link
Contributor

When using a new enough CMake (3.15+) prefer to use the
MSVC_RUNTIME_LIBRARY property on targets to select the runtime
library variant. This property is automatically set to the value
specified by CMAKE_MSVC_RUNTIME_LIBRARY. This proper requires that
the CMake Policy 91 is set to new (see CMP0091).

@google-cla google-cla bot added the cla: yes label Jul 31, 2021
@compnerd
Copy link
Contributor Author

CC: @haberman

When using a new enough CMake (3.15+) prefer to use the
`MSVC_RUNTIME_LIBRARY` property on targets to select the runtime library
variant.  This property is automatically set to the value specified by
`CMAKE_MSVC_RUNTIME_LIBRARY`.  This property requires that the CMake
Policy 91 is set to new (see CMP0091).
@compnerd
Copy link
Contributor Author

Anything else left before we could merge this @haberman?

@haberman haberman merged commit f79f956 into protocolbuffers:master Aug 28, 2021
@compnerd compnerd deleted the msvc-runtime-library branch August 29, 2021 01:29
@jtattermusch
Copy link
Contributor

This PR seems to be breaking gRPC build in a test job that tried to continuously test against protobuf's head.

https://source.cloud.google.com/results/invocations/7ea8aefa-6622-47c6-9c2f-092b0cc87f7c/targets/github%2Fgrpc%2Ftoplevel_run_tests_invocations%2Frun_tests_c_linux_dbg_native_buildonly/tests

internal b/198323179

-- 3.17.3.0
CMake Error at third_party/protobuf/cmake/CMakeLists.txt:187 (if):
  if given arguments:

    "CMAKE_VERSION" "VERSION_GREATER_EQUAL" "3.15"

  Unknown arguments specified

Looks like this change might be broken? Should we revert?

@compnerd
Copy link
Contributor Author

compnerd commented Sep 1, 2021

I don't think that this change is broken. The problem is likely the CMake version that is being used to build.

@snnn
Copy link
Contributor

snnn commented May 1, 2022

If I understand correctly, when protobuf_BUILD_SHARED_LIBS is OFF and the latest cmake is in-use, this change will force users using static CRT? Previously at least they have a protobuf_MSVC_STATIC_RUNTIME for us to choose. The line

    set(CMAKE_MSVC_RUNTIME_LIBRARY MultiThreaded$<$<CONFIG:Debug>:Debug>)

shouldn't exist at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants