Skip to content

Commit

Permalink
cleanup: fewer warnings for proto files (googleapis#5855)
Browse files Browse the repository at this point in the history
Disable some MSVC warnings on proto files. Also disable `clang-tidy` for
them in a simpler way, with fewer side-effects.
  • Loading branch information
coryan authored Feb 16, 2021
1 parent d756b07 commit cb61dda
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 64 deletions.
15 changes: 15 additions & 0 deletions cmake/CompileProtos.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,8 @@ function (google_cloud_cpp_install_proto_library_protos target source_dir)
endforeach ()
endfunction ()

include(EnableWerror)

function (google_cloud_cpp_proto_library libname)
cmake_parse_arguments(_opt "LOCAL_INCLUDE" "" "PROTO_PATH_DIRECTORIES"
${ARGN})
Expand Down Expand Up @@ -282,6 +284,19 @@ function (google_cloud_cpp_proto_library libname)
target_include_directories(
${libname} SYSTEM PUBLIC $<BUILD_INTERFACE:${CMAKE_CURRENT_BINARY_DIR}>
$<INSTALL_INTERFACE:include>)
google_cloud_cpp_silence_warnings_in_deps(${libname})
# Disable clang-tidy for generated code, note that the CXX_CLANG_TIDY
# property was introduced in 3.6, and we do not use clang-tidy with older
# versions
if (NOT ("${CMAKE_VERSION}" VERSION_LESS 3.6))
set_target_properties(${libname} PROPERTIES CXX_CLANG_TIDY "")
endif ()
if (MSVC)
# The protobuf-generated files have warnings under the default MSVC
# settings. We are not interested in these warnings, because we cannot
# fix them.
target_compile_options(${libname} PRIVATE "/wd4244" "/wd4251")
endif ()

# In some configs we need to only generate the protocol definitions from
# `*.proto` files. We achieve this by having this target depend on all proto
Expand Down
13 changes: 10 additions & 3 deletions cmake/EnableWerror.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -29,15 +29,22 @@ check_cxx_compiler_flag(-Wno-sign-conversion
GOOGLE_CLOUD_CPP_COMPILER_SUPPORTS_WNO_SIGN_CONVERSION)
check_cxx_compiler_flag(-Werror GOOGLE_CLOUD_CPP_COMPILER_SUPPORTS_WERROR)

function (google_cloud_cpp_silence_warnings_in_deps target)
if (MSVC)
target_compile_options(${target} PRIVATE "/experimental:external")
target_compile_options(${target} PRIVATE "/external:W0")
target_compile_options(${target} PRIVATE "/external:anglebrackets")
return()
endif ()
endfunction ()

function (google_cloud_cpp_add_common_options target)
google_cloud_cpp_silence_warnings_in_deps(${target})
if (MSVC)
target_compile_options(${target} PRIVATE "/W3")
if (GOOGLE_CLOUD_CPP_ENABLE_WERROR)
target_compile_options(${target} PRIVATE "/WX")
endif ()
target_compile_options(${target} PRIVATE "/experimental:external")
target_compile_options(${target} PRIVATE "/external:W0")
target_compile_options(${target} PRIVATE "/external:anglebrackets")
return()
endif ()
if (GOOGLE_CLOUD_CPP_COMPILER_SUPPORTS_WALL)
Expand Down
60 changes: 18 additions & 42 deletions external/googleapis/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -149,8 +149,7 @@ set(EXTERNAL_GOOGLEAPIS_PROTO_FILES
"google/type/quaternion.proto"
"google/type/timeofday.proto")

# Always disable clang-tidy for this generated code.
set(CMAKE_CXX_CLANG_TIDY "")
include(EnableWerror)

set(EXTERNAL_GOOGLEAPIS_BYPRODUCTS)
foreach (proto ${EXTERNAL_GOOGLEAPIS_PROTO_FILES})
Expand Down Expand Up @@ -182,10 +181,6 @@ if (PROTO_INCLUDE_DIR)
list(INSERT PROTOBUF_IMPORT_DIRS 0 "${PROTO_INCLUDE_DIR}")
endif ()

# Add flags to this library to automatically include them in all the libraries
# in this directory.
add_library(external_googleapis_common_flags INTERFACE)

include(SelectMSVCRuntime)

# Include the functions to compile proto files.
Expand Down Expand Up @@ -221,13 +216,10 @@ function (external_googleapis_add_library proto)
endforeach ()
list(LENGTH public_deps public_deps_length)
if (public_deps_length EQUAL 0)
target_link_libraries("google_cloud_cpp_${short_name}"
PRIVATE external_googleapis_common_flags)
target_link_libraries("google_cloud_cpp_${short_name}")
else ()
target_link_libraries(
"google_cloud_cpp_${short_name}"
PUBLIC ${public_deps}
PRIVATE external_googleapis_common_flags)
target_link_libraries("google_cloud_cpp_${short_name}"
PUBLIC ${public_deps})
endif ()
endfunction ()

Expand Down Expand Up @@ -380,8 +372,7 @@ target_link_libraries(
google-cloud-cpp::iam_v1_iam_policy_protos
google-cloud-cpp::iam_v1_policy_protos
google-cloud-cpp::rpc_status_protos
google-cloud-cpp::api_http_protos
PRIVATE external_googleapis_common_flags)
google-cloud-cpp::api_http_protos)

google_cloud_cpp_grpcpp_library(
google_cloud_cpp_bigtable_protos
Expand All @@ -406,8 +397,7 @@ target_link_libraries(
google-cloud-cpp::iam_v1_policy_protos
google-cloud-cpp::longrunning_operations_protos
google-cloud-cpp::rpc_status_protos
google-cloud-cpp::api_auth_protos
PRIVATE external_googleapis_common_flags)
google-cloud-cpp::api_auth_protos)

# TODO(#5660) - protobuf + gcc<6 + deprecated enums do not compile
if (NOT (("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU")
Expand Down Expand Up @@ -437,8 +427,7 @@ if (NOT (("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU")
google-cloud-cpp::api_resource_protos
google-cloud-cpp::longrunning_operations_protos
google-cloud-cpp::rpc_status_protos
google-cloud-cpp::type_latlng_protos
PRIVATE external_googleapis_common_flags)
google-cloud-cpp::type_latlng_protos)

google_cloud_cpp_grpcpp_library(
google_cloud_cpp_cloud_dialogflow_v2beta1_protos
Expand Down Expand Up @@ -467,8 +456,7 @@ if (NOT (("${CMAKE_CXX_COMPILER_ID}" STREQUAL "GNU")
google-cloud-cpp::api_resource_protos
google-cloud-cpp::longrunning_operations_protos
google-cloud-cpp::rpc_status_protos
google-cloud-cpp::type_latlng_protos
PRIVATE external_googleapis_common_flags)
google-cloud-cpp::type_latlng_protos)

list(APPEND external_googleapis_installed_libraries_list
google_cloud_cpp_cloud_dialogflow_v2_protos
Expand All @@ -488,8 +476,7 @@ target_link_libraries(
google-cloud-cpp::api_client_protos
google-cloud-cpp::api_field_behavior_protos
google-cloud-cpp::longrunning_operations_protos
google-cloud-cpp::rpc_status_protos
PRIVATE external_googleapis_common_flags)
google-cloud-cpp::rpc_status_protos)

google_cloud_cpp_grpcpp_library(
google_cloud_cpp_cloud_texttospeech_protos
Expand All @@ -502,8 +489,7 @@ target_link_libraries(
google_cloud_cpp_cloud_texttospeech_protos
PUBLIC google-cloud-cpp::api_annotations_protos
google-cloud-cpp::api_client_protos
google-cloud-cpp::api_field_behavior_protos
PRIVATE external_googleapis_common_flags)
google-cloud-cpp::api_field_behavior_protos)

google_cloud_cpp_grpcpp_library(
google_cloud_cpp_iam_protos
Expand All @@ -518,8 +504,7 @@ target_link_libraries(
PUBLIC google-cloud-cpp::api_annotations_protos
google-cloud-cpp::api_client_protos
google-cloud-cpp::api_field_behavior_protos
google-cloud-cpp::api_resource_protos
PRIVATE external_googleapis_common_flags)
google-cloud-cpp::api_resource_protos)

google_cloud_cpp_grpcpp_library(
google_cloud_cpp_logging_type_protos
Expand All @@ -529,10 +514,8 @@ google_cloud_cpp_grpcpp_library(
"${EXTERNAL_GOOGLEAPIS_SOURCE}"
"${PROTO_INCLUDE_DIR}")
external_googleapis_set_version_and_alias(logging_type_protos)
target_link_libraries(
google_cloud_cpp_logging_type_protos
PUBLIC google-cloud-cpp::api_annotations_protos
PRIVATE external_googleapis_common_flags)
target_link_libraries(google_cloud_cpp_logging_type_protos
PUBLIC google-cloud-cpp::api_annotations_protos)

google_cloud_cpp_grpcpp_library(
google_cloud_cpp_logging_protos
Expand All @@ -554,8 +537,7 @@ target_link_libraries(
google-cloud-cpp::api_monitored_resource_protos
google-cloud-cpp::api_resource_protos
google-cloud-cpp::logging_type_protos
google-cloud-cpp::rpc_status_protos
PRIVATE external_googleapis_common_flags)
google-cloud-cpp::rpc_status_protos)

google_cloud_cpp_grpcpp_library(
google_cloud_cpp_monitoring_protos
Expand Down Expand Up @@ -591,8 +573,7 @@ target_link_libraries(
google-cloud-cpp::api_monitored_resource_protos
google-cloud-cpp::api_resource_protos
google-cloud-cpp::rpc_status_protos
google-cloud-cpp::type_calendar_period_protos
PRIVATE external_googleapis_common_flags)
google-cloud-cpp::type_calendar_period_protos)

google_cloud_cpp_grpcpp_library(
google_cloud_cpp_pubsub_protos
Expand All @@ -607,8 +588,7 @@ target_link_libraries(
PUBLIC google-cloud-cpp::api_annotations_protos
google-cloud-cpp::api_client_protos
google-cloud-cpp::api_field_behavior_protos
google-cloud-cpp::api_resource_protos
PRIVATE external_googleapis_common_flags)
google-cloud-cpp::api_resource_protos)

google_cloud_cpp_grpcpp_library(
google_cloud_cpp_spanner_protos
Expand Down Expand Up @@ -636,8 +616,7 @@ target_link_libraries(
google-cloud-cpp::iam_v1_iam_policy_protos
google-cloud-cpp::iam_v1_policy_protos
google-cloud-cpp::longrunning_operations_protos
google-cloud-cpp::rpc_status_protos
PRIVATE external_googleapis_common_flags)
google-cloud-cpp::rpc_status_protos)

google_cloud_cpp_grpcpp_library(
google_cloud_cpp_storage_protos
Expand All @@ -653,16 +632,14 @@ target_link_libraries(
google-cloud-cpp::api_client_protos
google-cloud-cpp::api_field_behavior_protos
google-cloud-cpp::iam_v1_iam_policy_protos
google-cloud-cpp::iam_v1_policy_protos
PRIVATE external_googleapis_common_flags)
google-cloud-cpp::iam_v1_policy_protos)

# Install the libraries and headers in the locations determined by
# GNUInstallDirs
include(GNUInstallDirs)

install(
TARGETS ${external_googleapis_installed_libraries_list}
external_googleapis_common_flags
EXPORT googleapis-targets
RUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}
COMPONENT google_cloud_cpp_runtime
Expand All @@ -675,7 +652,6 @@ install(
# duplication).
install(
TARGETS ${external_googleapis_installed_libraries_list}
external_googleapis_common_flags
LIBRARY DESTINATION ${CMAKE_INSTALL_LIBDIR}
COMPONENT google_cloud_cpp_development
NAMELINK_ONLY
Expand Down
5 changes: 2 additions & 3 deletions generator/integration_tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ endif ()

include(CompileProtos)

# Do not run attempt to clang-tidy generated proto code.
# TODO(#5854) - enable clang-tidy for the non-generated code!
set(CMAKE_CXX_CLANG_TIDY "")

google_cloud_cpp_grpcpp_library(
Expand All @@ -48,8 +48,7 @@ target_link_libraries(
google-cloud-cpp::iam_v1_iam_policy_protos
google-cloud-cpp::iam_v1_policy_protos
google-cloud-cpp::longrunning_operations_protos
google-cloud-cpp::rpc_status_protos
PRIVATE external_googleapis_common_flags)
google-cloud-cpp::rpc_status_protos)

function (google_cloud_cpp_generator_define_integration_tests)
# The tests require googletest to be installed. Force CMake to use the
Expand Down
4 changes: 0 additions & 4 deletions google/cloud/storage/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -282,14 +282,10 @@ create_bazel_config(google_cloud_cpp_storage)

if (GOOGLE_CLOUD_CPP_STORAGE_ENABLE_GRPC)
find_package(gRPC REQUIRED)
# We need to disable clang tidy for the generated CC files.
set(saved_tidy "${CMAKE_CXX_CLANG_TIDY}")
set(CMAKE_CXX_CLANG_TIDY "")
google_cloud_cpp_proto_library(
google_cloud_cpp_storage_internal_protos
internal/grpc_resumable_upload_session_url.proto PROTO_PATH_DIRECTORIES
${CMAKE_CURRENT_SOURCE_DIR}/internal LOCAL_INCLUDE)
set(CMAKE_CXX_CLANG_TIDY "${saved_tidy}")
add_library(google-cloud-cpp::internal-storage-protos ALIAS
google_cloud_cpp_storage_internal_protos)
set_target_properties(
Expand Down
12 changes: 0 additions & 12 deletions google/cloud/storage/tests/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -58,18 +58,6 @@ if (GOOGLE_CLOUD_CPP_STORAGE_ENABLE_GRPC)
google_cloud_cpp_storage_tests_conformance_protos
conformance_tests.proto PROTO_PATH_DIRECTORIES
${CMAKE_CURRENT_SOURCE_DIR} LOCAL_INCLUDE)
if (CMAKE_VERSION VERSION_GREATER 3.6)
# This works because we're not using clang-tidy with older cmake
# versions.
set_target_properties(google_cloud_cpp_storage_tests_conformance_protos
PROPERTIES CXX_CLANG_TIDY "")
endif ()
if (MSVC)
# We're compiling generated code, so there's no use in MSVC failing on
# warnings.
target_compile_options(google_cloud_cpp_storage_tests_conformance_protos
PUBLIC "/wd4244")
endif ()
endif ()

foreach (fname ${storage_client_integration_tests})
Expand Down

0 comments on commit cb61dda

Please sign in to comment.