From 96ea32666448a054f70305b8b6cf1a7cd4821c5e Mon Sep 17 00:00:00 2001
From: Nate McMaster <natemcmaster@users.noreply.github.com>
Date: Thu, 13 Dec 2018 14:35:02 -0800
Subject: [PATCH] Document custom reference resolution (#4806)

---
 build/repo.targets                    |  6 +++-
 docs/README.md                        |  5 ++++
 docs/ReferenceResolution.md           | 34 +++++++++++++++++++++++
 eng/Baseline.xml                      |  6 ++++
 eng/Dependencies.props                | 28 +++++++++++--------
 eng/PatchConfig.props                 | 10 +++++++
 eng/ProjectReferences.props           |  6 +++-
 eng/Versions.props                    |  8 ++++++
 eng/targets/CSharp.Common.props       |  9 ++++++
 eng/targets/ResolveReferences.targets | 40 ++++++++++++++++++++++++---
 10 files changed, 135 insertions(+), 17 deletions(-)
 create mode 100644 docs/README.md
 create mode 100644 docs/ReferenceResolution.md
 create mode 100644 eng/Versions.props

diff --git a/build/repo.targets b/build/repo.targets
index 1bb8d78971f..59785014b5b 100644
--- a/build/repo.targets
+++ b/build/repo.targets
@@ -60,7 +60,11 @@
       <ProjectListFile>$(MSBuildThisFileDirectory)..\eng\ProjectReferences.props</ProjectListFile>
       <ProjectListContent>
       <![CDATA[
-<!-- This file is automatically generated. Run `build.cmd /t:GenerateProjectList` to update. -->
+<!--
+  This file is automatically generated. Run `build.cmd /t:GenerateProjectList` to update.
+
+  This file contains a map of assembly names to the projects that build them.
+-->
 <Project>
   <ItemGroup>
     @(_ProjectReferenceProvider->'<ProjectReferenceProvider Include="%(Identity)" ProjectPath="%24(RepositoryRoot)%(ProjectFileRelativePath)" />', '%0A    ')
diff --git a/docs/README.md b/docs/README.md
new file mode 100644
index 00000000000..98fe46e4032
--- /dev/null
+++ b/docs/README.md
@@ -0,0 +1,5 @@
+Contributor documentation
+=========================
+
+The primary audience for documentation in this folder is contributors to ASP.NET Core.
+If you are looking for documentation on to *use* ASP.NET Core, go to <https://docs.asp.net>.
diff --git a/docs/ReferenceResolution.md b/docs/ReferenceResolution.md
new file mode 100644
index 00000000000..e0c3dbf6f4d
--- /dev/null
+++ b/docs/ReferenceResolution.md
@@ -0,0 +1,34 @@
+`<Reference>` resolution
+========================
+
+Most project files in this repo should use `<Reference>` instead of `<ProjectReference>` or `<PackageReference>`.
+This was done to enable ASP.NET Core's unique requirements without requiring most ASP.NET Core contributors
+to understand the complex rules for how versions and references should work. The build system will resolve
+Reference items to the correct type and version of references based on our servicing and update rules.
+
+See [ResolveReferences.targets](/eng/targets/ResolveReferences.targets) for the exact implementation of custom
+`<Reference>` resolutions.
+
+The requirements that led to this system are:
+
+* Versions of external dependencies should be consistent.
+* Servicing updates of ASP.NET Core should minimize the number of assemblies which need to re-build and re-ship.
+* Newer versions of packages should not have lower dependency versions than previous releases.
+* Minimize the cascading effect of servicing updates where possible by keeping a consistent baseline of dependencies.
+
+## Recommendations for writing a .csproj
+
+* Use `<Reference>`
+* Do not use `<PackageReference>`
+* Only use `<ProjectReference>` in test projects
+* Name the .csproj file to match the assembly name.
+* Run `build.cmd /t:GenerateProjectList` when adding new projects
+* Use [eng/tools/BaseLineGenerator/](/eng/tools/BaselineGenerator/README.md) if you need to update baselines.
+
+## Important files
+
+* [eng/Baseline.xml](/eng/Baseline.xml) - this contains the 'baseline' of the latest servicing release for this branch. It should be modified and used to update the generated file, Baseline.Designer.props.
+* [eng/Dependencies.props](/eng/Dependencies.props) - contains a list of all package references that might be used in the repo.
+* [eng/PatchConfig.props](/eng/PatchConfig.props) - lists which assemblies or packages are patching in the current build.
+* [eng/ProjectReferences.props](/eng/ProjectReferences.props) - lists which assemblies or packages might be available to be referenced as a local project
+* [eng/Versions.props](/eng/Versions.props) - contains a list of versions which may be updated by automation.
diff --git a/eng/Baseline.xml b/eng/Baseline.xml
index 070816457bb..e27da11e3a6 100644
--- a/eng/Baseline.xml
+++ b/eng/Baseline.xml
@@ -1,3 +1,9 @@
+<!--
+
+This file contains a list of all the packages and their versions which were released in the last servicing
+build of ASP.NET Core 2.1.x. Update this list when preparing for a new patch.
+
+-->
 <Baseline Version="2.1.6">
   <Package Id="dotnet-dev-certs" Version="2.1.1" />
   <Package Id="dotnet-sql-cache" Version="2.1.1" />
diff --git a/eng/Dependencies.props b/eng/Dependencies.props
index b7b342f0e71..7b1eba3c54a 100644
--- a/eng/Dependencies.props
+++ b/eng/Dependencies.props
@@ -1,14 +1,29 @@
-<!-- This file is a work in progress as we merge repos and move content here from build/dependencies.props. -->
+<!--
 
+This file contains a list of all the external dependencies used in ASP.NET Core. These dependencies
+are expressed as `<LatestPackageReference>`. These are used as inputs reference resolution, and
+may be turned into `<PackageReference>` items in projects.
+
+`<BaselinePackageReference>` items should not be in this file. Those items appear in Baseline.Designer.props
+and are generated based on the last package release.
+-->
 <Project>
   <PropertyGroup>
     <MSBuildAllProjects>$(MSBuildAllProjects);$(MSBuildThisFileFullPath)</MSBuildAllProjects>
   </PropertyGroup>
 
+  <ItemDefinitionGroup>
+    <LatestPackageReference>
+      <!-- Required. Expected to be an exact package version. Wildcards are not allowed. -->
+      <Version></Version>
+    </LatestPackageReference>
+  </ItemDefinitionGroup>
+
+  <!-- This import is temporary and necessary while we work on https://github.com/aspnet/AspNetCore/issues/4246. -->
   <Import Project="dependencies.temp.props" />
 
+  <!-- These dependencies must use version variables because they may be overriden by ProdCon builds. -->
   <ItemGroup Label="ProdCon dependencies">
-    <!-- These dependencies must use version variables because they may be overriden by ProdCon builds. -->
     <LatestPackageReference Include="Microsoft.AspNetCore.BenchmarkRunner.Sources" Version="$(MicrosoftAspNetCoreBenchmarkRunnerSourcesPackageVersion)" />
     <LatestPackageReference Include="Microsoft.AspNetCore.Certificates.Generation.Sources" Version="$(MicrosoftAspNetCoreCertificatesGenerationSourcesPackageVersion)" />
     <LatestPackageReference Include="Microsoft.AspNetCore.Testing" Version="$(MicrosoftAspNetCoreTestingPackageVersion)" />
@@ -82,13 +97,4 @@
     <LatestPackageReference Include="xunit" Version="2.4.0" />
   </ItemGroup>
 
-  <ItemGroup Condition=" '$(IsTestProject)' == 'true' ">
-    <Reference Include="Microsoft.AspNetCore.Testing" />
-    <Reference Include="Microsoft.NET.Test.Sdk" />
-    <Reference Include="Moq" />
-    <Reference Include="xunit" />
-    <Reference Include="xunit.analyzers" />
-    <Reference Include="xunit.runner.visualstudio" />
-  </ItemGroup>
-
 </Project>
diff --git a/eng/PatchConfig.props b/eng/PatchConfig.props
index 91e93a17fe7..2fd81f182e3 100644
--- a/eng/PatchConfig.props
+++ b/eng/PatchConfig.props
@@ -1,3 +1,13 @@
+<!--
+
+This file contains a list of the package IDs which are patching in a given release.
+
+CAUTION: due to limitations in MSBuild, the format of the PackagesInPatch property is picky.
+When adding a new package, make sure the new line ends with a semicolon and starts with a space.
+Later on, this will be checked using this condition:
+
+    <IsPackageInThisPatch>$(PackagesInPatch.Contains(' $(PackageId);'))</IsPackageInThisPatch>
+-->
 <Project>
   <PropertyGroup>
     <MSBuildAllProjects>$(MSBuildAllProjects);$(MSBuildThisFileFullPath)</MSBuildAllProjects>
diff --git a/eng/ProjectReferences.props b/eng/ProjectReferences.props
index ea6ac86aac4..2b71425adf5 100644
--- a/eng/ProjectReferences.props
+++ b/eng/ProjectReferences.props
@@ -1,4 +1,8 @@
-<!-- This file is automatically generated. Run `build.cmd /t:GenerateProjectList` to update. -->
+<!--
+  This file is automatically generated. Run `build.cmd /t:GenerateProjectList` to update.
+
+  This file contains a map of assembly names to the projects that build them.
+-->
 <Project>
   <ItemGroup>
     <ProjectReferenceProvider Include="Microsoft.AspNetCore" ProjectPath="$(RepositoryRoot)src\DefaultBuilder\src\Microsoft.AspNetCore.csproj" />
diff --git a/eng/Versions.props b/eng/Versions.props
new file mode 100644
index 00000000000..8adc6dcbbd9
--- /dev/null
+++ b/eng/Versions.props
@@ -0,0 +1,8 @@
+<!--
+
+  This file defines the versions of external dependencies used by ASP.NET Core.
+  This file might be updated by automation.
+
+  TODO: move versions from build/dependencies.props into this location. Depends on https://github.com/aspnet/AspNetCore/issues/4246.
+-->
+<Project />
diff --git a/eng/targets/CSharp.Common.props b/eng/targets/CSharp.Common.props
index b3fc97e2d9b..6769d7508f6 100644
--- a/eng/targets/CSharp.Common.props
+++ b/eng/targets/CSharp.Common.props
@@ -11,4 +11,13 @@
     <PackageReference Include="Internal.AspNetCore.Sdk" PrivateAssets="All" Version="$(InternalAspNetCoreSdkPackageVersion)" />
   </ItemGroup>
 
+  <ItemGroup Condition=" '$(IsTestProject)' == 'true' ">
+    <Reference Include="Microsoft.AspNetCore.Testing" />
+    <Reference Include="Microsoft.NET.Test.Sdk" />
+    <Reference Include="Moq" />
+    <Reference Include="xunit" />
+    <Reference Include="xunit.analyzers" />
+    <Reference Include="xunit.runner.visualstudio" />
+  </ItemGroup>
+
 </Project>
diff --git a/eng/targets/ResolveReferences.targets b/eng/targets/ResolveReferences.targets
index 7ff54b7d33b..3c55bf6f8f6 100644
--- a/eng/targets/ResolveReferences.targets
+++ b/eng/targets/ResolveReferences.targets
@@ -1,3 +1,21 @@
+<!--
+
+  The targets in this file are used to implement custom <Reference> resolution.
+  For more details, see /docs/ReferenceResolution.md.
+
+  Properties which can be set by projects. If unset, these will be inferred.
+
+    * UseLatestPackageReferences = resolve `<Reference>` items to the latest version of PackageReferences in eng/Dependencies.props.
+    * UseProjectReferences = prefer project references to packages
+    * IsProjectReferenceProvider = when true, the assembly in this project should be available as a ProjectReferenceProvider (see below).
+
+  Items used by the resolution strategy:
+
+    * BaselinePackageReference = a list of packages that were reference in the last release of the project currently building
+    * LatestPackageReference = a list of the latest versions of packages
+    * Reference = a list of the references which are needed for compilation or runtime
+    * ProjectReferenceProvider = a list which maps of assembly names to the project file that produces it
+ -->
 <Project>
 
   <PropertyGroup>
@@ -31,16 +49,22 @@
   </PropertyGroup>
 
   <ItemGroup>
+    <!-- Packages which are implicitly defined by the .NET Core SDK. -->
     <_ImplicitPackageReference Include="@(PackageReference->WithMetadataValue('IsImplicitlyDefined', 'true'))" />
+    <!-- Capture a list of references which were set explicitly in the project. -->
     <_ExplicitPackageReference Include="@(PackageReference)" Exclude="@(_ImplicitPackageReference)" />
+    <!-- Special case: ignore the reference to Internal.AspNetCore.Sdk, which is defined in eng/targets/Cpp.Common.props. -->
     <_ExplicitPackageReference Remove="Internal.AspNetCore.Sdk" />
 
-    <UnusedProjectReferenceProvider Include="@(ProjectReferenceProvider)" Exclude="@(Reference)" />
+    <_UnusedProjectReferenceProvider Include="@(ProjectReferenceProvider)" Exclude="@(Reference)" />
 
-    <!-- Order matters. Projects should be used when possible instead of packages. -->
+    <!--
+      Turn Reference items into a ProjectReference when UseProjectReferences is true.
+      Order matters. This comes before package resolution because projects should be used when possible instead of packages.
+    -->
     <_ProjectReferenceByAssemblyName Condition="'$(UseProjectReferences)' == 'true'"
       Include="@(ProjectReferenceProvider)"
-      Exclude="@(UnusedProjectReferenceProvider)" />
+      Exclude="@(_UnusedProjectReferenceProvider)" />
 
     <ProjectReference Include="@(_ProjectReferenceByAssemblyName->'%(ProjectPath)')" />
 
@@ -51,12 +75,18 @@
     <Reference Remove="@(Reference)" />
   </ItemGroup>
 
+  <!--
+    This target resolves remaining Referene 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" BeforeTargets="CollectPackageReferences;ResolveAssemblyReferencesDesignTime;ResolveAssemblyReferences" Condition=" '$(TargetFramework)' != '' ">
     <ItemGroup>
       <Reference Include="@(_ReferenceTemp)" />
       <_ReferenceTemp Remove="@(_ReferenceTemp)" />
 
+      <!-- Identify if any references were present in the last release of this package, but have been removed. -->
       <UnusedBaselinePackageReference Include="@(BaselinePackageReference)" Exclude="@(Reference);@(_ProjectReferenceByAssemblyName)" />
+
       <!--
         MSBuild does not provide a way to join on matching identities in a Condition,
         but you can do a cartesian product of two item groups and filter out mismatched id's in a second pass.
@@ -101,7 +131,8 @@
       <_PrivatePackageReferenceWithVersion Remove="@(_PrivatePackageReferenceWithVersion)" />
       <_ImplicitPackageReference Remove="@(_ImplicitPackageReference)" />
     </ItemGroup>
-<!--
+
+<!-- TODO: when we finish https://github.com/aspnet/AspNetCore/issues/4246, introduce errors to force projects to use custom resolution.
     <Error Condition="@(_ExplicitPackageReference->Count()) != 0"
            Text="PackageReference items are not allowed. Use &lt;Reference&gt; instead. " /> -->
 
@@ -117,6 +148,7 @@
            Text="Could not resolve this reference. Could not locate the package or project for &quot;%(Reference.Identity)&quot;" />
   </Target>
 
+  <!-- These targets are used to generate the map of assembly name to project files. See also the /t:GenerateProjectList target in build/repo.targets. -->
   <Target Name="GetReferencesProvided" Returns="@(ProvidesReference)">
     <ItemGroup>
       <_TargetFramework Remove="@(_TargetFramework)" />