Cmake porting by owent · Pull Request #432 · open-telemetry/opentelemetry-cpp

owent · 2020-12-09T07:20:50Z

Add and install cmake config for all library targets and export target with the same name as bazel.

codecov · 2020-12-09T08:53:01Z

Codecov Report

Merging #432 (24d07d1) into master (7e06316) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #432   +/-   ##
=======================================
  Coverage   94.37%   94.37%           
=======================================
  Files         189      189           
  Lines        8410     8410           
=======================================
  Hits         7937     7937           
  Misses        473      473

Impacted Files	Coverage Δ
sdk/test/metrics/counter_aggregator_test.cc	`98.21% <0.00%> (-1.79%)`	⬇️
sdk/test/common/circular_buffer_test.cc	`100.00% <0.00%> (+1.02%)`	⬆️

maxgolov · 2020-12-11T17:12:41Z

+    endif()
+    if(VCPKG_TARGET_TRIPLET MATCHES "^.*windows-static$")
+      set(CMAKE_MSVC_RUNTIME_LIBRARY
+          "MultiThreaded$<$<CONFIG:Debug>:Debug>"


Why is it only Debug?

These codes mean set CMAKE_MSVC_RUNTIME_LIBRARY to MultiThreadedDebug when CMAKE_BUILD_TYPE is Debug , otherwise set CMAKE_MSVC_RUNTIME_LIBRARY to MultiThreaded . Just like https://github.com/microsoft/vcpkg/blob/master/scripts/toolchains/windows.cmake#L3 in vcpkg.

These codes are a old patch to compact for both x64/x86-windows-static and x64/x86-windows-static-md and can be replaced by the new codes set(CMAKE_MSVC_RUNTIME_LIBRARY "MultiThreaded$<$<CONFIG:Debug>:Debug>$<$<STREQUAL:${VCPKG_CRT_LINKAGE},dynamic>:DLL>" CACHE STRING "") now. I will update them later.

@maxgolov I have already update the cmake script and rebased from master again.

maxgolov

It looks good to me, but I'd like (selfishly) merge my way older PR first ( #374 ), that older one is a bigger PR that does changes in many cmake files. So if we merge this current PR, then yet again I would run into conflicts and have to spend yet another day to merge and retest.. sigh 😄

owent · 2020-12-19T03:22:19Z

It looks good to me, but I'd like (selfishly) merge my way older PR first ( #374 ), that older one is a bigger PR that does changes in many cmake files. So if we merge this current PR, then yet again I would run into conflicts and have to spend yet another day to merge and retest.. sigh 😄

Maybe I can create another PR witch is based of maxgolov/nostd_stl ?

maxgolov · 2020-12-22T20:05:06Z

@owt5008137 - No, it's now merged in the master. You're good. The changes look good to me in general. Thank you for your contribution! Would you be able to resolve the conflicts with the master and update the this PR?

@jsuereth @alolita - we need review from someone outside of Microsoft on this.

owent · 2020-12-23T07:49:23Z

@owt5008137 - No, it's now merged in the master. You're good. The changes look good to me in general. Thank you for your contribution! Would you be able to resolve the conflicts with the master and update the this PR?

@jsuereth @alolita - we need review from someone outside of Microsoft on this.

@maxgolov All conflicts are resolved. And I submit another commit to fix curl/curl.h not found when using custom built and installed libcurl. The old usage do not support cmake config files generated by curl and there is also a spelling mistake.

ThomsonTan · 2020-12-23T08:05:42Z

+include(GNUInstallDirs)
+include(CMakePackageConfigHelpers)


Don't include these 2 for non-posix?

The GNUInstallDirs module only defines some directory names to install. And CMakePackageConfigHelpers is used to generate the standardized cmake config files, they are used by find_package(NAME CONFIG) of cmake. These two modules are useful on both posix and non-posix platforms.

lalitb

LGTM. Thanks for adding install configurations.

lalitb · 2020-12-28T15:31:59Z

@owt5008137 - Can you please rebase the branch with master, so that we can merge the PR.

…_ROOT_PATH=<PROTOBUF INSTALL PREFIX DIR>``` with custom prebuilt protobuf. open-telemetry#424

…t with the same name as bazel

> See https://github.com/microsoft/vcpkg/blob/6c7f2d95d889ff9757555511ffe19d5157ebde07/scripts/toolchains/windows.cmake#L3

> Merge [open-telemetry#374](open-telemetry#374) and rebase from master again.

… libcurl

lalitb · 2020-12-29T09:01:54Z

@jsuereth -this is cmake- specific, and reviewed by @maxgolov and @ThomsonTan . Please let us know if this is ok to merge.

owent · 2020-12-29T10:42:15Z

@owt5008137 - Can you please rebase the branch with master, so that we can merge the PR.

done

…and-patch-dependencies Update otel/opentelemetry-collector Docker tag to v0.131.1

owent requested a review from a team December 9, 2020 07:20

owent force-pushed the cmake_porting branch 2 times, most recently from e5691fc to 1577238 Compare December 9, 2020 08:43

owent mentioned this pull request Dec 9, 2020

Compile error with google/protobuf/port_def.inc when using prebuilt protobuf #424

Closed

maxgolov reviewed Dec 11, 2020

View reviewed changes

owent force-pushed the cmake_porting branch from 4df2605 to 59af364 Compare December 14, 2020 03:43

maxgolov approved these changes Dec 18, 2020

View reviewed changes

owent force-pushed the cmake_porting branch from 59af364 to 881b19c Compare December 23, 2020 06:46

ThomsonTan approved these changes Dec 23, 2020

View reviewed changes

lalitb approved these changes Dec 23, 2020

View reviewed changes

lalitb requested a review from jsuereth December 23, 2020 10:02

owent added 6 commits December 29, 2020 10:49

Fix compile error when using cmake and ```-DWITH_OTLP=ON -DCMAKE_FIND…

65da5fd

…_ROOT_PATH=<PROTOBUF INSTALL PREFIX DIR>``` with custom prebuilt protobuf. open-telemetry#424

Add and install cmake config for all library targets and export targe…

039797b

…t with the same name as bazel

Fix test dependency, fix formatter for *.cmake.in

9a88c49

Using the same MSVC runtime as vcpkg as when using it.

72ffb06

> See https://github.com/microsoft/vcpkg/blob/6c7f2d95d889ff9757555511ffe19d5157ebde07/scripts/toolchains/windows.cmake#L3

Add new target opentelemetry-cpp::ostream_log_exporter to installer.

bd67bff

> Merge [open-telemetry#374](open-telemetry#374) and rebase from master again.

Fix curl/curl.h not found when using custom built and installed…

24d07d1

… libcurl

owent force-pushed the cmake_porting branch from a5da4fd to 24d07d1 Compare December 29, 2020 02:50

lalitb merged commit 98bbdd6 into open-telemetry:master Dec 29, 2020

owent mentioned this pull request Dec 30, 2020

Partly fix #424, compile error with custom prebuilt protobuf #428

Closed

ThomsonTan mentioned this pull request Jan 13, 2021

CMake find_package failed to import opentelemetry-cpp #513

Closed

owent deleted the cmake_porting branch May 28, 2021 02:07

owent mentioned this pull request Nov 24, 2021

REQUEST: New membership for @owent open-telemetry/community#920

Closed

6 tasks

GerHobbelt pushed a commit to GerHobbelt/opentelemetry-cpp that referenced this pull request Aug 31, 2025

Merge pull request open-telemetry#432 from malkia/renovate/all-minor-…

7b3723d

…and-patch-dependencies Update otel/opentelemetry-collector Docker tag to v0.131.1

		include(GNUInstallDirs)
		include(CMakePackageConfigHelpers)

Conversation

owent commented Dec 9, 2020

Uh oh!

codecov Bot commented Dec 9, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

maxgolov Dec 11, 2020

Choose a reason for hiding this comment

Uh oh!

owent Dec 12, 2020

Choose a reason for hiding this comment

Uh oh!

owent Dec 14, 2020

Choose a reason for hiding this comment

Uh oh!

maxgolov left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

owent commented Dec 19, 2020

Uh oh!

maxgolov commented Dec 22, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

owent commented Dec 23, 2020

Uh oh!

ThomsonTan Dec 23, 2020

Choose a reason for hiding this comment

Uh oh!

owent Dec 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lalitb left a comment

Choose a reason for hiding this comment

Uh oh!

lalitb commented Dec 28, 2020

Uh oh!

lalitb commented Dec 29, 2020

Uh oh!

owent commented Dec 29, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

codecov Bot commented Dec 9, 2020 •

edited

Loading

maxgolov left a comment •

edited

Loading

maxgolov commented Dec 22, 2020 •

edited

Loading

owent Dec 28, 2020 •

edited

Loading