Apply CMake best practices and remove stale references by bmehta001 · Pull Request #1416 · microsoft/cpp_client_telemetry

bmehta001 · 2026-03-18T08:34:30Z

Pure build/project cleanup

CMake improvements (3 files)

Cleanup / CMake hygiene

add LANGUAGES C CXX to the top-level project() call
switch build/config logging to message(STATUS ...) instead of bare message(...)
Fix message(FATAL_ERROR, ...) comma bug (invalid CMake syntax)
fix if(EXISTS ...) quoting in lib/, tests/functests/, and tests/unittests
simplify cross-version file copies to configure_file(... COPYONLY)
remove stale Visual Studio project/header references
update VS toolset fallback logic in Solutions/before.targets and Solutions/build.MIP.props
Update MATSDK_API_VERSION from 3.4 to 3.10 to match recent releases
Remove empty compiler-id blocks (Clang/Intel/MSVC with no content); GCC -fPIC flags preserved
Remove commented-out bondlite test block and stale TODO comments
Remove duplicate include_directories entry

Visual Studio project cleanup (8 files)

Remove stale header references that no longer exist in the repo:
- targetver.h (SampleCpp, SampleCppMini)
- LogManagerV1.hpp (Shared.vcxitems)
- MockILocalStorageReader.hpp (UnitTests.vcxproj)
- SanitizerBaseTests.cpp (FuncTests.vcxproj)
Modernize VS toolset: use `` fallback instead of hardcoded v141 (Solutions/before.targets, build.MIP.props)

Packaging

keep the package install-dir overrides that Debian/RPM packaging expects
adjust packaging messages / cleanup so the CMake changes stay behavior-correct

lalitb

see comments.

ThomsonTan

As a general suggestion, consider moving the cleanup changes into a separate PR to streamline the review process, reduce cognitive load, and accelerate merging by isolating functional changes from refactoring.

lalitb · 2026-03-21T00:52:32Z

This PR also duplicates the majority of changes from #1415 and parts of #1414. There is no unique content here worth preserving - the one thing it does differently from #1415 is the CMake quoting, and it does it incorrectly.

- 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]>

Copilot

Pull request overview

Build/project cleanup focused on improving CMake hygiene and removing stale Visual Studio project references, while modernizing toolset selection for newer VS installs.

Changes:

Standardize CMake logging to message(STATUS ...), improve if(EXISTS ...) quoting, and simplify test-data copying via configure_file(... COPYONLY).
Update SDK build version prefixing (MATSDK_API_VERSION to 3.10) and remove stale/commented build logic.
Remove references to deleted headers/sources from multiple .vcxproj / .vcxitems files and adjust MSBuild toolset selection defaults.

Reviewed changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
CMakeLists.txt	CMake best-practices cleanup (languages, STATUS messages), version bump, removes stale blocks/comments
lib/CMakeLists.txt	Consistent STATUS messages, quoted `EXISTS` checks, fix `message(FATAL_ERROR ...)` syntax
tools/ParseOsRelease.cmake	Quote `EXISTS` path; switch to `message(STATUS ...)`
tools/MakeTgz.cmake	Switch packaging output to `message(STATUS ...)`
tools/MakeDeb.cmake	Switch packaging output to `message(STATUS ...)`
tools/MakeRpm.cmake	Switch packaging output to `message(STATUS ...)`
tests/unittests/CMakeLists.txt	STATUS messages + quoting in `EXISTS` checks for optional test sources
tests/functests/CMakeLists.txt	STATUS messages, quoting in `EXISTS` checks, simplify test.json copy logic
tests/unittests/UnitTests.vcxproj	Remove stale project item reference to a non-existent test file
tests/functests/FuncTests.vcxproj	Remove stale header reference from project items
tests/functests/FuncTests.vcxproj.filters	Remove stale header reference from filters list
lib/shared/Shared.vcxitems	Remove stale header include from shared items
lib/shared/Shared.vcxitems.filters	Remove stale header include from shared items filters
examples/cpp/SampleCpp/SampleCpp.vcxproj	Remove stale `targetver.h` reference
examples/cpp/SampleCpp/SampleCpp.vcxproj.filters	Remove stale `targetver.h` reference from filters
examples/cpp/SampleCppMini/SampleCppMini.vcxproj	Remove stale `targetver.h` reference
examples/cpp/SampleCppMini/SampleCppMini.vcxproj.filters	Remove stale `targetver.h` reference from filters
Solutions/build.MIP.props	Switch toolset selection to default toolset variable
Solutions/before.targets	Update toolset fallback behavior to prefer default toolset variable

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

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]>

Co-authored-by: Copilot <[email protected]>

bmehta001 requested a review from a team as a code owner March 18, 2026 08:34

bmehta001 force-pushed the bhamehta/code-cleanup branch 2 times, most recently from 34e4948 to 790fdd0 Compare March 18, 2026 09:03

bmehta001 changed the title ~~Use best CMake practices, rm outdated headers, and update VS settings~~ Use best CMake practices, rm/update outdated code Mar 18, 2026

bmehta001 force-pushed the bhamehta/code-cleanup branch 18 times, most recently from 9a5a166 to 2feeb94 Compare March 19, 2026 05:40

bmehta001 self-assigned this Mar 19, 2026