Skip to content

Commit

Permalink
SetPlatform Negotiation: No global properties during GetTargetFramewo…
Browse files Browse the repository at this point in the history
…rks (#7803)

Fixes #7760

Context
#7511 figured out a better way to prevent over-evaluations when A(Platform=x86) -> B(Platform=x86, Platforms=x64,x86). It allowed setplatform negotiation to check "would project B have built as what A is currently building as if we didn't tell it anything?

Unfortunately, there's a bug when passing a global property because GetTargetFrameworks passes Platform and Configuration, despite not needing to. This PR is an attempt at resolving this by no longer passing those properties, as well as undefining them.

Changes Made
Remove additional properties during the MSBuild call to GetTargetFrameworks.

Testing
Notes
It's possible we can't fix it this way, and instead we'll need to create two msbuild calls. One that's the standard but only gets called when EnableDynamicPlatformResolution is false, and the other that only gets called when EnableDynamicPlatformResolution is true.
  • Loading branch information
benvillalobos authored Jul 16, 2022
1 parent 49032f6 commit d6e1a1d
Show file tree
Hide file tree
Showing 2 changed files with 19 additions and 3 deletions.
2 changes: 1 addition & 1 deletion src/Shared/PlatformNegotiation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ internal static string GetNearestPlatform(string referencedProjectPlatform, stri
// mappings on a per-ProjectReference basis.
Dictionary<string, string>? projectReferenceLookupTable = ExtractLookupTable(projectReferenceLookupTableMetadata, log);

HashSet<string> projectReferencePlatforms = new HashSet<string>();
HashSet<string> projectReferencePlatforms = new HashSet<string>(StringComparer.OrdinalIgnoreCase);
foreach (string s in projectReferencePlatformsMetadata.Split(MSBuildConstants.SemicolonChar, StringSplitOptions.RemoveEmptyEntries))
{
projectReferencePlatforms.Add(s);
Expand Down
20 changes: 18 additions & 2 deletions src/Tasks/Microsoft.Common.CurrentVersion.targets
Original file line number Diff line number Diff line change
Expand Up @@ -1780,7 +1780,23 @@ Copyright (C) Microsoft Corporation. All rights reserved.
Properties="%(_MSBuildProjectReferenceExistent.SetConfiguration); %(_MSBuildProjectReferenceExistent.SetPlatform)"
ContinueOnError="!$(BuildingProject)"
RemoveProperties="%(_MSBuildProjectReferenceExistent.GlobalPropertiesToRemove);TargetFramework;RuntimeIdentifier$(_GlobalPropertiesToRemoveFromProjectReferences)"
Condition="'%(_MSBuildProjectReferenceExistent.SkipGetTargetFrameworkProperties)' != 'true'"
Condition="'%(_MSBuildProjectReferenceExistent.SkipGetTargetFrameworkProperties)' != 'true' and '$(EnableDynamicPlatformResolution)' != 'true'"
SkipNonexistentTargets="true">
<Output TaskParameter="TargetOutputs" ItemName="_ProjectReferenceTargetFrameworkPossibilities" />
</MSBuild>

<!--
SetPlatform negotiation requires the 'GetTargetFrameworks' MSBuild call to NOT pass global properties. This is to verify
whether or not the referenced project would build as the same platform as the current project by default. The above
MSBuild call is kept for legacy scenarios that may depend on passing %(SetConfiguration) and %(SetPlatform).
-->
<MSBuild
Projects="@(_MSBuildProjectReferenceExistent)"
Targets="GetTargetFrameworks"
BuildInParallel="$(BuildInParallel)"
ContinueOnError="!$(BuildingProject)"
RemoveProperties="%(_MSBuildProjectReferenceExistent.GlobalPropertiesToRemove);TargetFramework;RuntimeIdentifier;Platform;Configuration$(_GlobalPropertiesToRemoveFromProjectReferences)"
Condition="'%(_MSBuildProjectReferenceExistent.SkipGetTargetFrameworkProperties)' != 'true' and '$(EnableDynamicPlatformResolution)' == 'true'"
SkipNonexistentTargets="true">
<Output TaskParameter="TargetOutputs" ItemName="_ProjectReferenceTargetFrameworkPossibilities" />
</MSBuild>
Expand Down Expand Up @@ -6670,4 +6686,4 @@ Copyright (C) Microsoft Corporation. All rights reserved.
<Import Project="$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.targets\ImportAfter\*" Condition="'$(ImportByWildcardAfterMicrosoftCommonTargets)' == 'true' and exists('$(MSBuildExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.targets\ImportAfter')"/>
<Import Project="$(MSBuildUserExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.targets\ImportAfter\*" Condition="'$(ImportUserLocationsByWildcardAfterMicrosoftCommonTargets)' == 'true' and exists('$(MSBuildUserExtensionsPath)\$(MSBuildToolsVersion)\Microsoft.Common.targets\ImportAfter')"/>

</Project>
</Project>

0 comments on commit d6e1a1d

Please sign in to comment.