Skip to content

[WIP] Adjust the standards for proto target to align with those of Protocol Buffers. #3800

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.

Sign up for GitHub

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

Open
owent wants to merge 9 commits into
base: main
Choose a base branch
from
+569 −76
Open

[WIP] Adjust the standards for proto target to align with those of Protocol Buffers. #3800

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
0ec2b1f
Reproduce #3799
owent Jan 6, 2026
c3edc6f
Fixes third_party_latest PATH
owent Jan 6, 2026
ed2b117
install latest third party libs for windows
owent Jan 7, 2026
43ce7c0
Fixes copyright and parallel
owent Jan 7, 2026
63110f9
try to reproduce link error
owent Jan 8, 2026
55a82c5
Allow install_thirdparty to set BUILD_SHARED_LIBS
owent Jan 8, 2026
a9184fe
Make the proto targets follow the CXX_STANDARD using by protobuf
owent Jan 9, 2026
1c29939
Fixes compiling
owent Jan 10, 2026
7cab857
Fixes spelling
owent Jan 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
50 changes: 50 additions & 0 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -587,6 +587,31 @@ jobs:
CXX_STANDARD: '23'
run: ./ci/do_ci.sh cmake.c++23.stl.test

cmake_test_different_std_for_protobuf_and_otel_gcc:
name: CMake C++23 test with protobuf built with C++17 (GCC)
runs-on: ubuntu-latest
steps:
- name: Harden the runner (Audit all outbound calls)
uses: step-security/harden-runner@20cf305ff2072d973412fa9b1e3a4f227bda3c76 # v2.14.0
with:
egress-policy: audit

- uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1
with:
submodules: 'recursive'
- name: setup
run: |
sudo -E ./ci/setup_ci_environment.sh
- name: install dependencies
env:
CXX_STANDARD: '17'
run: |
sudo -E ./ci/install_thirdparty.sh --install-dir /usr/local --tags-file install/cmake/third_party_latest --build-shared-libs "ON" --packages "zlib;abseil;protobuf;nlohmann-json;curl;grpc;googletest;benchmark"
- name: run tests
env:
CXX_STANDARD: '23'
run: ./ci/do_ci.sh cmake.different_std.test

cmake_otprotocol_test:
name: CMake test (with otlp-exporter)
runs-on: ubuntu-22.04
Expand Down Expand Up @@ -1044,6 +1069,31 @@ jobs:
- name: run otprotocol test
run: ./ci/do_ci.ps1 cmake.exporter.otprotocol.with_async_export.test

# Test case where protobuf is built with C++17 and OpenTelemetry with C++20 on MSVC will trigger a Internal compiler error
# We can enable this test case when MSVC on github runner is upgraded.
# cmake_test_different_std_for_protobuf_and_otel_msvc:
# name: CMake C++20 test with protobuf built with C++17 (MSVC)
# runs-on: windows-2022
# steps:
# - name: Harden the runner (Audit all outbound calls)
# uses: step-security/harden-runner@20cf305ff2072d973412fa9b1e3a4f227bda3c76 # v2.14.0
# with:
# egress-policy: audit
#
# - uses: actions/checkout@8e8c483db84b4bee98b60c0593521ed34d9990e8 # v6.0.1
# with:
# submodules: 'recursive'
# - name: setup
# run: |
# ./ci/setup_windows_ci_environment.ps1
# - name: install dependencies
# env:
# CXX_STANDARD: '17'
# run: |
# 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"
# - name: run cmake test
# run: ./ci/do_ci.ps1 cmake.different_std.test

windows_bazel:
name: Bazel Windows
runs-on: windows-2022
Expand Down
78 changes: 54 additions & 24 deletions ci/do_ci.ps1
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ switch ($action) {
cmake $SRC_DIR `
-DVCPKG_TARGET_TRIPLET=x64-windows `
-DOPENTELEMETRY_BUILD_DLL=1 `
"-DCMAKE_TOOLCHAIN_FILE=$VCPKG_DIR/scripts/buildsystems/vcpkg.cmake"
"-DCMAKE_TOOLCHAIN_FILE=$VCPKG_DIR/scripts/buildsystems/vcpkg.cmake"
$exit = $LASTEXITCODE
if ($exit -ne 0) {
exit $exit
Expand All @@ -90,7 +90,7 @@ switch ($action) {
-DCMAKE_CXX_STANDARD=20 `
-DVCPKG_TARGET_TRIPLET=x64-windows `
-DOPENTELEMETRY_BUILD_DLL=1 `
"-DCMAKE_TOOLCHAIN_FILE=$VCPKG_DIR/scripts/buildsystems/vcpkg.cmake"
"-DCMAKE_TOOLCHAIN_FILE=$VCPKG_DIR/scripts/buildsystems/vcpkg.cmake"
$exit = $LASTEXITCODE
if ($exit -ne 0) {
exit $exit
Expand Down Expand Up @@ -208,7 +208,7 @@ switch ($action) {
"-C $SRC_DIR/test_common/cmake/all-options-abiv1-preview.cmake" `
-DWITH_OPENTRACING=OFF `
-DVCPKG_TARGET_TRIPLET=x64-windows `
"-DCMAKE_TOOLCHAIN_FILE=$VCPKG_DIR/scripts/buildsystems/vcpkg.cmake"
"-DCMAKE_TOOLCHAIN_FILE=$VCPKG_DIR/scripts/buildsystems/vcpkg.cmake"
$exit = $LASTEXITCODE
if ($exit -ne 0) {
exit $exit
Expand Down Expand Up @@ -270,6 +270,35 @@ switch ($action) {
exit $exit
}
}
"cmake.different_std.test" {
cd "$BUILD_DIR"
cmake $SRC_DIR `
"-DCMAKE_PREFIX_PATH=$HOME/otel-third_party" `
"-DCMAKE_FIND_ROOT_PATH=$HOME/otel-third_party" `
-DCMAKE_BUILD_TYPE=Debug `
-DWITH_METRICS_EXEMPLAR_PREVIEW=ON `
-DWITH_ASYNC_EXPORT_PREVIEW=ON `
-DWITH_STL=CXX20 `
-DCMAKE_CXX_STANDARD=23 `
-DCMAKE_CXX_STANDARD_REQUIRED=ON `
-DWITH_OTLP_GRPC=ON `
-DWITH_OTLP_HTTP=ON `
-DWITH_OTLP_FILE=ON
$exit = $LASTEXITCODE
if ($exit -ne 0) {
exit $exit
}
cmake --build . -j $nproc --config Debug
$exit = $LASTEXITCODE
if ($exit -ne 0) {
exit $exit
}
ctest -C Debug
$exit = $LASTEXITCODE
if ($exit -ne 0) {
exit $exit
}
}
"cmake.build_example_plugin" {
cd "$BUILD_DIR"
cmake $SRC_DIR `
Expand Down Expand Up @@ -316,21 +345,22 @@ switch ($action) {
cd "$BUILD_DIR"

if (Test-Path Env:\CXX_STANDARD) {
$CXX_STANDARD = [int](Get-Item Env:\CXX_STANDARD).Value
} else {
$CXX_STANDARD = 17
$CXX_STANDARD = [int](Get-Item Env:\CXX_STANDARD).Value
}
else {
$CXX_STANDARD = 17
}
if (-not $CXX_STANDARD) {
$CXX_STANDARD = 17
$CXX_STANDARD = 17
}
Write-Host "Using CXX_STANDARD: $CXX_STANDARD"

$CMAKE_OPTIONS = @(
"-DCMAKE_CXX_STANDARD=$CXX_STANDARD",
"-DCMAKE_CXX_STANDARD_REQUIRED=ON",
"-DCMAKE_CXX_EXTENSIONS=OFF",
"-DVCPKG_TARGET_TRIPLET=x64-windows",
"-DCMAKE_TOOLCHAIN_FILE=$VCPKG_DIR/scripts/buildsystems/vcpkg.cmake"
"-DCMAKE_CXX_STANDARD=$CXX_STANDARD",
"-DCMAKE_CXX_STANDARD_REQUIRED=ON",
"-DCMAKE_CXX_EXTENSIONS=OFF",
"-DVCPKG_TARGET_TRIPLET=x64-windows",
"-DCMAKE_TOOLCHAIN_FILE=$VCPKG_DIR/scripts/buildsystems/vcpkg.cmake"
)

cmake $SRC_DIR `
Expand Down Expand Up @@ -390,10 +420,10 @@ switch ($action) {
cd "$BUILD_DIR\install_test"

cmake $CMAKE_OPTIONS `
"-DCMAKE_PREFIX_PATH=$INSTALL_TEST_DIR" `
"-DINSTALL_TEST_CMAKE_OPTIONS=$CMAKE_OPTIONS_STRING" `
"-DINSTALL_TEST_COMPONENTS=$EXPECTED_COMPONENTS_STRING" `
-S "$SRC_DIR\install\test\cmake"
"-DCMAKE_PREFIX_PATH=$INSTALL_TEST_DIR" `
"-DINSTALL_TEST_CMAKE_OPTIONS=$CMAKE_OPTIONS_STRING" `
"-DINSTALL_TEST_COMPONENTS=$EXPECTED_COMPONENTS_STRING" `
-S "$SRC_DIR\install\test\cmake"

$exit = $LASTEXITCODE
if ($exit -ne 0) {
Expand All @@ -414,10 +444,10 @@ switch ($action) {
Remove-Item -Recurse -Force "$INSTALL_TEST_DIR\*"

$CMAKE_OPTIONS = @(
"-DCMAKE_CXX_STANDARD=17",
"-DVCPKG_TARGET_TRIPLET=x64-windows",
"-DCMAKE_TOOLCHAIN_FILE=$VCPKG_DIR/scripts/buildsystems/vcpkg.cmake",
"-DOPENTELEMETRY_BUILD_DLL=1"
"-DCMAKE_CXX_STANDARD=17",
"-DVCPKG_TARGET_TRIPLET=x64-windows",
"-DCMAKE_TOOLCHAIN_FILE=$VCPKG_DIR/scripts/buildsystems/vcpkg.cmake",
"-DOPENTELEMETRY_BUILD_DLL=1"
)

cmake $SRC_DIR `
Expand Down Expand Up @@ -485,10 +515,10 @@ switch ($action) {
cd "$BUILD_DIR\install_test"

cmake $CMAKE_OPTIONS `
"-DCMAKE_PREFIX_PATH=$INSTALL_TEST_DIR" `
"-DINSTALL_TEST_CMAKE_OPTIONS=$CMAKE_OPTIONS_STRING" `
"-DINSTALL_TEST_COMPONENTS=$EXPECTED_COMPONENTS_STRING" `
-S "$SRC_DIR\install\test\cmake"
"-DCMAKE_PREFIX_PATH=$INSTALL_TEST_DIR" `
"-DINSTALL_TEST_CMAKE_OPTIONS=$CMAKE_OPTIONS_STRING" `
"-DINSTALL_TEST_COMPONENTS=$EXPECTED_COMPONENTS_STRING" `
-S "$SRC_DIR\install\test\cmake"

$exit = $LASTEXITCODE
if ($exit -ne 0) {
Expand Down
16 changes: 16 additions & 0 deletions ci/do_ci.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,22 @@ elif [[ "$1" == "cmake.c++23.stl.test" ]]; then
eval "$MAKE_COMMAND"
make test
exit 0
elif [[ "$1" == "cmake.different_std.test" ]]; then
cd "${BUILD_DIR}"
rm -rf *
cmake "${CMAKE_OPTIONS[@]}" \
-DWITH_METRICS_EXEMPLAR_PREVIEW=ON \
-DCMAKE_CXX_FLAGS="-Werror $CXXFLAGS" \
-DWITH_ASYNC_EXPORT_PREVIEW=ON \
Copy link

Copilot AI Jan 6, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The PowerShell script sets DWITH_STL=CXX20 (line 278) and also explicitly sets DCMAKE_CXX_STANDARD=20 (line 279). However, the bash script only sets DWITH_STL=CXX23 (line 343) without explicitly setting CMAKE_CXX_STANDARD. This inconsistency could lead to different behaviors. Consider either adding CMAKE_CXX_STANDARD to the bash script or removing it from the PowerShell script to maintain consistency, or align both to use the same C++ standard version.

Suggested change
-DWITH_ASYNC_EXPORT_PREVIEW=ON \
-DWITH_ASYNC_EXPORT_PREVIEW=ON \
-DCMAKE_CXX_STANDARD=23 \

Copilot uses AI. Check for mistakes.
Copy link
Member Author

@owent owent Jan 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The environment already set CMAKE_CXX_STANDARD, it should not be set here again.

-DBUILD_SHARED_LIBS=ON \
-DWITH_STL=CXX23 \
-DWITH_OTLP_GRPC=ON \
-DWITH_OTLP_HTTP=ON \
-DWITH_OTLP_FILE=ON \
"${SRC_DIR}"
eval "$MAKE_COMMAND"
make test
exit 0
elif [[ "$1" == "cmake.legacy.test" ]]; then
cd "${BUILD_DIR}"
rm -rf *
Expand Down
Loading
Loading