From fb28ce3ec2531327d6f98bfb7d44ad160393a925 Mon Sep 17 00:00:00 2001
From: Doug Bunting <6431421+dougbu@users.noreply.github.com>
Date: Tue, 21 Jul 2020 15:21:30 -0700
Subject: [PATCH] Simplify ref/ assembly generation (#24136)

* Simplify ref/ assembly generation
    - followup 1/2 for 5266918ed2be
    - correct the Razor.Tools project
      - `%(Reference.Version)` metadata does not bleed through into `@(PackageReference)` items
      - much more work to do so than to add this special case
      - remove `$(Razor_NewtonsoftJsonPackageVersion)`
    - remove RTMVersions project and use RepoTasks instead
      - make it an error if RepoTasks is not restored before anything else builds
- add items and properties for System.Security.AccessControl

nits:
- remove invalid (ignored) metadata in Directory.Build.props and AzureAppServices.SiteExtension project
- improve / extend a couple of comments
- move `@(Reference)` items together in Microsoft.AspNetCore.Razor.Tools
---
 Directory.Build.props                         |  4 +--
 eng/Build.props                               |  8 ------
 eng/Dependencies.props                        |  2 ++
 eng/RTMVersions/Directory.Build.props         |  6 ----
 eng/RTMVersions/Directory.Build.targets       |  1 -
 eng/RTMVersions/RTMVersions.csproj            | 28 -------------------
 eng/Version.Details.xml                       |  5 ++++
 eng/Versions.props                            | 13 +++++++--
 eng/targets/ResolveReferences.targets         | 12 ++++----
 eng/tools/RepoTasks/RepoTasks.csproj          | 10 +++++++
 .../Microsoft.AspNetCore.Razor.Tools.csproj   |  7 ++---
 ...Core.AzureAppServices.SiteExtension.csproj |  8 +++---
 12 files changed, 41 insertions(+), 63 deletions(-)
 delete mode 100644 eng/RTMVersions/Directory.Build.props
 delete mode 100644 eng/RTMVersions/Directory.Build.targets
 delete mode 100644 eng/RTMVersions/RTMVersions.csproj

diff --git a/Directory.Build.props b/Directory.Build.props
index 21ae56027b..a4335939ce 100644
--- a/Directory.Build.props
+++ b/Directory.Build.props
@@ -151,9 +151,7 @@
       Include="$(RepoRoot)src\Analyzers\Internal.AspNetCore.Analyzers\src\Internal.AspNetCore.Analyzers.csproj"
       ReferenceOutputAssembly="false"
       OutputItemType="Analyzer"
-      PrivateAssets="All"
-      Version="$(InternalAspNetCoreAnalyzersPackageVersion)"
-      IsImplicitlyDefined="true" />
+      PrivateAssets="All" />
   </ItemGroup>
 
   <!-- Compilation options which apply to all languages. Language-specific options should be set in eng/targets/$(lang).Common.props -->
diff --git a/eng/Build.props b/eng/Build.props
index 3740546dd6..f0cb8db0b8 100644
--- a/eng/Build.props
+++ b/eng/Build.props
@@ -44,10 +44,6 @@
     <!-- Project selection can be overridden on the command line by passing in -projects. -->
     <When Condition="'$(ProjectToBuild)' != ''">
       <ItemGroup>
-        <!-- Include RTMVersions.csproj unless this invocation is building RepoTasks.csproj or won't compile C#. -->
-        <ProjectToBuild Include="$(RepoRoot)eng\RTMVersions\RTMVersions.csproj"
-            Exclude="@(ProjectToExclude)"
-            Condition=" $(ProjectToBuild.EndsWith('.csproj')) AND !$(ProjectToBuild.EndsWith('RepoTasks.csproj')) " />
         <ProjectToBuild Include="$(ProjectToBuild)" Exclude="@(ProjectToExclude);$(RepoRoot)**\bin\**\*;$(RepoRoot)**\obj\**\*">
           <RestoreInParallel Condition="'%(Extension)' == '.npmproj'">false</RestoreInParallel>
         </ProjectToBuild>
@@ -125,11 +121,8 @@
         <!--
           Use caution to avoid deep recursion. If the globbing pattern picks up something which exceeds MAX_PATH,
           the entire pattern will silently fail to evaluate correctly.
-
-          Include RTMVersions.csproj when building any managed projects.
         -->
         <DotNetProjects Include="
-                          $(RepoRoot)eng\RTMVersions\RTMVersions.csproj;
                           $(RepoRoot)src\Framework\App.Ref\src\Microsoft.AspNetCore.App.Ref.csproj;
                           $(RepoRoot)src\Framework\App.Runtime\src\Microsoft.AspNetCore.App.Runtime.csproj;
                           $(RepoRoot)src\Framework\App.Ref.Internal\src\Microsoft.AspNetCore.App.Ref.Internal.csproj;
@@ -178,7 +171,6 @@
                           $(RepoRoot)**\obj\**\*;"
                         Condition=" '$(BuildMainlyReferenceProviders)' != 'true' " />
         <DotNetProjects Include="
-                          $(RepoRoot)eng\RTMVersions\RTMVersions.csproj;
                           $(RepoRoot)src\DefaultBuilder\**\src\*.csproj;
                           $(RepoRoot)src\Features\JsonPatch\**\src\*.csproj;
                           $(RepoRoot)src\DataProtection\**\src\*.csproj;
diff --git a/eng/Dependencies.props b/eng/Dependencies.props
index a00e929607..0e3a756de7 100644
--- a/eng/Dependencies.props
+++ b/eng/Dependencies.props
@@ -71,6 +71,8 @@ and are generated based on the last package release.
     <LatestPackageReference Include="System.Reflection.Metadata" />
     <LatestPackageReference Include="System.Runtime.CompilerServices.Unsafe" />
     <LatestPackageReference Include="System.Runtime.InteropServices.RuntimeInformation" />
+    <!-- System.Security.AccessControl should only be referenced in Dependencies.props and RTMVersions.csproj. -->
+    <LatestPackageReference Include="System.Security.AccessControl" />
     <LatestPackageReference Include="System.Security.Cryptography.Cng" />
     <LatestPackageReference Include="System.Security.Cryptography.Pkcs" />
     <LatestPackageReference Include="System.Security.Cryptography.Xml" />
diff --git a/eng/RTMVersions/Directory.Build.props b/eng/RTMVersions/Directory.Build.props
deleted file mode 100644
index 75a2439073..0000000000
--- a/eng/RTMVersions/Directory.Build.props
+++ /dev/null
@@ -1,6 +0,0 @@
-<Project>
-  <!-- Minimize what gets set to avoid useless references in this simple project. -->
-  <Import Project="..\Common.props" />
-  <Import Project="..\Versions.props" />
-  <Import Project="..\Dependencies.props" />
-</Project>
diff --git a/eng/RTMVersions/Directory.Build.targets b/eng/RTMVersions/Directory.Build.targets
deleted file mode 100644
index 058246e408..0000000000
--- a/eng/RTMVersions/Directory.Build.targets
+++ /dev/null
@@ -1 +0,0 @@
-<Project />
diff --git a/eng/RTMVersions/RTMVersions.csproj b/eng/RTMVersions/RTMVersions.csproj
deleted file mode 100644
index 486d02b1e4..0000000000
--- a/eng/RTMVersions/RTMVersions.csproj
+++ /dev/null
@@ -1,28 +0,0 @@
-<Project Sdk="Microsoft.NET.Sdk">
-  <!--
-    Gather project references for compilation against RTM packages. %(RTMVersion) is set for about a dozen packages
-    in all servicing builds. Cannot reference two versions of a package, mandating this separate package.
-  -->
-  <PropertyGroup>
-    <TargetFramework>net5.0</TargetFramework>
-    <IsPackable>false</IsPackable>
-
-    <!-- Don't bother building anything here. We only need to ensure the RTM packages are on disk. -->
-    <DebugType>none</DebugType>
-    <IncludeBuildOutput>false</IncludeBuildOutput>
-    <CopyBuildOutputToPublishDirectory>false</CopyBuildOutputToPublishDirectory>
-    <CopyBuildOutputToOutputDirectory>false</CopyBuildOutputToOutputDirectory>
-    <CopyOutputSymbolsToOutputDirectory>false</CopyOutputSymbolsToOutputDirectory>
-    <GenerateDependencyFile>false</GenerateDependencyFile>
-
-    <!-- This project should not be referenced via the `<Reference>` implementation. -->
-    <IsProjectReferenceProvider>false</IsProjectReferenceProvider>
-  </PropertyGroup>
-
-  <ItemGroup>
-    <PackageReference Include="@(LatestPackageReference->HasMetadata('RTMVersion'))" Version="%(RTMVersion)" />
-  </ItemGroup>
-
-  <!-- Arcade SDK calls Test target on every project in the repo but provides an empty fallback. Do same here. -->
-  <Target Name="Test" />
-</Project>
diff --git a/eng/Version.Details.xml b/eng/Version.Details.xml
index 5e6d20b8c0..a59af60d1b 100644
--- a/eng/Version.Details.xml
+++ b/eng/Version.Details.xml
@@ -233,6 +233,11 @@
       <Uri>https://github.com/dotnet/runtime</Uri>
       <Sha>f37dd6fc8595e130909dcb3085a56342d04aa20c</Sha>
     </Dependency>
+    <!-- System.Security.AccessControl should only be referenced in Dependencies.props and RTMVersions.csproj. -->
+    <Dependency Name="System.Security.AccessControl" Version="5.0.0-preview.8.20361.2">
+      <Uri>https://github.com/dotnet/runtime</Uri>
+      <Sha>f37dd6fc8595e130909dcb3085a56342d04aa20c</Sha>
+    </Dependency>
     <Dependency Name="System.Security.Cryptography.Cng" Version="5.0.0-preview.8.20361.2">
       <Uri>https://github.com/dotnet/runtime</Uri>
       <Sha>f37dd6fc8595e130909dcb3085a56342d04aa20c</Sha>
diff --git a/eng/Versions.props b/eng/Versions.props
index 5faede0115..07b3a5246d 100644
--- a/eng/Versions.props
+++ b/eng/Versions.props
@@ -116,6 +116,8 @@
     <SystemReflectionMetadataPackageVersion>5.0.0-preview.8.20361.2</SystemReflectionMetadataPackageVersion>
     <SystemResourcesExtensionsPackageVersion>5.0.0-preview.8.20361.2</SystemResourcesExtensionsPackageVersion>
     <SystemRuntimeCompilerServicesUnsafePackageVersion>5.0.0-preview.8.20361.2</SystemRuntimeCompilerServicesUnsafePackageVersion>
+    <!-- System.Security.AccessControl should only be referenced in Dependencies.props and RTMVersions.csproj. -->
+    <SystemSecurityAccessControlPackageVersion>5.0.0-preview.8.20361.2</SystemSecurityAccessControlPackageVersion>
     <SystemSecurityCryptographyCngPackageVersion>5.0.0-preview.8.20361.2</SystemSecurityCryptographyCngPackageVersion>
     <SystemSecurityCryptographyPkcsPackageVersion>5.0.0-preview.8.20361.2</SystemSecurityCryptographyPkcsPackageVersion>
     <SystemSecurityCryptographyXmlPackageVersion>5.0.0-preview.8.20361.2</SystemSecurityCryptographyXmlPackageVersion>
@@ -163,6 +165,14 @@
     Upshot is we need Major.Minor.0 runtime packages for compilation and the targeting pack and Major.Minor.Latest
     runtime packages for everything else. This is not an issue for assemblies available in Microsoft.NETCore.App.Ref or
     Microsoft.Extensions.Internal.Transport because it is next to impossible we would service those packages.
+
+    System.Security.AccessControl should only be referenced in Dependencies.props and RTMVersions.csproj. Because
+    it's a transitive reference, we reship the ref/ assembly in Microsoft.AspNetCore.App.Ref. dotnet/runtime ships
+    the implementation assemblies in Microsoft.NETCore.App.Runtime.* packages.
+
+    If testing this configuration prior to servicing, update the versions of dependencies too. E.g. change
+    `$(SystemSecurityPrincipalWindowsV0PackageVersion)` if you change `$(SystemSecurityAccessControlV0PackageVersion)`
+    because System.Security.AccessControl will otherwise be loadable. This should not be necessary in servicing.
   -->
   <PropertyGroup Condition=" '$(IsServicingBuild)' == 'true' ">
     <MicrosoftWin32RegistryV0PackageVersion>$(MicrosoftWin32RegistryPackageVersion.Split('.')[0]).$(MicrosoftWin32RegistryPackageVersion.Split('.')[1]).0</MicrosoftWin32RegistryV0PackageVersion>
@@ -252,9 +262,6 @@
     <MonoCecilPackageVersion>0.11.2</MonoCecilPackageVersion>
     <NewtonsoftJsonBsonPackageVersion>1.0.2</NewtonsoftJsonBsonPackageVersion>
     <NewtonsoftJsonPackageVersion>12.0.2</NewtonsoftJsonPackageVersion>
-    <!-- Begin: STOP!!! Razor need to reference the version of JSON that our HOSTS support. -->
-    <Razor_NewtonsoftJsonPackageVersion>9.0.1</Razor_NewtonsoftJsonPackageVersion>
-    <!-- End: STOP!!! Razor need to reference the version of JSON that our HOSTS support. -->
     <NSwagApiDescriptionClientPackageVersion>13.0.4</NSwagApiDescriptionClientPackageVersion>
     <SeleniumSupportPackageVersion>3.12.1</SeleniumSupportPackageVersion>
     <SeleniumWebDriverMicrosoftDriverPackageVersion>17.17134.0</SeleniumWebDriverMicrosoftDriverPackageVersion>
diff --git a/eng/targets/ResolveReferences.targets b/eng/targets/ResolveReferences.targets
index 209b42d1d1..d8d30907c8 100644
--- a/eng/targets/ResolveReferences.targets
+++ b/eng/targets/ResolveReferences.targets
@@ -127,7 +127,7 @@
   </Target>
 
   <!--
-    This target resolves remaining Referene items to Packages, if possible. If not, they are left as Reference items fo the SDK to resolve.
+    This target resolves remaining Reference items to Packages, if possible. If not, they are left as Reference items fo the SDK to resolve.
     This executes on NuGet restore and during DesignTimeBuild. It should not run in the outer, cross-targeting build.
    -->
   <Target Name="ResolveCustomReferences"
@@ -205,21 +205,23 @@
   </Target>
 
   <!--
-    Muck with @(ResolvedCompileFileDefinitions) items between generation and use in order to compile against RTM lib/
+    Change @(ResolvedCompileFileDefinitions) items between generation and use in order to compile against RTM lib/
     or ref/ assemblies. The approach works for all TFMs because it happens after a specific assembly is chosen for
     compilation; no need to restrict this to (say) the default TFM.
 
-    Condition checks for RTMVersions.csproj.nuget.g.props file only to ensure restore operation is complete.
-
     This target could get confused if the layout changes for one of the dozen special-cased packages during servicing.
     E.g. ResolvePackageAssets picks a compatible assembly from the 5.0.1 package and _UseRTMReferenceAssemblies
     changes the path to one not present in the 5.0.0 package. Fortunately, this will break the build with complaints
     about the "invalid strong name".
   -->
   <Target Name="_UseRTMReferenceAssemblies"
-      Condition=" '$(EnableCustomReferenceResolution)' == 'true' AND EXISTS('$(RepoRoot)eng\RTMVersions\obj\RTMVersions.csproj.nuget.g.props') "
+      Condition=" '$(MSBuildProjectName)' != 'RepoTasks' "
+      AfterTargets="ResolvePackageAssets"
       BeforeTargets="GenerateBuildDependencyFile;GeneratePublishDependencyFile;ILLink;ResolveLockFileReferences"
       DependsOnTargets="ResolvePackageAssets">
+    <Error Condition=" !EXISTS('$(RepoRoot)artifacts\obj\RepoTasks\RepoTasks.csproj.nuget.g.props') "
+        Text="The eng/tools/RepoTasks project must be restored before building other projects." />
+
     <JoinItems Left="@(ResolvedCompileFileDefinitions)"
         Right="@(LatestPackageReference->HasMetadata('RTMVersion'))"
         LeftKey="Filename"
diff --git a/eng/tools/RepoTasks/RepoTasks.csproj b/eng/tools/RepoTasks/RepoTasks.csproj
index e71a1d7591..798c6c9fb3 100644
--- a/eng/tools/RepoTasks/RepoTasks.csproj
+++ b/eng/tools/RepoTasks/RepoTasks.csproj
@@ -36,5 +36,15 @@
     <Reference Include="Microsoft.Deployment.WindowsInstaller.Package">
       <HintPath>$(WiXSdkPath)\Microsoft.Deployment.WindowsInstaller.Package.dll</HintPath>
     </Reference>
+
+    <!--
+      Gather project references for compilation against RTM packages. %(RTMVersion) is set for about a dozen packages
+      in all servicing builds. Cannot reference two versions of a package, mandating this separation from projects
+      using the relevant packages.
+    -->
+    <PackageReference Include="@(LatestPackageReference->HasMetadata('RTMVersion'))"
+        IncludeAssets="None"
+        PrivateAssets="All"
+        Version="%(RTMVersion)" />
   </ItemGroup>
 </Project>
diff --git a/src/Razor/Microsoft.AspNetCore.Razor.Tools/src/Microsoft.AspNetCore.Razor.Tools.csproj b/src/Razor/Microsoft.AspNetCore.Razor.Tools/src/Microsoft.AspNetCore.Razor.Tools.csproj
index a12f1b8624..0e4954984b 100644
--- a/src/Razor/Microsoft.AspNetCore.Razor.Tools/src/Microsoft.AspNetCore.Razor.Tools.csproj
+++ b/src/Razor/Microsoft.AspNetCore.Razor.Tools/src/Microsoft.AspNetCore.Razor.Tools.csproj
@@ -36,13 +36,10 @@
     <Compile Include="$(SharedSourceRoot)CommandLineUtils\**\*.cs" />
   </ItemGroup>
 
-  <ItemGroup>
-    <Reference Include="Newtonsoft.Json" Version="$(Razor_NewtonsoftJsonPackageVersion)" />
-    <Reference Include="Microsoft.Css.Parser" Version="$(MicrosoftCssParserPackageVersion)" />
-  </ItemGroup>
-
   <ItemGroup>
     <Reference Include="Microsoft.CodeAnalysis.Razor" />
+    <Reference Include="Microsoft.Css.Parser" />
+    <Reference Include="Newtonsoft.Json" />
   </ItemGroup>
 
   <!-- This makes it so that the runtimeconfig.json is included as part of the build output of the project that references this project. -->
diff --git a/src/SiteExtensions/LoggingAggregate/src/Microsoft.AspNetCore.AzureAppServices.SiteExtension/Microsoft.AspNetCore.AzureAppServices.SiteExtension.csproj b/src/SiteExtensions/LoggingAggregate/src/Microsoft.AspNetCore.AzureAppServices.SiteExtension/Microsoft.AspNetCore.AzureAppServices.SiteExtension.csproj
index 4d043ed846..ebe593e808 100644
--- a/src/SiteExtensions/LoggingAggregate/src/Microsoft.AspNetCore.AzureAppServices.SiteExtension/Microsoft.AspNetCore.AzureAppServices.SiteExtension.csproj
+++ b/src/SiteExtensions/LoggingAggregate/src/Microsoft.AspNetCore.AzureAppServices.SiteExtension/Microsoft.AspNetCore.AzureAppServices.SiteExtension.csproj
@@ -24,10 +24,10 @@
   </PropertyGroup>
 
   <ItemGroup>
-    <Reference Include="Microsoft.AspNetCore.AzureAppServices.SiteExtension.2.1" Version="$(MicrosoftAspNetCoreAzureAppServicesSiteExtension21PackageVersion)" PrivateAssets="All" />
-    <Reference Include="Microsoft.AspNetCore.AzureAppServices.SiteExtension.2.2" Version="$(MicrosoftAspNetCoreAzureAppServicesSiteExtension22PackageVersion)" PrivateAssets="All" />
-    <Reference Include="Microsoft.AspNetCore.AzureAppServices.SiteExtension.3.1.x64" Version="$(MicrosoftAspNetCoreAzureAppServicesSiteExtension31PackageVersion)" PrivateAssets="All" />
-    <Reference Include="Microsoft.AspNetCore.AzureAppServices.SiteExtension.3.1.x86" Version="$(MicrosoftAspNetCoreAzureAppServicesSiteExtension31PackageVersion)" PrivateAssets="All" />
+    <Reference Include="Microsoft.AspNetCore.AzureAppServices.SiteExtension.2.1" PrivateAssets="All" />
+    <Reference Include="Microsoft.AspNetCore.AzureAppServices.SiteExtension.2.2" PrivateAssets="All" />
+    <Reference Include="Microsoft.AspNetCore.AzureAppServices.SiteExtension.3.1.x64" PrivateAssets="All" />
+    <Reference Include="Microsoft.AspNetCore.AzureAppServices.SiteExtension.3.1.x86" PrivateAssets="All" />
     <!-- When updating this add the previous SiteExtension(s) to the list above -->
     <PackageReference Include="Microsoft.AspNetCore.AzureAppServices.SiteExtension.5.0.x86" Version="$(PackageVersion)" PrivateAssets="All" />
     <PackageReference Include="Microsoft.AspNetCore.AzureAppServices.SiteExtension.5.0.x64" Version="$(PackageVersion)" PrivateAssets="All" />