Home: Honor TreatWarningsAsErrors during NuGet Package creation

[nkolev92]

From an internal thread;

E:\A\_work\3324\s\bin\2\win-x86\dotnet\sdk\2.1.500-preview-009365\Sdks\NuGet.Build.Tasks.Pack\build\NuGet.Build.Tasks.Pack.targets(202,5): error NU5118: File 'E:\A\_work\3324\s\TestAssets\TestPackages\dotnet-portable-v1-prefercli\bin\Release\netcoreapp1.1\dotnet-portable-v1-prefercli.runtimeconfig.json' is not added because the package already contains file 'lib\netcoreapp1.1\dotnet-portable-v1-prefercli.runtimeconfig.json' [E:\A\_work\3324\s\TestAssets\TestPackages\dotnet-portable-v1-prefercli\dotnet-portable-v1-prefercli.csproj] [E:\A\_work\3324\s\build.proj]
2018-10-13T10:52:25.2756630Z            Successfully created package 'E:\A\_work\3324\s\/bin/2/win-x86/test//packages/dotnet-portable-v1-prefercli.1.0.0.nupkg'.

The package is still created even though it's elevated to an error.

Note that the exit code should not be 0!

[rrelyea]

(in #8576, @ericstj suggests that we also have a problem in incremental builds)

[nkolev92]

I'd imagine https://github.com/NuGet/Home/issues/8576#issuecomment-530947105 explains it, but they should both be done together.

[tompazourek]

I recently created a duplicate of the issue #9373. I got burned by this. NU5123 happened which caused a package to be built and pushed. It wasn't possible to restore it because of the long paths, which caused other problems down the line, all completely avoidable. I have TreatWarningsAsErrors set up exactly for this reason. I managed to fix the issue, but because of this behavior, it can happen anytime again (just rename some folders and there it is). There's no way for me to stop this if TreatWarningsAsErrors doesn't work.

I wouldn't actually mind that the package is created. What's important to me is that the program exit code is not 0. Currently it's 0 and appears all successful. I'm calling it from MSBuild (/t:Pack /p:NoBuild=true), and it says "Build Successful" as well... That means that the build/pack is all green in CI and I don't notice the errors unless I specifically dig for errors in successful tasks (which is not something developers usually do).

If the program exit code wasn't 0, I would see the step as failed, and I don't care if it created some package somewhere... it'd be a failed build, the package would not be pushed. It would be better if it didn't produce the package at all, but the important thing for me is just for it to fail somehow.

[nkolev92]

Yeah, the exit code was implied. Adding it explicitly :)

[AArnott]

From linked bug, here's what I believe is the source of the bug:

The bug appears to be that the PackTask uses return true; in non-exceptional cases.
But the MSBuild Task pattern is to instead return !Log.HasLoggedErrors;, as can be clearly seen.

[nkolev92]

This will be in 5.7/3.1.400.