Skip to content

Commit 00ca2ea

Browse files
owentowent
committed
Make the proto targets follow the CXX_STANDARD using by protobuf
Fixes the compatibility for old version of protobuf with C++14 used Fixes DEFINED CMAKE_CXX_STANDARD checking
1 parent c10590e commit 00ca2ea

File tree

3 files changed

+183
-67
lines changed

3 files changed

+183
-67
lines changed

‎.github/workflows/ci.yml‎

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1071,28 +1071,28 @@ jobs:
10711071

10721072
# Test case where protobuf is built with C++17 and OpenTelemetry with C++20 on MSVC will trigger a Internal compiler error
10731073
# We can enable this test case when MSVC on github runner is upgraded.
1074-
cmake_test_different_std_for_protobuf_and_otel_msvc:
1075-
name: CMake C++20 test with protobuf built with C++17 (MSVC)
1076-
runs-on: windows-2022
1077-
steps:
1078-
- name: Harden the runner (Audit all outbound calls)
1079-
uses: step-security/harden-runner@20cf305ff2072d973412fa9b1e3a4f227bda3c76 # v2.14.0
1080-
with:
1081-
egress-policy: audit
1082-
1083-
- uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1
1084-
with:
1085-
submodules: 'recursive'
1086-
- name: setup
1087-
run: |
1088-
./ci/setup_windows_ci_environment.ps1
1089-
- name: install dependencies
1090-
env:
1091-
CXX_STANDARD: '17'
1092-
run: |
1093-
pwsh -File ./ci/install_thirdparty.ps1 --install-dir "$HOME/otel-third_party" --tags-file install/cmake/third_party_latest --build-type Debug --build-shared-libs "ON" --packages "zlib;abseil;protobuf;nlohmann-json;curl;grpc;googletest;benchmark"
1094-
- name: run cmake test
1095-
run: ./ci/do_ci.ps1 cmake.different_std.test
1074+
# cmake_test_different_std_for_protobuf_and_otel_msvc:
1075+
# name: CMake C++20 test with protobuf built with C++17 (MSVC)
1076+
# runs-on: windows-2022
1077+
# steps:
1078+
# - name: Harden the runner (Audit all outbound calls)
1079+
# uses: step-security/harden-runner@20cf305ff2072d973412fa9b1e3a4f227bda3c76 # v2.14.0
1080+
# with:
1081+
# egress-policy: audit
1082+
#
1083+
# - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1
1084+
# with:
1085+
# submodules: 'recursive'
1086+
# - name: setup
1087+
# run: |
1088+
# ./ci/setup_windows_ci_environment.ps1
1089+
# - name: install dependencies
1090+
# env:
1091+
# CXX_STANDARD: '17'
1092+
# run: |
1093+
# pwsh -File ./ci/install_thirdparty.ps1 --install-dir "$HOME/otel-third_party" --tags-file install/cmake/third_party_latest --build-type Debug --build-shared-libs "ON" --packages "zlib;abseil;protobuf;nlohmann-json;curl;grpc;googletest;benchmark"
1094+
# - name: run cmake test
1095+
# run: ./ci/do_ci.ps1 cmake.different_std.test
10961096

10971097
windows_bazel:
10981098
name: Bazel Windows

‎cmake/opentelemetry-proto.cmake‎

Lines changed: 44 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,21 @@
22
# SPDX-License-Identifier: Apache-2.0
33

44
#
5-
# The dependency on opentelemetry-proto can be provided by order
6-
# of decreasing priority, options are:
5+
# The dependency on opentelemetry-proto can be provided by order of decreasing
6+
# priority, options are:
77
#
8-
# 1 - Fetch from local source directory defined by the OTELCPP_PROTO_PATH variable
8+
# 1 - Fetch from local source directory defined by the OTELCPP_PROTO_PATH
9+
# variable
910
#
10-
# 2 - Fetch from the opentelemetry-proto git submodule (opentelemetry-cpp/third_party/opentelemetry-proto)
11+
# 2 - Fetch from the opentelemetry-proto git submodule
12+
# (opentelemetry-cpp/third_party/opentelemetry-proto)
1113
#
12-
# 3 - Fetch from github using the git tag set in opentelemetry-cpp/third_party_release
14+
# 3 - Fetch from github using the git tag set in
15+
# opentelemetry-cpp/third_party_release
1316
#
1417

15-
set(OPENTELEMETRY_PROTO_SUBMODULE "${opentelemetry-cpp_SOURCE_DIR}/third_party/opentelemetry-proto")
18+
set(OPENTELEMETRY_PROTO_SUBMODULE
19+
"${opentelemetry-cpp_SOURCE_DIR}/third_party/opentelemetry-proto")
1620

1721
if(OTELCPP_PROTO_PATH)
1822
if(NOT EXISTS
@@ -21,29 +25,32 @@ if(OTELCPP_PROTO_PATH)
2125
FATAL_ERROR
2226
"OTELCPP_PROTO_PATH does not point to a opentelemetry-proto repository")
2327
endif()
24-
message(STATUS "fetching opentelemetry-proto from OTELCPP_PROTO_PATH=${OTELCPP_PROTO_PATH}")
25-
FetchContent_Declare(
26-
opentelemetry-proto
27-
SOURCE_DIR ${OTELCPP_PROTO_PATH}
28+
message(
29+
STATUS
30+
"fetching opentelemetry-proto from OTELCPP_PROTO_PATH=${OTELCPP_PROTO_PATH}"
2831
)
32+
FetchContent_Declare(opentelemetry-proto SOURCE_DIR ${OTELCPP_PROTO_PATH})
2933
set(opentelemetry-proto_PROVIDER "fetch_source")
30-
# If the opentelemetry-proto directory is a general directory then we don't have a good way to determine the version. Set it as unknown.
34+
# If the opentelemetry-proto directory is a general directory then we don't
35+
# have a good way to determine the version. Set it as unknown.
3136
set(opentelemetry-proto_VERSION "unknown")
3237
elseif(EXISTS ${OPENTELEMETRY_PROTO_SUBMODULE}/.git)
33-
message(STATUS "fetching opentelemetry-proto from git submodule")
34-
FetchContent_Declare(
35-
opentelemetry-proto
36-
SOURCE_DIR ${OPENTELEMETRY_PROTO_SUBMODULE}
37-
)
38+
message(STATUS "fetching opentelemetry-proto from git submodule")
39+
FetchContent_Declare(opentelemetry-proto SOURCE_DIR
40+
${OPENTELEMETRY_PROTO_SUBMODULE})
3841
set(opentelemetry-proto_PROVIDER "fetch_source")
39-
string(REGEX REPLACE "^v([0-9]+\\.[0-9]+\\.[0-9]+)$" "\\1" opentelemetry-proto_VERSION "${opentelemetry-proto_GIT_TAG}")
42+
string(REGEX
43+
REPLACE "^v([0-9]+\\.[0-9]+\\.[0-9]+)$" "\\1"
44+
opentelemetry-proto_VERSION "${opentelemetry-proto_GIT_TAG}")
4045
else()
4146
FetchContent_Declare(
42-
opentelemetry-proto
43-
GIT_REPOSITORY https://github.com/open-telemetry/opentelemetry-proto.git
44-
GIT_TAG "${opentelemetry-proto_GIT_TAG}")
47+
opentelemetry-proto
48+
GIT_REPOSITORY https://github.com/open-telemetry/opentelemetry-proto.git
49+
GIT_TAG "${opentelemetry-proto_GIT_TAG}")
4550
set(opentelemetry-proto_PROVIDER "fetch_repository")
46-
string(REGEX REPLACE "^v([0-9]+\\.[0-9]+\\.[0-9]+)$" "\\1" opentelemetry-proto_VERSION "${opentelemetry-proto_GIT_TAG}")
51+
string(REGEX
52+
REPLACE "^v([0-9]+\\.[0-9]+\\.[0-9]+)$" "\\1"
53+
opentelemetry-proto_VERSION "${opentelemetry-proto_GIT_TAG}")
4754
endif()
4855

4956
FetchContent_MakeAvailable(opentelemetry-proto)
@@ -191,9 +198,24 @@ endif()
191198
# opentelemetry_exporter_otlp_grpc_client as a static library.
192199
if(TARGET protobuf::libprotobuf)
193200
get_target_property(protobuf_lib_type protobuf::libprotobuf TYPE)
201+
get_target_property(protobuf_lib_compile_features protobuf::libprotobuf
202+
INTERFACE_COMPILE_FEATURES)
203+
if(DEFINED CMAKE_CXX_STANDARD AND protobuf_lib_compile_features MATCHES
204+
"(^|\\s)cxx_std_([0-9]+)(|$\\s)")
205+
# Wether the protobuf library using C++20 or higher may have different ABI.
206+
# We need to make sure our proto targets are using the same C++ standard.
207+
if(${CMAKE_MATCH_2} LESS 20 AND CMAKE_CXX_STANDARD GREATER_EQUAL 20)
208+
set(protobuf_lib_compile_features_cxx_std ${CMAKE_MATCH_2})
209+
elseif(${CMAKE_MATCH_2} GREATER_EQUAL 20 AND CMAKE_CXX_STANDARD LESS 20)
210+
set(protobuf_lib_compile_features_cxx_std ${CMAKE_MATCH_2})
211+
endif()
212+
message(
213+
STATUS
214+
"protobuf::libprotobuf detected compile features cxx_std_${CMAKE_MATCH_2}."
215+
)
216+
endif()
194217
else()
195218
set(protobuf_lib_type "SHARED_LIBRARY")
196-
target_link_libraries(opentelemetry_proto PUBLIC ${Protobuf_LIBRARIES})
197219
foreach(protobuf_lib_file ${Protobuf_LIBRARIES})
198220
if(protobuf_lib_file MATCHES
199221
"(^|[\\\\\\/])[^\\\\\\/]*protobuf[^\\\\\\/]*\\.(a|lib)$")

‎cmake/tools.cmake‎

Lines changed: 117 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -15,12 +15,19 @@ if(NOT PATCH_PROTOBUF_SOURCES_OPTIONS_SET)
1515
if(MSVC)
1616
unset(PATCH_PROTOBUF_SOURCES_OPTIONS CACHE)
1717
set(PATCH_PROTOBUF_SOURCES_OPTIONS
18+
/wd4100
1819
/wd4244
1920
/wd4251
2021
/wd4267
2122
/wd4309
2223
/wd4668
24+
/wd4702
25+
/wd4715
26+
/wd4800
27+
/wd4090
2328
/wd4946
29+
/wd4996
30+
/wd5054
2431
/wd6001
2532
/wd6244
2633
/wd6246)
@@ -43,8 +50,15 @@ if(NOT PATCH_PROTOBUF_SOURCES_OPTIONS_SET)
4350
unset(PATCH_PROTOBUF_SOURCES_OPTIONS CACHE)
4451
include(CheckCXXCompilerFlag)
4552
check_append_cxx_compiler_flag(
46-
PATCH_PROTOBUF_SOURCES_OPTIONS -Wno-type-limits
47-
-Wno-deprecated-declarations -Wno-unused-parameter)
53+
PATCH_PROTOBUF_SOURCES_OPTIONS
54+
-Wno-type-limits
55+
-Wno-sign-compare
56+
-Wno-sign-conversion
57+
-Wno-shadow
58+
-Wno-uninitialized
59+
-Wno-conversion
60+
-Wno-deprecated-declarations
61+
-Wno-unused-parameter)
4862
endif()
4963
set(PATCH_PROTOBUF_SOURCES_OPTIONS_SET TRUE)
5064
if(PATCH_PROTOBUF_SOURCES_OPTIONS)
@@ -56,41 +70,121 @@ if(NOT PATCH_PROTOBUF_SOURCES_OPTIONS_SET)
5670
endif()
5771

5872
function(patch_protobuf_sources)
59-
if(PATCH_PROTOBUF_SOURCES_OPTIONS)
60-
foreach(PROTO_SRC ${ARGN})
61-
unset(PROTO_SRC_OPTIONS)
62-
get_source_file_property(PROTO_SRC_OPTIONS ${PROTO_SRC} COMPILE_OPTIONS)
63-
if(PROTO_SRC_OPTIONS)
64-
list(APPEND PROTO_SRC_OPTIONS ${PATCH_PROTOBUF_SOURCES_OPTIONS})
73+
if(protobuf_lib_compile_features_cxx_std)
74+
if(MSVC)
75+
set(__additional_cxx_standard
76+
"/std:c++${protobuf_lib_compile_features_cxx_std}")
77+
else()
78+
set(__additional_cxx_standard
79+
"-std=c++${protobuf_lib_compile_features_cxx_std}")
80+
endif()
81+
endif()
82+
83+
foreach(PROTO_SRC ${ARGN})
84+
unset(PROTO_SRC_OPTIONS)
85+
set(__proto_src_options_changed FALSE)
86+
get_source_file_property(PROTO_SRC_OPTIONS ${PROTO_SRC} COMPILE_OPTIONS)
87+
if(PROTO_SRC_OPTIONS)
88+
set(__need_cxx_standard TRUE)
89+
if(NOT protobuf_lib_compile_features_cxx_std
90+
OR PROTO_SRC_OPTIONS MATCHES
91+
"(/std:|-std=)(c|gnu)\\+\\+${protobuf_lib_compile_features_cxx_std}"
92+
)
93+
set(__need_cxx_standard FALSE)
6594
else()
66-
set(PROTO_SRC_OPTIONS ${PATCH_PROTOBUF_SOURCES_OPTIONS})
95+
if(MSVC)
96+
string(REGEX REPLACE "/std:c\\+\\+[0-9a-zA-Z_]+" "" PROTO_SRC_OPTIONS
97+
"${PROTO_SRC_OPTIONS}")
98+
endif()
99+
string(REGEX REPLACE "-std=(c|gnu)\\+\\+[0-9a-zA-Z_]+" ""
100+
PROTO_SRC_OPTIONS "${PROTO_SRC_OPTIONS}")
101+
set(__need_cxx_standard TRUE)
102+
endif()
103+
104+
foreach(TEST_OPTION ${PATCH_PROTOBUF_SOURCES_OPTIONS})
105+
if(NOT "${TEST_OPTION}" IN_LIST PROTO_SRC_OPTIONS)
106+
list(APPEND PROTO_SRC_OPTIONS "${TEST_OPTION}")
107+
set(__proto_src_options_changed TRUE)
108+
endif()
109+
endforeach()
110+
if(__need_cxx_standard)
111+
list(APPEND PROTO_SRC_OPTIONS "${__additional_cxx_standard}")
112+
set(__proto_src_options_changed TRUE)
113+
endif()
114+
else()
115+
set(PROTO_SRC_OPTIONS ${PATCH_PROTOBUF_SOURCES_OPTIONS})
116+
if(__additional_cxx_standard)
117+
list(APPEND PROTO_SRC_OPTIONS "${__additional_cxx_standard}")
67118
endif()
119+
set(__proto_src_options_changed TRUE)
120+
endif()
68121

122+
if(__proto_src_options_changed)
69123
set_source_files_properties(
70124
${PROTO_SRC} PROPERTIES COMPILE_OPTIONS "${PROTO_SRC_OPTIONS}")
71-
endforeach()
72-
unset(PROTO_SRC)
73-
unset(PROTO_SRC_OPTIONS)
74-
endif()
125+
endif()
126+
endforeach()
75127
endfunction()
76128

77129
function(patch_protobuf_targets)
78-
if(PATCH_PROTOBUF_SOURCES_OPTIONS)
130+
if(protobuf_lib_compile_features_cxx_std)
79131
foreach(PROTO_TARGET ${ARGN})
80-
unset(PROTO_TARGET_OPTIONS)
81-
get_target_property(PROTO_TARGET_OPTIONS ${PROTO_TARGET} COMPILE_OPTIONS)
82-
if(PROTO_TARGET_OPTIONS)
83-
list(APPEND PROTO_TARGET_OPTIONS ${PATCH_PROTOBUF_SOURCES_OPTIONS})
132+
set_target_properties(
133+
${PROTO_TARGET} PROPERTIES CXX_STANDARD
134+
${protobuf_lib_compile_features_cxx_std})
135+
endforeach()
136+
137+
if(MSVC)
138+
set(__additional_cxx_standard
139+
"/std:c++${protobuf_lib_compile_features_cxx_std}")
140+
else()
141+
set(__additional_cxx_standard
142+
"-std=c++${protobuf_lib_compile_features_cxx_std}")
143+
endif()
144+
endif()
145+
146+
foreach(PROTO_TARGET ${ARGN})
147+
set(__proto_target_options_changed FALSE)
148+
get_target_property(PROTO_TARGET_OPTIONS ${PROTO_TARGET} COMPILE_OPTIONS)
149+
if(PROTO_TARGET_OPTIONS)
150+
set(__need_cxx_standard TRUE)
151+
if(NOT protobuf_lib_compile_features_cxx_std
152+
OR PROTO_TARGET_OPTIONS MATCHES
153+
"(/std:|-std=)(c|gnu)\\+\\+${protobuf_lib_compile_features_cxx_std}"
154+
)
155+
set(__need_cxx_standard FALSE)
84156
else()
85-
set(PROTO_TARGET_OPTIONS ${PATCH_PROTOBUF_SOURCES_OPTIONS})
157+
if(MSVC)
158+
string(REGEX REPLACE "/std:c\\+\\+[0-9a-zA-Z_]+" ""
159+
PROTO_TARGET_OPTIONS "${PROTO_TARGET_OPTIONS}")
160+
endif()
161+
string(REGEX REPLACE "-std=(c|gnu)\\+\\+[0-9a-zA-Z_]+" ""
162+
PROTO_TARGET_OPTIONS "${PROTO_TARGET_OPTIONS}")
163+
set(__need_cxx_standard TRUE)
86164
endif()
87165

166+
foreach(TEST_OPTION ${PATCH_PROTOBUF_SOURCES_OPTIONS})
167+
if(NOT "${TEST_OPTION}" IN_LIST PROTO_TARGET_OPTIONS)
168+
list(APPEND PROTO_TARGET_OPTIONS "${TEST_OPTION}")
169+
set(__proto_target_options_changed TRUE)
170+
endif()
171+
endforeach()
172+
if(__need_cxx_standard)
173+
list(APPEND PROTO_TARGET_OPTIONS "${__additional_cxx_standard}")
174+
set(__proto_target_options_changed TRUE)
175+
endif()
176+
else()
177+
set(PROTO_TARGET_OPTIONS ${PATCH_PROTOBUF_SOURCES_OPTIONS})
178+
if(__additional_cxx_standard)
179+
list(APPEND PROTO_TARGET_OPTIONS "${__additional_cxx_standard}")
180+
endif()
181+
set(__proto_target_options_changed TRUE)
182+
endif()
183+
if(__proto_target_options_changed)
88184
set_target_properties(
89185
${PROTO_TARGET} PROPERTIES COMPILE_OPTIONS "${PROTO_TARGET_OPTIONS}")
90-
endforeach()
91-
unset(PROTO_TARGET)
92-
unset(PROTO_TARGET_OPTIONS)
93-
endif()
186+
endif()
187+
endforeach()
94188
endfunction()
95189

96190
function(project_build_tools_get_imported_location OUTPUT_VAR_NAME TARGET_NAME)

0 commit comments

Comments
 (0)