Home: Pack needs support for PackageReference specific NoWarn

[nkolev92]

@iwillspeak commented on Wed, 19 Aug 2020 12:36:47 GMT
We have a dependency on a package with no release build available. Building
release builds of our packages results in the NU5104 warning being issued.

Previously we were able to suppress this warning with NoWarn="NU5104" on
the PackageReference. With the 3.1.401 SDK this no longer works. The
exit status of dotnet pack is now 1, causing our build pipeline to fail.

A minimal reproduction is in this Gist:

https://gist.github.com/iwillspeak/2d503703a579f5623fd5e5fb410a29f1

This can be worked around by installing an older SDK (e.g. 3.1.301) in our CI
pipeline, or by removing the <TreatWarningsAsErrors> property.

I can't find any reference to this being a deliberate change. Is this a regression?
How can we ignore the warning / error in this case? Ideally we would still be
able to use <TreatWarningsAsErrors> for other warnings.

[nkolev92]

@joeloff commented on Wed, 19 Aug 2020 22:19:46 GMT
I tried to repro this on machine with 3.1.401, but couldn't. I don't see anything obvious that would have regressed this. Have you looked at a binlog to see if the NoWarn property is perhaps getting cleared out by another target.

cc @nkolev92 in case he knows of any changes around NuGet.

Update: I can repro when I set this on the PackageReference, but if I set it only as a project property like below, it is suppressed

  <PropertyGroup>
    <TargetFramework>netcoreapp3.1</TargetFramework>
    <!--<TreatWarningsAsErrors>true</TreatWarningsAsErrors>-->
    <NoWarn>$(NoWarn);NU5104</NoWarn>
    <Version>1.2.3</Version>
  </PropertyGroup>

[nkolev92]

@iwillspeak commented on Thu, 20 Aug 2020 08:00:37 GMT
I've updated the Gist with a script that reproduces the change in exit code with docker containers.

On my machine running $ ./test_script.sh produces output similar to:

with SDK 3.1.201
Microsoft (R) Build Engine version 16.5.0+d4cbfca49 for .NET Core
Copyright (C) Microsoft Corporation. All rights reserved.

  Restore completed in 2.27 sec for /test/nu5104bug.csproj.
  nu5104bug -> /test/bin/Debug/netstandard2.0/nu5104bug.dll
  Successfully created package '/test/bin/Debug/nu5104bug.1.2.3.nupkg'.
/usr/share/dotnet/sdk/3.1.201/Sdks/NuGet.Build.Tasks.Pack/build/NuGet.Build.Tasks.Pack.targets(198,5): error NU5104: A stable release of a package should not have a prerelease dependency. Either modify the version spec of dependency "Lucene.Net [4.8.0-beta00011, )" or update the version field in the nuspec. [/test/nu5104bug.csproj]
++ passed. Exit code: 0 ++

with SDK 3.1.301
Microsoft (R) Build Engine version 16.6.0+5ff7b0c9e for .NET Core
Copyright (C) Microsoft Corporation. All rights reserved.

  Determining projects to restore...
  Restored /test/nu5104bug.csproj (in 1.91 sec).
  nu5104bug -> /test/bin/Debug/netstandard2.0/nu5104bug.dll
  Successfully created package '/test/bin/Debug/nu5104bug.1.2.3.nupkg'.
/usr/share/dotnet/sdk/3.1.301/Sdks/NuGet.Build.Tasks.Pack/build/NuGet.Build.Tasks.Pack.targets(198,5): error NU5104: A stable release of a package should not have a prerelease dependency. Either modify the version spec of dependency "Lucene.Net [4.8.0-beta00011, )" or update the version field in the nuspec. [/test/nu5104bug.csproj]
++ passed. Exit code: 0 ++

with SDK 3.1.302
Microsoft (R) Build Engine version 16.6.0+5ff7b0c9e for .NET Core
Copyright (C) Microsoft Corporation. All rights reserved.

  Determining projects to restore...
  Restored /test/nu5104bug.csproj (in 2.11 sec).
  nu5104bug -> /test/bin/Debug/netstandard2.0/nu5104bug.dll
  Successfully created package '/test/bin/Debug/nu5104bug.1.2.3.nupkg'.
/usr/share/dotnet/sdk/3.1.302/Sdks/NuGet.Build.Tasks.Pack/build/NuGet.Build.Tasks.Pack.targets(198,5): error NU5104: A stable release of a package should not have a prerelease dependency. Either modify the version spec of dependency "Lucene.Net [4.8.0-beta00011, )" or update the version field in the nuspec. [/test/nu5104bug.csproj]
++ passed. Exit code: 0 ++

with SDK 3.1.401
Microsoft (R) Build Engine version 16.7.0-preview-20360-03+188921e2f for .NET
Copyright (C) Microsoft Corporation. All rights reserved.

  Determining projects to restore...
  Restored /test/nu5104bug.csproj (in 1.95 sec).
  nu5104bug -> /test/bin/Debug/netstandard2.0/nu5104bug.dll
/usr/share/dotnet/sdk/3.1.401/Sdks/NuGet.Build.Tasks.Pack/build/NuGet.Build.Tasks.Pack.targets(198,5): error NU5104: A stable release of a package should not have a prerelease dependency. Either modify the version spec of dependency "Lucene.Net [4.8.0-beta00011, )" or update the version field in the nuspec. [/test/nu5104bug.csproj]
-- failed. Exit code: 1 --

[nkolev92]

@iwillspeak commented on Thu, 20 Aug 2020 08:07:02 GMT

Update: I can repro when I set this on the PackageReference, but if I set it only as a project property like below, it is suppressed

  <PropertyGroup>
    <TargetFramework>netcoreapp3.1</TargetFramework>
    <!--<TreatWarningsAsErrors>true</TreatWarningsAsErrors>-->
    <NoWarn>$(NoWarn);NU5104</NoWarn>
    <Version>1.2.3</Version>
  </PropertyGroup>

I believe that this would not fail the build as TreatWarningsAsErrors is not enabled. You are
right however. The test script in the Gist passes for 401 with the following properties:

  <PropertyGroup>
    <TargetFramework>netstandard2.0</TargetFramework>
    <Version>1.2.3</Version>
    <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
    <NoWarn>$(NoWarn);NU5104</NoWarn>
  </PropertyGroup>

I guess that when we tried setting that property in our larger solution where we first
encountered this issue we didn't see it 'fix' things because it will need to be set on
all projects that reference that package, directly or indirectly. I'm going to test this
theory now by setting <NoWarn>$(NoWarn);NU5104</NoWarn> with a
Directory.Build.props file.

Edit: Setting this at the root level of our solution with a Directory.Buld.props file
does suppress that warning.

[nkolev92]

@dasMulli commented on Thu, 20 Aug 2020 08:19:21 GMT
However the feature was designed / supposed to work on the PackageReference item so that you can suppress individual packages that you know about and not disable this warning/error globally so you don't run into issues while updating.
@nkolev92 do you know about any changes in this area for restore task input?

[nkolev92]

@iwillspeak commented on Thu, 20 Aug 2020 08:22:02 GMT
Yeah, I'd rather not disable it globally. That package is the _only_ pre-release dependency we have. It would be nice to
prevent other pre-release dependencies slipping in.

[nkolev92]

@nkolev92 commented on Wed, 26 Aug 2020 19:00:03 GMT
I finally got some time to investigate this one.

I think the root cause is that pack doesn't respect per PackageReference NoWarn.

Now that behavior is not a regression. It's always been broken/not supported.

The reason why it appeared to work before was because pack always returned an exit code 0 and I fixed that in 3.1.400, https://github.com/NuGet/NuGet.Client/pull/3367.

I will move this issue to NuGet/Home, but for now the recommendation is suppress it globally.

[nkolev92]

We'd be happy to consider a contribution for this, so marking it as up for grabs.
Code pointers: https://github.com/NuGet/NuGet.Client/blob/dev/src/NuGet.Core/NuGet.Commands/PackCollectorLogger.cs might need to work more like https://github.com/NuGet/NuGet.Client/blob/dev/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/RestoreCollectorLogger.cs.

[dasMulli]

Does this mean that the second example in https://docs.microsoft.com/en-us/nuget/consume-packages/package-references-in-project-files#suppressing-nuget-warnings no longer works as documented or never worked properly?

[nkolev92]

@dasMulli

They work in restore, but they don't work in pack.

It's only pack that doesn't seem to support package specific warnings.

[mholo65]

@nkolev92 if this is still up-for-grabs I can take a stab at it. Perfect for #hacktoberfest

[aortiz-msft]

@mholo65 - it is definitely up-for-grabs. That'd be great!

[nkolev92]

@mholo65

Yep, for ideas look at https://github.com/NuGet/NuGet.Client/blob/dev/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/RestoreCollectorLogger.cs as it implements the restore side equivalent of https://github.com/NuGet/NuGet.Client/blob/dev/src/NuGet.Core/NuGet.Commands/PackCollectorLogger.cs.

Both rely on https://github.com/NuGet/NuGet.Client/blob/dev/src/NuGet.Core/NuGet.Commands/RestoreCommand/Logging/WarningPropertiesCollection.cs right now.
You might need to add some new types there that represent the specific package warning messages.

Finally refer to the resources for development https://github.com/NuGet/NuGet.Client/blob/dev/CONTRIBUTING.md#resources-for-nugetclient-development!