dotnet / dotnet/msbuild

Change Waves Warnings Don't Report to Error/Warning Window in VS and Log Many Times

Open
#5,902 7 comments 0 reactions 0 assignees View on GitHub
bug changewaves triaged
Dominant language
C#
Stars
5.5k
Forks
1.5k
Avg merge
1d 13h
Merged PRs (30d)
133

Description

### The Problem
There is currently no mechanism for MSBuild to log a warning **once per build**.
- CLI builds and VS builds log differently.
- The ideal location for us to throw this warning for it to happen once is in `BeginBuild`. This is complicated because VS has zero loggers attached at the time BeginBuild is called. So while this works for CLI builds, this doesn't work at the moment for VS (see Actual Behavior).
- Should we decide to log the warning once per project, or after MSBuild worker nodes have spawned, we will see these warnings **many** times unless deduplicated.

Currently, ChangeWaves warnings get logged at evaluation time when [setting built-in properties](https://github.com/dotnet/msbuild/blob/master/src/Build/Evaluation/Evaluator.cs#L1074). These warnings have two issues with them:
1. The warning gets logged many times per build
2. The warning does not show in the Error/Warning window in VS.

### The Solution
Create a task that runs **once per project** that checks if the user set some invalid change wave, and warn appropriately. This should cut down the number of warnings.
-----

### Steps to Reproduce
in a dev cmd prompt:
`set MSBuildDisableFeaturesFromVersion=16.5`
`devenv someSolution.sln`
Build your project and view the output window

### Expected Behavior
Error/Warning window shows the thrown warning.

### Actual Behavior
![image](https://user-images.githubusercontent.com/4691428/99858214-e2085e80-2b41-11eb-9aeb-df48881d3880.png)
![image](https://user-images.githubusercontent.com/4691428/99858229-ec2a5d00-2b41-11eb-80b2-54207f7550a0.png)

------
A Path Forward (old)
The [new diagnostic API](https://devdiv.visualstudio.com/DevDiv/_wiki/wikis/DevDiv.wiki/10963/Error-List-Remote-Diagnostic-API) is supposed to be able to condense errors/warnings of the same type. See "Identifier/Supersedes integers" in the doc linked. I don't think this is something we can implement at the MSBuild layer. Project System could consider adding this to their Error/Warning window loggers.

There is also the option of attaching a logger just before `BeginBuild` is called, having it only watch for warning events, and detaching it after the `BeginBuild` call here: https://dev.azure.com/devdiv/DevDiv/_git/VS?path=%2Fsrc%2Fenv%2Fvscore%2Fpackage%2FBuildManager%2FBuildManagerAccessor.cs&version=GBmain&line=1740&lineEnd=1741&lineStartColumn=1&lineEndColumn=1&lineStyle=plain&_a=contents

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.