Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MSBuildWarningsAsMessages should allow comma separation #7094

Open
rainersigwald opened this issue Nov 29, 2021 · 4 comments
Open

MSBuildWarningsAsMessages should allow comma separation #7094

rainersigwald opened this issue Nov 29, 2021 · 4 comments

Comments

@rainersigwald
Copy link
Member

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 #7089 (comment)

@Forgind
Copy link
Member

Forgind commented Nov 29, 2021

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
Copy link
Member

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

@benvillalobos
Copy link
Member

benvillalobos commented Dec 6, 2021

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
Copy link
Member

ericstj commented Jan 3, 2024

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: dotnet/machinelearning#6935

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

No branches or pull requests

4 participants