Double Evaluation Fix (#2595)

* Get properties for ProjectReference in parallel In order to avoid batching the _GetProjectReferenceTargetFrameworkProperties target for each reference, the ProjectReference protocol can be amended to return an item from GetTargetFrameworkProperties instead of a semicolon-delimited list of key-value pairs. This allows a single build request to be sent to the engine, and allows resolving references in parallel on multiprocess builds. * Identify inner-build in consuming project * Call GetTargetFrameworks target to identify target frameworks in referenced projects. Since TargetFrameworks is not set, this will avoid a 2nd evaluation when the project single targets. * Adds dependency on NuGet GetReferenceNearestTargetFrameworkTask. * Add NuGet ImportAfter targets to bootstrapped build This is needed to get the import to get the automatic import of NuGet.targets.
dotnet · Oct 11, 2017 · 57ae27c · gore01 · Nov 24, 2017 · 57ae27c
1 parent f5a692a
commit 57ae27c
Show file tree

Hide file tree

Showing 7 changed files with 133 additions and 46 deletions.
diff --git a/dir.props b/dir.props
@@ -43,7 +43,7 @@
     <MicroBuildVersion>0.2.0</MicroBuildVersion>
     <GitVersioningVersion>1.6.35</GitVersioningVersion>
     <NuSpecReferenceGeneratorVersion>1.4.2</NuSpecReferenceGeneratorVersion>
-    <NuGetVersion>4.5.0-preview1-4518</NuGetVersion>
+    <NuGetVersion>4.5.0-preview1-4527</NuGetVersion>
   </PropertyGroup>
 
    <!-- Common repo directories -->

diff --git a/documentation/ProjectReference-Protocol.md b/documentation/ProjectReference-Protocol.md
@@ -1,4 +1,4 @@
-# The `ProjectReference` Protocol
+# The `ProjectReference` Protocol
 
 The MSBuild engine doesn't have a notion of a “project reference”—it only provides the [`MSBuild` task](https://docs.microsoft.com/en-us/visualstudio/msbuild/msbuild-task) to allow cross-project communication.
 
@@ -31,7 +31,9 @@ There are empty hooks in the default targets for
 
 `AssignProjectConfiguration` runs when building in a solution context, and ensures that the right `Configuration` and `Platform` are assigned to each reference. For example, if a solution specifies (using the Solution Build Manager) that for a given solution configuration, a project should always be built `Release`, that is applied inside MSBuild in this target.
 
-`PrepareProjectReferences` then runs, ensuring that each referenced project exists (creating the item `@(_MSBuildProjectReferenceExistent)`) and determining the parameters it needs to produce a compatible build by calling its `GetTargetFrameworkProperties` target.
+`PrepareProjectReferences` then runs, ensuring that each referenced project exists (creating the item `@(_MSBuildProjectReferenceExistent)`).
+
+`_ComputeProjectReferenceTargetFrameworkMatches` calls `GetTargetFrameworks` in existent ProjectReferences and determines the parameters needed to produce a compatible build by calling the `AssignReferenceProperties` task for each reference that multitargets.
 
 `ResolveProjectReferences` does the bulk of the work, building the referenced projects and collecting their outputs.
 
@@ -47,16 +49,25 @@ These targets are all defined in `Microsoft.Common.targets` and are defined in M
 
 If implementing a project with an “outer” (determine what properties to pass to the real build) and “inner” (fully specified) build, only `GetTargetFrameworkProperties` is required in the “outer” build. The other targets listed can be “inner” build only.
 
-* `GetTargetFrameworkProperties` determines what properties should be passed to the “main” target.
-  * **New** for MSBuild 15/Visual Studio 2017. Supports the cross-targeting feature allowing a project to have multiple `TargetFrameworks`.
+* `GetTargetFrameworks` tells referencing projects what options are available to the build.
+  * It returns an item with metadata `TargetFrameworks` indicating what TargetFrameworks are available in the project, as well as boolean metadata `HasSingleTargetFramework` and `IsRidAgnostic`.
+  * **New** in MSBuild 15.5.
+* `GetTargetFrameworkProperties` determines what properties should be passed to the “main” target for a given `ReferringTargetFramework`.
+  * **Deprecated** in MSBuild 15.5.
+  * New for MSBuild 15/Visual Studio 2017. Supports the cross-targeting feature allowing a project to have multiple `TargetFrameworks`.
   * **Conditions**: only when metadata `SkipGetTargetFrameworkProperties` for each reference is not true.
   * Skipped for `*.vcxproj` by default.
-* `GetTargetPath` should the path of the project's output, but _not_ build that output.
+  * This should return either
+    * a string of the form `TargetFramework=$(NearestTargetFramework);ProjectHasSingleTargetFramework=$(_HasSingleTargetFramework);ProjectIsRidAgnostic=$(_IsRidAgnostic)`, where the value of `NearestTargetFramework` will be used to formulate `TargetFramework` for the following calls and the other two properties are booleans, or
+    * an item with metadata `DesiredTargetFrameworkProperties` (key-value pairs of the form `TargetFramework=net46`), `HasSingleTargetFramework` (boolean), and `IsRidAgnostic` (boolean).
+* `GetTargetPath` should return the path of the project's output, but _not_ build that output.
   * **Conditions**: this is used for builds inside Visual Studio, but not on the command line.
   * It's also used when the property `BuildProjectReferences` is `false`, manually indicating that all `ProjectReferences` are up to date and shouldn't be (re)built.
+  * This should return a single item that is the primary output of the project, with metadata describing that output. See [`TargetPathWithTargetPlatformMoniker`](https://github.com/Microsoft/msbuild/blob/080ef976a428f6ff7bf53ca5dd4ee637b3fe949c/src/Tasks/Microsoft.Common.CurrentVersion.targets#L1834-L1842) for the default metadata.
 * **Default** targets should do the full build and return an assembly to be referenced.
   * **Conditions**: this is _not_ called when building inside Visual Studio. Instead, Visual Studio builds each project in isolation but in order, so the path returned from `GetTargetPath` can be assumed to exist at consumption time.
   * If the `ProjectReference` defines the `Targets` metadata, it is used. If not, no target is passed, and the default target of the reference (usually `Build`) is built.
+  * The return value of this target should be identical to that of `GetTargetPath`.
 * `GetNativeManifest` should return a manifest suitable for passing to the `ResolveNativeReferences` target.
 * `GetCopyToOutputDirectoryItems` should return the outputs of a project that should be copied to the output of a referencing project.
 * `Clean` should delete all outputs of the project.
@@ -68,4 +79,4 @@ As with all MSBuild logic, targets can be added to do other work with `ProjectRe
 
 In particular, NuGet depends on being able to identify referenced projects' package dependencies, and calls some targets that are imported through `Microsoft.Common.targets` to do so. At the time of writing this this is in [`NuGet.targets`](https://github.com/NuGet/NuGet.Client/blob/79264a74262354c1a8f899c2c9ddcaff58afaf62/src/NuGet.Core/NuGet.Build.Tasks/NuGet.targets).
 
-`Microsoft.AppxPackage.targets` adds a dependency on the target `GetPackagingOutputs`.
+`Microsoft.AppxPackage.targets` adds a dependency on the target `GetPackagingOutputs`.
diff --git a/src/.nuget/project.json b/src/.nuget/project.json
@@ -6,7 +6,7 @@
     "Microsoft.Net.Compilers": "2.3.1",
     "MicroBuild.Core": "0.2.0",
     "Microsoft.DotNet.BuildTools.GenAPI": "1.0.0-beta2-00731-01",
-    "NuGet.Build.Tasks": "4.5.0-preview1-4518"
+    "NuGet.Build.Tasks": "4.5.0-preview1-4527"
   },
   "frameworks": {
     "net46": {}

diff --git a/src/Tasks/Microsoft.Common.CrossTargeting.targets b/src/Tasks/Microsoft.Common.CrossTargeting.targets
@@ -20,6 +20,21 @@ Copyright (C) Microsoft Corporation. All rights reserved.
 
   <Import Project="$(CustomBeforeMicrosoftCommonCrossTargetingTargets)" Condition="'$(CustomBeforeMicrosoftCommonCrossTargetingTargets)' != '' and Exists('$(CustomBeforeMicrosoftCommonCrossTargetingTargets)')"/>
 
+  <Target Name="GetTargetFrameworks"
+          Returns="@(_ThisProjectBuildMetadata)">
+    <ItemGroup>
+      <_ThisProjectBuildMetadata Include="$(MSBuildProjectFullPath)">
+        <TargetFrameworks Condition="'$(TargetFrameworks)' != ''">$(TargetFrameworks)</TargetFrameworks>
+        <TargetFrameworks Condition="'$(TargetFrameworks)' == ''">$(TargetFramework)</TargetFrameworks>
+        <HasSingleTargetFramework>true</HasSingleTargetFramework>
+        <HasSingleTargetFramework Condition="'$(IsCrossTargetingBuild)' == 'true'">false</HasSingleTargetFramework>
+        <!-- indicate to caller that project is RID agnostic so that a global property RuntimeIdentifier value can be removed -->
+        <IsRidAgnostic>false</IsRidAgnostic>
+        <IsRidAgnostic Condition=" '$(RuntimeIdentifier)' == '' and '$(RuntimeIdentifiers)' == '' ">true</IsRidAgnostic>
+      </_ThisProjectBuildMetadata>
+    </ItemGroup>
+  </Target>
+
   <Target Name="_ComputeTargetFrameworkItems" Returns="@(InnerOutput)">
     <ItemGroup>
       <_TargetFramework Include="$(TargetFrameworks)" />

diff --git a/src/Tasks/Microsoft.Common.CurrentVersion.targets b/src/Tasks/Microsoft.Common.CurrentVersion.targets
@@ -1516,17 +1516,26 @@ Copyright (C) Microsoft Corporation. All rights reserved.
     ====================================================================================
                                         _GetProjectReferenceTargetFrameworkProperties
 
-    Builds the GetTargetFrameworkProperties target of all existent project references,
-    passing $(TargetFrameworkMoniker) as $(ReferringTargetFramework) and sets the
-    SetTargetFramework metadata of the project reference to the value that is returned.
-
-    This allows a cross-targeting project to select how it should be configured to
-    build against the most appropriate target for the referring target framework.
+    Builds the GetTargetFrameworks target of all existent project references to get a list
+    of all supported TargetFrameworks of the referenced projects. Calls the
+    GetReferenceNearestTargetFrameworkTask to determine the closest match for each project.
+    This allows a cross-targeting project to select how it should be configured to build
+    against the most appropriate target for the referring target framework.
 
     ======================================================================================
   -->
-  <Target Name="_GetProjectReferenceTargetFrameworkProperties"
-          Outputs="%(_MSBuildProjectReferenceExistent.Identity)">
+  <Target Name="_GetProjectReferenceTargetFrameworkProperties">
+    <!--
+      Select the moniker to send to each project reference  if not already set. NugetTargetMoniker (NTM) is preferred by default over 
+      TargetFrameworkMoniker (TFM) because it is required to disambiguate the UWP case where TFM is fixed at .NETCore,Version=v5.0 and 
+      has floating NTM=UAP,Version=vX.Y.Z. However, in other cases (classic PCLs), NTM contains multiple values and that will cause the MSBuild
+      invocation below to fail by passing invalid properties. Therefore we do not use the NTM if it contains a semicolon.
+    -->
+    <PropertyGroup Condition="'$(ReferringTargetFrameworkForProjectReferences)' == ''">
+      <ReferringTargetFrameworkForProjectReferences Condition="'$(NugetTargetMoniker)' != '' and !$(NuGetTargetMoniker.Contains(';'))">$(NugetTargetMoniker)</ReferringTargetFrameworkForProjectReferences>
+      <ReferringTargetFrameworkForProjectReferences Condition="'$(NugetTargetMoniker)' == ''">$(TargetFrameworkMoniker)</ReferringTargetFrameworkForProjectReferences>
+    </PropertyGroup>
+
     <!--
       Honor SkipGetTargetFrameworkProperties=true metadata on project references
       to mean that the project reference is known not to target multiple frameworks
@@ -1559,49 +1568,79 @@ Copyright (C) Microsoft Corporation. All rights reserved.
     </ItemGroup>
 
     <!--
-      Select the moniker to send to each project reference  if not already set. NugetTargetMoniker (NTM) is preferred by default over 
-      TargetFrameworkMoniker (TFM) because it is required to disambiguate the UWP case where TFM is fixed at .NETCore,Version=v5.0 and 
-      has floating NTM=UAP,Version=vX.Y.Z. However, in other cases (classic PCLs), NTM contains multiple values and that will cause the MSBuild
-      invocation below to fail by passing invalid properties. Therefore we do not use the NTM if it contains a semicolon.
+       Get reference target framework lists.
+       Note: A future optimization could cache the closest match and set the target framework on
+       this MSBuild task invocation. This would (optimistically) save an evaluation of the referenced
+       project when the answer is the same.
     -->
-    <PropertyGroup Condition="'$(ReferringTargetFrameworkForProjectReferences)' == ''">
-      <ReferringTargetFrameworkForProjectReferences Condition="'$(NugetTargetMoniker)' != '' and !$(NuGetTargetMoniker.Contains(';'))">$(NugetTargetMoniker)</ReferringTargetFrameworkForProjectReferences>
-      <ReferringTargetFrameworkForProjectReferences Condition="'$(NugetTargetMoniker)' == ''">$(TargetFrameworkMoniker)</ReferringTargetFrameworkForProjectReferences>
-    </PropertyGroup>
-
     <MSBuild
-        Projects="%(_MSBuildProjectReferenceExistent.Identity)"
-        Targets="GetTargetFrameworkProperties"
+        Projects="@(_MSBuildProjectReferenceExistent)"
+        Targets="GetTargetFrameworks"
         BuildInParallel="$(BuildInParallel)"
-        Properties="%(_MSBuildProjectReferenceExistent.SetConfiguration); %(_MSBuildProjectReferenceExistent.SetPlatform); ReferringTargetFramework=$(ReferringTargetFrameworkForProjectReferences)"
+        Properties="%(_MSBuildProjectReferenceExistent.SetConfiguration); %(_MSBuildProjectReferenceExistent.SetPlatform)"
         ContinueOnError="!$(BuildingProject)"
         RemoveProperties="%(_MSBuildProjectReferenceExistent.GlobalPropertiesToRemove);TargetFramework;RuntimeIdentifier"
-        Condition="'%(_MSBuildProjectReferenceExistent.SkipGetTargetFrameworkProperties)' != 'true'">
-
-      <Output TaskParameter="TargetOutputs" PropertyName="_ProjectReferenceTargetFrameworkProperties" />
+        Condition="'%(_MSBuildProjectReferenceExistent.SkipGetTargetFrameworkProperties)' != 'true'"
+        SkipNonexistentTargets="true">
+      <Output TaskParameter="TargetOutputs" ItemName="_ProjectReferenceTargetFrameworkPossibilities" />
     </MSBuild>
 
+    <!-- For each reference, get closest match -->
+    <GetReferenceNearestTargetFrameworkTask AnnotatedProjectReferences="@(_ProjectReferenceTargetFrameworkPossibilities)"
+                                            CurrentProjectTargetFramework="$(ReferringTargetFrameworkForProjectReferences)"
+                                            CurrentProjectName="$(MSBuildProjectName)"
+                                            Condition="'@(_ProjectReferenceTargetFrameworkPossibilities->Count())' != '0' and '$(ReferringTargetFrameworkForProjectReferences)' != ''">
+      <Output ItemName="AnnotatedProjects" TaskParameter="AssignedProjects" />
+    </GetReferenceNearestTargetFrameworkTask>
+
     <ItemGroup>
-      <_MSBuildProjectReferenceExistent Condition="'%(_MSBuildProjectReferenceExistent.Identity)' == '%(Identity)' and '$(_ProjectReferenceTargetFrameworkProperties)' != ''">
-        <SetTargetFramework>$(_ProjectReferenceTargetFrameworkProperties)</SetTargetFramework>
+      <!--
+         If the task was skipped or the current TargetFramework is empty, AnnotatedProjects will be empty.
+         In this case, copy _ProjectReferenceTargetFrameworkPossibilities as is. See:
+            https://github.com/dotnet/sdk/issues/416
+      -->
+      <AnnotatedProjects Include="@(_ProjectReferenceTargetFrameworkPossibilities)"
+                         Condition="'$(ReferringTargetFrameworkForProjectReferences)' == ''" />
+      <!-- If the NearestTargetFramework property was set and the project multi-targets, SetTargetFramework must be set. -->
+      <AnnotatedProjects Condition="'@(AnnotatedProjects)' == '%(Identity)' and '%(AnnotatedProjects.NearestTargetFramework)' != '' and '%(AnnotatedProjects.HasSingleTargetFramework)' != 'true'">
+        <SetTargetFramework>TargetFramework=%(AnnotatedProjects.NearestTargetFramework)</SetTargetFramework>
+      </AnnotatedProjects>
 
-        <UndefineProperties Condition="$(_ProjectReferenceTargetFrameworkProperties.Contains(`ProjectHasSingleTargetFramework=true`))">%(_MSBuildProjectReferenceExistent.UndefineProperties);TargetFramework;ProjectHasSingleTargetFramework</UndefineProperties>
-        <!-- Unconditionally remove the property that was set as a marker to indicate that for this call we should remove TargetFramework -->
-        <UndefineProperties Condition="!$(_ProjectReferenceTargetFrameworkProperties.Contains(`ProjectHasSingleTargetFramework=true`))">%(_MSBuildProjectReferenceExistent.UndefineProperties);ProjectHasSingleTargetFramework</UndefineProperties>
-      </_MSBuildProjectReferenceExistent>
+      <!--
+         If the NearestTargetFramework property was not set or the project has a single TargetFramework, we need to Undefine
+         TargetFramework to avoid another project evaluation.
+      -->
+      <AnnotatedProjects Condition="'@(AnnotatedProjects)' == '%(Identity)' and ('%(AnnotatedProjects.NearestTargetFramework)' == '' or '%(AnnotatedProjects.HasSingleTargetFramework)' == 'true')">
+        <UndefineProperties>%(AnnotatedProjects.UndefineProperties);TargetFramework</UndefineProperties>
+      </AnnotatedProjects>
+
+      <!-- If the project is RID agnostic, undefine the RuntimeIdentifier property to avoid another evaluation. -->
+      <AnnotatedProjects Condition="'@(AnnotatedProjects)' == '%(Identity)' and '%(AnnotatedProjects.IsRidAgnostic)' == 'true'">
+        <UndefineProperties>%(AnnotatedProjects.UndefineProperties);RuntimeIdentifier</UndefineProperties>
+      </AnnotatedProjects>
+
+      <!-- 
+         Remove the items we've touched from _MSBuildProjectReferenceExistent. This will leave all projects where 
+         SkipGetTargetFrameworkProperties was set. Then add all AnnotatedProjects back.
+      -->
+      <_MSBuildProjectReferenceExistent Remove="@(_MSBuildProjectReferenceExistent)" Condition="'%(_MSBuildProjectReferenceExistent.SkipGetTargetFrameworkProperties)' != 'true'" />
+      <_MSBuildProjectReferenceExistent Include="@(AnnotatedProjects)" />
     </ItemGroup>
+  </Target>
 
+  <Target Name="GetTargetFrameworks"
+          Returns="@(_ThisProjectBuildMetadata)">
     <ItemGroup>
-      <_MSBuildProjectReferenceExistent Condition="'%(_MSBuildProjectReferenceExistent.Identity)' == '%(Identity)' and '$(_ProjectReferenceTargetFrameworkProperties)' != ''">
-        <UndefineProperties Condition="$(_ProjectReferenceTargetFrameworkProperties.Contains(`ProjectIsRidAgnostic=true`))">%(_MSBuildProjectReferenceExistent.UndefineProperties);RuntimeIdentifier;ProjectIsRidAgnostic</UndefineProperties>
-        <!-- Unconditionally remove the property that was set as a marker to indicate that for this call we should remove RuntimeIdentifier -->
-        <UndefineProperties Condition="!$(_ProjectReferenceTargetFrameworkProperties.Contains(`ProjectIsRidAgnostic=true`))">%(_MSBuildProjectReferenceExistent.UndefineProperties);ProjectIsRidAgnostic</UndefineProperties>
-      </_MSBuildProjectReferenceExistent>
+      <_ThisProjectBuildMetadata Include="$(MSBuildProjectFullPath)">
+        <TargetFrameworks Condition="'$(TargetFrameworks)' != ''">$(TargetFrameworks)</TargetFrameworks>
+        <TargetFrameworks Condition="'$(TargetFrameworks)' == ''">$(TargetFramework)</TargetFrameworks>
+        <HasSingleTargetFramework>true</HasSingleTargetFramework>
+        <HasSingleTargetFramework Condition="'$(IsCrossTargetingBuild)' == 'true'">false</HasSingleTargetFramework>
+        <!-- indicate to caller that project is RID agnostic so that a global property RuntimeIdentifier value can be removed -->
+      <IsRidAgnostic>false</IsRidAgnostic>
+      <IsRidAgnostic Condition=" '$(RuntimeIdentifier)' == '' and '$(RuntimeIdentifiers)' == '' ">true</IsRidAgnostic>
+      </_ThisProjectBuildMetadata>
     </ItemGroup>
-
-    <PropertyGroup>
-      <_ProjectReferenceTargetFrameworkProperties />
-    </PropertyGroup>
   </Target>
 
   <!--

diff --git a/targets/BootStrapMSBuild.proj b/targets/BootStrapMSBuild.proj
@@ -124,6 +124,7 @@
       <NuGetCommonExtensions Include="$(ProjectDir)packages\nuget.versioning\$(NuGetVersion)\lib\netstandard1.0\**\*.*" />
       <NuGetCommonExtensions Include="$(ProjectDir)packages\nuspec.referencegenerator\$(NuGetVersion)\lib\netstandard1.3\**\*.*" />
       <NuGetCommonExtensions Include="$(ProjectDir)packages\Newtonsoft.Json\9.0.1\lib\netstandard1.0\**\*.*" />
+      <BootstrapDependencies Include="$(ProjectDir)targets\bootstrapDependencies\Core\**\*.*" />
     </ItemGroup>
 
     <RemoveDir
@@ -140,6 +141,9 @@
     <Copy SourceFiles="@(NuGetCommonExtensions)"
           DestinationFiles="@(NuGetCommonExtensions -> '$(BootstrapDestination)%(RecursiveDir)%(FileName)%(Extension)')" />
 
+    <Copy SourceFiles="@(BootstrapDependencies)"
+          DestinationFiles="@(BootstrapDependencies -> '$(BootstrapDestination)%(RecursiveDir)%(FileName)%(Extension)')" />
+
     <!-- Microsoft.Portable.CSharp.targets imports this file with a capital T -->
     <Copy SourceFiles="$(DeploymentDir)\Microsoft.CSharp.targets"
           DestinationFiles="$(BootstrapDestination)\Microsoft.CSharp.Targets" />