Skip to content

Conversation

GangWang01
Copy link
Member

Fixes #12248

Context

see #12248 (comment)

Changes Made

Don't register loggers during creating ProjectCollection when doing preprocess because the project isn't built and loggers are not needed.

Testing

Add new tests.

Notes

@GangWang01 GangWang01 force-pushed the logger-itemspec-compare branch from f07c5d1 to 5a98631 Compare August 29, 2025 09:32
@GangWang01 GangWang01 marked this pull request as ready for review August 29, 2025 09:58
@Copilot Copilot AI review requested due to automatic review settings August 29, 2025 09:58
Copy link
Contributor

@Copilot 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

Fixes a crash when using MSBuild's preprocess functionality by preventing logger registration during preprocessing operations. The change ensures that loggers are only registered when actually building projects, not when preprocessing them.

  • Modifies ProjectCollection constructor to conditionally register loggers based on preprocess mode
  • Adds comprehensive test coverage for preprocessing with various verbosity and parallelization options

Reviewed Changes

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

File Description
src/MSBuild/XMake.cs Conditionally passes null instead of loggers to ProjectCollection when preprocessing
src/MSBuild.UnitTests/XMake_Tests.cs Adds new test method with multiple parameter combinations to verify preprocessing works correctly

@@ -1393,7 +1393,7 @@ .. distributedLoggerRecords.Select(d => d.CentralLogger)

projectCollection = new ProjectCollection(
globalProperties,
evaluationLoggers,
isPreprocess ? null : evaluationLoggers,
Copy link
Member

Choose a reason for hiding this comment

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

this deserves a comment, why the condition is there

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.

Unhandled exception when -pp:File.xml property combined with -v:diag
2 participants