Rearchitect vcpkg port for official submission#1
Open
bmehta001 wants to merge 52 commits into
Open
Conversation
8c1d2b5 to
0999317
Compare
7a5f685 to
0d9bb76
Compare
6d9b5c4 to
c857e4b
Compare
e6fd7e3 to
9e2b8fe
Compare
- 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 <223556219+Copilot@users.noreply.github.com>
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 <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Validate MATSDK_BUILD_VERSION before deriving CPack version components and remove now-dead DAYNUMBER calculations from package scripts. Files changed: - CMakeLists.txt - tools/MakeDeb.cmake - tools/MakeRpm.cmake - tools/MakeTgz.cmake Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
# Conflicts: # lib/CMakeLists.txt
The previous merge from main left a conflict marker block in lib/CMakeLists.txt's trailing commented-link section. Remove the marker block and keep the cleaned-up file content. Files changed: - lib/CMakeLists.txt Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Keep package script version cleanup out of the CMake cleanup PR; package version alignment can be handled separately before the next release. Files changed: - CMakeLists.txt - tools/MakeDeb.cmake - tools/MakeRpm.cmake - tools/MakeTgz.cmake Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add an explicit VS2026/v145 command-line build entry alongside the existing VS2022/v143 script so Windows consumers can choose the right supported toolset without guessing from MSBuild internals. Files changed: - docs/cpp-start-windows.md - build-all-v145.bat - tools/vcvars.cmd - Solutions/before.targets Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This reverts commit b99ff14.
* Initial plan * test: stabilize sqlite reservation timeout test --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Lalit Kumar Bhasin <lalit_fin@yahoo.com> Co-authored-by: bmehta001 <bmehta001@users.noreply.github.com>
…soft#1457) * sqlite: upgrade vendored snapshot 3.34.1 -> 3.53.1 (CVE fixes for modules#248, modules#266) The in-tree SQLite snapshot has been on 3.34.1 (March 2021) since commit 03c78ec. That leaves us exposed to a series of CVEs reported against the modules repo as cpp_client_telemetry_modules#248 and microsoft#266: CVE-2022-35737 String overflow in printf APIs (fixed 3.39.2) CVE-2023-7104 Heap buffer overflow in sqlite3session.c (fixed 3.44.0) CVE-2024-0232 Use-after-free in JSON parser (fixed 3.44.2) CVE-2025-6965 Aggregate function memory corruption (fixed 3.50.2) CVE-2025-70873 zipfile extension info disclosure (post-3.51.1) CVE-2025-7709 FTS5 integer overflow -> controlled OOB (post-3.51.1) Upgrading to the current stable amalgamation (3.53.1, May 2025) addresses all of the above and avoids the trap of landing on an interim version that later requires another bump for a newer CVE. Source: https://www.sqlite.org/2026/sqlite-amalgamation-3530100.zip SHA3-256 3c07136e4f6b5dd0c395be86455014039597bc65b6851f7111e88f71b6e06114 sqlite/sqlite GitHub tag version-3.53.1 (commit ccd445d76a9362c63add000354fac84ba9022176) Files refreshed from the upstream amalgamation: sqlite/sqlite3.h 604 KB -> 690 KB sqlite/sqlite3ext.h 41 KB -> 39 KB sqlite/sqlite3_debug.c sqlite/sqlite3_retail.c sqlite/shell.c 676 KB -> 1182 KB The MEMSET_S patches from PR microsoft#1286 are re-applied. Four of the five original patch sites still exist verbatim in 3.53.1 (pcacheFetchFinishWithInit, sqlite3VdbeCreate, whereClauseInsert, sqlite3WhereBegin); the fifth (sqlite3ExprListAppend zEName clear) was refactored upstream into a single `*pItem = zeroItem;` struct assignment and therefore no longer needs to be wrapped. Trade-off: the previous sqlite3_debug.c was built with `--linemacros` and carried 802 `#line` directives to map back to the unamalgamated `tsrc/*.c` files; sqlite3_retail.c had them stripped. The upstream amalgamation download ships without `#line` directives (matching the previous retail flavor), and we have no Tcl-based regeneration step locally, so for this upgrade both `sqlite3_debug.c` and `sqlite3_retail.c` are copies of the upstream amalgamation. The retail/debug split (and the `sqlite/sqlite3.c` wrapper that selects between them via NDEBUG) is preserved so downstream project files do not need to change; only the source-file mapping in debug builds is affected, and that capability was never tested for the retail flavor. cgmanifest.json: bumped the SQLite registration `CommitHash` to the github.com/sqlite/sqlite mirror commit for tag `version-3.53.1`. sqlite/version.txt: updated the source URL and rewrote the Integration steps section to reflect the new amalgamation-zip layout (the previous `sqlite-win10-all-*.zip` no longer exists for current versions). Closes microsoft/cpp_client_telemetry_modules#248 Closes microsoft/cpp_client_telemetry_modules#266 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Document SQLite CVE fixed-in versions Clarify the historical fixed-in versions for CVE-2023-7104, CVE-2024-0232, and CVE-2025-7709 so the PR audit trail matches public CVE metadata. The upgrade target remains SQLite 3.53.1. Files changed: - none (metadata-only correction in PR description) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Potential fix for pull request finding Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> * Fix SQLite release MT runtime checks Release.vc14x.MT-sqlite builds already optimize for size, so leaving /RTC1 enabled makes modern MSVC reject the SQLite project with D8016. Match the normal Release configs by using default runtime checks. Files changed: - sqlite/sqlite.vcxproj Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Clarify SQLite refresh integration notes Document that SQLite refreshes should review all five historical MEMSET_S patch sites from PR microsoft#1286, even though only four remain in the current amalgamation, and keep both desktop and UWP project files in sync. Files changed: - sqlite/version.txt Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * sqlite: define _GNU_SOURCE in wrapper before libc headers The vendored SQLite amalgamation defines _GNU_SOURCE itself (line 253 of sqlite3_retail.c / sqlite3_debug.c) so it can call mremap() and reference MREMAP_MAYMOVE on glibc-Linux. That self-definition is too late, however: the wrapper sqlite/sqlite3.c includes memset_s.h *before* the amalgamation, and memset_s.h includes <string.h>, which pulls in glibc's <features.h> -- which locks in the non-GNU feature set before the amalgamation's #define ever runs. Result: compiling sqlite/sqlite3.c standalone under gcc/glibc fails with error: 'mremap' undeclared (sqlite3_retail.c line 40724) error: 'MREMAP_MAYMOVE' undeclared (sqlite3_retail.c line 45789) This is pre-existing -- the same failure reproduces against the prior 3.34.1 amalgamation -- and doesn't break the SDK's normal Linux build because lib/CMakeLists.txt:303-310 imports system libsqlite3 there; the vendored .c is only compiled on Windows (where _GNU_SOURCE is irrelevant) and Android (where bionic correctly leaves HAVE_MREMAP=0). But anyone trying to compile the vendored source standalone on glibc-Linux (for testing parity, downstream packaging, or future static-linking work) hits it. Defining _GNU_SOURCE at the top of the wrapper, before any include, fixes the standalone glibc build and is a no-op everywhere else: - Windows MSVC: _GNU_SOURCE is unused. - Android (bionic): HAVE_MREMAP gate is `__linux__ && _GNU_SOURCE`, but bionic doesn't export mremap regardless; the amalgamation's secondary runtime checks handle that. - macOS: not __linux__, so HAVE_MREMAP stays 0. The #ifndef guard avoids -Wmacro-redefined if the build system already sets it. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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 <223556219+Copilot@users.noreply.github.com>
The previous submodule pin (2022.02.02) cannot find Visual Studio 2026 (VS 18) and rejects local vcpkg builds with 'Unable to find a valid Visual Studio instance'. Update to the 2026.04.27 release so the bundled vcpkg supports VS 2026 toolchains. Validated by running tests/vcpkg/test-vcpkg-windows.ps1 with VS 2026, x64-windows-static triplet: vcpkg installed mstelemetry from the pinned GitHub SHA together with sqlite3, zlib, and nlohmann-json, the consumer linked, and all 7 integration assertions passed. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
aeecb06 to
91520cf
Compare
Comment 3319614678: tools/ports/mstelemetry/vcpkg.json used an older port version while the source tree reports 3.10.100.1. Updated the port manifest version to match Version.hpp. Verified at lib/include/public/Version.hpp. Comment 3319614708: iOS simulator execution failure handling was unreachable under set -e. Changed the simctl spawn call to an if/else so the failure path is reachable. Verified at tests/vcpkg/test-vcpkg-ios.sh. Comment 3319614736: Windows vcpkg test passed CMake args through a space-joined cmd string. Generate a temporary batch file and quote each CMake argument individually after vcvarsall initializes the environment. Verified by running tests/vcpkg/test-vcpkg-windows.ps1 locally. Comment 3319614765: installed MSTelemetryConfig.cmake only exists for vcpkg mode, so the MATSDK_USE_VCPKG_DEPS guard around find_dependency was redundant. Removed the guard. Verified at cmake/MSTelemetryConfig.cmake.in and lib/CMakeLists.txt install path. Comment 3319614807: vcpkg workflow used unconditional cancel-in-progress. Match repo workflow behavior and cancel only pull_request runs. Verified at .github/workflows/test-vcpkg.yml. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comments 3338908128/8154/8170/8187/8210/8228/8247/8270/8288: CMake range syntax requires CMake 3.12+ to parse and contradicted the 3.5+ compatibility goal. Replace the changed project files with plain VERSION 3.5 minimums. Verified by CMake configure on Windows. Comments 3338908329/8357: CXX_EXTRA_WARN_FLAGS was defined but not applied after preserving microsoft#1416's CMAKE_CXX_STANDARD cleanup. Append it to Release and Debug CXX flags without reintroducing -std=c++11. Verified at CMakeLists.txt. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment 3338950616: BUILD_SHARED_LIBS was compared to the literal string ON. Use normal CMake boolean evaluation so TRUE, YES, and 1 are handled correctly. Verified at lib/CMakeLists.txt. Comment 3338950649: sample helper only checked libmat.a/libmat.dylib before falling back to a Linux multiarch libdir. Include libmat.so checks for shared Linux installs. Verified at examples/cmake/MSTelemetrySample.cmake. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comments 3338994983/3338995007: Android legacy builds compile bundled sqlite3/zlib as static libraries, which can be linked into shared mat when BUILD_SHARED_LIBS is ON. Enable POSITION_INDEPENDENT_CODE on both bundled targets to avoid relocation failures. Verified at lib/CMakeLists.txt. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment 3339078002: dependency docs described curl as Linux-only, but the port manifest enables curl on any non-Windows, non-Apple, non-Android platform. Broaden the docs wording to match the vcpkg platform expression. Verified at tools/ports/mstelemetry/vcpkg.json and docs/building-with-vcpkg.md. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comments 3339140694/3339140734: ODWLogger.mm inferred PrivacyGuard/Sanitizer availability from header presence, which could compile hard references even when BUILD_PRIVACYGUARD or BUILD_SANITIZER disabled the corresponding wrapper sources. Drive the macros from CMake instead and default them to unavailable outside that build path. Verified at lib/CMakeLists.txt and wrappers/obj-c/ODWLogger.mm. Validation: - CMake configure on Windows legacy path - Material self-review found no issues Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Local review found the overlay port was still downloading an older source snapshot, so vcpkg validation could pass without exercising the current PR code. Update the REF/SHA512 to the current PR source archive and validate the Windows overlay-port consumer against that archive. Validation: - tests/vcpkg/test-vcpkg-windows.ps1 -VcpkgRoot tools/vcpkg - Material self-review found no issues Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment 3339226785: --simulator always builds arm64-ios-simulator, which cannot run on Intel macOS. Add an early Apple Silicon capability check that also allows Rosetta shells on Apple Silicon via hw.optional.arm64. Verified at tests/vcpkg/test-vcpkg-ios.sh. Validation: - bash -n tests/vcpkg/test-vcpkg-ios.sh - Material self-review found no issues Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment 3339346904: APPLE_HTTP was defined in two locations. Remove the duplicate later block and keep the definition near the BUILD_APPLE_HTTP option. Verified at CMakeLists.txt. Comment 3339346945: ODWLogger.h exposes PrivacyGuard/Sanitizer init-config types even when module-backed wrappers are disabled. Always compile the init-config/support classes with BUILD_OBJC_WRAPPER, and keep only the module-backed wrapper implementations conditional. Verified at lib/CMakeLists.txt. Validation: - CMake configure on Windows legacy path - Material self-review found no issues Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Update the overlay port REF/SHA512 so vcpkg validation downloads the current PR source snapshot that includes the latest review fixes. Validation: - tests/vcpkg/test-vcpkg-windows.ps1 -VcpkgRoot tools/vcpkg Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment 3339435283: the simctl create hint omitted the required runtime identifier. Replace the invalid one-liner with guidance to use Xcode Devices and Simulators or list device/runtime identifiers before creating a simulator. Verified at tests/vcpkg/test-vcpkg-ios.sh. Validation: - bash -n tests/vcpkg/test-vcpkg-ios.sh - Material self-review found no issues Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comments 3339555065/3339555113: Package.swift defined MATSDK_DATAVIEWER_AVAILABLE and MATSDK_SANITIZER_AVAILABLE, but no Swift sources read those compilation conditions. Remove the unused defines and keep source exclusion as the availability mechanism. Keep MATSDK_PRIVACYGUARD_AVAILABLE because Logger.swift uses it. Verified at wrappers/swift/Package.swift and wrappers/swift/Sources/OneDSSwift/Logger.swift. Comment 3339555134: no code change. add_compile_definitions requires newer CMake than the 3.5 compatibility target, so zlib must keep add_definitions here. Verified against CMake compatibility requirements. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Update the overlay port REF/SHA512 so the downloaded source snapshot includes the latest Swift package cleanup. This keeps overlay-port validation aligned with the source-bearing PR commits. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Comment 3344040029: the vcpkg port supported any non-UWP target even though the repo only supports Windows, Linux, macOS, iOS, and Android. Restrict the supports expression to those platforms and still exclude UWP. Verified at tools/ports/mstelemetry/vcpkg.json. Comment 3344040085: sqlite3_bundled comment implied this SDK enables -ffast-math. Reword it to describe guarding against toolchain/environment finite-math-only flags. Verified at lib/CMakeLists.txt. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Update the overlay port REF/SHA512 so vcpkg validation downloads the source snapshot that includes the latest supports-expression and comment fixes. Validation: - tests/vcpkg/test-vcpkg-windows.ps1 -VcpkgRoot tools/vcpkg 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.
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.
Other changes
After merging