8000 [CMAKE] Add CMake scripts to find or fetch curl and find zlib by dbarker · Pull Request #3526 · open-telemetry/opentelemetry-cpp · GitHub
[go: up one dir, main page]

Skip to content

Conversation

dbarker
Copy link
Member
@dbarker dbarker commented Jul 9, 2025

Fixes # (issue)

Changes

  • Add cmake script to find or fetch curl
  • Add cmake script to find zlib
  • Test fetching curl from github in CI

For significant contributions please make sure you have completed the following items:

  • CHANGELOG.md updated for non-trivial changes
  • Unit tests have been added
  • Changes in public API reviewed

Copy link
netlify bot commented Jul 9, 2025

Deploy Preview for opentelemetry-cpp-api-docs canceled.

Name Link
🔨 Latest commit da13a6d
🔍 Latest deploy log https://app.netlify.com/projects/opentelemetry-cpp-api-docs/deploys/687569e9f65f1400083d5e94

Copy link
codecov bot commented Jul 9, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.98%. Comparing base (62ba4de) to head (da13a6d).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #3526      +/-   ##
==========================================
- Coverage   90.01%   89.98%   -0.02%     
==========================================
  Files         219      219              
  Lines        7044     7044              
==========================================
- Hits         6340     6338       -2     
- Misses        704      706       +2     

see 2 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dbarker dbarker marked this pull request as ready for review July 10, 2025 01:21
@dbarker dbarker requested a review from a team as a code owner July 10, 2025 01:21
@lalitb lalitb requested a review from Copilot July 14, 2025 17:28
Copy link
@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR modernizes the CMake build system by adding dedicated scripts to find or fetch curl and zlib dependencies, replacing the legacy variable-based approach with modern CMake imported targets.

  • Introduces new CMake scripts for curl and zlib dependency management with fallback to FetchContent
  • Replaces legacy ${CURL_LIBRARIES} and ${ZLIB_LIBRARIES} with modern imported targets CURL::libcurl and ZLIB::ZLIB
  • Updates CI configuration to test fetching curl from GitHub instead of using system packages

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
cmake/curl.cmake New script to find or fetch curl with FetchContent fallback
cmake/zlib.cmake New script to find zlib with modern target support
CMakeLists.txt Reorganizes dependency detection and integrates new cmake scripts
ext/src/http/client/curl/CMakeLists.txt Simplifies curl linking by removing legacy target detection logic
ext/test/http/CMakeLists.txt Replaces complex curl target detection with direct CURL::libcurl usage
ext/test/w3c_tracecontext_http_test_server/CMakeLists.txt Updates curl linking to use modern imported target
exporters/zipkin/CMakeLists.txt Updates curl linking to use modern imported target
examples/http/CMakeLists.txt Updates curl linking to use modern imported target
.github/workflows/ci.yml Removes libcurl4-openssl-dev from CI dependencies to test fetching

@marcalff marcalff merged commit 4da9b67 into open-telemetry:main Jul 15, 2025
70 checks passed
@dbarker dbarker deleted the cmake_cleanup_curl_and_zlib branch July 15, 2025 15:33
malkia added a commit to malkia/opentelemetry-cpp that referenced this pull request Jul 15, 2025
[CMAKE] Add CMake scripts to find or fetch curl and find zlib (open-telemetry#3526)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

0