Reduce binary footprint: minimal SQLite and selectable curl TLS backend#1499
Open
bmehta001 wants to merge 12 commits into
Open
Reduce binary footprint: minimal SQLite and selectable curl TLS backend#1499bmehta001 wants to merge 12 commits into
bmehta001 wants to merge 12 commits into
Conversation
On the CPP11/curl path (non-Apple, non-Windows), the built-in libcurl HTTP client was always compiled and curl was a hard find_package(CURL REQUIRED) dependency -- pulling in curl and a TLS backend (OpenSSL/mbedTLS) even for hosts that already have their own HTTP stack. Add option(BUILD_CURL_HTTP_CLIENT ON). When OFF, the curl block is skipped (no find_package(CURL), no link, no -DHAVE_MAT_CURL_HTTP_CLIENT) and the build instead defines -DMATSDK_NO_DEFAULT_HTTP_CLIENT. mat/config.h then undefines HAVE_MAT_DEFAULT_HTTP_CLIENT centrally (regardless of the config preset), which the SDK already handles end-to-end: HttpClientFactory and HttpClient_Curl.cpp compile out, and LogManagerImpl's existing !HAVE_MAT_DEFAULT_HTTP_CLIENT branch requires the host to supply an IHttpClient via CFG_MODULE_HTTP_CLIENT. Default ON keeps existing behavior unchanged. Apple/Windows are unaffected (they use native HTTP stacks and never enter the curl block). Validated on WSL x64-linux: with OFF, libmat has no curl symbols and a consumer links with no -lcurl/-lTLS (1.43 MB stripped, vs 4.39 MB with curl+mbedTLS and 10.65 MB with curl+OpenSSL). Files changed: - CMakeLists.txt: BUILD_CURL_HTTP_CLIENT option + gating - lib/include/mat/config.h: central HAVE_MAT_DEFAULT_HTTP_CLIENT opt-out Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…tprint The SDK uses SQLite only for its offline event-storage cache (plain tables, transactions, WAL, autovacuum/VACUUM, a few PRAGMAs, and one custom UTF-8 function), so most SQLite subsystems are dead weight. Add an option to compile a private SQLite from the vendored amalgamation with a set of amalgamation-safe strip flags (single source of truth: MATSDK_SQLITE_MINIMAL_DEFS), removing the external sqlite3 dependency and shrinking SQLite ~10.2% (.text) / ~12.5% (object). - Root CMakeLists.txt: add option(MATSDK_MINIMAL_SQLITE) (default OFF). In vcpkg mode, skip find_package(unofficial-sqlite3) when bundling, and emit a clear FATAL_ERROR pointing at the system-sqlite/minimal-sqlite features when neither provides SQLite (e.g. a bare [core] install). - lib/CMakeLists.txt: define MATSDK_SQLITE_MINIMAL_DEFS, compute MATSDK_BUNDLE_SQLITE (minimal OR vendored-Android), and build a single sqlite3_bundled. The strip flags are applied ONLY when MATSDK_MINIMAL_SQLITE is ON, so the default Android legacy build keeps its existing unstripped bundled SQLite. Warnings are disabled on the vendored target (/w on MSVC, -w on GCC/Clang for the stripped build) so the SDK's -Werror/-WX does not fire on amalgamation code. A static mat propagates the PRIVATE sqlite3_bundled through its link interface, so export+install it. - MSTelemetryConfig.cmake.in: skip find_dependency(unofficial-sqlite3) when bundled. - vcpkg port: add a minimal-sqlite feature (-DMATSDK_MINIMAL_SQLITE=ON) and move sqlite3 into a default system-sqlite feature so [core,minimal-sqlite] drops it. - docs/building-with-vcpkg.md: document the feature, the size win, and the static-absorption symbol-visibility caveat. SQLITE_OMIT_AUTOINIT and SQLITE_DEFAULT_MEMSTATUS=0 are deliberately NOT stripped: the former because skipSqliteInitAndShutdown lets the host skip the SDK's explicit sqlite3_initialize() (which the host cannot do against a private SQLite), the latter because the SDK arms a soft heap limit via sqlite3_soft_heap_limit64() that is only enforced while memory statistics are enabled. Validated: vendored Linux Debug (77 offline-storage/SQLite unit tests pass on the debug amalgamation), vcpkg [core,minimal-sqlite] consumer (links MSTelemetry::sqlite3_bundled, runs 10/10, external sqlite3 dropped), default vcpkg path regression (system-sqlite intact), and MSVC compile/link of sqlite3_bundled+mat. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR introduces opt-in build knobs intended to reduce the SDK’s footprint in host-embedded scenarios by (1) allowing builds without the built-in libcurl HTTP client and (2) optionally replacing the external SQLite dependency with a private, feature-stripped bundled SQLite.
Changes:
- Added
BUILD_CURL_HTTP_CLIENTto omit the built-in curl/TLS HTTP client and require a host-suppliedIHttpClient. - Added
MATSDK_MINIMAL_SQLITEand corresponding vcpkgminimal-sqlitefeature to build a stripped, bundled SQLite instead of depending on vcpkg’ssqlite3. - Updated the vcpkg port and documentation to reflect the new SQLite feature split and minimal-SQLite option.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tools/ports/cpp-client-telemetry/vcpkg.json | Moves sqlite3 into a default system-sqlite feature and adds minimal-sqlite. |
| tools/ports/cpp-client-telemetry/portfile.cmake | Wires vcpkg minimal-sqlite feature into -DMATSDK_MINIMAL_SQLITE=ON. |
| lib/include/mat/config.h | Adds MATSDK_NO_DEFAULT_HTTP_CLIENT override to disable HAVE_MAT_DEFAULT_HTTP_CLIENT. |
| lib/CMakeLists.txt | Implements bundled/minimal SQLite build, link selection, and installation/export logic. |
| docs/building-with-vcpkg.md | Documents the new vcpkg minimal-sqlite feature and behavior. |
| CMakeLists.txt | Adds MATSDK_MINIMAL_SQLITE and BUILD_CURL_HTTP_CLIENT; adjusts vcpkg SQLite discovery flow. |
| cmake/MSTelemetryConfig.cmake.in | Skips unofficial-sqlite3 dependency discovery when SQLite is bundled. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+487
to
+493
| if(MATSDK_BUNDLE_SQLITE) | ||
| # Private minimal SQLite instead of the vcpkg sqlite3 package. PRIVATE so the | ||
| # SDK-internal copy is not re-exported to consumers. | ||
| target_link_libraries(mat PRIVATE sqlite3_bundled) | ||
| else() | ||
| target_link_libraries(mat PUBLIC unofficial::sqlite3::sqlite3) | ||
| endif() |
Contributor
Author
There was a problem hiding this comment.
This is the static-library case CMake already handles: target_link_libraries(mat PRIVATE sqlite3_bundled) on a static mat places the dependency in INTERFACE_LINK_LIBRARIES wrapped in \$<LINK_ONLY:...>, so a downstream target that links only MSTelemetry::mat still pulls in MSTelemetry::sqlite3_bundled automatically (CMake propagates a static library's PRIVATE link deps because the archive itself does no linking). It is exported in the same set for exactly this reason.
Verified empirically: a vcpkg [core,minimal-sqlite] consumer that links only MSTelemetry::mat (x64-linux, static) builds, links, and runs 10/10 with no unresolved sqlite3_* symbols; the installed MSTelemetryTargets.cmake shows INTERFACE_LINK_LIBRARIES "\$<LINK_ONLY:MSTelemetry::sqlite3_bundled>;...". Making it PUBLIC would instead propagate the target's usage requirements (include dirs/defs) as a public contract, which is not wanted for an internal impl detail. Keeping it PRIVATE + LINK_ONLY is the correct semantic.
Comments 2 and 3 are addressed in 0977e64 (sqlite3_bundled is now exported only for a static mat; the status message reflects the SQLite path).
Comment on lines
+599
to
+605
| # A static libmat propagates its PRIVATE static dependencies through its link | ||
| # interface, so the bundled SQLite must be part of the same export set and be | ||
| # installed alongside mat for downstream find_package() consumers to link. | ||
| set(MATSDK_INSTALL_TARGETS mat) | ||
| if(MATSDK_BUNDLE_SQLITE AND TARGET sqlite3_bundled) | ||
| list(APPEND MATSDK_INSTALL_TARGETS sqlite3_bundled) | ||
| endif() |
| find_package(ZLIB REQUIRED) | ||
| find_package(nlohmann_json CONFIG REQUIRED) | ||
| message(STATUS "Using vcpkg-provided sqlite3, zlib, nlohmann-json") | ||
| message(STATUS "Using vcpkg-provided zlib, nlohmann-json") |
- lib/CMakeLists.txt: only add sqlite3_bundled to the install/export set when mat is a STATIC_LIBRARY. A shared mat absorbs the private SQLite into libmat and does not propagate the PRIVATE dependency, so exporting the separate archive there was unnecessary and could let a consumer link a second SQLite copy. For a static mat the archive must stay exported because the static library propagates its PRIVATE dependency through its link interface (\$<LINK_ONLY:...>). - CMakeLists.txt: make the vcpkg dependency-mode status message reflect whether the external sqlite3 package or the private minimal SQLite is used. Verified with an isolated CMake export test: static mat exports m+sq (consumer linking only the namespaced lib resolves sq); shared mat exports only m and install(EXPORT) succeeds with sq excluded. Re-ran the vcpkg [core,minimal-sqlite] consumer (static x64-linux): 10/10. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment on lines
+488
to
+490
| # Private minimal SQLite instead of the vcpkg sqlite3 package. PRIVATE so the | ||
| # SDK-internal copy is not re-exported to consumers. | ||
| target_link_libraries(mat PRIVATE sqlite3_bundled) |
Comment on lines
+23
to
+29
| # minimal-sqlite feature -> build a private feature-stripped SQLite instead of | ||
| # linking the external sqlite3 package. Maps to -DMATSDK_MINIMAL_SQLITE=ON/OFF. | ||
| vcpkg_check_features( | ||
| OUT_FEATURE_OPTIONS FEATURE_OPTIONS | ||
| FEATURES | ||
| minimal-sqlite MATSDK_MINIMAL_SQLITE | ||
| ) |
Contributor
Author
There was a problem hiding this comment.
Intentionally not exposed as a vcpkg feature. minimal-sqlite is a transparent, drop-in change (it swaps one SQLite implementation for another with identical behavior), so it is safe to toggle from a manifest. BUILD_CURL_HTTP_CLIENT=OFF is not drop-in: it removes the only built-in HTTP client, so the consumer must supply their own IHttpClient via CFG_MODULE_HTTP_CLIENT or telemetry silently cannot upload. Shipping that as a published-port feature would let a consumer enable it and get a port that compiles but fails to send data at runtime, with no build-time signal.
BUILD_CURL_HTTP_CLIENT therefore stays a CMake build option for direct-source embedders (e.g. ONNX Runtime / Foundry Local, which build the SDK via CMake/FetchContent and already provide their own transport), where the host owns the integration. The vcpkg port keeps a working default HTTP client (and its curl dependency) so find_package(MSTelemetry) consumers get a functional SDK out of the box.
The comment-accuracy point (line 490) is addressed in 534be72.
Correct the inline comment: a PRIVATE link of the bundled SQLite suppresses propagation of its include dirs / compile definitions, but a static mat still propagates the archive for linking via \$<LINK_ONLY:...> (hence it is exported for static builds); a shared mat absorbs it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…efault-http-client feature Make the HTTP-client footprint a consumer choice instead of hardcoding curl[openssl]: - vcpkg.json: replace the base curl[openssl] dependency with three features -- curl-openssl (default; libcurl + OpenSSL), curl-mbedtls (libcurl + mbedTLS), and no-default-http-client (omit the built-in client). curl-openssl is a default feature so a plain install keeps current behavior; [core,no-default-http-client] drops curl entirely. - portfile.cmake: map the no-default-http-client feature to -DBUILD_CURL_HTTP_CLIENT=OFF via INVERTED_FEATURES. - CMakeLists.txt: when the built-in client is enabled in vcpkg mode but libcurl is not found, emit a clear FATAL_ERROR pointing at the curl-openssl/curl-mbedtls/ no-default-http-client features (instead of a bare find_package failure). - docs: document the size ladder (OpenSSL ~10.6MB / mbedTLS ~4.4MB / no-curl ~1.4MB) and the exact mbedTLS recipe -- crucially, the consumer must ALSO list curl with default-features:false at the top level, because vcpkg only honors curl's default-features:false for top-level dependencies (otherwise curl's ssl default pulls OpenSSL in transitively alongside mbedTLS). Validated on WSL with vcpkg: default resolves curl[openssl]+sqlite3; the documented mbedTLS recipe builds with mbedTLS only (no libssl/libcrypto, libcurl carries no OpenSSL symbols) and the consumer runs; [core,no-default-http-client] drops curl from the graph; [core,minimal-sqlite,no-default-http-client] drops curl and the external sqlite3. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment on lines
384
to
396
| # The TLS backend (OpenSSL/mbedTLS) is selected by the vcpkg port's | ||
| # curl-openssl (default) / curl-mbedtls features; the SDK just links libcurl. | ||
| find_package(CURL QUIET) | ||
| if(NOT CURL_FOUND) | ||
| message(FATAL_ERROR | ||
| "libcurl was not found but the built-in HTTP client is enabled. The " | ||
| "vcpkg port provides the curl HTTP client through the curl-openssl " | ||
| "(default) or curl-mbedtls feature, or you can omit it with the " | ||
| "no-default-http-client feature (host supplies its own IHttpClient). " | ||
| "Install cpp-client-telemetry with its default features, with " | ||
| "[core,curl-mbedtls], or with [core,no-default-http-client].") | ||
| endif() | ||
| list(APPEND LIBS CURL::libcurl) |
Contributor
Author
There was a problem hiding this comment.
Fixed in c00ce55: switched to find_package(CURL CONFIG QUIET) and now gate on TARGET CURL::libcurl before linking, so the vcpkg CURLConfig (which defines the imported target) is used rather than module FindCURL. Verified the default curl-openssl consumer still configures, links, and runs.
Comment on lines
+23
to
+35
| # Feature -> CMake option mapping: | ||
| # * minimal-sqlite -> -DMATSDK_MINIMAL_SQLITE=ON (private feature-stripped SQLite). | ||
| # * no-default-http-client (INVERTED) -> -DBUILD_CURL_HTTP_CLIENT=OFF (omit the | ||
| # built-in libcurl client; host supplies its own IHttpClient). When the feature | ||
| # is absent the built-in curl client is built (ON), with its TLS backend chosen | ||
| # by the curl-openssl (default) / curl-mbedtls features in vcpkg.json. | ||
| vcpkg_check_features( | ||
| OUT_FEATURE_OPTIONS FEATURE_OPTIONS | ||
| FEATURES | ||
| minimal-sqlite MATSDK_MINIMAL_SQLITE | ||
| INVERTED_FEATURES | ||
| no-default-http-client BUILD_CURL_HTTP_CLIENT | ||
| ) |
Contributor
Author
There was a problem hiding this comment.
Added in c00ce55: a portfile guard that fails fast when more than one of curl-openssl/curl-mbedtls/no-default-http-client is selected, with guidance to use the [core,...] form. Verified it passes single selections and fires on curl-openssl+curl-mbedtls and curl-openssl+no-default-http-client (the 'forgot [core]' mixing case).
…ard (Copilot review) - CMakeLists.txt: in vcpkg mode use find_package(CURL CONFIG QUIET) and gate on TARGET CURL::libcurl. Forcing CONFIG selects the vcpkg-provided CURLConfig (which defines the imported target) rather than the module FindCURL, which on some CMake versions does not define CURL::libcurl and would fail at link. - portfile.cmake: fail fast when more than one of curl-openssl/curl-mbedtls/ no-default-http-client is selected. vcpkg cannot express mutual exclusivity, so a consumer requesting e.g. curl-mbedtls without [core] keeps the default curl-openssl and would union both TLS backends; the guard now errors with guidance to use the [core,...] form. Validated: the guard passes single selections and fires on curl-openssl+curl-mbedtls and curl-openssl+no-default-http-client; the default (curl-openssl) vcpkg consumer still configures via CURL CONFIG, links, and runs. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Drop the ability to build without the built-in libcurl HTTP client. That option only benefited consumers that already ship their own IHttpClient; the SDK's named consumers (and Apple/Windows, which use NSURLSession/WinInet) never needed it, and it added a fragile feature plus a config-flow opt-out. The TLS-backend selection (curl-openssl default / curl-mbedtls) and minimal-SQLite remain. - CMakeLists.txt: remove option(BUILD_CURL_HTTP_CLIENT) and the no-curl else branch; the curl HTTP client is always built on the CPP11/curl path again (keeping the find_package(CURL CONFIG) + TARGET CURL::libcurl hardening). - lib/include/mat/config.h: remove the MATSDK_NO_DEFAULT_HTTP_CLIENT -> HAVE_MAT_DEFAULT_HTTP_CLIENT opt-out. - vcpkg.json: remove the no-default-http-client feature. - portfile.cmake: remove the INVERTED_FEATURES mapping; the mutual-exclusivity guard now covers just curl-openssl vs curl-mbedtls. - docs: drop the no-curl row/section; note the size figures are worst-case (without consumer-side --gc-sections). Validated: vcpkg.json parses, CMake configures cleanly, and the mat target builds and links with the curl client compiled in. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
bmehta001
changed the title
Contributor
Author
|
Updated scope: removed the no-curl ( |
| | zlib | `zlib` | `ZLIB::ZLIB` | All | | ||
| | nlohmann JSON | `nlohmann-json` | `nlohmann_json::nlohmann_json` | All | | ||
| | libcurl | `curl[openssl]` | `CURL::libcurl` | Non-Windows, non-Apple | | ||
| | libcurl | `curl[openssl]` or `curl[mbedtls]` | `CURL::libcurl` | Non-Windows, non-Apple (selectable; optional) | |
…kg packages
macOS/iOS ship libsqlite3 and libz as system libraries, so pulling and statically
linking the vcpkg sqlite3 + zlib packages added ~1 MB of redundant code to Apple
binaries. Link the system libraries instead -- consistent with the SDK's own Swift
Package (which links .linkedLibrary("sqlite3"/"z")) and with how analogous telemetry
SDKs (e.g. sentry-native) gate these deps off Apple platforms.
- vcpkg.json: gate the zlib dependency and the system-sqlite feature's sqlite3
dependency to "!osx & !ios" so they are not installed on Apple.
- CMakeLists.txt: on APPLE in vcpkg mode, find_package(SQLite3)/find_package(ZLIB)
(CMake's modules resolve to the OS libraries) and set MATSDK_APPLE_SYSTEM_DEPS.
- lib/CMakeLists.txt: link SQLite::SQLite3 + ZLIB::ZLIB on Apple; never bundle a
private SQLite on Apple (MATSDK_MINIMAL_SQLITE is a no-op there since the system
lib is already smaller).
- MSTelemetryConfig.cmake.in: re-find system SQLite3 on Apple, the vcpkg
unofficial-sqlite3 elsewhere.
- docs: note the Apple system-lib behavior.
Validated: non-Apple paths unchanged -- Linux vendored mat builds, and the Linux
vcpkg consumer's generated config resolves unofficial-sqlite3 (if(OFF)) and runs.
The Apple build itself needs validation on macOS/iOS CI (no Mac available here);
the risk is whether find_package(SQLite3) resolves the system lib under the vcpkg
Apple triplets' find-root settings.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment on lines
+35
to
+38
| message(FATAL_ERROR | ||
| "curl-openssl (default) and curl-mbedtls are mutually exclusive but both were " | ||
| "selected. To use mbedTLS, drop the default with the [core,...] form, e.g. " | ||
| "cpp-client-telemetry[core,curl-mbedtls].") |
Comment on lines
+384
to
+387
| message(FATAL_ERROR | ||
| "libcurl was not found. The vcpkg port provides the curl HTTP client " | ||
| "through the curl-openssl (default) or curl-mbedtls feature. Install " | ||
| "cpp-client-telemetry with its default features or with [core,curl-mbedtls].") |
Comment on lines
+279
to
+280
| 1. `[core,curl-mbedtls]` (the `"default-features": false` form) drops the SDK's | ||
| default `curl-openssl` feature, so the SDK no longer *requests* OpenSSL. |
The curl-openssl/curl-mbedtls guidance in the port's fatal-error messages and docs recommended cpp-client-telemetry[core,curl-mbedtls], but the [core,...] form (default-features:false) drops ALL default features -- including system-sqlite -- not just curl-openssl. That example yields a config-time failure with no SQLite backend selected. Update both FATAL_ERROR messages (portfile.cmake mutual-exclusivity guard, CMakeLists.txt libcurl-not-found) and the docs prose to show a complete, working feature set ([core,curl-mbedtls,system-sqlite]) and to note that [core,...] also drops system-sqlite, so a SQLite backend must be re-selected. Files: tools/ports/cpp-client-telemetry/portfile.cmake, CMakeLists.txt, docs/building-with-vcpkg.md Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
| | zlib | `zlib` | `ZLIB::ZLIB` | Non-Apple. **macOS/iOS link the system `libz`** | | ||
| | nlohmann JSON | `nlohmann-json` | `nlohmann_json::nlohmann_json` | All | | ||
| | libcurl | `curl[openssl]` | `CURL::libcurl` | Non-Windows, non-Apple | | ||
| | libcurl | `curl[openssl]` or `curl[mbedtls]` | `CURL::libcurl` | Non-Windows, non-Apple (selectable; optional) | |
| ] | ||
| }, | ||
| "curl-mbedtls": { | ||
| "description": "Built-in libcurl HTTP client with the mbedTLS backend instead of OpenSSL (smaller footprint). Affects Linux/Android only. Use [core,curl-mbedtls] to drop the default OpenSSL curl.", |
Two doc/manifest accuracy fixes from Copilot review: - The dependency table labeled libcurl 'optional' for non-Windows/non-Apple, but since the no-curl option was removed, Linux/Android vcpkg builds always require curl (only the TLS backend is selectable). Relabel as required. - The curl-mbedtls feature description recommended [core,curl-mbedtls], which drops all defaults (incl. system-sqlite); note that a SQLite backend must be re-selected to avoid a configure-time failure. Files: docs/building-with-vcpkg.md, tools/ports/cpp-client-telemetry/vcpkg.json Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment on lines
+23
to
+42
| # curl-openssl (default) and curl-mbedtls choose the TLS backend for the built-in | ||
| # HTTP client and are mutually exclusive. vcpkg cannot express mutual exclusivity, | ||
| # so fail fast if both are selected (e.g. requesting curl-mbedtls without [core] | ||
| # keeps the default curl-openssl, which would union both TLS backends). | ||
| set(_matsdk_http_features "") | ||
| foreach(_matsdk_http_feature curl-openssl curl-mbedtls) | ||
| if(_matsdk_http_feature IN_LIST FEATURES) | ||
| list(APPEND _matsdk_http_features ${_matsdk_http_feature}) | ||
| endif() | ||
| endforeach() | ||
| list(LENGTH _matsdk_http_features _matsdk_http_feature_count) | ||
| if(_matsdk_http_feature_count GREATER 1) | ||
| message(FATAL_ERROR | ||
| "curl-openssl (default) and curl-mbedtls are mutually exclusive but both were " | ||
| "selected. To use mbedTLS, drop the defaults with the [core,...] form and " | ||
| "re-select a SQLite backend (the [core,...] form also drops the default " | ||
| "system-sqlite feature), e.g. " | ||
| "cpp-client-telemetry[core,curl-mbedtls,system-sqlite] " | ||
| "(or minimal-sqlite in place of system-sqlite).") | ||
| endif() |
Comment on lines
+384
to
+389
| message(FATAL_ERROR | ||
| "libcurl was not found. The vcpkg port provides the curl HTTP client " | ||
| "through the curl-openssl (default) or curl-mbedtls feature. Install " | ||
| "cpp-client-telemetry with its default features, or, for mbedTLS, with " | ||
| "[core,curl-mbedtls,system-sqlite] -- the [core,...] form also drops the " | ||
| "default system-sqlite feature, so re-select system-sqlite or minimal-sqlite.") |
Comment on lines
+426
to
+433
| message(FATAL_ERROR | ||
| "SQLite was not found and the minimal SQLite is not enabled. The vcpkg " | ||
| "port provides SQLite through one of two features: 'system-sqlite' " | ||
| "(default, links the external sqlite3 package) or 'minimal-sqlite' " | ||
| "(builds a private feature-stripped SQLite). Install " | ||
| "cpp-client-telemetry with its default features, or with " | ||
| "[core,minimal-sqlite]. For a direct CMake build, pass " | ||
| "-DMATSDK_MINIMAL_SQLITE=ON or ensure unofficial-sqlite3 is discoverable.") |
Comment on lines
+8
to
+12
| if(@MATSDK_APPLE_SYSTEM_DEPS@) | ||
| find_dependency(SQLite3) | ||
| elseif(NOT @MATSDK_BUNDLE_SQLITE@) | ||
| find_dependency(unofficial-sqlite3 CONFIG) | ||
| endif() |
Comment on lines
+336
to
+352
| ```json | ||
| { | ||
| "dependencies": [ | ||
| { | ||
| "name": "cpp-client-telemetry", | ||
| "default-features": false, | ||
| "features": [ "minimal-sqlite" ] | ||
| } | ||
| ] | ||
| } | ||
| ``` | ||
|
|
||
| Use the `[core,minimal-sqlite]` form (here, `"default-features": false` is the | ||
| `[core]` part) so the default `system-sqlite` feature — and its `sqlite3` | ||
| dependency — is dropped. Requesting `minimal-sqlite` *without* `[core]` still | ||
| pulls in the default `system-sqlite`; that is harmless (the external `sqlite3` is | ||
| installed but unused) but does not save the dependency. |
Five fixes from the Copilot review on the curl/SQLite feature interactions (all stem from vcpkg's [core,...] form dropping ALL default features, not just one): - portfile.cmake: fail fast on Linux/Android when no curl TLS backend is selected (verified: a real vcpkg install of [core,minimal-sqlite] now stops at the portfile with a complete [core,curl-openssl,system-sqlite] example, instead of a later, opaque libcurl-not-found error). - CMakeLists.txt libcurl message: show how to re-select a curl backend (not only mbedTLS) under [core,...], alongside a SQLite backend. - CMakeLists.txt SQLite message: include the valid [core,system-sqlite] path, not only [core,minimal-sqlite]. - MSTelemetryConfig.cmake.in: find_dependency(CURL CONFIG) so the exported package config uses the vcpkg CURLConfig that defines CURL::libcurl (the target MSTelemetryTargets references), matching the unofficial-sqlite3/ nlohmann_json CONFIG siblings and the root CMakeLists CURL CONFIG lookup. - docs: minimal-sqlite manifest example re-selects curl-openssl so the Linux/Android manifest actually configures. Files: CMakeLists.txt, cmake/MSTelemetryConfig.cmake.in, docs/building-with-vcpkg.md, tools/ports/cpp-client-telemetry/portfile.cmake Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment on lines
+34
to
+42
| if(_matsdk_http_feature_count GREATER 1) | ||
| message(FATAL_ERROR | ||
| "curl-openssl (default) and curl-mbedtls are mutually exclusive but both were " | ||
| "selected. To use mbedTLS, drop the defaults with the [core,...] form and " | ||
| "re-select a SQLite backend (the [core,...] form also drops the default " | ||
| "system-sqlite feature), e.g. " | ||
| "cpp-client-telemetry[core,curl-mbedtls,system-sqlite] " | ||
| "(or minimal-sqlite in place of system-sqlite).") | ||
| endif() |
The mutual-exclusivity check (curl-openssl vs curl-mbedtls) previously ran on all platforms. Since curl-openssl is a default feature and the curl dependency is platform-filtered to linux|android, a cross-platform manifest that enables curl-mbedtls without [core] would falsely fail the port on Windows/macOS/iOS -- where curl is not used (WinInet / Apple HTTP) and neither feature pulls curl. Wrap both the mutual-exclusivity (count>1) and no-curl (count==0) checks in a single VCPKG_TARGET_IS_LINUX/ANDROID block so they only fire where the curl backend selection is actually meaningful. Verified on x64-linux: [core,minimal- sqlite] still fails with the no-curl message, and [curl-mbedtls] (no core) still fails with the mutual-exclusivity message. Files: tools/ports/cpp-client-telemetry/portfile.cmake Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three changes that reduce the SDK's binary footprint and dependency surface when it is embedded into a host application (e.g. ONNX Runtime / Foundry Local). All are opt-in or platform-scoped, so the default vcpkg install and the default CMake build are unchanged (built-in curl HTTP client + external
sqlite3).1.
MATSDK_MINIMAL_SQLITE(default OFF) — private feature-stripped SQLiteThe SDK uses SQLite only for its offline event-storage cache (plain tables, transactions, WAL, autovacuum/
VACUUM, a few PRAGMAs, and one custom UTF-8 function).MATSDK_MINIMAL_SQLITE=ONcompiles a private SQLite from the vendored amalgamation with a set of amalgamation-safe strip flags (single source of truth:MATSDK_SQLITE_MINIMAL_DEFS), removing the externalsqlite3dependency and shrinking SQLite ~10.2% (.text) / ~12.5% (stripped object).minimal-sqlitefeature;sqlite3moves into a defaultsystem-sqlitefeature, so[core,minimal-sqlite]drops the external dependency.SQLITE_OMIT_AUTOINIT(theskipSqliteInitAndShutdownconfig lets the host skip the SDK's explicitsqlite3_initialize(), which the host cannot do against a private SQLite) andSQLITE_DEFAULT_MEMSTATUS=0(the SDK arms a soft heap limit viasqlite3_soft_heap_limit64()that is only enforced while memory statistics are on).matpropagates the private SQLite through its link interface, so it is exported/installed asMSTelemetry::sqlite3_bundled(static-only; a sharedmatabsorbs it). The footprint doc notes the static-absorption symbol-visibility caveat.2. Selectable curl TLS backend —
curl-openssl(default) /curl-mbedtlsOn Linux/Android the built-in HTTP client links libcurl, which pulls a TLS backend. The port now exposes the choice as two mutually-exclusive vcpkg features:
curl-openssl(default) —curl[core,openssl], unchanged behavior.curl-mbedtls—curl[core,mbedtls]for a smaller TLS footprint.Because vcpkg only honors
default-features: falsefor top-level dependencies, selecting mbedTLS requires the[core,curl-mbedtls,...]form plus a top-levelcurl[core,mbedtls]override in the consumer manifest (documented, with a verified recipe). The port fails fast on Linux/Android if both or neither curl feature is selected; these guards are scoped to Linux/Android, since Windows (WinInet) and Apple (NSURLSession) do not link curl.3. Apple: link the system
libsqlite3/libzon macOS/iOSmacOS/iOS ship
libsqlite3andlibz, so the port no longer pulls the vcpkgsqlite3/zlibpackages there (gated!osx & !iosin the manifest). The build usesfind_package(SQLite3)/find_package(ZLIB)(SQLite::SQLite3/ZLIB::ZLIB) — the same system libraries the SDK's Swift Package already links — removing ~1 MB of redundant bundled binaries on Apple.minimal-sqlitetherefore has no effect on Apple.Footprint impact
Validation
MATSDK_MINIMAL_SQLITE=ON[core,minimal-sqlite]consumerMSTelemetry::sqlite3_bundled, runs 10/10; external sqlite3 droppedsystem-sqlite+curl-openssl)curl-mbedtlsrecipevcpkg install --dry-run+ full build resolve tocurl[core,mbedtls], no OpenSSL built[core,minimal-sqlite]fails fast (no curl backend);[curl-mbedtls]w/o[core]fails (mutual-exclusive); both messages give complete examplessqlite3_bundled+matcompile/link.text/ −12.5% stripped objectNote: the Apple system-libs path uses CMake's standard
FindSQLite3/FindZLIB; it is validated by the macOS/iOS CI jobs (cannot be built locally without a Mac).Reviewed for correctness, security (the SQLite strip is security-neutral-to-positive —
SQLITE_DQS=0hardens identifier handling), and design; driven through the Copilot review loop.Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com