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

NoWarn the new .NET SDK netstandard1.x warning doesn't work #41787

Closed
ViktorHofer opened this issue Jun 25, 2024 · 14 comments
Closed

NoWarn the new .NET SDK netstandard1.x warning doesn't work #41787

ViktorHofer opened this issue Jun 25, 2024 · 14 comments
Labels
Area-NetSDK untriaged Request triage from a team member

Comments

@ViktorHofer
Copy link
Member

/vmr/.dotnet/sdk/9.0.100-preview.7.24325.1/Sdks/Microsoft.NET.Sdk/targets/Microsoft.NET.TargetFrameworkInference.targets(187,5): error NETSDK1215: Targeting .NET Standard prior to 2.0 is no longer recommended. See https://aka.ms/dotnet/dotnet-standard-guidance for more details. [/vmr/src/source-build-reference-packages/src/referencePackages/src/humanizer.core/2.2.0/Humanizer.Core.2.2.0.csproj::TargetFramework=netstandard1.0]

<NoWarn>$(NoWarn);NETSDK1215</NoWarn> doesn't work. I see there's a separate msbuild property to disable the check but why can't I use NoWarn?

@marcpopMSFT @terrajobst

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-NetSDK untriaged Request triage from a team member labels Jun 25, 2024
@KalleOlaviNiemitalo
Copy link
Contributor

Why does your log show "error NETSDK1215" rather than a warning?

Microsoft.Common.CurrentVersion.targets sets MSBuildWarningsAsMessages to match $(NoWarn), which should downgrade the warning. But does your project or build environment set some options that interfere with this?

@ViktorHofer
Copy link
Member Author

Presumably because we upgrade warnings to errors via the TreatWarningsAsErrors=true flag?

@ViktorHofer
Copy link
Member Author

ViktorHofer commented Jun 25, 2024

The

Log.LogWarning(message);
task is used which doesn't add a warning code, it just logs a string.

I think we would need to use https://apisof.net/catalog/e25be74c0e0f3ae3f0230429a6d2c04b to log a warning with a warning code in order for NoWarn / MSBuildWarningsAsMessages to work.

cc @rainersigwald

@baronfel
Copy link
Member

not quite @ViktorHofer - that method is actually not using the MSBuild Logger, it's using the SDK version of Logger:

public void LogWarning(string format, params string[] args)
=> Log(CreateMessage(MessageLevel.Warning, format, args));
. We use this a lot of places to extract a code from a format string and prevent having to keep things in sync. So it should be applying the code as expected.

@ViktorHofer
Copy link
Member Author

Ah I see. So the SDK sends the warning level correctly to msbuild but for some reason NoWarn doesn't work.

@rainersigwald
Copy link
Member

rainersigwald commented Jun 25, 2024

Can I get a repro or a log? I wonder if there's a mix of NoWarn/MSBuildWarningsAsMessages involved, that's caused friction before.

@marcpopMSFT
Copy link
Member

FWIW, I cannot reproduce this. I had tested NoWarn with the original PR and it was able to remove the warning. Here's the project file I have though note that if I remove NoWarn, I don't get NETSDK1215 as an error which is suspicious:

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

  <PropertyGroup>
    <TargetFramework>netstandard1.6</TargetFramework>
    <CheckNotRecommendedTargetFramework>true</CheckNotRecommendedTargetFramework>
    <NoWarn>$(NoWarn);NETSDK1215</NoWarn>
    <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
  </PropertyGroup>

</Project>

@ViktorHofer
Copy link
Member Author

Interesting, this didn't work for SBRP. I'll take a look again tomorrow.

@marcpopMSFT
Copy link
Member

There might be some other setting that's interacting strangely with nowarn.

@rainersigwald
Copy link
Member

IIRC we've also seen some timing things, maybe it's setting NoWarn after it's already been read into MSBuildWarningsAsMessages?

@terrajobst
Copy link
Member

terrajobst commented Jun 26, 2024

Damn. Not being able to suppress this warning makes the warning problematic. If we can't make NoWarn work, we should add a property that allows suppressing the target that raises it.

@ViktorHofer
Copy link
Member Author

We already have a property switch: https://github.com/dotnet/source-build-reference-packages/blob/0b53e839fa2f09a5994cc6006533dcc3d45a4226/Directory.Build.props#L20-L21

This issue is just about making sure that NoWarn also works.

@terrajobst
Copy link
Member

Awesome, glad we did that!

@ViktorHofer
Copy link
Member Author

I just tried this out in a sample app and NoWarn works. I noticed this in the source-build-reference-packages repo so I assume that somewhere there we are accidentally overriding other NoWarn entries. Sorry for the noise everyone.

@ViktorHofer ViktorHofer closed this as not planned Won't fix, can't repro, duplicate, stale Jul 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-NetSDK untriaged Request triage from a team member
Projects
None yet
Development

No branches or pull requests

6 participants