Skip to content

Conversation

@surayya-MS
Copy link
Member

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 .

dotnet-maestro bot and others added 30 commits October 30, 2024 14:02
…6.13.0.46

NuGet.Build.Tasks
 From Version 6.12.0-rc.127 -> To Version 6.13.0-preview.1.46
This is based only on code inspection / static analysis / searching around. It is not based on profiling.
…t#11504)

Part of dotnet#11160

### Context
While profiling some of Eric's PRs and one of my experiments, I've
noticed following:
<img width="953" alt="terminall_loger_get_width_height_cost"
src="https://github.com/user-attachments/assets/bb3478c3-e669-4911-b6b0-0c834e38305e"
/>
Terminal width/height are behind a lock and repeated access is quite
expensive.

### Changes Made
I've set it so that the update is happening only once every ~second or
so. This makes the cost negligible.
Note that this appears to be ~1.5% CPU time saved on the main node,
which is the one under heaviest load due to IPC with all the other
nodes.

### Testing
None, this should be non-disruptive change.

### Notes
Refresh frequency is up to discussion. Making it twice a second should
be fine as well.
This reverts commit aa508b8.

No need to worry about speed on this path so let's just match existing behavior.
…6.14.0.66 (dotnet#11552)

NuGet.Build.Tasks
 From Version 6.14.0-preview.1.53 -> To Version 6.14.0-preview.1.66

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: Gang Wang <[email protected]>
Fixes dotnet#9176

Context
Occasionally temp files fail to delete because of virus checkers, so generate MSB5018 warning.
The commit d7788d6#diff-3abd8382aac3bdfa59d5c1ca41dd089795d6ca539a00b3c50eab4fd6a0996314 including string lockedFileMessage = LockCheck.GetLockedFileMessage(fileName); doesn't output the related process.

Changes Made
Add the warning MSB5018
* Switch to AwesomeAssertions

* Account for library breaking change
JaynieBai and others added 19 commits April 16, 2025 09:24
* Add DisallowNull for LoadType property

* Enable Nullable Reference Types

* Fix issues after enable nullable

* Fix System.IndexOutOfRangeException: Index was outside the bounds of the array

* Enable nullable for dile TaskLoader.cs

* Apply suggestions from code review

Co-authored-by: Copilot <[email protected]>

---------

Co-authored-by: YuliiaKovalova <[email protected]>
Co-authored-by: Copilot <[email protected]>
* deleted file import

* deleted unused props files

* bring back import, just unconditional

* removed props files references from more files

* another props removal
And some minor clean-up changes:

* added readonly to a field
* added parameter names to function parameters
* log telemetry load failure

* fix buildmanager state in test

* fix usings broken by merge

* fix race condition
…418.8 (dotnet#11749)

Microsoft.SourceBuild.Intermediate.roslyn , Microsoft.Net.Compilers.Toolset
 From Version 4.14.0-3.25210.2 -> To Version 4.14.0-3.25218.8

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Co-authored-by: YuliiaKovalova <[email protected]>
Add these info to etl traces:

Add a cancellation started event to better track the timeline of cancellation (when a customer attempts to cancel a build)
Add Target List to build project start event ((now it's only on build stop event)
…: Build ID 11466882 (dotnet#11761)

Localized file check-in by OneLocBuild Task: Build definition ID 9434: Build ID 11466940
…ence-packages build 20250423.3 (dotnet#11757)

Microsoft.SourceBuild.Intermediate.source-build-reference-packages
 From Version 9.0.0-alpha.1.25209.1 -> To Version 9.0.0-alpha.1.25223.3

Co-authored-by: dotnet-maestro[bot] <dotnet-maestro[bot]@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 28, 2025 15:00
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.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

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 fixes a regression where the BinaryLogger was not being included for C++ projects on Visual Studio load and also corrects an input concatenation issue in the GenerateResource task.

  • Ensures the BinaryLogger is always appended when the loggers collection is null.
  • Revises the concatenation of task input arrays to use standard collection operations.

Reviewed Changes

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

File Description
src/Tasks/GenerateResource.cs Replaces an invalid bracket/spread syntax with Concat to combine inputs.
src/Build/BackEnd/BuildManager/BuildManager.cs Corrects the concatenation expression to always include the BinaryLogger.
Comments suppressed due to low confidence (2)

src/Tasks/GenerateResource.cs:1708

  • The revised expression correctly combines 'References' and 'AdditionalInputs', ensuring proper behavior. Please verify that the changed type (from ITaskItem[] to IEnumerable) is acceptable in this context.
            var inputs = (this.References ?? []).Concat(this.AdditionalInputs ?? []);

src/Build/BackEnd/BuildManager/BuildManager.cs:681

  • This change ensures that the BinaryLogger is always included even when 'loggers' is null. Double-check that the new array literal syntax '[logger]' is supported by the target framework.
                return (loggers ?? []).Concat([logger]);

@surayya-MS surayya-MS changed the base branch from main to vs17.14 April 28, 2025 15:10
@surayya-MS surayya-MS requested a review from a team as a code owner April 28, 2025 15:10
@surayya-MS surayya-MS closed this Apr 28, 2025
@surayya-MS
Copy link
Member Author

Closing this as we want to target 17.14 #11774

@surayya-MS surayya-MS deleted the binlog-vs-load branch April 28, 2025 15:15
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.

VS binlogs MSBUILDDEBUGENGINE regression AB#2395325