Merge pull request #15917 from weshaggard/FixReferenceOrder

Update Reference and ProjectReference ordering in our targets.

10 files changed, 81 insertions, 69 deletions

diff --git a/Tools-Override/FrameworkTargeting.targets b/Tools-Override/FrameworkTargeting.targets
index 21f9f35e9a..dcb87fe7ac 100644
--- a/Tools-Override/FrameworkTargeting.targets
+++ b/Tools-Override/FrameworkTargeting.targets
@@ -1,17 +1,6 @@
 <?xml version="1.0" encoding="utf-8"?>
 <Project ToolsVersion="12.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
 
-  <PropertyGroup Condition="'$(IncludeDefaultReferences)' == ''">
-    <IncludeDefaultReferences Condition="'$(MSBuildProjectExtension)' == '.csproj'">true</IncludeDefaultReferences>
-    <IncludeDefaultReferences Condition="'$(MSBuildProjectExtension)' == '.vbproj'">true</IncludeDefaultReferences>
-  </PropertyGroup>
-
-  <ItemGroup>
-    <!-- netstandard is a default reference whenever building for NETStandard or building an implementation assembly -->
-    <DefaultReference Condition="$(NuGetTargetMoniker.StartsWith('.NETStandard')) OR '$(IsReferenceAssembly)' != 'true'"
-                      Include="netstandard" />
-  </ItemGroup>
-
   <ItemGroup>
     <TargetingPackDirs Include="$(RefPath)" />
     <AdditionalReferencePaths Include="@(TargetingPackDirs)" />
@@ -21,6 +10,8 @@
     <ContractOutputPath>$(RefPath)</ContractOutputPath>
     <FrameworkPathOverride>$(ContractOutputPath)</FrameworkPathOverride>
     <AssemblySearchPaths>$(AssemblySearchPaths);$(ContractOutputPath);{RawFileName}</AssemblySearchPaths>
+    <!-- Disable RAR from transitively discovering depdencies for References -->
+    <_FindDependencies>false</_FindDependencies>
   </PropertyGroup>
 
   <PropertyGroup Condition=" '$(TargetFrameworkIdentifier)' == ''
@@ -63,27 +54,32 @@
     <NuGetTargetMoniker Condition="'$(NuGetTargetMoniker)' == ''">.NETCoreApp,Version=v1.0</NuGetTargetMoniker>
   </PropertyGroup>
 
-  <Target Name="AddDefaultReferences"
-          Condition="'$(IncludeDefaultReferences)' =='true'"
-          BeforeTargets="BeforeResolveReferences">
-    <ItemGroup>
-      <!-- Include default references when specified and they exist -->
-      <Reference Condition="Exists('$(RefPath)%(Identity).dll')" Include="@(DefaultReference)" />
+  <PropertyGroup Condition="'$(IncludeDefaultReferences)' == ''">
+    <IncludeDefaultReferences Condition="'$(MSBuildProjectExtension)' == '.csproj'">true</IncludeDefaultReferences>
+    <IncludeDefaultReferences Condition="'$(MSBuildProjectExtension)' == '.vbproj'">true</IncludeDefaultReferences>
+  </PropertyGroup>
+
+  <Target Name="SetupDefaultReferences">
+    <ItemGroup Condition="'$(IncludeDefaultReferences)' =='true'">
+      <!-- netstandard is a default reference whenever building for NETStandard or building an implementation assembly -->
+      <DefaultReference Condition="$(NuGetTargetMoniker.StartsWith('.NETStandard')) OR '$(IsReferenceAssembly)' != 'true'"
+                        Include="netstandard" />
     </ItemGroup>
   </Target>
 
-  <Target Name="MarkReferencePrivateFalse"
-          BeforeTargets="ResolveAssemblyReferences;AssignProjectConfiguration">
+  <Target Name="UpdateReferenceItems"
+          DependsOnTargets="SetupDefaultReferences"
+          BeforeTargets="BeforeResolveReferences"
+  >
+    <ItemGroup>
+      <Reference Include="@(DefaultReference)" />
+    </ItemGroup>
+
     <ItemGroup>
       <!-- Simple name references will be resolved from the targeting pack folders and should never be copied to output -->
       <Reference Condition="'%(Reference.Extension)' != '.dll'">
         <Private>false</Private>
       </Reference>
-
-      <!-- Project references for non-test assemblies should never be copied to the output. -->
-      <ProjectReference Condition="'$(IsTestProject)' != 'true'">
-        <Private>false</Private>
-      </ProjectReference>
     </ItemGroup>
   </Target>
 
@@ -127,6 +123,28 @@
   <Import Project="$(MSBuildToolsPath)\Microsoft.VisualBasic.targets"
           Condition="'$(TargetFrameworkIdentifier)' != '.NETPortable' and '$(MSBuildProjectExtension)' == '.vbproj'" />
 
+  <PropertyGroup>
+    <ResolveReferencesDependsOn>
+      AddProjectReferencesDynamically;
+      $(ResolveReferencesDependsOn);
+    </ResolveReferencesDependsOn>
+    <CleanDependsOn>
+      AddProjectReferencesDynamically;
+      $(CleanDependsOn);
+    </CleanDependsOn>
+  </PropertyGroup>
+  <!--
+    Common targets don't provide a good place to enable adding new ProjectReference items in targets that work
+    with both clean, build, and rebuild entry point targets. We cannot hook off of AssignProjectConfigurations
+    because it is conditioned on "'@(ProjectReference)'!=''" which gets evalulated before the BeforeTargets run
+    so adding ProjectReference as part of a BeforeTarget make still have the AssignProjectConfiguration skipped.
+    To help with this problem we are creating a new target and correctly hooking it up in the resolve and clean
+    depends on target chains.
+
+    For information on evaulation of targets ordering see https://msdn.microsoft.com/en-us/library/ee216359.aspx.
+  -->
+  <Target Name="AddProjectReferencesDynamically" DependsOnTargets="$(AddProjectReferencesDynamicallyDependsOn)" />
+
   <PropertyGroup Condition="'$(TargetFrameworkIdentifier)' != '.NETFramework' and '$(OutputType)' == 'exe'">
     <!-- RAR thinks all EXEs require binding redirects.  That's not the case for CoreCLR -->
     <AutoUnifyAssemblyReferences>true</AutoUnifyAssemblyReferences>
diff --git a/Tools-Override/resolveContract.targets b/Tools-Override/resolveContract.targets
index 9a76265298..f8f98711f1 100644
--- a/Tools-Override/resolveContract.targets
+++ b/Tools-Override/resolveContract.targets
@@ -2,15 +2,14 @@
 <Project ToolsVersion="12.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
 
   <PropertyGroup Condition="'$(ResolveMatchingContract)' == 'true'">
-    <ResolveReferencesDependsOn>
+    <AddProjectReferencesDynamicallyDependsOn>
       ResolveMatchingContract;
+      $(AddProjectReferencesDynamicallyDependsOn);
+    </AddProjectReferencesDynamicallyDependsOn>
+    <ResolveReferencesDependsOn>
       $(ResolveReferencesDependsOn);
       VerifyMatchingContract
     </ResolveReferencesDependsOn>
-    <CleanDependsOn>
-      ResolveMatchingContract;
-      $(CleanDependsOn);
-    </CleanDependsOn>
   </PropertyGroup>
 
   <Target Name="ResolveMatchingContract">
diff --git a/Tools-Override/tests.targets b/Tools-Override/tests.targets
index 068a31a252..0e926536ab 100644
--- a/Tools-Override/tests.targets
+++ b/Tools-Override/tests.targets
@@ -99,6 +99,25 @@
     </ItemGroup>
   </Target>
 
+  <Target Name="AddDefaultTestReferences" BeforeTargets="SetupDefaultReferences">
+    <ItemGroup Condition="'$(IsTestProject)'=='true' and '$(IncludeDefaultReferences)' == 'true'">
+      <TargetingPackExclusions Include="System.Runtime.WindowsRuntime.UI.Xaml" /> <!-- Harmless, but causes PRI targets to run -->
+      <TargetingPackExclusions Include="@(ReferenceFromRuntime)"/>
+
+      <!-- Whitelisted runtime assemblies that are OK to reference. -->
+      <ReferenceFromRuntime Include="xunit.core" />
+      <ReferenceFromRuntime Include="Xunit.NetCore.Extensions" />
+      <ReferenceFromRuntime Include="xunit.assert" />
+      <ReferenceFromRuntime Include="xunit.abstractions" />
+      <ReferenceFromRuntime Include="xunit.performance.core" />
+
+      <!-- Reference everything in the targeting pack directory -->
+      <TargetingPackItems Include="%(TargetingPackDirs.Identity)/*.dll" />
+
+      <DefaultReference Include="%(TargetingPackItems.Filename)" Exclude="@(TargetingPackExclusions)" />
+    </ItemGroup>
+  </Target>
+
   <!-- Generate the script to run the tests.  The script performs two high-level steps:
        1.  Copies the common test runtime dependencies calculated in DiscoverTestDependencies to the test
            execution directory. Each copy command no-ops if the file already exists in the test execution
diff --git a/buildvertical.targets b/buildvertical.targets
index 3c841f0a7e..891061899d 100644
--- a/buildvertical.targets
+++ b/buildvertical.targets
@@ -27,7 +27,7 @@
       <_projectBuildConfigurations>
         <AdditionalProperties>Configuration=%(Identity);%(_projectBuildConfigurations.AdditionalProperties)</AdditionalProperties>
       </_projectBuildConfigurations>
-      
+
       <!-- transform back to project -->
       <_projectWithConfiguration Include="@(_projectBuildConfigurations->'%(OriginalItemSpec)')" />
     </ItemGroup>
@@ -83,7 +83,7 @@
 
   <!-- Runs in a leaf project (csproj) to determine best configuration for ProjectReferences -->
   <Target Name="AnnotateProjectReference"
-          BeforeTargets="BeforeResolveReferences"
+          BeforeTargets="AssignProjectConfiguration"
           Condition="'@(ProjectReference)' != ''"
           Inputs="%(ProjectReference.Identity)"
           Outputs="unused">
@@ -116,6 +116,11 @@
       <ProjectReference>
         <AdditionalProperties>Configuration=$(_projectReferenceConfiguration);%(ProjectReference.AdditionalProperties)</AdditionalProperties>
       </ProjectReference>
+
+      <!-- Project references for non-test assemblies should never be copied to the output. -->
+      <ProjectReference Condition="'$(IsTestProject)' != 'true'">
+        <Private>false</Private>
+      </ProjectReference>
     </ItemGroup>
   </Target>
 
diff --git a/dir.props b/dir.props
index b4a3ae00b0..d18b2c5562 100644
--- a/dir.props
+++ b/dir.props
@@ -54,8 +54,6 @@
     <BuildToolsTargetsDesktop Condition="'$(RunningOnCore)' != 'true'">true</BuildToolsTargetsDesktop>
   </PropertyGroup>
 
-  <Import Project="$(MSBuildThisFileDirectory)targetingpacks.props" />
-
   <!-- Enable the analyzers for this repo -->
   <PropertyGroup>
     <EnableDotnetAnalyzers Condition="'$(EnableDotnetAnalyzers)'==''">true</EnableDotnetAnalyzers>
diff --git a/referenceFromRuntime.targets b/referenceFromRuntime.targets
index 9f0bc48d24..df0b90a15e 100644
--- a/referenceFromRuntime.targets
+++ b/referenceFromRuntime.targets
@@ -9,8 +9,8 @@
     </ItemGroup>
   </Target>
 
-  <Target Name="AddRuntimeProjectReference" 
-          BeforeTargets="AnnotateProjectReference"
+  <Target Name="AddRuntimeProjectReference"
+          BeforeTargets="AddProjectReferencesDynamically"
           Condition="'$(IsTestProject)'!='true' AND '@(ReferenceFromRuntime)' != ''">
     <Error Condition="'$(IsReferenceAssembly)' != 'true' AND '$(RuntimeProjectFile)' == ''" Text="RuntimeProjectFile must be specified when using ReferenceFromRuntime from source projects." />
     <Error Condition="'$(IsReferenceAssembly)' == 'true'" Text="ReferenceFromRuntime may not be used from reference assemblies." />
@@ -22,8 +22,8 @@
       </ProjectReference>
     </ItemGroup>
   </Target>
-  
-  <Target Name="FilterReferenceFromRuntime" 
+
+  <Target Name="FilterReferenceFromRuntime"
           AfterTargets="ResolveProjectReferences"
           Condition="'$(IsTestProject)'!='true' AND '@(ReferenceFromRuntime)' != ''">
     <ItemGroup>
diff --git a/src/System.Linq.Expressions/tests/System.Linq.Expressions.Tests.csproj b/src/System.Linq.Expressions/tests/System.Linq.Expressions.Tests.csproj
index 59cefeb5fb..e9c670df12 100644
--- a/src/System.Linq.Expressions/tests/System.Linq.Expressions.Tests.csproj
+++ b/src/System.Linq.Expressions/tests/System.Linq.Expressions.Tests.csproj
@@ -253,5 +253,8 @@
     <Compile Include="ILReader\LocalsSignatureParser.cs" />
     <Compile Include="ILReader\SigParser.cs" />
   </ItemGroup>
+  <ItemGroup>
+    <ReferenceFromRuntime Include="xunit.execution.dotnet" />
+  </ItemGroup>
   <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
 </Project>
\ No newline at end of file
diff --git a/src/System.Linq.Expressions/tests/TestExtensions/TestOrderer.cs b/src/System.Linq.Expressions/tests/TestExtensions/TestOrderer.cs
index 35985108d8..5168d7733a 100644
--- a/src/System.Linq.Expressions/tests/TestExtensions/TestOrderer.cs
+++ b/src/System.Linq.Expressions/tests/TestExtensions/TestOrderer.cs
@@ -16,7 +16,7 @@ namespace System.Linq.Expressions.Tests
             Dictionary<int, List<TTestCase>> queue = new Dictionary<int, List<TTestCase>>();
             foreach (TTestCase testCase in testCases)
             {
-                Xunit.Abstractions.IAttributeInfo orderAttribute = ReflectionAbstractionExtensions.GetCustomAttributes(testCase.TestMethod.Method, typeof(TestOrderAttribute)).FirstOrDefault();
+                Xunit.Abstractions.IAttributeInfo orderAttribute = testCase.TestMethod.Method.GetCustomAttributes(typeof(TestOrderAttribute)).FirstOrDefault();
                 int order;
                 if (orderAttribute == null || (order = orderAttribute.GetConstructorArguments().Cast<int>().First()) == 0)
                 {
diff --git a/src/System.Runtime.Serialization.Json/tests/Performance/System.Runtime.Serialization.Json.Performance.Tests.csproj b/src/System.Runtime.Serialization.Json/tests/Performance/System.Runtime.Serialization.Json.Performance.Tests.csproj
index 83e7f2739b..623c621b5e 100644
--- a/src/System.Runtime.Serialization.Json/tests/Performance/System.Runtime.Serialization.Json.Performance.Tests.csproj
+++ b/src/System.Runtime.Serialization.Json/tests/Performance/System.Runtime.Serialization.Json.Performance.Tests.csproj
@@ -16,7 +16,7 @@
     <Compile Include="JsonNetPerformanceTest.cs" />
   </ItemGroup>
   <ItemGroup>
-    <None Include="project.json" />
+    <ReferenceFromRuntime Include="Newtonsoft.Json" />
   </ItemGroup>
   <Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
 </Project>
\ No newline at end of file
diff --git a/targetingpacks.props b/targetingpacks.props
deleted file mode 100644
index 9f4df8b4e7..0000000000
--- a/targetingpacks.props
+++ /dev/null
@@ -1,30 +0,0 @@
-<?xml version="1.0" encoding="utf-8"?>
-<Project ToolsVersion="14.0" xmlns="http://schemas.microsoft.com/developer/msbuild/2003">
-
-  <!-- Adds test references to the targeting pack and runtime assemblies. -->
-  <Target Name="AddTestTargetingPackReferences" BeforeTargets="ResolveAssemblyReferences">
-    <ItemGroup Condition="'$(IsTestProject)'=='true'">
-      <TargetingPackExclusions Include="System.Private.CoreLib" />
-      <TargetingPackExclusions Include="System.Runtime.WindowsRuntime.UI.Xaml" /> <!-- Harmless, but causes PRI targets to run -->
-      <TargetingPackExclusions Include="@(ReferenceFromRuntime)"/>
-
-      <!-- Whitelisted runtime assemblies that are OK to reference. -->
-      <ReferenceFromRuntime Include="xunit.core" />
-      <ReferenceFromRuntime Include="Xunit.NetCore.Extensions" />
-      <ReferenceFromRuntime Include="xunit.assert" />
-      <ReferenceFromRuntime Include="xunit.abstractions" />
-      <ReferenceFromRuntime Include="xunit.performance.core" />
-      <ReferenceFromRuntime Include="xunit.execution.dotnet" Condition="'$(_bc_TargetGroup)'!='netfx'"/>
-      <ReferenceFromRuntime Include="Newtonsoft.Json" Condition="'$(SkipIncludeNewtonsoftJson)'!='true'" />
-
-      <!-- Add Reference's to all files in the targeting pack folder, and to whitelisted items from the group above in the runtime folder. -->
-      <TargetingPackItems Include="%(TargetingPackDirs.Identity)/*" Condition="'$(NoTargetingPackReferences)' != 'true'" />
-    </ItemGroup>
-
-    <ItemGroup>
-      <Reference Include="%(TargetingPackItems.Filename)" Exclude="@(TargetingPackExclusions)"> <!-- TODO: System.Private.CoreLib shouldn't even be in the targeting pack. -->
-        <Private>false</Private>
-      </Reference>
-    </ItemGroup>
-  </Target>
-</Project>