Skip to content

Fix graph target list for multitargetting projects using BuildProjectReferences=false #8565

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
merged 1 commit into from
Apr 5, 2023
Merged

Fix graph target list for multitargetting projects using BuildProjectReferences=false #8565

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
65 changes: 65 additions & 0 deletions src/Build.UnitTests/Graph/ProjectGraph_Tests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2508,6 +2508,71 @@ public void MultipleProjectReferencesSameFileDifferentTargets()
targetLists[project2].ShouldBe(new[] { "SomeDefaultTarget2", "SomeOtherTarget" });
}

[Fact]
public void MultitargettingTargetsWithBuildProjectReferencesFalse()
{
// This test should emulate Microsoft.Managed.After.targets's handling of multitargetting projects.
ProjectGraph graph = Helpers.CreateProjectGraph(
env: _env,
dependencyEdges: new Dictionary<int, int[]>()
{
{ 1, new[] { 2 } },
},
globalProperties: new Dictionary<string, string> { { "BuildProjectReferences", "false" } },
extraContentForAllNodes: """
<PropertyGroup>
<TargetFrameworks>netcoreapp3.1;net6.0;net7.0</TargetFrameworks>
</PropertyGroup>

<PropertyGroup Condition="'$(TargetFrameworks)' != '' and '$(TargetFramework)' == ''">
<IsCrossTargetingBuild>true</IsCrossTargetingBuild>
</PropertyGroup>

<PropertyGroup>
<InnerBuildProperty>TargetFramework</InnerBuildProperty>
<InnerBuildPropertyValues>TargetFrameworks</InnerBuildPropertyValues>
</PropertyGroup>

<PropertyGroup Condition="'$(IsGraphBuild)' == 'true' and '$(IsCrossTargetingBuild)' != 'true'">
<_MainReferenceTargetForBuild Condition="'$(BuildProjectReferences)' == '' or '$(BuildProjectReferences)' == 'true'">.projectReferenceTargetsOrDefaultTargets</_MainReferenceTargetForBuild>
<_MainReferenceTargetForBuild Condition="'$(_MainReferenceTargetForBuild)' == ''">GetTargetPath</_MainReferenceTargetForBuild>

<ProjectReferenceTargetsForBuild>$(_MainReferenceTargetForBuild);GetNativeManifest;$(_RecursiveTargetForContentCopying);$(ProjectReferenceTargetsForBuild)</ProjectReferenceTargetsForBuild>
</PropertyGroup>
<PropertyGroup Condition="'$(IsGraphBuild)' == 'true' and '$(IsCrossTargetingBuild)' == 'true'">
<ProjectReferenceTargetsForBuild>.default;$(ProjectReferenceTargetsForBuild)</ProjectReferenceTargetsForBuild>
</PropertyGroup>

<ItemGroup Condition="'$(IsGraphBuild)' == 'true'">
<ProjectReferenceTargets Include="Build" Targets="GetTargetFrameworks" OuterBuild="true" SkipNonexistentTargets="true" Condition="'$(IsCrossTargetingBuild)' != 'true'" />
<ProjectReferenceTargets Include="Build" Targets="$(ProjectReferenceTargetsForBuild)" Condition=" '$(ProjectReferenceTargetsForBuild)' != '' " />

<ProjectReferenceTargets Include="Build" Targets="GetTargetFrameworksWithPlatformForSingleTargetFramework" SkipNonexistentTargets="true" Condition="'$(IsCrossTargetingBuild)' != 'true'" />
</ItemGroup>

<Target Name="Build" />
<Target Name="GetTargetPath" />
<Target Name="GetNativeManifest" />
<Target Name="GetTargetFrameworks" />
<Target Name="GetTargetFrameworksWithPlatformForSingleTargetFramework" />
""");

IReadOnlyDictionary<ProjectGraphNode, ImmutableList<string>> targetLists = graph.GetTargetLists(Array.Empty<string>());

targetLists[GetOuterBuild(graph, 1)].ShouldBe(new[] { "Build" });
foreach (ProjectGraphNode inner in GetInnerBuilds(graph, 1))
{
targetLists[inner].ShouldBe(new[] { "Build" });
}

targetLists[GetOuterBuild(graph, 2)].ShouldBe(new[] { "GetTargetFrameworks" });
foreach (ProjectGraphNode inner in GetInnerBuilds(graph, 2))
{
// GetTargetFrameworks actually shouldn't be here...
Copy link
Member

Choose a reason for hiding this comment

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

Ok, I'll bite. Why is it? :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because we need to rewrite how this all works :)

But specifically it's this line:

<ProjectReferenceTargets Include="Build" Targets="GetTargetFrameworks" OuterBuild="true" SkipNonexistentTargets="true" Condition="'$(IsCrossTargetingBuild)' != 'true'" />

I couldn't think of a way to ensure that only applies from outer build to outer build and not the inner build.

targetLists[inner].ShouldBe(new[] { "GetTargetFrameworks", "GetTargetPath", "GetNativeManifest", "GetTargetFrameworksWithPlatformForSingleTargetFramework" });
}
}

public void Dispose()
{
_env.Dispose();
Expand Down
27 changes: 15 additions & 12 deletions src/Tasks/Microsoft.Managed.After.targets
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,17 +36,13 @@ Copyright (C) Microsoft Corporation. All rights reserved.
Properties for extension of ProjectReferenceTargets.
Append any current value which may have been provided in a Directory.Build.props since the intent was likely to append, not prepend.
-->
<PropertyGroup Condition="'$(IsGraphBuild)' == 'true'">
<!-- Empty case is for outer builds which do not import the target files that set BuildProjectReferences -->
<PropertyGroup Condition="'$(IsGraphBuild)' == 'true' and '$(IsCrossTargetingBuild)' != 'true'">
<!-- Empty case is for builds which do not import the target files that set BuildProjectReferences -->
<_MainReferenceTargetForBuild Condition="'$(BuildProjectReferences)' == '' or '$(BuildProjectReferences)' == 'true'">.projectReferenceTargetsOrDefaultTargets</_MainReferenceTargetForBuild>
<_MainReferenceTargetForBuild Condition="'$(_MainReferenceTargetForBuild)' == ''">GetTargetPath</_MainReferenceTargetForBuild>

<ProjectReferenceTargetsForBuild>$(_MainReferenceTargetForBuild);GetNativeManifest;$(_RecursiveTargetForContentCopying);$(ProjectReferenceTargetsForBuild)</ProjectReferenceTargetsForBuild>

<ProjectReferenceTargetsForClean>Clean;$(ProjectReferenceTargetsForClean)</ProjectReferenceTargetsForClean>

<ProjectReferenceTargetsForRebuild>$(ProjectReferenceTargetsForClean);$(ProjectReferenceTargetsForBuild);$(ProjectReferenceTargetsForRebuild)</ProjectReferenceTargetsForRebuild>

<!-- Publish has the same logic as Build for the main reference target except it also takes $(NoBuild) into account. -->
<_MainReferenceTargetForPublish Condition="'$(NoBuild)' == 'true'">GetTargetPath</_MainReferenceTargetForPublish>
<_MainReferenceTargetForPublish Condition="'$(NoBuild)' != 'true'">$(_MainReferenceTargetForBuild)</_MainReferenceTargetForPublish>
Expand All @@ -58,23 +54,30 @@ Copyright (C) Microsoft Corporation. All rights reserved.

<ProjectReferenceTargetsForGetCopyToPublishDirectoryItems>GetCopyToPublishDirectoryItems;$(ProjectReferenceTargetsForGetCopyToPublishDirectoryItems)</ProjectReferenceTargetsForGetCopyToPublishDirectoryItems>
</PropertyGroup>
<PropertyGroup Condition="'$(IsGraphBuild)' == 'true' and '$(IsCrossTargetingBuild)' == 'true'">
<ProjectReferenceTargetsForBuild>.default;$(ProjectReferenceTargetsForBuild)</ProjectReferenceTargetsForBuild>
</PropertyGroup>
<PropertyGroup Condition="'$(IsGraphBuild)' == 'true'">
<ProjectReferenceTargetsForClean>Clean;$(ProjectReferenceTargetsForClean)</ProjectReferenceTargetsForClean>
<ProjectReferenceTargetsForRebuild>$(ProjectReferenceTargetsForClean);$(ProjectReferenceTargetsForBuild);$(ProjectReferenceTargetsForRebuild)</ProjectReferenceTargetsForRebuild>
</PropertyGroup>

<ItemGroup Condition="'$(IsGraphBuild)' == 'true'">
<ProjectReferenceTargets Include="Build" Targets="$(ProjectReferenceTargetsForBuildInOuterBuild)" Condition=" '$(ProjectReferenceTargetsForBuildInOuterBuild)' != '' " OuterBuild="true" />
<ProjectReferenceTargets Include="Build" Targets="GetTargetFrameworks" OuterBuild="true" SkipNonexistentTargets="true" />
<ProjectReferenceTargets Include="Build" Targets="GetTargetFrameworks" OuterBuild="true" SkipNonexistentTargets="true" Condition="'$(IsCrossTargetingBuild)' != 'true'" />
<ProjectReferenceTargets Include="Build" Targets="$(ProjectReferenceTargetsForBuild)" Condition=" '$(ProjectReferenceTargetsForBuild)' != '' " />

<ProjectReferenceTargets Include="Clean" Targets="$(ProjectReferenceTargetsForCleanInOuterBuild)" Condition=" '$(ProjectReferenceTargetsForCleanInOuterBuild)' != '' " OuterBuild="true" />
<ProjectReferenceTargets Include="Clean" Targets="GetTargetFrameworks" OuterBuild="true" SkipNonexistentTargets="true" />
<ProjectReferenceTargets Include="Clean" Targets="GetTargetFrameworks" OuterBuild="true" SkipNonexistentTargets="true" Condition="'$(IsCrossTargetingBuild)' != 'true'" />
Copy link
Contributor

Choose a reason for hiding this comment

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

I know you just added the condition here and below, but why are we Include'ing Build and Clean multiple times with different Targets lists? I would read that as that we should execute each target multiple times, but that doesn't sound right.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't follow the question. Can you clarify?

Copy link
Contributor Author

@dfederm dfederm Apr 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Maybe it would help if you read it this way?

<ProjectReferenceTargets Include="X" Targets="Y" />

Means "If I'm called with and entry target X, I call Y on my ProjectRefernces".

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, so when you say:

<ProjectReferenceTargets Include="Clean" Targets="GetTargetFrameworks" />
<ProjectReferenceTargets Include="Clean" Targets="GetTargetFrameworksWithPlatformForSingleTargetFramework" />

what you're saying is "If this project builds the Clean target as its entrypoint, build the other two." That makes more sense, thanks.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yup, exactly!

<ProjectReferenceTargets Include="Clean" Targets="$(ProjectReferenceTargetsForClean)" Condition=" '$(ProjectReferenceTargetsForClean)' != '' " />

<!--
Note: SkipNonexistentTargets="true" on the following three items means that an outer build node's call to its existent GetTargetFrameworks target will fail if its inner build nodes don't define GetTargetFrameworksWithPlatformForSingleTargetFrameworks.
This is necessary since the P2P protocol cannot express the targets called from the outer build to the inner build.
-->
<ProjectReferenceTargets Include="Build" Targets="GetTargetFrameworksWithPlatformForSingleTargetFramework" SkipNonexistentTargets="true" />
<ProjectReferenceTargets Include="Clean" Targets="GetTargetFrameworksWithPlatformForSingleTargetFramework" SkipNonexistentTargets="true" />
<ProjectReferenceTargets Include="Rebuild" Targets="GetTargetFrameworksWithPlatformForSingleTargetFramework" SkipNonexistentTargets="true" />
<ProjectReferenceTargets Include="Build" Targets="GetTargetFrameworksWithPlatformForSingleTargetFramework" SkipNonexistentTargets="true" Condition="'$(IsCrossTargetingBuild)' != 'true'" />
<ProjectReferenceTargets Include="Clean" Targets="GetTargetFrameworksWithPlatformForSingleTargetFramework" SkipNonexistentTargets="true" Condition="'$(IsCrossTargetingBuild)' != 'true'" />
<ProjectReferenceTargets Include="Rebuild" Targets="GetTargetFrameworksWithPlatformForSingleTargetFramework" SkipNonexistentTargets="true" Condition="'$(IsCrossTargetingBuild)' != 'true'" />

<ProjectReferenceTargets Include="Rebuild" Targets="$(ProjectReferenceTargetsForRebuild)" Condition=" '$(ProjectReferenceTargetsForRebuild)' != '' " />

Expand All @@ -83,4 +86,4 @@ Copyright (C) Microsoft Corporation. All rights reserved.
<ProjectReferenceTargets Include="GetCopyToPublishDirectoryItems" Targets="$(ProjectReferenceTargetsForGetCopyToPublishDirectoryItems)" Condition=" '$(ProjectReferenceTargetsForGetCopyToPublishDirectoryItems)' != '' " />
</ItemGroup>

</Project>
</Project>