Skip to content

Conversation

@surayya-MS
Copy link
Member

@surayya-MS surayya-MS commented Apr 23, 2025

Fixes #11678

Context

Regression in Visual Studio 17.13 (worked in 17.12).
Binlog is not created for C++ project on Visual Studio load.

The bug is on this line:

return (loggers ?? [logger]);

The PR that changed this line:
https://github.com/dotnet/msbuild/pull/10758/files#diff-2b0716a511d8f4ee690ebd5c3a59dec1e3f9a5eab4ab2a80a1018820a658accbL671

The code before and after

- return (loggers ?? Enumerable.Empty<ILogger>()).Concat(new[] { logger });
+ return (loggers ?? [logger]);

Before logger (BinaryLogger here) was always included.

Changes Made

Made sure to include the BinaryLogger.

Testing

Manual testing:

  1. Get any C++ projec. I got it from the feedback ticket
  2. In the terminal set MSBUILDDEBUGENGINE and MSBUILDDEBUGPATH
  3. in the same terminal open the C++ project with devenv
  4. Check the MSBUILDDEBUGPATH for the binlogs

For using the correct msbuild

  1. Use build.cmd script
  2. Use ~\msbuild\artifacts\bin\bootstrap\net472\MSBuild\Current instead of {Visual Studio path }\MSBuild\Current

Notes

There is also a same mistake with misplaced brackets here:
https://github.com/dotnet/msbuild/pull/10758/files#diff-9ee98aebd9b1aea9900e0b2859bdcbe6b6bdda285f4b5771d9bdeb8b2f480b8dL1708

- var inputs = (this.References ?? Enumerable.Empty<ITaskItem>()).Concat(this.AdditionalInputs ?? Enumerable.Empty<ITaskItem>());
+ ITaskItem[] inputs = this.References ?? [..(this.AdditionalInputs ?? [])];

Reverting this causes a test to fail .

@Copilot Copilot AI review requested due to automatic review settings April 23, 2025 15:43
Copy link
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

Fixes the regression where the BinaryLogger was not being included in the loggers for C++ projects under Visual Studio 17.13.

  • Uses the null-coalescing operator to default to an empty collection if loggers is null
  • Concatenates the new BinaryLogger to always include it in the logger collection

@surayya-MS surayya-MS changed the title Cpp binlog fix [vs17.13] Binlog not produced for C++ project on Visual Studio Load Fix Apr 23, 2025
@surayya-MS surayya-MS requested a review from Copilot April 23, 2025 15:43
Copy link
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

This PR addresses a regression in Visual Studio 17.13 where a BinaryLogger was not being appended when loading C++ projects by ensuring that the logger is always included.

  • Fixes the bug by modifying the null check and concatenation on the loggers collection
  • Updates the array initialization syntax using C# 12 patterns

@surayya-MS surayya-MS requested a review from a team as a code owner April 23, 2025 15:50
@surayya-MS surayya-MS changed the base branch from vs17.13 to main April 28, 2025 14:53
@surayya-MS surayya-MS changed the base branch from main to vs17.13 April 28, 2025 14:53
@surayya-MS surayya-MS closed this Apr 28, 2025
@surayya-MS surayya-MS deleted the cpp-binlog-fix branch April 28, 2025 15:00
@surayya-MS
Copy link
Member Author

surayya-MS commented Apr 28, 2025

Closing this PR because we want to target 17.14 instead #11774

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants