From ab7f3f2018379994c3f14cc93addc1aae06b6c5f Mon Sep 17 00:00:00 2001
From: Doug Bunting <6431421+dougbu@users.noreply.github.com>
Date: Thu, 18 Jun 2020 15:59:34 -0700
Subject: [PATCH] Enable `/warnAsError` (#23072)

* Enable `/warnAsError` in Windows builds
  - already enabled in non-Windows builds because override existed only in build.ps1
* Allow some warnings related to closed issues
  - common `<NoWarn>$(NoWarn);CS1591</NoWarn>` case unchanged
    - /Directory.Build.props ensures that warning remains a warning but doesn't hide it
* !fixup! Root build.sh _does_ disable warnings as errors
  - variable eng/common/tools.sh uses named `warn_as_error`
---
 Directory.Build.props                           | 17 +++++++----------
 build.ps1                                       |  7 +------
 build.sh                                        |  4 ----
 eng/Workarounds.props                           |  7 -------
 ...pNetCore.Mvc.Razor.RuntimeCompilation.csproj |  3 ---
 .../src/Microsoft.NET.Sdk.Razor.csproj          |  2 --
 src/Razor/test/testassets/Directory.Build.props |  3 ---
 7 files changed, 8 insertions(+), 35 deletions(-)

diff --git a/Directory.Build.props b/Directory.Build.props
index d901404a13..4b42f630ad 100644
--- a/Directory.Build.props
+++ b/Directory.Build.props
@@ -36,7 +36,7 @@
     <!-- Workaround issue with ComponentsAnalyzer throwing for interfaces -->
     <DisableImplicitComponentsAnalyzers>true</DisableImplicitComponentsAnalyzers>
   </PropertyGroup>
-  
+
   <!-- Disable logging of some task parameters or metadata to reduce binlog size.
        Reenable logging of any particular item by changing the value of the property below to 'false'
        The format is as follows:
@@ -48,22 +48,22 @@
     <!-- ItemsToHash is used for incremental building and hashes input properties to a file
          This is not generally useful for day-to-day build debugging. -->
     <DisableLogTaskParameter_Hash_ItemsToHash>$(TrimTaskParameters)</DisableLogTaskParameter_Hash_ItemsToHash>
-    
+
     <!-- JoinItems takes input ItemGroups. The output ItemGroup is logged. -->
     <DisableLogTaskParameter_JoinItems_Right>$(TrimTaskParameters)</DisableLogTaskParameter_JoinItems_Right>
     <DisableLogTaskParameter_JoinItems_Left>$(TrimTaskParameters)</DisableLogTaskParameter_JoinItems_Left>
-    
+
     <!-- ConvertToAbsolutePaths - The output parameter (AbsolutePaths) is interesting
          while the input Path is not generally useful. The output itemgroup's metadata
          is not altered by the task. -->
     <DisableLogTaskParameter_ConvertToAbsolutePath_Paths>$(TrimTaskParameters)</DisableLogTaskParameter_ConvertToAbsolutePath_Paths>
     <DisableLogTaskParameterItemMetadata_ConvertToAbsolutePath_Paths>$(TrimTaskParameters)</DisableLogTaskParameterItemMetadata_ConvertToAbsolutePath_Paths>
-    
+
     <!-- The standard msbuild Copy task does not use Metadata and thus the input/outputs
          item metadata is not relevant -->
      <DisableLogTaskParameterItemMetadata_Copy_SourceFiles>$(TrimTaskParameters)</DisableLogTaskParameterItemMetadata_Copy_SourceFiles>
      <DisableLogTaskParameterItemMetadata_Copy_DestinationFiles>$(TrimTaskParameters)</DisableLogTaskParameterItemMetadata_Copy_DestinationFiles>
-     
+
      <!-- Reference metadata for GenerateDepsFile, Csc, RAR, etc. are sometimes useful, but extraordinarily large
           when building against a shared framework where the number of input assemblies is very large.
           Avoid logging these by default. -->
@@ -101,14 +101,11 @@
     <TreatWarningsAsErrors>true</TreatWarningsAsErrors>
     <!-- Don't make missing XML docs a fatal build error, but still surface so we have visibility into undocumented APIs. -->
     <WarningsNotAsErrors>$(WarningsNotAsErrors);CS1591</WarningsNotAsErrors>
+
     <!-- xUnit1004 = warns about skipped tests. Make this a non-fatal build warning. -->
     <WarningsNotAsErrors>$(WarningsNotAsErrors);xUnit1004</WarningsNotAsErrors>
-    <!-- Working around https://github.com/NuGet/Home/issues/8467 -->
-    <NoWarn>$(NoWarn);NU5131</NoWarn>
-    <!-- Needed until we resolve https://github.com/dotnet/aspnetcore-internal/issues/3103 -->
-    <NoWarn>$(NoWarn);NU5048</NoWarn>
 
-    <!-- TODO: Figure out why these warnings are occuring -->
+    <!-- Ignore specific Roslyn warnings for now, https://github.com/dotnet/aspnetcore/issues/22090 -->
     <NoWarn Condition="'$(IsAnalyzersProject)' == 'true'">$(NoWarn);RS1024;RS1025;RS1026</NoWarn>
   </PropertyGroup>
 
diff --git a/build.ps1 b/build.ps1
index d31ab318cc..96e5cb7bf0 100644
--- a/build.ps1
+++ b/build.ps1
@@ -358,17 +358,12 @@ if (-not $foundJdk -and $RunBuild -and ($All -or $BuildJava) -and -not $NoBuildJ
 # Initialize global variables need to be set before the import of Arcade is imported
 $restore = $RunRestore
 
-# Though VS Code may indicate $nodeReuse, $warnAsError and $msbuildEngine are unused, tools.ps1 uses them.
+# Though VS Code may indicate $nodeReuse and $msbuildEngine are unused, tools.ps1 uses them.
 
 # Disable node reuse - Workaround perpetual issues in node reuse and custom task assemblies
 $nodeReuse = $false
 $env:MSBUILDDISABLENODEREUSE=1
 
-# Our build often has warnings that we can't fix, like "MSB3026: Could not copy" due to race
-# conditions in building C++
-# Fixing this is tracked by https://github.com/dotnet/aspnetcore-internal/issues/601
-$warnAsError = $false
-
 # Use `dotnet msbuild` by default
 $msbuildEngine = 'dotnet'
 
diff --git a/build.sh b/build.sh
index 80dbb7f291..3c3cab39ce 100755
--- a/build.sh
+++ b/build.sh
@@ -306,10 +306,6 @@ restore=$run_restore
 nodeReuse=false
 export MSBUILDDISABLENODEREUSE=1
 
-# Our build often has warnings that we can't fix
-# Fixing this is tracked by https://github.com/dotnet/aspnetcore-internal/issues/601
-warn_as_error=false
-
 # Ensure passing neither --bl nor --nobl on CI avoids errors in tools.sh. This is needed because we set both variables
 # to false by default i.e. they always exist. (We currently avoid binary logs but that is made visible in the YAML.)
 if [[ "$ci" == true && "$exclude_ci_binary_log" == false ]]; then
diff --git a/eng/Workarounds.props b/eng/Workarounds.props
index eeb9004c6d..a56cf44b46 100644
--- a/eng/Workarounds.props
+++ b/eng/Workarounds.props
@@ -18,13 +18,6 @@
     <TestRunnerAdditionalArguments>$(TestRunnerAdditionalArguments) -nocolor</TestRunnerAdditionalArguments>
   </PropertyGroup>
 
-  <PropertyGroup>
-    <!-- Working around https://github.com/NuGet/Home/issues/8467 -->
-    <NoWarn>$(NoWarn);NU5131</NoWarn>
-    <!-- Workaround until https://github.com/dotnet/aspnetcore-internal/issues/3103 is resolved -->
-    <NoWarn>$(NoWarn);NU5048</NoWarn>
-  </PropertyGroup>
-
   <!-- Workaround https://github.com/dotnet/roslyn/issues/27975 -->
   <PropertyGroup>
     <!-- We use the compiler toolset that comes from NuGet Packages rather than the SDK built-in.
diff --git a/src/Mvc/Mvc.Razor.RuntimeCompilation/src/Microsoft.AspNetCore.Mvc.Razor.RuntimeCompilation.csproj b/src/Mvc/Mvc.Razor.RuntimeCompilation/src/Microsoft.AspNetCore.Mvc.Razor.RuntimeCompilation.csproj
index 62a1e9eece..565757c491 100644
--- a/src/Mvc/Mvc.Razor.RuntimeCompilation/src/Microsoft.AspNetCore.Mvc.Razor.RuntimeCompilation.csproj
+++ b/src/Mvc/Mvc.Razor.RuntimeCompilation/src/Microsoft.AspNetCore.Mvc.Razor.RuntimeCompilation.csproj
@@ -4,9 +4,6 @@
     <TargetFramework>$(DefaultNetCoreTargetFramework)</TargetFramework>
     <NoWarn>$(NoWarn);CS1591</NoWarn>
 
-    <!-- !!! Temporary workaround while we switch TFMs. -->
-    <NoWarn>$(NoWarn);NU5129</NoWarn>
-
     <GenerateDocumentationFile>true</GenerateDocumentationFile>
     <PackageTags>aspnetcore;aspnetcoremvc;razor</PackageTags>
     <IsPackable>true</IsPackable>
diff --git a/src/Razor/Microsoft.NET.Sdk.Razor/src/Microsoft.NET.Sdk.Razor.csproj b/src/Razor/Microsoft.NET.Sdk.Razor/src/Microsoft.NET.Sdk.Razor.csproj
index 3156331456..efe499aaca 100644
--- a/src/Razor/Microsoft.NET.Sdk.Razor/src/Microsoft.NET.Sdk.Razor.csproj
+++ b/src/Razor/Microsoft.NET.Sdk.Razor/src/Microsoft.NET.Sdk.Razor.csproj
@@ -13,8 +13,6 @@
     <NoWarn>$(NoWarn);NU5100</NoWarn>
     <!-- It is intentional to not include build/netstandard2.0/Microsoft.NET.Sdk.Razor.targets -->
     <NoWarn>$(NoWarn);NU5129</NoWarn>
-    <!-- Ignore file copy warnings -->
-    <NoWarn>$(NoWarn);MSB3030;MSB4181</NoWarn>
     <!-- Cannot run pubternal check on net46 -->
     <DisablePubternalApiCheck>true</DisablePubternalApiCheck>
     <!-- Need to build this project in source build -->
diff --git a/src/Razor/test/testassets/Directory.Build.props b/src/Razor/test/testassets/Directory.Build.props
index b1bd192bc1..ece4b1e256 100644
--- a/src/Razor/test/testassets/Directory.Build.props
+++ b/src/Razor/test/testassets/Directory.Build.props
@@ -26,9 +26,6 @@
 
     <!-- Working around an issue in XDT transforms -->
     <AspNetCoreHostingModel>OutOfProcess</AspNetCoreHostingModel>
-
-    <!-- Working around https://github.com/NuGet/Home/issues/8467 -->
-    <NoWarn>$(NoWarn);NU5131</NoWarn>
   </PropertyGroup>
 
   <!-- Don't use the server when building in place. This locks up rzc.dll -->