Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Double Evaluation Fix #2595

Merged
merged 13 commits into from
Oct 11, 2017
Merged

Double Evaluation Fix #2595

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
ba8fca1
Get properties for ProjectReference in parallel
rainersigwald Aug 18, 2017
ff79ea2
Backward compat for GetTargetFrameworkProperties
rainersigwald Aug 18, 2017
8729689
Clarify existing ProjectReference protocol returns
rainersigwald Aug 22, 2017
984f381
Document new GetTargetFrameworkProperties return
rainersigwald Aug 22, 2017
c36bfb3
Documentation for protocol change
rainersigwald Aug 24, 2017
bb44739
Identify inner-build in consuming project
rainersigwald Aug 24, 2017
f066173
Move logic out of NuGet
AndyGerlicher Oct 10, 2017
caf92b2
Update comments
AndyGerlicher Oct 10, 2017
62094fd
Fix issue when consuming project TFM is null.
AndyGerlicher Oct 11, 2017
21b9a95
Update NuGet.Build.Tasks
AndyGerlicher Oct 11, 2017
15afc90
Add NuGet ImportAfter targets to bootstrapped build
AndyGerlicher Oct 11, 2017
00e2059
Comment cleanup
AndyGerlicher Oct 11, 2017
e3ee36f
Preserve project references that opt-out
AndyGerlicher Oct 11, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 17 additions & 6 deletions documentation/ProjectReference-Protocol.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# The `ProjectReference` Protocol
# The `ProjectReference` Protocol
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are the changes to this file up-to-date after the changes to the protocol and interface with NuGet that were made in this PR?


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.

Expand Down Expand Up @@ -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.

Expand All @@ -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.
Expand All @@ -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`.
15 changes: 15 additions & 0 deletions src/Tasks/Microsoft.Common.CrossTargeting.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)" />
Expand Down
105 changes: 64 additions & 41 deletions src/Tasks/Microsoft.Common.CurrentVersion.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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. For each project, call
GetReferenceNearestTargetFrameworkTask to determine the closest match. This allows a
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reword this to say "Call GetReferenceNearestTargetFrameworkTask to determine the closest match for each project." This way it won't imply that the task is called once for project.

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
Expand Down Expand Up @@ -1558,50 +1567,64 @@ Copyright (C) Microsoft Corporation. All rights reserved.
</_MSBuildProjectReferenceExistent>
</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.
-->
<PropertyGroup Condition="'$(ReferringTargetFrameworkForProjectReferences)' == ''">
<ReferringTargetFrameworkForProjectReferences Condition="'$(NugetTargetMoniker)' != '' and !$(NuGetTargetMoniker.Contains(';'))">$(NugetTargetMoniker)</ReferringTargetFrameworkForProjectReferences>
<ReferringTargetFrameworkForProjectReferences Condition="'$(NugetTargetMoniker)' == ''">$(TargetFrameworkMoniker)</ReferringTargetFrameworkForProjectReferences>
</PropertyGroup>

<!-- Get reference target framework lists -->
<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>

<ItemGroup>
<_MSBuildProjectReferenceExistent Condition="'%(_MSBuildProjectReferenceExistent.Identity)' == '%(Identity)' and '$(_ProjectReferenceTargetFrameworkProperties)' != ''">
<SetTargetFramework>$(_ProjectReferenceTargetFrameworkProperties)</SetTargetFramework>
<!-- 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>

<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>
<ItemGroup>
<AnnotatedProjects Include="@(_ProjectReferenceTargetFrameworkPossibilities)"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest adding a comment here explaining this logic (possibly linking to dotnet/sdk#416)

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>

<!-- 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>

<_MSBuildProjectReferenceExistent Remove="@(_MSBuildProjectReferenceExistent)" />
<_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>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is always going to be false when we land in the non-cross-targeting version of GetTargetFrameworks.

<!-- 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>

<!--
Expand Down