Modernize CMake embedding and self-contained dependencies

[bmehta001]

Problem

Source embedding via add_subdirectory() / FetchContent should let ORT / ORT GenAI-style consumers link one stable target without patching 1DS sources, manually wiring curl/SQLite/zlib, inheriting SDK warning flags, or passing nonstandard Apple architecture variables.

Changes

Uniform embedding surface

• Add build-tree MSTelemetry::mat.
• Add explicit MATSDK_LIBRARY_TYPE=STATIC|SHARED without mutating parent BUILD_SHARED_LIBS.
• Add canonical namespaced MATSDK_BUILD_* options; legacy BUILD_* inputs remain compatibility inputs.
• Default SDK warnings-as-errors ON standalone and OFF embedded; CI explicitly tests ON.

Dependency providers

• Add MATSDK_SQLITE_PROVIDER=AUTO|SYSTEM|MINIMAL|VENDORED.
• Add MATSDK_ZLIB_PROVIDER=AUTO|SYSTEM|VENDORED.
• Keep parent-provided MATSDK_CURL_TARGET, MATSDK_SQLITE_TARGET, and MATSDK_ZLIB_TARGET hooks.
• Add MATSDK_CURL_PROVIDER=PACKAGE|FETCH and MATSDK_CURL_TLS_BACKEND=MBEDTLS|OPENSSL.
• Fetch pinned SHA256-verified static curl + mbedTLS on Linux when requested.
• Preserve system CA discovery, HTTP proxy support, PIC, hidden symbols, and HTTP(S)-only feature minimization.
• Install SDK-owned bundled/fetched dependency archives beside static libmat.

Build correctness and isolation

• Use standard CMAKE_OSX_ARCHITECTURES, CMAKE_OSX_SYSROOT, and CMAKE_OSX_DEPLOYMENT_TARGET; retain legacy Apple input translation.
• Make warning, Werror, ARC, visibility, optimization, and dead-strip flags target-local.
• Keep vendored SQLite/zlib outside SDK Werror while retaining function/data sections.
• Link Apple system SQLite/zlib from the selected SDK without host find_package() paths.
• Exclude /usr/local/include from all cross-compiles.
• Select HTTP/2 only when curl reports runtime support; otherwise request HTTP/1.1.
• Check curl option/info failures and preserve construction errors.
• Fix response-code/socket types, Android constructor initialization/order, and calloc argument order.
• Route legacy installation through cmake --install.

Consumer CI

Add a source-embedding matrix covering:

• Linux system static, system shared, and fully self-contained static
• Windows self-contained static
• macOS arm64 and universal
• iOS device and simulator
• Android arm64-v8a/API 23

All consumer targets build with warnings-as-errors. Existing vcpkg CI continues to cover installed find_package(MSTelemetry) consumers.

Local validation

• Linux add_subdirectory + FetchContent, system and self-contained providers: build/run 10/10.
• Windows add_subdirectory + FetchContent with vendored SQLite/zlib: build/run 10/10.
• Android FetchContent arm64-v8a/API 23 under Werror: cross-compile passed; no host /usr/local/include.
• Explicit shared Linux build passed; bundled sqlite/zlib symbols hidden (nm -D).
• Static source install contains mat + bundled SQLite/zlib.
• Static fetched install contains mat + curl + mbedTLS + SQLite/zlib archives.
• Static consumer link does not inherit SDK strip/dead-strip flags.
• Legacy BUILD_*, BUILD_SHARED_LIBS, and MAC_ARCH=universal compatibility verified.
• Windows vcpkg static and dynamic package/consumer runtime tests passed.
• Pinned v3.10.173.1 production port fallback passed without local source override.
• Workflow YAML and shell scripts parse; repeated material reviews completed.

[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)