Skip to content

Rearchitect vcpkg port for official submission#1414

Open
bmehta001 wants to merge 10 commits intomicrosoft:mainfrom
bmehta001:bhamehta/vcpkg-port-rearchitect
Open

Rearchitect vcpkg port for official submission#1414
bmehta001 wants to merge 10 commits intomicrosoft:mainfrom
bmehta001:bhamehta/vcpkg-port-rearchitect

Conversation

@bmehta001
Copy link
Copy Markdown
Contributor

@bmehta001 bmehta001 commented Mar 16, 2026

Addresses #1412, #1390, and #781
Also addresses ##1339

  • Rearchitects the vcpkg port from a shell-script-based build with different behavior for different platforms, into a pure CMake workflow for official submission to the vcpkg registry that should works on all supported platforms
  • Have created vcpkg tests and a sample project where this library was imported into ONNXRuntime and successfully tested these and all existing unit/functional tests where applicable, on Windows, WSL, Android Simulator on WSL with NDK, macOS, and iOS Simulator on macOS

Changes:

vcpkg

  • For portfile.cmake, use vcpkg_from_github() + vcpkg_cmake_configure() + vcpkg_cmake_install() + vcpkg_cmake_config_fixup()
  • Added vcpkg.json manifest with dependency declarations (sqlite3, zlib, nlohmann-json, curl), platform constraints (!uwp), and optional feature flags (azmon, privacyguard, cds, signals, sanitizer, liveeventinspector).
    • zlib in vcpkg does not have the SIMD patches - have added more at the end for how to fix this.
  • Added usage file (vcpkg usage instructions shown after install)
  • Removed: CONTROL, TODO.md, get_repo_name.sh, response_file_linux.txt, response_file_mac.txt, v142-build.patch, since they are now redundant or outdated

CMake (had to modify to fix some behavior in legacy builds and for vcpkg)

  • Use MATSDK_USE_VCPKG_DEPS option to detect vcpkg toolchain and thus
    • Resolve dependencies with find_package() ; when OFF, use existing vendored/system-installed workflow
    • Skip manual compiler/platform flag configuration (architecture, paths, etc.) for vcpkg builds
    • Added CMake package config (cmake/MSTelemetryConfig.cmake.in) to generate find_package(MSTelemetry CONFIG) support with transitive dependency re-finding (find_dependency() for sqlite3, zlib, nlohmann-json, curl, pthreads as needed).
    • Added install/export targets — install(TARGETS mat EXPORT MSTelemetryTargets ...) with GNUInstallDirs, write_basic_package_version_file(), and configure_package_config_file() to specify paths in standardized way
    • Deduplicated and simplified dependency linking — replaced the tangled shared/static forking with a clear three-way branch: vcpkg mode (imported targets), Android legacy (bundled source compilation), and Linux/macOS legacy
      (-lsqlite3 -lz).
    • Added Android HTTP client selection (HttpClient_Android.cpp is now compiled on Android instead of HttpClient_Curl.cpp)
    • Link Apple frameworks like CoreFoundation, Foundation, CFNetwork, Network, SystemConfiguration, UIKit (iOS) / IOKit (macOS) via target_link_libraries() with -framework syntax
    • Make compatible with CMake 3.5+ without losing compatibility with older CMake versions

Build flags

  • Removed ZLIB_WINAPI from vcpkg builds since vcpkg's zlib does not use WINAPI/STDCALL.

Obj-C/Swift wrapper resilience

  • Added compile-time feature detection to ODWLogger.mm to conditionally compile Privacy Guard and Sanitizer integration. When modules aren't available (e.g., submodule not cloned), calls log a warning instead of failing to compile.
  • Updated Package.swift to add Privacy Guard and Sanitizer compilation conditions.
  • Updated Swift wrapper to use #if canImport() for optional module imports.

Updated samples

  • Added shared MSTelemetrySample.cmake for ease of updating all sample CMakeLists.txt files now include this shared module that handles both vcpkg (find_package) and legacy (vendored) dependency resolution.
  • Removed hardcoded install paths from all sample CMakeLists.txt files, since it was causing local builds to fail

Test infrastructure (tests/vcpkg/)

  • Added standalone vcpkg consumer test (main.cpp, CMakeLists.txt, vcpkg.json) using core APIs (LogManager, EventProperties, PII annotations, event priority) and verifies find_package(MSTelemetry CONFIG) works correctly.
  • Added platform-specific test scripts:
    - test-vcpkg-windows.ps1 — auto-detects host architecture (x64/ARM64), finds VS via vswhere, builds with x64-windows-static or arm64-windows-static, runs 8 integration tests.
    - test-vcpkg-linux.sh — detects host triplet, builds and runs tests.
    - test-vcpkg-macos.sh — builds with host-native triplet, runs tests.
    - test-vcpkg-ios.sh — builds for both arm64-ios (device) and simulator (arm64-ios-simulator), verifies Mach-O binary architecture.
    - test-vcpkg-android.sh — cross-compiles for configurable ABI (arm64-v8a, armeabi-v7a, x86_64, x86), verifies ELF binary production.
  • Added README.md with usage instructions for all platforms.

Pipelines

  • Added vcpkg tests to pipelines

Other changes

  • Added .gitattributes to marks tools/ports/, tests/vcpkg/, and docs/building-with-vcpkg.md with export-ignore so they are excluded from GitHub tarballs and so vcpkg files don't affect SHA needed for vcpkg, since vcpkg doesn't need the port files themselves when downloading the GitHub source tarball
  • Updated .gitignore to ignores vcpkg test build artifacts.
  • Updated docs (docs/building-with-vcpkg.md) to cover all platforms, overlay port usage, feature flags, and troubleshooting.

After merging

  • Need a separate commit to update the SHA and point REF at a release tag (I assume will be 3.10.42.1)
  • Then will officially submit port to vcpkg registry
    • May be possible to set it so that future releases will automatically result in updated vcpkg ports

For zlib vcpkg lacking SIMD patches:

  • zlib-ng does have the SIMD (and many other patches!), but setting ZLIB_COMPAT=on to use that would without changing any existing zlib calls would force downstream vcpkg users to also adopt zlib-ng. Downstream users can override zlib for each triplet themselves with something like:
set(ZLIB_COMPAT ON)

for the triplet .cmake file

Then, in vcpkg.json for the downstream consuming project, a user can use:

   {
     "dependencies": [
       "onnxruntime",
       "mstelemetry"
     ],
     "overrides": [
       { "name": "zlib", "version": "[email protected]" }
     ]
   }

Another option for future consideration would be to use zlib-ng directly with ZLIB_COMPAT OFF (giving us a different namespace and preventing any package conflicts, duplicate symbols, and any issues that can come from that). This would also require updating the codebase to use the updated header files, which means that to prevent breaking the standard CMake build, we would have to also updated the vendored version to use zlib-ng (which would make updating the version easier but would require extensive testing).

@bmehta001 bmehta001 requested a review from a team as a code owner March 16, 2026 09:28
@bmehta001 bmehta001 self-assigned this Mar 16, 2026
@bmehta001 bmehta001 force-pushed the bhamehta/vcpkg-port-rearchitect branch 6 times, most recently from 8c1d2b5 to 0999317 Compare March 19, 2026 04:34
Copy link
Copy Markdown
Contributor

@lalitb lalitb left a comment

Choose a reason for hiding this comment

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

This PR bundles the vcpkg port rewrite with changes that are already under review in #1415 and partially in #1416. That makes this very hard to review.
Please keep these separate. The recommended path:

  1. Let #1415 land first (after addressing its open feedback)
  2. Either close this PR and open a new one rebased on top of #1415 containing only the vcpkg-unique changes or scope this PR down to just those changes and let the CI fail for now until #1415 merges. Either way, the vcpkg work should not carry duplicate bug fixes that are being reviewed and corrected elsewhere.

@bmehta001
Copy link
Copy Markdown
Contributor Author

bmehta001 commented Mar 28, 2026

@lalitb Thanks for the reviews so far. I am not sure if you were on the email thread where I described how I split up the vcpkg PR into 3 to make it easier to review and merge. Unfortunately, I cannot make the branches completely separate, if I want to be able to ensure that the cumulative changes still pass all tests.

So, I separated the pipeline + test fixes into #1415, then created a new branch from that PR with additional CMake changes + cleanup in #1416 (fixed some syntax issues, removed outdated files, used standard CMake style that the vcpkg PR then follows, and necessary compilation fixes). The original vcpkg PR (#1414) contains all these changes plus the changes for a vcpkg port.

Thus, the order that they should be reviewed and merged in are #1415 -> #1416 -> #1414. I will cherry-pick/rebase/merge from main for the later PRs as needed as we go through PR revisions and each previous PR is merged in.

It would be great if we can focus on just #1415 for now. If this is too confusing, I can close the #1416 / #1414 PRs for now and re-open them once #1415 is merged.

Happy to discuss more over email/Teams/a call, if that is easier

@bmehta001 bmehta001 force-pushed the bhamehta/vcpkg-port-rearchitect branch 5 times, most recently from 7a5f685 to 0d9bb76 Compare April 1, 2026 16:55
@bmehta001 bmehta001 force-pushed the bhamehta/vcpkg-port-rearchitect branch 8 times, most recently from 6f874e4 to 5405100 Compare April 29, 2026 21:37
- Modernize CMakeLists.txt: flatten nesting, deduplicate, use
  consistent quoting and variable patterns
- Remove stale header references from vcxproj and vcxitems files
- Simplify test CMakeLists.txt files
- Fix CMake conventions in packaging scripts

Co-authored-by: Copilot <[email protected]>
@bmehta001 bmehta001 force-pushed the bhamehta/vcpkg-port-rearchitect branch from ffc37ed to ed30a8a Compare April 29, 2026 21:42
bmehta001 and others added 3 commits April 30, 2026 06:57
Keep the before.targets toolset selection deterministic on newer
Visual Studio hosts, but only set the MIP props fallback when a
consumer has not already chosen a toolset. While addressing the
MSBuild review comments, also point the optional module test
conditions and source paths at the real lib/modules locations and
use CPACK_PACKAGE_FILE_NAME for the RPM status message.

Co-authored-by: Copilot <[email protected]>
@bmehta001 bmehta001 force-pushed the bhamehta/vcpkg-port-rearchitect branch from 6d9b5c4 to c857e4b Compare May 2, 2026 01:58
@bmehta001 bmehta001 linked an issue May 2, 2026 that may be closed by this pull request
Use a real ZLIB package in the non-vcpkg Linux/macOS path instead of a bare imported z target with no location. This restores the regular debug test path on Ubuntu/macOS while keeping the vendored-vs-vcpkg dependency split intact.

Files changed:
- lib/CMakeLists.txt
- tests/functests/CMakeLists.txt
- tests/unittests/CMakeLists.txt

Co-authored-by: Copilot <[email protected]>
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.

cpp_client_telemetry fails to build with CMake 4.0

2 participants