Document that public Microsoft.Build.Tasks.CallTarget is not the live implementation#13670
Document that public Microsoft.Build.Tasks.CallTarget is not the live implementation#13670jankratochvilcz wants to merge 4 commits intomainfrom
Conversation
…entation The MSBuild engine resolves CallTarget to the intrinsic implementation in Microsoft.Build.BackEnd.IntrinsicTasks.CallTarget. The public Microsoft.Build.Tasks.CallTarget type is retained only for backwards compatibility in case a third-party task derives from it. Add a remarks note so future readers and contributors don't try to enlighten or otherwise modify this dead-code path. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds documentation clarifying that the public Microsoft.Build.Tasks.CallTarget type is retained for compatibility and is not the intrinsic implementation used when MSBuild executes the <CallTarget> element.
Changes:
- Adds a
<remarks>note toMicrosoft.Build.Tasks.CallTargetexplaining that MSBuild uses an intrinsicCallTargetimplementation at build time. - Documents that the public type exists primarily for backward compatibility (e.g., third-party derivation).
| /// | ||
| /// NOTE: This class is not the implementation that actually runs at build time. The MSBuild engine | ||
| /// resolves <CallTarget> to the intrinsic implementation in | ||
| /// Microsoft.Build.BackEnd.IntrinsicTasks.CallTarget. This type is retained only for backwards |
There was a problem hiding this comment.
The remarks reference Microsoft.Build.BackEnd.IntrinsicTasks.CallTarget, but the intrinsic implementation currently lives in src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/CallTarget.cs under namespace Microsoft.Build.BackEnd (type Microsoft.Build.BackEnd.CallTarget). As written, the doc points readers to a non-existent type/namespace; please update the fully qualified type name (and consider using <c>...</c> or a <see cref> if it won’t break doc generation).
| /// Microsoft.Build.BackEnd.IntrinsicTasks.CallTarget. This type is retained only for backwards | |
| /// <c>Microsoft.Build.BackEnd.CallTarget</c>. This type is retained only for backwards |
There was a problem hiding this comment.
Review Summary
Documentation-only change — no behavioral or breaking-change concerns. The intent is correct and valuable: future contributors should know that Microsoft.Build.Tasks.CallTarget is dead code retained only for third-party derivation.
Issue found
Incorrect namespace in the comment. The comment references Microsoft.Build.BackEnd.IntrinsicTasks.CallTarget, but the actual fully-qualified name of the intrinsic implementation is Microsoft.Build.BackEnd.CallTarget. "IntrinsicTasks" is the folder path, not the namespace. See inline comment for details and a suggested fix.
Verification
I confirmed the claim is otherwise accurate by inspecting:
TaskExecutionHost.cs:971— the engine explicitly redirectsMicrosoft.Build.Tasks.CallTarget→ the intrinsicCallTargettype viaIntrinsicTaskFactory.src/Build/BackEnd/Components/RequestBuilder/IntrinsicTasks/CallTarget.cs:13— namespace isMicrosoft.Build.BackEnd, and the file already has a comment noting it replaces the Tasks version.
Generated by Expert Code Review (on open) for issue #13670 · ● 1.4M
…ntrinsicTasks) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Both reviewers correctly caught that the namespace I gave was wrong — the intrinsic implementation lives in I didn't go with |
Co-authored-by: Rainer Sigwald <rsigwald@gmail.com>
Per @rainersigwald's comment on #13616: the public
Microsoft.Build.Tasks.CallTargetclass is not the implementation that actually runs at build time — the MSBuild engine resolves<CallTarget>to the intrinsic implementation inMicrosoft.Build.BackEnd.IntrinsicTasks.CallTarget(which is already enlightened for multithreaded mode). This type is retained only in case some third-party task derives from it.This PR adds a
<remarks>note to that effect on the public class so future readers/contributors don't try to enlighten or otherwise modify this dead-code path.No behavioral change.