Skip to content

Fix unresolved P2P references losing Configuration in solution builds#13458

Closed
jimpark wants to merge 31 commits into
dotnet:mainfrom
jimpark:main
Closed

Fix unresolved P2P references losing Configuration in solution builds#13458
jimpark wants to merge 31 commits into
dotnet:mainfrom
jimpark:main

Conversation

@jimpark

@jimpark jimpark commented Mar 27, 2026

Copy link
Copy Markdown

Fixes #13453

Context

When building a solution, AssignProjectConfiguration sets
GlobalPropertiesToRemove=Configuration;Platform on project references
not found in the .sln file. This strips Configuration from child
projects, causing them to fall back to their default (typically "Debug")
instead of inheriting the parent's configuration. This breaks any
solution where ProjectReference targets exist outside the solution and
the active configuration differs from the child's default — most
visibly with custom C++ configurations like "Debug Unicode".

Changes Made

Under ChangeWave 18.6, unresolved references no longer have
Configuration and Platform added to GlobalPropertiesToRemove. Child
projects inherit these properties from the parent's global properties,
matching the existing behavior for non-solution builds.

  • src/Tasks/AssignProjectConfiguration.cs — Gate the
    GlobalPropertiesToRemove logic behind ChangeWave 18.6
  • documentation/wiki/ChangeWaves.md — Document the new entry

Testing

  • Added 4 new tests in AssignProjectConfigurationChangeWave_Tests.cs
    covering unresolved references with and without the ChangeWave,
    resolved references with spaces in configuration names, and
    preservation of pre-existing GlobalPropertiesToRemove metadata.
  • Verified 4 prior space-in-configuration tests in MSBuild_Tests.cs
    still pass.
  • Root cause confirmed via programmatic analysis of the user's binlog.

Notes

This is a behavioral change gated behind ChangeWave 18.6. Users who
rely on unresolved references building with their default configuration
can opt out by setting MSBuildDisableFeaturesFromVersion=18.6.

Copilot AI review requested due to automatic review settings March 27, 2026 18:26

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses solution-build project-reference configuration propagation by changing how AssignProjectConfiguration handles unresolved P2P references, so referenced projects no longer lose the parent Configuration/Platform in solution builds (gated behind ChangeWave 18.6).

Changes:

  • Gate the unresolved-reference GlobalPropertiesToRemove=Configuration;Platform behavior behind ChangeWave 18.6 (old behavior only when 18.6 is disabled).
  • Add unit tests validating the new ChangeWave 18.6 behavior for unresolved references and configuration names containing spaces.
  • Document the new ChangeWave 18.6 entry in ChangeWaves.md.

Reviewed changes

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

File Description
src/Tasks/AssignProjectConfiguration.cs Stops stripping Configuration/Platform for unresolved solution references when ChangeWave 18.6 is enabled.
src/Tasks.UnitTests/AssignProjectConfigurationChangeWave_Tests.cs Adds targeted tests around the ChangeWave 18.6 behavior and config/platform metadata handling.
documentation/wiki/ChangeWaves.md Adds ChangeWave 18.6 documentation entry for this behavior change.

Comment thread src/Tasks/AssignProjectConfiguration.cs Outdated
Comment thread src/Tasks.UnitTests/AssignProjectConfigurationChangeWave_Tests.cs
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@jimpark

jimpark commented Mar 27, 2026

Copy link
Copy Markdown
Author

@dotnet-policy-service agree

@jimpark

jimpark commented Mar 29, 2026

Copy link
Copy Markdown
Author

CI failures in Microsoft.Build.Engine.UnitTests appear to be flaky and unrelated to this PR:

  • ba4fdfb — Windows Core failed, Linux Core passed
  • 5c2f7b9 — Windows Core passed, Linux Core failed (no code changes, just CI rerun)

The failures rotate between platforms with identical code. All failures are in Engine.UnitTests, not in
Tasks.UnitTests where this PR's test changes live. The 5 most recent merged PRs (#13450, #13443, #13408, #13389,
#13375) all passed Linux Core cleanly.

Retriggered CI — happy to retrigger again if needed.

@jimpark

jimpark commented Apr 29, 2026

Copy link
Copy Markdown
Author

The current CI failure appears unrelated to this PR.

This branch only changes src/Tasks/AssignProjectConfiguration.cs,
src/Tasks.UnitTests/AssignProjectConfigurationChangeWave_Tests.cs, documentation/wiki/ChangeWaves.md, and
template_feed/content/Microsoft.CheckTemplate/.template.config/template.json.

The failing job is in Microsoft.Build.Engine.UnitTests on net472 x86, where
ProcessLoggingEventConcurrentWithShutdown_DoesNotThrow crashes with:

 Microsoft.Build.Exceptions.InternalLoggerException
 inner exception: System.ArgumentNullException: Parameter "BuildEventContext" cannot be null
 at Microsoft.Build.BackEnd.Logging.ParallelConsoleLogger.MessageHandler(...)

This looks like a pre-existing logging race/bug that my PR is surfacing rather than causing. I don’t think it should be bundled into this PR, but I can open a separate follow-up PR for the logger fix if that would help unblock this one.

@rainersigwald

Copy link
Copy Markdown
Member

I am very concerned about changing this behavior (even though this makes more sense than the existing behavior, I strongly suspect this would break user builds). I am not comfortable taking this change.

@jimpark

jimpark commented Jun 3, 2026

Copy link
Copy Markdown
Author

I appreciate the caution regarding breaking changes, but the risk here seems minimal because the current behavior actively blocks the feature's intended use. We are attempting to move to a decentralized build model where each .vcxproj owns its dependencies, avoiding massive .sln files. This bug is the sole blocker for that migration. Since it stops users from successfully utilizing project-level dependency resolution, it's highly unlikely that there are existing builds relying on the current, broken state.

Would you reconsider and reopen the PR? I would love to figure out a safe way to land this, even if it means placing the behavior behind a specific MSBuild feature flag to entirely isolate the risk

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.

MSBuild v18 truncates configuration names with spaces when passing to Project References

3 participants