Update CMakeLists.txt - Check protobuf abseil dependency only when there is no CMAKE_TOOLCHAIN_FILE)

[meastp]

I'm using vcpkg and WITH_STL, and stumbled upon this error when building an updated version. I'm not entirely sure how to solve this properly, but it does look like it is only valid when building the protobuf library itself. Also, in my opinion, the check should not depend on the WITH_ABSEIL option, as this means something different and unrelated to just providing a dependency ("I want to use the absl:: types in the otel-cpp API").

I noticed the NOT DEFINED CMAKE_TOOLCHAIN_FILE elsewhere in CMakeLists.txt, perhaps adding that check is sufficient and meaningful?

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.66%. Comparing base (497eaf4) to head (bd2d0bc).
Report is 130 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3048      +/-   ##
==========================================
+ Coverage   87.12%   87.66%   +0.55%     
==========================================
  Files         200      190      -10     
  Lines        6109     5866     -243     
==========================================
- Hits         5322     5142     -180     
+ Misses        787      724      -63     

see 124 files with indirect coverage changes

[owent]

You should use WITH_ABSEIL here because protobuf depends on it, and we need to maintain compatibility by using the same version.

[ThomsonTan]

@owent mentioned, WITH_ABSEIL is needed even CMAKE_TOOLCHAIN_FILE is defined (like vcpkg is used).

Does this fix the issue? I think we need more details on it.

[meastp]

You should use WITH_ABSEIL here because protobuf depends on it, and we need to maintain compatibility by using the same version.

@owent @ThomsonTan
I did assume that not defining CMAKE_TOOLCHAIN_FILE means that no package manager is used (i.e. building dependencies like protobuf and abseil is part of the build). It is only when you build protobuf AND otel-cpp that you do have control over the abseil version used. You can not possibly know or check for consistent abseil versions when protobuf was externally built.

I think some of the cmake configuration does assume that dependencies are built using the otel-cpp build, and those assumptions shouldn't affect builds using a package manager, imho.

[owent]

You should use WITH_ABSEIL here because protobuf depends on it, and we need to maintain compatibility by using the same version.

@owent @ThomsonTan I did assume that not defining CMAKE_TOOLCHAIN_FILE means that no package manager is used (i.e. building dependencies like protobuf and abseil is part of the build). It is only when you build protobuf AND otel-cpp that you do have control over the abseil version used. You can not possibly know or check for consistent abseil versions when protobuf was externally built.

I think some of the cmake configuration does assume that dependencies are built using the otel-cpp build, and those assumptions shouldn't affect builds using a package manager, imho.

Sorry, I didn't understand your point. If you use an external protobuf version >= 3.22.0, it must be built with either bundle or external abseil-cpp. We include this check to inform users to enable abseil-cpp when using otel-cpp; otherwise, there may be conflicts in the search paths for some headers.

[meastp]

We include this check to inform users to enable abseil-cpp when using otel-cpp; otherwise, there may be conflicts in the search paths for some headers.

@owent You are assuming that abseil headers are actually used (when building otel-cpp when protobuf was externally built (with abseil)). This is not true when building otel-cpp with WITH_STL.

I the point is to inform users and not fail the build, I'd like the message to have a non-fatal severity. :)

[owent]

We include this check to inform users to enable abseil-cpp when using otel-cpp; otherwise, there may be conflicts in the search paths for some headers.

@owent You are assuming that abseil headers are actually used (when building otel-cpp when protobuf was externally built (with abseil)). This is not true when building otel-cpp with WITH_STL.

I the point is to inform users and not fail the build, I'd like the message to have a non-fatal severity. :)

Protobuf version >= 3.22.0 uses abseil-cpp headers, which conflict with the bundled abseil-cpp headers in otel-cpp located in opentelemetry/nostd/internal/absl. This check is activated only for OTLP exporters that depend on protobuf. I believe the dependency declarations are already minimal.

[meastp]

@owent ok, then - as long as it doesn't interfere with WITH_STL. Thank you for patiently explaining :)