You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
CMake SDK consumers need to build and consume selected targets from a shared CMake project, while native build tools must not operate on the same build tree concurrently.
This change adds CMakeProjectReference with CMakeTargets, selects artifacts directly from the CMake File API target descriptions, and serializes configure/build execution with a user-scoped cross-process mutex. Existing NativeProjectReference directory-based artifact handling remains supported.
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🟡 Changes recommended
There are build-blocking issues (unused using with warnings-as-errors, and invalid MSBuild metadata method usage in ProjectReference.targets) that must be corrected before merge.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Lite Findings: 2 · 1
New issues introduced by this change (3)
Severity
Finding
src/Microsoft.DotNet.CMake.Sdk/sdk/ProjectReference.targets — The %(CMakeProjectReference.CMakeTargets.Replace(...)) syntax is not a valid MSBuild metadata…
src/Microsoft.DotNet.CMake.Sdk/src/ExecWithMutex.cs — System.IO is not used in this file, and the repo builds with TreatWarningsAsErrors=true, so…
src/Microsoft.DotNet.CMake.Sdk/src/GetCMakeArtifactsFromFileApi.cs — When CMakeTargets is specified, this task doesn’t use config.Directories, but it still fails if…
What changed in this PR
This PR extends the CMake SDK’s project reference model so consumers can build/copy artifacts from specific CMake targets (via the CMake File API) while preventing concurrent native configure/build operations on the same build tree using a cross-process, user-scoped mutex.
Changes:
Add CMakeProjectReference with CMakeTargets metadata and integrate it into restore/build/copy flows.
Update File API artifact discovery to support selecting artifacts by CMake target name(s) in addition to source-directory matching.
Introduce ExecWithMutex and use it to serialize CMake Configure/CoreCompile execution per build output directory.
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🟡 Changes recommended
MSBuild property passing currently breaks multi-target CMakeTargets (semicolon parsing) and target selection can silently succeed on typos, risking missing native outputs without failing the build.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Lite Findings: 1 · 1
New issues introduced by this change (2)
Severity
Finding
src/Microsoft.DotNet.CMake.Sdk/sdk/ProjectReference.targets — CMakeTargets is documented/treated as semicolon-separated, but it’s being embedded directly into…
src/Microsoft.DotNet.CMake.Sdk/src/GetCMakeArtifactsFromFileApi.cs — When CMakeTargets is specified, the task silently succeeds even if one or more requested targets do…
Issues resolved since last review (3)
Severity
Finding
src/Microsoft.DotNet.CMake.Sdk/src/GetCMakeArtifactsFromFileApi.cs — When CMakeTargets is specified, this task doesn’t use config.Directories, but it still fails if… View resolved comment
src/Microsoft.DotNet.CMake.Sdk/src/ExecWithMutex.cs — System.IO is not used in this file, and the repo builds with TreatWarningsAsErrors=true, so… View resolved comment
src/Microsoft.DotNet.CMake.Sdk/sdk/ProjectReference.targets — The %(CMakeProjectReference.CMakeTargets.Replace(...)) syntax is not a valid MSBuild metadata… View resolved comment
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
The comment above this block notes that CMake’s File API “routinely emits” "."/".." segments, but dirSource is only canonicalized when it is not rooted. If CMake returns an absolute directory path containing "."/"..", the comparison against normalizedSourceDir can fail unexpectedly. Canonicalize dirSource for both rooted and non-rooted cases.
ReferencedCMakeTargets is passed through the MSBuild task Properties list without escaping. If CMakeTargets contains multiple targets separated by ';', only the first target will be assigned to ReferencedCMakeTargets and the remaining segments will be parsed as additional (invalid) properties, breaking artifact selection/copy.
The reason will be displayed to describe this comment to others. Learn more.
Copilot review overview
🟡 Changes recommended
The MSBuild invocation that builds CMake project references can batch incorrectly (cross-product) and may build the wrong projects with the wrong CMakeBuildTarget values.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Lite Findings: 1 · 1
New issues introduced by this change (1)
Severity
Finding
src/Microsoft.DotNet.CMake.Sdk/sdk/ProjectReference.targets — The MSBuild invocation for CMake project references is batching over…
Pre-existing issues (1)
Severity
Finding
src/Microsoft.DotNet.CMake.Sdk/src/GetCMakeArtifactsFromFileApi.cs — When CMakeTargets is specified, the task silently succeeds even if one or more requested targets do… View comment
Issues resolved since last review (1)
Severity
Finding
src/Microsoft.DotNet.CMake.Sdk/sdk/ProjectReference.targets — CMakeTargets is documented/treated as semicolon-separated, but it’s being embedded directly into… View resolved comment
CMake target names are case-sensitive, but this lookup uses OrdinalIgnoreCase (and the selection below uses StringComparer.OrdinalIgnoreCase). A request whose casing differs from the real target can be treated as found here even though the generated cmake --build --target command fails on case-sensitive generators. Match with ordinal/case-sensitive comparisons consistently.
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
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.
CMake SDK consumers need to build and consume selected targets from a shared CMake project, while native build tools must not operate on the same build tree concurrently.
This change adds
CMakeProjectReferencewithCMakeTargets, selects artifacts directly from the CMake File API target descriptions, and serializes configure/build execution with a user-scoped cross-process mutex. ExistingNativeProjectReferencedirectory-based artifact handling remains supported.Validation:
To double check: