Modernize CMake embedding and self-contained dependencies

[bmehta001]

Problem

Source embedding through add_subdirectory() or FetchContent should let ORT /
ORT GenAI-style consumers link one stable target without patching 1DS sources,
manually wiring dependencies, inheriting SDK warning flags, or using custom
platform controls where CMake already has standard ones.

Changes

Standard CMake surface

• Provide MSTelemetry::mat in both build and installed package trees.
• Use standard BUILD_SHARED_LIBS for static/shared selection.
• Add namespaced MATSDK_BUILD_* options while retaining existing BUILD_*
  inputs as compatibility aliases.
• Keep tests enabled by default for standalone builds and disabled when embedded.
• Use CMAKE_OSX_ARCHITECTURES, CMAKE_OSX_SYSROOT, and
  CMAKE_OSX_DEPLOYMENT_TARGET; legacy Apple inputs are translated at the edge.
• Add CMake presets and thin build wrappers. Direct CMake builds retain the
  existing 3.15 floor; preset wrappers require CMake 3.21.

Explicit dependency providers

• Add MATSDK_SQLITE_PROVIDER=AUTO|SYSTEM|MINIMAL|VENDORED|NONE.
• Add MATSDK_ZLIB_PROVIDER=AUTO|SYSTEM|VENDORED.
• Add MATSDK_CURL_PROVIDER=SYSTEM|FETCH and
  MATSDK_CURL_TLS_BACKEND=MBEDTLS|OPENSSL.
• Consume canonical CURL::libcurl, SQLite::SQLite3, and ZLIB::ZLIB targets,
  including targets supplied by a parent project.
• Support a pinned, SHA256-verified, static HTTP(S)-only curl + mbedTLS build
  with proxy, IPv6, system CA discovery, PIC, and hidden symbols.
• Export the required private archives for static installed-package consumers.
• Use Apple system SQLite/zlib without host include leakage. The imported
  targets are GLOBAL and their creation is shared (cmake/MatsdkAppleSystemDeps.cmake)
  between the build-time and installed-package configs, so a consumer that
  calls find_package(MSTelemetry) in one directory and links
  MSTelemetry::mat from a sibling directory can still resolve them, and the
  two configs cannot drift out of sync with each other.

Legacy input handling

• USE_CURL (Android) still works: translated to
  MATSDK_ANDROID_HTTP_CLIENT=CURL when the canonical option isn't set.
• INSTALL_LIB_DIR and BUILD_STATIC_SQLITE are narrow, internal knobs whose
  old semantics don't map cleanly onto the new layout/provider model; rather
  than silently reinterpreting or silently ignoring them, setting either now
  prints an explicit DEPRECATION message pointing at the replacement.
• The Android AAR CMake target rename (maesdk -> mat) has no compatibility
  target: it was never installed/exported, so it was never a public interface.

Android and target isolation

• Route the Android Gradle/AAR build through the same root mat target instead
  of maintaining a second source graph.
• Default Android to the Java/JNI transport; retain explicit native curl
  opt-ins through the Android curl vcpkg features.
• Install the Java bridge sources with Android packages and expose their path as
  MSTelemetry_ANDROID_JAVA_SOURCE_DIR.
• Keep warning, warnings-as-errors, visibility, ARC, optimization, and
  dead-strip policy target-local so parent and vendored targets do not inherit it.
• Exclude host /usr/local/include paths from cross-compiles.

Correctness and consumer coverage

• Select HTTP/2 only when libcurl reports support; otherwise request HTTP/1.1.
• Check curl option/info failures and preserve construction errors.
• Fix curl response/socket types, Android constructor initialization, and
  calloc argument ordering.
• Add source-embedding CI for Linux system/static/shared/self-contained,
  Windows, macOS arm64/universal, iOS device/simulator, and Android API 23.
• Continue exercising installed packages through Linux/macOS embedding jobs and
  the existing Windows/Linux/macOS/iOS/Android vcpkg matrix.

Validation

• Windows FetchContent consumer with vendored SQLite/zlib: built and ran 10/10.
• Linux static system-dependency package: built, installed, and consumed through
  find_package(MSTelemetry); runtime checks passed 10/10.
• Android Java transport: root library built and installed, Java bridge sources
  were installed, and an installed-package native consumer linked.
• Android test-app CMake adapter configured directly with its Room/SQLite split.
• Curl response-cap regression tests passed 2/2.
• Preset JSON, PowerShell, and shell wrapper syntax checks passed.
• Code review found and fixed a real issue: the installed static-package config
  created the Apple-system SQLite::SQLite3/ZLIB::ZLIB imported targets
  without GLOBAL, inconsistent with the build-time config's identical
  construct. A consumer calling find_package(MSTelemetry) in one directory
  and linking MSTelemetry::mat from a sibling directory would have failed to
  resolve them at generate time. Fixed, and the target-creation logic was
  deduplicated into a shared, installed cmake helper so the two configs cannot
  diverge again. Re-verified: Windows FetchContent build/run 10/10, Linux
  static install confirms the shared helper installs alongside
  MSTelemetryConfig.cmake, and a standalone simulation of the generated
  Apple-branch config confirms both targets are created with
  IMPORTED_GLOBAL=TRUE. Final review pass reported no further findings.

[Copilot]

Pull request overview

This PR improves the CMake “source embedding” experience (via add_subdirectory() / FetchContent) so consumers can link the SDK through the same MSTelemetry::mat target name used by vcpkg/installed workflows, and optionally obtain a self-contained non-vcpkg libcurl on Linux.

Changes:

• Add a build-tree MSTelemetry::mat alias for the in-tree mat target.
• Add non-vcpkg dependency override cache variables (MATSDK_CURL_TARGET, MATSDK_SQLITE_TARGET, MATSDK_ZLIB_TARGET) and a Linux-only MATSDK_CURL_PROVIDER=FETCH path with selectable TLS backend.
• Add embedding documentation and a CMake smoke project that links MSTelemetry::mat via add_subdirectory().

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
CMakeLists.txt	Adds cache variables and validation for non-vcpkg dependency overrides and fetched-curl selection.
lib/CMakeLists.txt	Adds build-tree MSTelemetry::mat alias and wires SQLite/zlib override targets into platform link logic.
cmake/MatsdkFetchCurl.cmake	Implements Linux-only FetchContent build of pinned static curl (+ optional mbedTLS/OpenSSL).
docs/embedding-with-cmake.md	Documents embedding with MSTelemetry::mat and the non-vcpkg override/fetch options.
tests/embedding/CMakeLists.txt	Adds an embedding smoke CMake project that links MSTelemetry::mat.

Comments suppressed due to low confidence (1)

cmake/MatsdkFetchCurl.cmake:116

• FetchContent URL_HASH uses SHA1 for the pinned curl download. SHA1 is cryptographically weak for verifying downloaded archives; prefer SHA256 (or stronger) to reduce the risk of a collision-based substitution.

  FetchContent_Declare(
    matsdk_curl
    URL ${MATSDK_CURL_URL}
    URL_HASH SHA1=${MATSDK_CURL_SHA1})
  FetchContent_MakeAvailable(matsdk_curl)

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

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

Comments suppressed due to low confidence (1)

CMakeLists.txt:379

• The fetched dependency integrity pins use SHA1 (MATSDK_MBEDTLS_SHA1). SHA1 is collision-prone; prefer SHA256 (or stronger) for FetchContent URL_HASH to reduce supply-chain risk. This change will require updating both the cache variable(s) and the URL_HASH algorithm in MatsdkFetchCurl.cmake.

set(MATSDK_MBEDTLS_URL "https://github.com/Mbed-TLS/mbedtls/releases/download/mbedtls-3.6.7/mbedtls-3.6.7.tar.bz2" CACHE STRING
  "URL for the mbedTLS dependency used by MATSDK_CURL_PROVIDER=FETCH and MATSDK_CURL_TLS_BACKEND=MBEDTLS")
set(MATSDK_MBEDTLS_SHA1 "e892e98cff90cf7e0736c525bf357626a2513c4c" CACHE STRING
  "SHA1 for MATSDK_MBEDTLS_URL")

[Copilot]

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (5)

cmake/MatsdkFetchCurl.cmake:73

• Same as above for the ON toggles: these should be set as CACHE entries so the fetched curl/mbedTLS builds deterministically pick them up (especially options like BUILD_STATIC_LIBS / HTTP_ONLY).

  foreach(option IN ITEMS
      BUILD_STATIC_LIBS
      DISABLE_PACKAGE_CONFIG_AND_INSTALL
      CURL_DISABLE_INSTALL
      HTTP_ONLY
      CURL_DISABLE_ALTSVC
      CURL_DISABLE_HSTS
      CURL_DISABLE_COOKIES
      CURL_DISABLE_NETRC
      CURL_DISABLE_MIME
      CURL_DISABLE_DOH
      CURL_DISABLE_AWS
      CURL_DISABLE_BEARER_AUTH
      CURL_DISABLE_DIGEST_AUTH
      CURL_DISABLE_KERBEROS_AUTH
      CURL_DISABLE_NEGOTIATE_AUTH)
    set(${option} ON)
  endforeach()

cmake/MatsdkFetchCurl.cmake:54

• The feature/option toggles are set as normal variables (e.g., set(${option} OFF)), which may not reliably control the fetched subprojects' option()/CACHE settings across all CMake versions and scopes. To ensure curl/mbedTLS actually build with the intended minimal feature set, set these as CACHE entries (with FORCE) before FetchContent_MakeAvailable().

This issue also appears on line 56 of the same file.

  foreach(option IN ITEMS
      BUILD_SHARED_LIBS
      BUILD_TESTING
      ENABLE_PROGRAMS
      ENABLE_TESTING
      GEN_FILES
      UNSAFE_BUILD
      INSTALL_MBEDTLS_HEADERS
      MBEDTLS_FATAL_WARNINGS
      USE_SHARED_MBEDTLS_LIBRARY
      LINK_WITH_PTHREAD
      BUILD_CURL_EXE
      BUILD_EXAMPLES
      BUILD_LIBCURL_DOCS
      BUILD_MISC_DOCS
      ENABLE_CURL_MANUAL
      CURL_ENABLE_EXPORT_TARGET
      CURL_USE_OPENSSL
      CURL_USE_PKGCONFIG
      CURL_USE_CMAKECONFIG
      CURL_ZLIB
      CURL_BROTLI
      CURL_ZSTD
      USE_LIBIDN2
      CURL_USE_LIBPSL
      CURL_USE_LIBSSH2
      CURL_USE_LIBSSH
      CURL_USE_GSSAPI
      CURL_USE_GSASL
      USE_NGHTTP2
      USE_NGTCP2
      USE_QUICHE
      ENABLE_ARES
      ENABLE_UNIX_SOCKETS)
    set(${option} OFF)
  endforeach()

CMakeLists.txt:391

• MATSDK_CURL_TLS_BACKEND is validated even when MATSDK_CURL_PROVIDER is PACKAGE (or curl isn't used on the current platform). This makes otherwise-valid configurations fail due to an unused setting. Gate the validation on MATSDK_CURL_PROVIDER=FETCH.

string(TOUPPER "${MATSDK_CURL_TLS_BACKEND}" MATSDK_CURL_TLS_BACKEND_UPPER)
if(NOT MATSDK_CURL_TLS_BACKEND_UPPER STREQUAL "MBEDTLS" AND NOT MATSDK_CURL_TLS_BACKEND_UPPER STREQUAL "OPENSSL")
  message(FATAL_ERROR "MATSDK_CURL_TLS_BACKEND must be MBEDTLS or OPENSSL; got '${MATSDK_CURL_TLS_BACKEND}'.")
endif()

CMakeLists.txt:393

• The MATSDK_*_TARGET overrides are described as applying to non-vcpkg builds, but they are validated unconditionally. This can make vcpkg configurations fail due to unused override variables. Consider gating this validation on NOT MATSDK_USE_VCPKG_DEPS.

  if(${_matsdk_dependency_target_var} AND NOT TARGET "${${_matsdk_dependency_target_var}}")

docs/embedding-with-cmake.md:26

• The docs mention MATSDK_CURL_PROVIDER=FETCH but don’t note that it will fail if the consuming build already defines CURL::libcurl (e.g., via find_package(CURL)), because the embedded curl build needs to create that target name. Calling this out will save consumers time when integrating into larger superbuilds.

When the CPP11 PAL uses the curl HTTP transport outside vcpkg, the SDK normally
calls `find_package(CURL)` and links `CURL::libcurl` when that imported target is
available. On Linux, set `MATSDK_CURL_PROVIDER=FETCH` to let the SDK download and
build a pinned static curl dependency instead:

[Copilot]

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (3)

lib/http/HttpClient_Curl.hpp:317

• CURLOPT_WRITEFUNCTION expects a function pointer; passing it as a void* (via (void *)&WriteVectorCallback) is undefined behavior on platforms where data pointers and function pointers differ. Prefer passing a function-pointer type (e.g., curl_write_callback) and pass data pointers as void* explicitly.

        else if (!SetOption(CURLOPT_WRITEFUNCTION, (void *)&WriteVectorCallback)
            || !SetOption(CURLOPT_HEADERDATA, (void *)&respHeaders)
            || !SetOption(CURLOPT_WRITEDATA, (void *)&respBody))
        {

cmake/MatsdkFetchCurl.cmake:54

• The curl/mbedTLS option toggles are set with plain set(...) inside a CMake function(). FetchContent builds the dependency via add_subdirectory(), which typically does not inherit function-scope variables, so these toggles may be ignored and the fetched curl build could end up using its defaults (shared libs, extra protocols/features, installs, etc.). Consider switching to a mechanism guaranteed to affect the dependency configure (e.g., cache variables scoped/restored around the FetchContent call, or a wrapper toolchain/options file).

  foreach(option IN ITEMS
      BUILD_SHARED_LIBS
      BUILD_TESTING
      ENABLE_PROGRAMS
      ENABLE_TESTING
      GEN_FILES
      UNSAFE_BUILD
      INSTALL_MBEDTLS_HEADERS
      MBEDTLS_FATAL_WARNINGS
      USE_SHARED_MBEDTLS_LIBRARY
      LINK_WITH_PTHREAD
      BUILD_CURL_EXE
      BUILD_EXAMPLES
      BUILD_LIBCURL_DOCS
      BUILD_MISC_DOCS
      ENABLE_CURL_MANUAL
      CURL_ENABLE_EXPORT_TARGET
      CURL_USE_OPENSSL
      CURL_USE_PKGCONFIG
      CURL_USE_CMAKECONFIG
      CURL_ZLIB
      CURL_BROTLI
      CURL_ZSTD
      USE_LIBIDN2
      CURL_USE_LIBPSL
      CURL_USE_LIBSSH2
      CURL_USE_LIBSSH
      CURL_USE_GSSAPI
      CURL_USE_GSASL
      USE_NGHTTP2
      USE_NGTCP2
      USE_QUICHE
      ENABLE_ARES
      ENABLE_UNIX_SOCKETS)
    set(${option} OFF)
  endforeach()

lib/http/HttpClient_Curl.hpp:240

• This guard can leak a previous successful HTTP status code into the !curl || !m_isConfigured failure path (because it only overwrites res when it is exactly CURLE_OK). That can make Send() report success even though it immediately fails initialization. Consider overriding res when it is CURLE_OK or when it looks like an HTTP status code (>=100), while still preserving a prior CURL error from construction/configuration.

        if(!curl || !m_isConfigured)
        {
            if (res == CURLE_OK)
            {
                res = CURLE_FAILED_INIT;
            }
            DispatchEvent(OnSendFailed);

[Copilot]

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

lib/http/HttpClient_Curl.hpp:318

• Send() attempts to collect response headers into respHeaders via CURLOPT_HEADERDATA, but no CURLOPT_HEADERFUNCTION is set. In that case, libcurl will not use respHeaders (and GetResponseHeaders() will always return empty). Set an explicit header callback (the existing WriteVectorCallback works) so the respHeaders buffer is actually populated.

        else if (!SetOption(CURLOPT_WRITEFUNCTION,
                static_cast<curl_write_callback>(&WriteVectorCallback))
            || !SetOption(CURLOPT_HEADERDATA, static_cast<void*>(&respHeaders))
            || !SetOption(CURLOPT_WRITEDATA, static_cast<void*>(&respBody)))

[Copilot]

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated 1 comment.

[Copilot]

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (3)

lib/http/HttpClient_Curl.hpp:322

• respHeaders is never populated: the code sets CURLOPT_HEADERDATA but never sets CURLOPT_HEADERFUNCTION, so libcurl has no callback to write headers into respHeaders. As a result, HttpClient_Curl.cpp always sees an empty header map from GetResponseHeaders(). Set CURLOPT_HEADERFUNCTION (and keep CURLOPT_HEADERDATA) so headers are captured separately from the body.

        else if (!SetOption(CURLOPT_WRITEFUNCTION,
                static_cast<curl_write_callback>(&WriteVectorCallback))
            || !SetOption(CURLOPT_HEADERDATA, static_cast<void*>(&respHeaders))
            || !SetOption(CURLOPT_WRITEDATA, static_cast<void*>(&respBody)))
        {

lib/http/HttpClient_Curl.hpp:283

• On the pre-7.45.0 libcurl path (CURLINFO_LASTSOCKET), lastSocket can be -1 even when curl_easy_getinfo returns CURLE_OK. That leaves sockextr as CURL_SOCKET_BAD, but the code proceeds to WaitOnSocket() and treats poll() errors as success. Treat CURL_SOCKET_BAD as a connect failure before using the socket.

        {
            CURLcode infoResult;
#if LIBCURL_VERSION_NUM >= 0x072D00 // Version 7.45.00
            infoResult = curl_easy_getinfo(curl, CURLINFO_ACTIVESOCKET, &sockextr);
#else
            long lastSocket = -1;
            infoResult = curl_easy_getinfo(curl, CURLINFO_LASTSOCKET, &lastSocket);
            if (infoResult == CURLE_OK)
            {
                sockextr = static_cast<curl_socket_t>(lastSocket);
            }
#endif
            if(CURLE_OK != infoResult)
            {
                res = static_cast<long>(infoResult);
                DispatchEvent(OnConnectFailed);     // couldn't connect - stage 2
                TRACE("Error #2: %s\n", curl_easy_strerror(infoResult));
                goto cleanup;
            }

.github/workflows/test-embedding.yml:137

• The Android embedding workflow uses ANDROID_NDK_LATEST_HOME, but the repo’s existing Android tooling uses ANDROID_NDK_HOME (e.g., tests/vcpkg/test-vcpkg-android.sh). Using a different env var here risks expanding to an empty toolchain path and breaking CI on runners where ANDROID_NDK_LATEST_HOME isn’t set. Prefer the same ANDROID_NDK_HOME variable for consistency.

      run: >
        cmake -G Ninja -S tests/embedding -B build-embedding
        -DCMAKE_TOOLCHAIN_FILE=${ANDROID_NDK_LATEST_HOME}/build/cmake/android.toolchain.cmake
        -DANDROID_ABI=arm64-v8a

[Copilot]

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

lib/http/HttpClient_Curl.hpp:290

• The per-operation httpConnTimeout parameter is never used; Send() still waits using the compile-time HTTP_CONN_TIMEOUT constant. This makes httpConnTimeout ineffective for callers and can lead to unexpected timeout behavior when a non-default value is provided.

        /* wait for the socket to become ready for sending */
        sockfd = sockextr;
        socketWaitResult = WaitOnSocket(sockfd, 0, HTTP_CONN_TIMEOUT * 1000L);
        if(socketWaitResult <= 0 || isAborted)
        {

lib/http/HttpClient_Curl.hpp:393

• res is a long, but the trace uses %d (int). If tracing is ever enabled, this is undefined behavior on LP64 platforms. Use %ld (or cast) to match the argument type.

        // We got some response from server. Dump the contents.
        TRACE("HTTP response code %d\n", res);
        DispatchEvent(OnResponse);

[Copilot]

Pull request overview

Copilot reviewed 26 out of 26 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

cmake/MatsdkFetchCurl.cmake:72

• Same scoping issue as above: setting these dependency options to ON as non-cache variables may not affect the FetchContent-added subproject. Use CACHE BOOL (FORCE) so curl/mbedTLS configuration is deterministic.

    set(${option} ON)

cmake/MatsdkFetchCurl.cmake:53

• The curl/mbedTLS option toggles are set as normal variables inside a function. Because FetchContent adds the dependency via add_subdirectory(), these values may not propagate into the fetched project's directory scope, so curl could ignore them and build unwanted components (tests, tools, shared libs, etc.). Set them as CACHE variables (FORCE) so the fetched subproject reliably honors them.

This issue also appears on line 72 of the same file.

    set(${option} OFF)

[Copilot]

Pull request overview

Copilot reviewed 39 out of 39 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (3)

cmake/MSTelemetryConfig.cmake.in:28

• Same directory-scope concern as SQLite::SQLite3 above: define ZLIB::ZLIB as an IMPORTED GLOBAL target so it remains visible wherever MSTelemetry targets are linked.

        add_library(ZLIB::ZLIB INTERFACE IMPORTED)
        set_property(TARGET ZLIB::ZLIB PROPERTY INTERFACE_LINK_LIBRARIES z)

tools/setup-buildtools.sh:4

• The script detects yum via a hard-coded /bin/yum path. On many RPM-based distros (e.g., CentOS/RHEL), yum is at /usr/bin/yum, so this check can incorrectly fall through to the apt-get branch and fail. Prefer checking the command on PATH and use modern $(...) + a POSIX-compatible string compare for the release check.
  cmake/MSTelemetryConfig.cmake.in:11
• This config file can be loaded from a subdirectory; creating SQLite::SQLite3 as a non-GLOBAL imported target makes it directory-scoped and can lead to target visibility/link-interface issues. Make the imported target GLOBAL (matching the MSTelemetry::* dependency targets defined below).

This issue also appears on line 27 of the same file.

        add_library(SQLite::SQLite3 INTERFACE IMPORTED)
        set_property(TARGET SQLite::SQLite3 PROPERTY INTERFACE_LINK_LIBRARIES sqlite3)