Don't require applications using jaeger exporter to know about libcurl

[astitcher]

Fixes #1517

Changes

Changed link of libcurl (CMake CURL::libcurl) to opentelemetry_http_client_curl to be PRIVATE. This fixes the build issue for me on my Linux system using shared libraries, however I've not tested this anywhere else.

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: astitcher / name: Andrew Stitcher (710d483)

[codecov]

Codecov Report

Merging #1518 (60b0262) into main (30b4fcd) will decrease coverage by 0.07%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #1518      +/-   ##
==========================================
- Coverage   84.68%   84.61%   -0.06%     
==========================================
  Files         155      155              
  Lines        4782     4782              
==========================================
- Hits         4049     4046       -3     
- Misses        733      736       +3     

Impacted Files	Coverage Δ
ext/src/http/client/curl/http_client_curl.cc	79.93% <0.00%> (-1.13%)	⬇️

[esigo]

@astitcher thanks for the PR, could you please fix the format as explained here?

[astitcher]

@astitcher thanks for the PR, could you please fix the format as explained here?

Hmm, I didn't see anywhere there that referenced the cmake formatting spec or the tool to do it - can you be more specific?

[esigo]

@astitcher thanks for the PR, could you please fix the format as explained here?

Hmm, I didn't see anywhere there that referenced the cmake formatting spec or the tool to do it - can you be more specific?

This part I mean:

git checkout -b feature
# edit files
tools/format.sh
git commit
git push fork feature

so running tools/format.sh should do the job.

[astitcher]

Thanks - looks like I need to install cmake-format...

[ThomsonTan]

Thanks - looks like I need to install cmake-format...

For one file change, you may also try apply the diff mentioned int he build log (see below link) which should fix the format.

https://github.com/open-telemetry/opentelemetry-cpp/runs/7522769448?check_suite_focus=true

[esigo]

LGTM
Thanks for the PR :)

[owent]

If we build opentelemetry_http_client_curl as static library, we still required to let the linker knows and links curl before opentelemetry_http_client_curl if any library use it. This PR breaks this dependency, I wonder if is there any better solution to solve this problem?

[astitcher]

Are you sure this is a problem? I built opentelemetry-cpp with the jaeger exporter as a static library on my Linux machine and CMake did the right thing (CMake is documented to treat static and shared transitive dependencies differently).
The import module that cmake generated added libcurl and libthrift to the libraries l;inked for the final executable which is the previous behavior requiring the application to add find_package(CURL ...) and find_package(thrift ...).
@owent could you try building/installing with main and seeing if you actually have a problem compared to the previous situation - because for me static libraries are unchanged.

[owent]

Are you sure this is a problem? I built opentelemetry-cpp with the jaeger exporter as a static library on my Linux machine and CMake did the right thing (CMake is documented to treat static and shared transitive dependencies differently). The import module that cmake generated added libcurl and libthrift to the libraries l;inked for the final executable which is the previous behavior requiring the application to add find_package(CURL ...) and find_package(thrift ...). @owent could you try building/installing with main and seeing if you actually have a problem compared to the previous situation - because for me static libraries are unchanged.

You are right, sorry for my mistake and please ignore my reply.
But where is the document about the transitive dependencies of linking?I only find the document about definitions.