Skip to content

Move core ProjectReferenceTargets from Managed-only to Common targets#13427

Open
dfederm wants to merge 1 commit intodotnet:mainfrom
dfederm:projref-targets-fixes
Open

Move core ProjectReferenceTargets from Managed-only to Common targets#13427
dfederm wants to merge 1 commit intodotnet:mainfrom
dfederm:projref-targets-fixes

Conversation

@dfederm
Copy link
Copy Markdown
Contributor

@dfederm dfederm commented Mar 21, 2026

Summary

Move the core Build/Clean/Rebuild ProjectReferenceTargets protocol from Microsoft.Managed.After.targets into Microsoft.Common.CurrentVersion.targets so it applies to all project types that import the common targets, not just managed languages (C#, VB, F#). This aligns with the static graph spec which prescribes that common protocols be defined in the common targets.

What changed

Targets files

  • Microsoft.Common.CurrentVersion.targets: Now contains the core ProjectReferenceTargets for Build, Clean, and Rebuild, including OuterBuild items, SkipNonexistentTargets items, and the extensibility properties (ProjectReferenceTargetsForBuild, etc.)
  • Microsoft.Common.CrossTargeting.targets: Now contains the cross-targeting outer build protocol (.default dispatch) that was previously inlined in Managed.After.targets
  • Microsoft.Managed.After.targets: Retains only managed-specific extensions: Publish, DeployOnBuild, GetCopyToPublishDirectoryItems, WPF graph isolation, and InnerBuildProperty/InnerBuildPropertyValues

DeployOnBuild refactor

DeployOnBuild handling changed from property-level composition (appending Publish targets to $(ProjectReferenceTargetsForBuild)) to item-level composition (separate ProjectReferenceTargets items). This is functionally equivalent since the static graph engine merges all items with the same Include value, and eliminates a subtle duplication in the old code where ProjectReferenceTargetsForRebuild included publish targets twice.

Documentation

  • Updated stale docs.microsoft.comlearn.microsoft.com URL
  • Converted absolute GitHub links to relative paths
  • Added prose hints for deep-link references into large targets files
  • Fixed typo in SkipNonexistentTargets comment

Testing

Unit tests (11 new, all passing)

New file src/Build.UnitTests/Graph/ProjectReferenceTargetsProtocol_Tests.cs covering:

  • Non-managed project gets core Build/Clean/Rebuild protocol
  • Managed project protocol unchanged after move
  • DeployOnBuild property→item migration
  • Cross-targeting .default dispatch
  • BuildProjectReferences=false and NoBuild=true fallbacks
  • Clean/Rebuild propagation through multi-project chains

Manual validation

Tested with old (VS 18 Canary) vs new (bootstrap) MSBuild:

  • Key result: Non-managed .proj with /graph /isolate — old MSBuild produces MSB4252 isolation violation ("target not specified in ProjectReferenceTargets"), new MSBuild resolves the graph correctly
  • -getItem:ProjectReferenceTargets /p:IsGraphBuild=true on non-managed .proj: old returns empty, new returns full Build/Clean/Rebuild protocol from Microsoft.Common.CurrentVersion.targets
  • C++ graph build binlogs produced for manual inspection

Move the core Build/Clean/Rebuild ProjectReferenceTargets protocol from
Microsoft.Managed.After.targets into Microsoft.Common.CurrentVersion.targets
so it applies to all project types that import the common targets, not just
managed languages (C#, VB, F#). This aligns with the static graph spec which
prescribes that common protocols be defined in the common targets.

Extract the cross-targeting outer build protocol (using .default dispatch) into
Microsoft.Common.CrossTargeting.targets where it naturally belongs alongside the
DispatchToInnerBuilds target.

Refactor DeployOnBuild handling from property-level composition (appending
Publish targets to the ProjectReferenceTargetsForBuild property) to item-level
composition (separate ProjectReferenceTargets items). This is functionally
equivalent since the static graph engine merges all items with the same Include
value, and eliminates a subtle duplication in the old code.

Microsoft.Managed.After.targets retains managed-specific extensions: Publish,
DeployOnBuild, GetCopyToPublishDirectoryItems, WPF graph isolation, and the
InnerBuildProperty/InnerBuildPropertyValues definitions.

Also fixes:
- Update docs.microsoft.com URL to learn.microsoft.com
- Convert absolute GitHub links to relative paths in protocol docs
- Add prose hints for deep-link references into large targets files
- Fix typo: GetTargetFrameworksWithPlatformForSingleTargetFrameworks (plural)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings March 21, 2026 00:14
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Moves the static-graph ProjectReferenceTargets protocol for core Build/Clean/Rebuild from managed-only targets into the common MSBuild targets so it applies to all project types that import the common targets, aligning implementation with the static graph spec.

Changes:

  • Added core static-graph ProjectReferenceTargets (Build/Clean/Rebuild) to Microsoft.Common.CurrentVersion.targets.
  • Added cross-targeting (outer build) static-graph ProjectReferenceTargets dispatch protocol to Microsoft.Common.CrossTargeting.targets.
  • Refactored managed DeployOnBuild propagation from property composition to item-based composition and added new unit tests + doc updates.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/Tasks/Microsoft.Managed.After.targets Removes core Build/Clean/Rebuild protocol and keeps managed-only Publish/DeployOnBuild extensions.
src/Tasks/Microsoft.Common.CurrentVersion.targets Defines core Build/Clean/Rebuild ProjectReferenceTargets protocol for static graph builds.
src/Tasks/Microsoft.Common.CrossTargeting.targets Defines cross-targeting (outer build) static graph protocol for Build/Clean/Rebuild.
src/Build.UnitTests/Graph/ProjectReferenceTargetsProtocol_Tests.cs Adds unit tests validating protocol behavior across managed/non-managed and cross-targeting scenarios.
documentation/ProjectReference-Protocol.md Updates links and clarifies references to protocol/comment locations in targets files.

Comment on lines 7 to +12
This file defines common build logic for all managed languaged: C#, VisualBasic, F#
It is imported after the common targets have been imported.

The core ProjectReferenceTargets for Build, Clean, and Rebuild are defined in
Microsoft.Common.CurrentVersion.targets so they apply to all project types.
This file adds managed-specific extensions (Publish, DeployOnBuild, WPF, etc.).
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the file header comment, "managed languaged" is a typo. Please change it to "managed languages" while touching this header so the comment is accurate and professional.

Copilot uses AI. Check for mistakes.
Comment on lines +30 to +34
private TestEnvironment _env;

public ProjectReferenceTargetsProtocolTests(ITestOutputHelper output)
{
_env = TestEnvironment.Create(output);
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test class constructor passes the ITestOutputHelper directly into TestEnvironment.Create(...) but doesn't store it (and the created TestEnvironment) in readonly fields like other graph tests do. Storing the output helper as a field and using it consistently (e.g., for MockLogger diagnostics) makes failures much easier to diagnose and matches the prevailing pattern in this test suite.

Suggested change
private TestEnvironment _env;
public ProjectReferenceTargetsProtocolTests(ITestOutputHelper output)
{
_env = TestEnvironment.Create(output);
private readonly TestEnvironment _env;
private readonly ITestOutputHelper _output;
public ProjectReferenceTargetsProtocolTests(ITestOutputHelper output)
{
_output = output;
_env = TestEnvironment.Create(_output);

Copilot uses AI. Check for mistakes.
Comment on lines +321 to +322
// The outer build uses CrossTargeting protocol
string outerBuildContent = MultitargetingSpec + CrossTargetingProtocol + CommonCurrentVersionTargetsProtocol + AllDummyTargets;
Copy link

Copilot AI Mar 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the cross-targeting test, outerBuildContent includes CommonCurrentVersionTargetsProtocol. In actual cross-targeting builds, managed projects import Microsoft.Common.CrossTargeting.targets (e.g., via Microsoft.CSharp.CrossTargeting.targets) and do not also import Microsoft.Common.CurrentVersion.targets in the outer build. Including the inner-build protocol in the synthetic outer build can change which ProjectReferenceTargets items exist and may make the test less representative (or accidentally mask protocol issues). Consider modeling the outer build with just the cross-targeting protocol + multitargeting spec, and reserving CommonCurrentVersionTargetsProtocol for inner builds / non-cross-targeting projects.

Suggested change
// The outer build uses CrossTargeting protocol
string outerBuildContent = MultitargetingSpec + CrossTargetingProtocol + CommonCurrentVersionTargetsProtocol + AllDummyTargets;
// The outer build uses CrossTargeting protocol (no current-version protocol in real outer builds)
string outerBuildContent = MultitargetingSpec + CrossTargetingProtocol + AllDummyTargets;

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants