Skip to content

Avoid project evaluations for single tfms #8377

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

Merged
ericstj merged 3 commits into from
Jan 27, 2022
Merged

Avoid project evaluations for single tfms #8377

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
1c2b168
Avoid project evaluations for single tfms
ViktorHofer Jan 22, 2022
4dff20d
Update BinPlace.targets
ViktorHofer Jan 22, 2022
e717649
Update BinPlace.targets
ViktorHofer Jan 22, 2022
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
20 changes: 19 additions & 1 deletion src/Microsoft.DotNet.Build.Tasks.TargetFramework.Sdk/src/ChooseBestP2PTargetFrameworkTask.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,25 @@ public override bool Execute()
Log.LogError($"Not able to find a compatible supported target framework for {referringTargetFramework} in Project {Path.GetFileName(projectReference.ItemSpec)}. The Supported Configurations are {string.Join(", ", targetFrameworks)}");
}

projectReference.SetMetadata("SetTargetFramework", "TargetFramework=" + bestTargetFramework);
// Mimic msbuild's Common.targets behavior: https://github.com/dotnet/msbuild/blob/3c8fb11a080a5a15199df44fabf042a22e9ad4da/src/Tasks/Microsoft.Common.CurrentVersion.targets#L1842-L1853
if (projectReference.GetMetadata("HasSingleTargetFramework") != "true")
{
projectReference.SetMetadata("SetTargetFramework", "TargetFramework=" + bestTargetFramework);
}
else
{
// If the project has a single TargetFramework, we need to Undefine TargetFramework to avoid another project evaluation.
string undefineProperties = projectReference.GetMetadata("UndefineProperties");
projectReference.SetMetadata("UndefineProperties", undefineProperties + ";TargetFramework");
}

if (projectReference.GetMetadata("IsRidAgnostic") == "true")
Copy link
Member

@ericstj ericstj Jan 25, 2022

Choose a reason for hiding this comment

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

What sets this metadata? I don't think we should ever flow RID between projects, it should always be inferred from the active TargetFramework value.

Copy link
Member Author

@ViktorHofer ViktorHofer Jan 25, 2022

Choose a reason for hiding this comment

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

IsRidAgnostic is set by msbuild: https://github.com/dotnet/msbuild/blob/8872ed650643d12340fb22ece223c1aacf5a30a0/src/Tasks/Microsoft.Common.CrossTargeting.targets#L48.

Our projects don't set a runtime identifier so it doesn't matter but as I mimicked the SDK's behavior (see the comment with the link) I included this part as well, for completeness.

Copy link
Member Author

@ViktorHofer ViktorHofer Jan 26, 2022

Choose a reason for hiding this comment

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

@ericstj does that make sense? If there isn't any other feedback I would like to merge this in :)

Copy link
Member

@ericstj ericstj Jan 27, 2022

Choose a reason for hiding this comment

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

Oh, that's interesting. I hadn't realized they touched the RuntimeIdentifier property across projects.
If a project doesn't specify a any RID it is built without RIDs. If a project specifies a RID it will be built with the RID of the parent, should that be globally specified.

{
// If the project is RID agnostic, undefine the RuntimeIdentifier property to avoid another evaluation. -->
string undefineProperties = projectReference.GetMetadata("UndefineProperties");
projectReference.SetMetadata("UndefineProperties", undefineProperties + ";RuntimeIdentifier");
}

projectReference.SetMetadata("SkipGetTargetFrameworkProperties", "true");
}

Expand Down
18 changes: 12 additions & 6 deletions src/Microsoft.DotNet.Build.Tasks.TargetFramework.Sdk/src/build/BinPlace.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,13 @@
<Target Name="BinPlace"
DependsOnTargets="GetBinPlaceTargetFramework;BinPlaceFiles;BinPlaceProps"
AfterTargets="CopyFilesToOutputDirectory"
Condition="'$(EnableBinPlacing)' == 'true' OR '@(BinPlaceDir)' != ''" />
Condition="'$(EnableBinPlacing)' == 'true' or '@(BinPlaceDir)' != ''" />

<Target Name="BinPlaceFiles"
Condition="'@(BinPlaceDir)' != ''"
DependsOnTargets="GetBinPlaceItems"
Inputs="@(BinPlaceDir);%(BinPlaceDir.ItemName);%(BinPlaceDir.Identity)"
Outputs="unused" >
Outputs="unused">

<PropertyGroup>
<_BinPlaceItemName>%(BinPlaceDir.ItemName)</_BinPlaceItemName>
Expand Down Expand Up @@ -117,11 +117,17 @@
</Target>

<Target Name="GetBinPlaceTargetFramework">
<!-- find which, if any, supported target framework of this project is best
for each binplace targetFramework -->
<ItemGroup>
<!-- This needs to be a separate item as batching doesn't work when passing in the ValueOrDefault result directly. -->
<_supportedTargetFramework Include="$(TargetFrameworks)" Condition="'$(TargetFrameworks)' != ''" />
<_supportedTargetFramework Include="$(TargetFramework)" Condition="'@(_supportedTargetFramework)' == '' and '$(TargetFramework)' != ''" />
</ItemGroup>

<!-- Find which, if any, supported target framework of this project is best
for each binplace targetFramework. -->
<ChooseBestTargetFrameworksTask BuildTargetFrameworks="@(BinPlaceTargetFrameworks)"
SupportedTargetFrameworks="$(TargetFrameworks)"
RuntimeGraph="$(RuntimeGraph)" >
SupportedTargetFrameworks="@(_supportedTargetFramework)"
RuntimeGraph="$(RuntimeGraph)">
<Output TaskParameter="BestTargetFrameworks" ItemName="_bestBinPlaceTargetFrameworks" />
</ChooseBestTargetFrameworksTask>

Expand Down