Rearchitect vcpkg port for official submission#1414
Open
bmehta001 wants to merge 106 commits intomicrosoft:mainfrom
Open
Rearchitect vcpkg port for official submission#1414bmehta001 wants to merge 106 commits intomicrosoft:mainfrom
bmehta001 wants to merge 106 commits intomicrosoft:mainfrom
Conversation
Refactor the CMake build system to support vcpkg-provided dependencies via find_package() when MATSDK_USE_VCPKG_DEPS is enabled. This is auto-detected from the vcpkg toolchain but can also be set explicitly. Changes: - CMakeLists.txt: Add vcpkg detection, guard iOS/macOS manual flags and compiler flags behind vcpkg check, add find_package for sqlite3/zlib/ nlohmann-json/curl, initialize BUILD_IOS/BUILD_APPLE_HTTP for config template, guard bondlite/include path for builds without submodules - lib/CMakeLists.txt: Refactor dependency linking to use vcpkg CMake targets, add install(TARGETS/EXPORT) for CMake package config, fix Apple framework linking for both macOS and iOS (add CFNetwork, Network, UIKit), make /usr/local/include and ZLIB_WINAPI conditional on vcpkg - cmake/MSTelemetryConfig.cmake.in: New config template for downstream find_package(MSTelemetry CONFIG) with platform-conditional deps - tools/ports/mstelemetry/: Rewrite portfile.cmake to use pure CMake (vcpkg_cmake_configure/install/config_fixup), replace CONTROL with vcpkg.json manifest, remove legacy build scripts and patches, add usage - tests/vcpkg/: Integration test suite with consumer project and per-platform test scripts (Windows/Linux/macOS/iOS) - docs/building-with-vcpkg.md: Rewrite with modern instructions Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Pin portfile REF + SHA512 so vcpkg manifest mode works without --head - Rewrite test scripts to use CMake vcpkg toolchain integration instead of a separate vcpkg install step (avoids manifest mode conflicts) - Windows script: use array splatting for cmake args, support -VcpkgRoot parameter to handle vcvarsall.bat overriding VCPKG_ROOT - All platforms: check for vcpkg.cmake toolchain instead of vcpkg binary - Update README with corrected 3-step process and vcvarsall troubleshooting Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Pin portfile REF to latest commit (55d7124) with correct SHA512 - Change curl dependency platform from 'linux' to '!windows & !osx & !ios' to cover any non-Windows/non-Apple platform (e.g., FreeBSD) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Fix C++ stdlib headers not found on macOS CLT: detect when headers exist only in the SDK sysroot and add -isystem path automatically - Add missing -framework SystemConfiguration for ODWReachability - Add MSTEL_LOCAL_SOURCE env override in portfile for local dev testing - Add standalone consumer test project (tests/vcpkg-standalone/) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add export-ignore for tools/ports/ and tests/vcpkg/ in .gitattributes. GitHub tarballs (used by vcpkg_from_github) will no longer include these directories, breaking the chicken-and-egg cycle where portfile SHA512 changes invalidate the tarball hash. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Now that .gitattributes excludes tools/ports/ from archives, future portfile changes won't alter the tarball hash. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…tion The config template used @BUILD_IOS@ and @BUILD_APPLE_HTTP@ with mixed YES/NO/ON/OFF values from the CMake build system. This is fragile because CMake's boolean semantics can vary. Normalize both to canonical ON/OFF before configure_package_config_file() and use explicit string comparison in the template for robustness. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
FATAL_ERROR if MATSDK_BUILD_VERSION is not set or empty, preventing write_basic_package_version_file from silently producing a broken version file. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Apple frameworks: extract 5 common frameworks, differ only UIKit vs IOKit - Merge tests/vcpkg-standalone/ into tests/vcpkg/ (7 tests now) - Use ON/OFF for BUILD_IOS/BUILD_APPLE_HTTP from init, remove normalization - Simplify MATSDK_BUILD_VERSION guard - Config template uses native boolean eval (safe with guaranteed ON/OFF) - Net -217 lines Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This was temporary scaffolding for local testing without pushing code. vcpkg provides built-in mechanisms (--head, --editable, overlay ports) for local development. Official ports should only use vcpkg_from_github. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
REPO and HEAD_REF now reference the canonical upstream. REF and SHA512 still need updating after this branch is merged to main. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Link Android NDK log library when CMAKE_SYSTEM_NAME=Android - Share Linux thread/dl linking with Android - Update config template to find Threads on Android - Android uses curl (CPP11 PAL) same as Linux, already covered by vcpkg.json platform filter Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Supports arm64-v8a (default), armeabi-v7a, x86_64, x86 ABIs. Uses VCPKG_CHAINLOAD_TOOLCHAIN_FILE to chain the NDK toolchain with vcpkg's toolchain. Cross-compiled binary verified by existence and can be deployed to device via adb for runtime testing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…1, document duplication - Restore robust MATSDK_BUILD_VERSION guard (check both undefined and empty) - Update portfile REF to tagged release v3.10.42.1 (SHA512 placeholder) - Update vcpkg.json version-semver to 3.10.42.1 - Add comments noting intentional C++ stdlib header workaround duplication Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- point the temporary pre-merge port at a specific tested commit on the branch fork - add BUILD_VERSION so the generated CMake package version matches the port version - use version-string for the 4-part SDK version in vcpkg.json Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- remove --head from the documented install commands - note that --head is only for explicit unreleased branch/tag testing Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Instead of duplicating the entire include_directories() call in both branches of the vcpkg conditional, keep a single include_directories() with all internal paths and gate only /usr/local/include behind if(NOT MATSDK_USE_VCPKG_DEPS). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add #ifndef guards in config-default.h to prevent HAVE_MAT_AI, HAVE_MAT_UTC, HAVE_MAT_SIGNALS, HAVE_MAT_SANITIZER redefinition warnings when both __has_include and CMake add_definitions set them. This was causing build failures with /WX (warnings-as-errors). - Add clarifying comments to lib/CMakeLists.txt distinguishing legacy include_directories() from modern target_include_directories(). - Add detailed comments to MSTelemetryConfig.cmake.in explaining the CURL dependency logic and why @var@ substitution is used. - Remove CMAKE_REQUIRED_LIBRARIES from legacy link lines (it is a CMake internal for check_* macros, not a link dependency). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Add section for installing from the official vcpkg registry - Keep overlay port section for development/pre-release use - Add vcpkg manifest mode example (vcpkg.json snippet) - Add Android platform instructions with NDK requirement - Add Platforms column to dependency table - Improve feature descriptions - Update links to current Microsoft Learn URLs Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Use LANGUAGES keyword in project() for explicitness
- Add CMAKE_CXX_STANDARD_REQUIRED and CMAKE_CXX_EXTENSIONS
- Use get_filename_component for compiler dir (more robust)
- Use message(STATUS ...) instead of message("-- ...")
- Guard CRT matching with MSVC check
- Scope macOS header workaround to target via target_compile_options
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- message(STATUS ...) instead of message("-- ...") everywhere
(root, lib, azmon, ParseOsRelease, MakeDeb/Rpm/Tgz, tests)
- project(MSTelemetry LANGUAGES C CXX) with CMAKE_CXX_STANDARD_REQUIRED
and CMAKE_CXX_EXTENSIONS OFF
- Quote all EXISTS paths with variable expansions (CMP0054)
(lib/CMakeLists.txt, tests/functests, tests/unittests)
- CMAKE_SYSTEM_NAME STREQUAL "Android" instead of bare ANDROID
- get_filename_component() for macOS CLT header workaround
- Remove redundant if(NOT CURL_FOUND) after find_package(CURL REQUIRED)
- Fix message(FATAL_ERROR, ...) to message(FATAL_ERROR ...) in swift wrapper
- find_package(Threads) extended to cover Android
- Portfile REPO restored to microsoft/cpp_client_telemetry
- Static CRT linkage guard scoped to MSVC in test consumer
Verified: legacy CMake build (mat target) passes, vcpkg 7/7 tests pass.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ortfile - Restore accidentally deleted if-block bodies in tests/functests/CMakeLists.txt - Remove outdated FIXME comment in tools/MakeTgz.cmake (arch already present) - Use vcpkg \ variable instead of hardcoded BUILD_VERSION - Switch portfile REPO to bmehta001 fork for branch validation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
REPO: bmehta001/cpp_client_telemetry REF: ee6e6a0 (bhamehta/vcpkg-port-rearchitect branch) HEAD_REF: bhamehta/vcpkg-port-rearchitect This lets vcpkg tests build from the actual branch source code. Revert to microsoft/cpp_client_telemetry with a release tag before submitting the port upstream. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fixes z_size_t typedef missing from zconf.h.cmakein (CMake template was out of sync with the actual zconf.h in 1.2.11). Also addresses known CVEs in the old version. The Chromium SIMD patches (crc_folding.c, fill_window_sse.c, x86.c/h) and simd_stub.c are retained as-is — the SDK compiles simd_stub.c which stubs out all SIMD calls. Verified: Windows legacy CMake build (mat target) passes. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Condense verbose tutorial-style comments to terse developer notes - Remove duplicate PUBLIC keyword in legacy shared-lib link line - Shorten macOS CLT workaround comments in root and test CMakeLists Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Match the pattern used by macOS, iOS, Android, and Windows scripts. Detects aarch64 for arm64-linux, defaults to x64-linux. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Android uses HttpClient_Android, not curl. Exclude Android from curl dependency in vcpkg.json platform expression, config template, and docs. - Simplify config template: capture MATSDK_NEEDS_CURL boolean at build time instead of re-deriving from three separate variables. - Use 'version' instead of 'version-string' in vcpkg.json per vcpkg maintainer guidelines (dot-separated numeric versions). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Remove unused INSTALL_BIN/LIB/INC_DIR variables (GNUInstallDirs used) - Remove CMAKE_SYSTEM_INFO_FILE status message (always empty) - Collapse empty Clang/Intel/MSVC compiler branches to single GCC check - Remove commented-out bondlite tests block - Remove dead INSTALL_LIB_DIR assignments in packaging section Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Runs vcpkg consumer tests on all supported platforms: - Windows (x64-windows-static) - Linux (x64-linux) - macOS (native arch) - iOS (arm64-ios cross-compile) - Android (arm64-v8a cross-compile) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
macos-13 runners were removed from GitHub Actions on Dec 4, 2025. Also update actions/checkout to v4 and deployment target to 14.0. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace 'iPad Pro (11-inch) (4th generation)' with 'iPad Air 11-inch (M2)' for macos-14 to avoid ambiguous xcodebuild destination matching. Use 'iPad Pro 11-inch (M4)' for macos-15. Matrix: macos-14: iPhone 15, iPad Air 11-inch (M2) macos-15: iPhone 16, iPad Pro 11-inch (M4) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fixes ambiguous destination matching on macos-14 where xcodebuild picks the 'Any iOS Device' placeholder over the simulator when multiple OS versions exist. Adding OS=latest forces selection of the newest concrete simulator runtime. Reverts simulator name changes — the original names work correctly with the explicit OS specifier. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
OS=latest is unreliable in newer Xcode versions. Instead, query xcrun simctl for the newest available OS version matching the requested simulator name. This eliminates ambiguous destination matching that caused xcodebuild to pick 'Any iOS Device'. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Replace the Python-based OS version detection with a simple grep for the simulator UUID. Using -destination id=<UUID> is the most reliable way to target a specific simulator, per xcodebuild docs and StackOverflow consensus. No Python dependency needed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The test HTTP server binds with AF_INET (IPv4 only), but NSURLSession on iOS 18 / macOS 15 resolves 'localhost' to ::1 (IPv6) first. The IPv6 connection is refused, and the IPv4 fallback combined with keepalive=false causes intermittent -1005 'network connection was lost' errors when NSURLSession tries to reuse a server-closed connection. Using 127.0.0.1 directly bypasses the IPv6 attempt entirely. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The default NSURLSession configuration caches and reuses TCP connections. When the test HTTP server closes connections (keepalive=false), NSURLSession can attempt to send on a stale connection, causing -1005 'network connection was lost' errors. Ephemeral configuration disables connection caching, ensuring each request gets a fresh connection. This also benefits production use since the SDK's upload pattern (infrequent batched POSTs) does not benefit from persistent connections. Verified locally: 39/39 iOS func tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- PalTests.SystemTime: Widen upper bound from 550ms to 1000ms (sleep 369ms) - PalTests.MonotonicTime: Widen upper bound from 950ms to 1500ms (sleep 789ms) Both still verify clocks work correctly; the wider bounds accommodate loaded CI runners hosting iOS simulators. - APITest.LogManager_Initialize_DebugEventListener: Poll for up to 5s for the storage-full callback to arrive, since it fires asynchronously on the worker thread after Flush() returns. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…down The NSURLSession was a file-level static shared across all SDK instances. On teardown, CancelAllRequests returned before NSURLSession completion handlers had finished executing on Apple's dispatch queue. The HttpClient_Apple object (and its mutex) was then destroyed, and a stale completion handler would crash trying to lock the destroyed mutex: 'mutex lock failed: Invalid argument' Fix: each HttpClient_Apple now owns its own ephemeral NSURLSession. The destructor calls [invalidateAndCancel], which cancels all outstanding tasks and blocks until every in-flight completion handler has finished, guaranteeing no callbacks fire after the object is destroyed. The header stores the session as void* since .hpp cannot reference Obj-C types; the .mm file casts via __bridge_retained/__bridge_transfer for proper ARC memory management. Verified locally: 39/39 iOS tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
When a new commit is pushed to the same branch, any in-progress workflow run for that branch is automatically cancelled, avoiding wasted CI time. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
0ad2247 to
79f1782
Compare
Move [invalidateAndCancel] from the destructor into CancelAllRequests(), which runs while HttpClientManager and its callbacks are still alive (during TelemetrySystem::onStop). This eliminates the theoretical window where invalidateAndCancel could fire a stale completion handler after the HttpClientManager is destroyed. The session is recreated lazily via ensureSession()/GetOrCreateSession() if the SDK is reinitialized. The destructor retains invalidateAndCancel as a safety net for cases where CancelAllRequests was not called before destruction. Verified locally: 39/39 iOS tests pass. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
v1 and v2 are deprecated; v3 will be soon. Update all active workflow files to v4 for Node.js 20 support and latest fixes. Disabled (.off) workflows are left unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- actions/checkout: v1/v2/v3 -> v4 (in .off files) - actions/setup-java: v3 -> v5 - android-actions/setup-android: v2 -> v3 - microsoft/setup-msbuild: v1.1 -> v2 - actions/upload-artifact: v2 -> v4 (in .off file) All updates move to Node.js 20+ runtimes, addressing GitHub's deprecation of Node 16-based actions. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
setup-android@v3 installs cmdline-tools at 16.0 (not 7.0) and adds sdkmanager to PATH automatically. Remove the hardcoded working-directory and call sdkmanager directly. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Replace hardcoded C:\Users\runneradmin with $Env:USERPROFILE in build-android.yml and codeql-analysis.yml - Update vs-version from [16,) (VS 2019+) to [17,) (VS 2022+) in test-win-latest.yml to match the windows-2022 runner Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…raint - Change HEAD_REF from commit SHA to 'main' (vcpkg convention) - Add comment noting BUILD_APPLE_HTTP must stay ON for macOS/iOS since curl is excluded from vcpkg.json on those platforms Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The queue cleanup must not run after detach() — the detached thread still needs the shutdown item to exit its event loop. Cleaning up after detach deletes the shutdown signal, causing the thread to hang and then access destroyed members (use-after-free). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
m_runningLatency is read without locks at lines 219 and 376, while written from other threads. Making it std::atomic eliminates the undefined behavior without requiring additional locks (which could deadlock through upload callbacks). The existing locks remain for other shared state they protect. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…sary) Now that m_runningLatency is std::atomic, the write doesn't need to be inside the lock scope. Restore the original position from main to minimize the diff. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Move m_runningLatency and m_scheduledUploadTime back to before the LOCKGUARD scope, matching the original code on main. The .cpp now has zero diff from main — all threading fixes are in the .hpp (std::atomic declaration). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
bad9bcf to
ae941f9
Compare
MSVC rejects passing std::atomic<T> to variadic functions (deleted copy constructor). Use explicit .load() to pass the underlying EventLatency value. 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.
Addresses #1412, #1390, and #781
Changes:
vcpkg
CMake (had to modify to fix some behavior in legacy builds and for vcpkg)
(-lsqlite3 -lz).
Build flags
Obj-C/Swift wrapper resilience
Updated samples
Test infrastructure (tests/vcpkg/)
- 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.
Bug fixes:
- Fix restartRecoversEventsFromStorage iOS functest failure
- FHttpRequestApple::Cancel() called [session getTasksWithCompletionHandler:] to cancel every task on the shared static NSURLSession.The completion handler callback fires asynchronously on an internal dispatch queue, so when one SDK session tears down and a new session starts sending requests, the stale callback arrives and cancels the new session's in-flight tasks. This caused receivedRequests.size() == 0 in the second block of the restart test. The fix removes the blanket cancel and keeps only [m_dataTask cancel], which cancels the specific task.
Pipelines
Other changes
After merging
For zlib vcpkg lacking SIMD patches:
for the triplet .cmake file
Then, in vcpkg.json for the downstream consuming project, a user can use:
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).