search

dotnet / msbuild

The Microsoft Build Engine (MSBuild) is the build platform for .NET and Visual Studio.
https://docs.microsoft.com/visualstudio/msbuild/msbuild
MIT License
5.2k stars 1.35k forks source link

MSBuildWarningsAsMessages should allow comma separation #7094

Open rainersigwald opened 2 years ago

rainersigwald commented 2 years ago

We don't:

<Project Sdk="Microsoft.NET.Sdk">

  <PropertyGroup>
    <OutputType>Exe</OutputType>
    <TargetFramework>net6.0</TargetFramework>
    <ImplicitUsings>enable</ImplicitUsings>
    <Nullable>enable</Nullable>
    <NoWarn>NAT011,NAT012</NoWarn>
  </PropertyGroup>

  <Target Name="AlwaysRun">
    <Warning Code="NAT011" Text="You fail" />
    <Warning Code="NAT012" Text="Other Fail" />
  </Target>

</Project>
C:\Users\forgind\Desktop\Archives\Bug-specific\myTemp>dotnet build myTemp.csproj /t:AlwaysRun
Microsoft (R) Build Engine version 17.0.0+c9eb9dd64 for .NET
Copyright (C) Microsoft Corporation. All rights reserved.

  Determining projects to restore...
  All projects are up-to-date for restore.
C:\Users\forgind\Desktop\Archives\Bug-specific\myTemp\myTemp.csproj(12,5): warning NAT011: You fail
C:\Users\forgind\Desktop\Archives\Bug-specific\myTemp\myTemp.csproj(13,5): warning NAT012: Other Fail

Build succeeded.

C:\Users\forgind\Desktop\Archives\Bug-specific\myTemp\myTemp.csproj(12,5): warning NAT011: You fail
C:\Users\forgind\Desktop\Archives\Bug-specific\myTemp\myTemp.csproj(13,5): warning NAT012: Other Fail
    2 Warning(s)
    0 Error(s)

Time Elapsed 00:00:00.69

_Originally posted by @Forgind in https://github.com/dotnet/msbuild/pull/7089#discussion_r758470716_

Forgind commented 2 years ago

Note that the same is true for MSBuildWarningsAsErrors—that is, using commas as delimiters doesn't work. In testing that, it also seemed that when I promoted the warnings to errors, the build still "succeeded." I think that was a bug you fixed at some point, @BenVillalobos? Do you remember what version that was in? I seem to be using 17.0.0.52104, and I'd thought it was before that.

benvillalobos commented 2 years ago

That was fixed around 16.10. I don't see that behavior in 17.1.0-preview-21572-11+b1e1a581a

benvillalobos commented 2 years ago

Also tested MSBuildWarningsAsErrors with this repro on 17.0.0-preview-21308-06+420c91c69 and the build failed like it should (using semicolons as the delimiter)

ericstj commented 7 months ago

Can confirm this is still broken in MSBuild version 17.8.3+195e7f5a3 for .NET. CSC honors comma separators but MSBuild does not.

Here's a simple repro. project.csproj

<Project Sdk="Microsoft.NET.Sdk">
  <PropertyGroup>
    <TargetFramework>net8.0</TargetFramework>
    <NoWarn>$(NoWarn);MYWARN0001;BOGUS0001,MYWARN0002,CS1030,BOGUS0001</NoWarn>
  </PropertyGroup>

  <Target Name="WarnTest" AfterTargets="BeforeBuild">
    <Warning Code="MYWARN0001" Text="This is a warning." />
    <Warning Code="MYWARN0002" Text="This is another warning." />
  </Target>
</Project>

class.cs

#warning Warning from C#

The CS1030 is suppressed since CSC handles the comma separated values. MYWARN0001 is suppressed since it's surrounded by semi-colons. MYWARN0002 is not suppressed.

I just broke our official build because of this: https://github.com/dotnet/machinelearning/pull/6935