Skip to content

Commit

Permalink
fixing global properties error in Dynamic platform resolution (#8222)
Browse files Browse the repository at this point in the history
Context
In graph projects, if project A (x86) references project B (AnyCPU), we use the same global properties dictionary in both cases. This means that project A will get Platform:AnyCPU as a global property.

Platform should be the only property that changes, so anything that prevents the parent project from getting its global properties changed by dependent projects would unblock VS.

Note that we cannot use one dictionary and change the property while building B in the above example, then revert the property because that would still affect B; anyone using the project graph object would be getting incorrect information.

From a performance perspective, there should only be ~6-7 properties in the dictionary, so copying it should not be a major hit.

Changes Made
This change prevents the global properties dictionary from being passed directly if EnableDynamicPlatformResolution is true.

Testing
Unit test and possibly manual tests, though I didn't ask.
Co-authored-by: Forgind <[email protected]>
Co-authored-by: Rainer Sigwald <[email protected]>
  • Loading branch information
MIchaelRShea authored Mar 2, 2023
1 parent e0f4434 commit 47a7121
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 2 deletions.
27 changes: 27 additions & 0 deletions src/Build.UnitTests/Graph/GetCompatiblePlatformGraph_Tests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,33 @@ namespace Microsoft.Build.Graph.UnitTests
public class ProjectGraphSetPlatformTests
{

[Fact]
public void ValidateGlobalPropertyCopyByValueNotReference()
{
using (var env = TestEnvironment.Create())
{

TransientTestFile entryProject = CreateProjectFile(env, 1, extraContent: @"<PropertyGroup>
<EnableDynamicPlatformResolution>true</EnableDynamicPlatformResolution>
<Platform>x64</Platform>
<PlatformLookupTable>win32=x64</PlatformLookupTable>
</PropertyGroup>
<ItemGroup>
<ProjectReference Include=""$(MSBuildThisFileDirectory)2.proj"" />
</ItemGroup>");
var proj2 = env.CreateFile("2.proj", @"
<Project>
<PropertyGroup>
<EnableDynamicPlatformResolution>true</EnableDynamicPlatformResolution>
<Platforms>AnyCPU</Platforms>
</PropertyGroup>
</Project>");

ProjectGraph graph = new ProjectGraph(entryProject.Path);
GetFirstNodeWithProjectNumber(graph, 1).ProjectInstance.GlobalProperties.ContainsKey("Platform").ShouldBeFalse();
}
}

[Fact]
public void ValidateSetPlatformOverride()
{
Expand Down
5 changes: 3 additions & 2 deletions src/Build/Graph/ProjectInterpretation.cs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ public IEnumerable<ReferenceInfo> GetReferences(ProjectInstance requesterInstanc

var projectReferenceFullPath = projectReferenceItem.GetMetadataValue(FullPathMetadataName);

var referenceGlobalProperties = GetGlobalPropertiesForItem(projectReferenceItem, requesterInstance.GlobalPropertiesDictionary, globalPropertiesModifiers);
var referenceGlobalProperties = GetGlobalPropertiesForItem(projectReferenceItem, requesterInstance.GlobalPropertiesDictionary, ConversionUtilities.ValidBooleanTrue(requesterInstance.GetPropertyValue(EnableDynamicPlatformResolutionMetadataName)), globalPropertiesModifiers);

var requesterPlatform = "";
var requesterPlatformLookupTable = "";
Expand Down Expand Up @@ -324,6 +324,7 @@ public GlobalPropertyPartsForMSBuildTask AddPropertyToUndefine(string propertyTo
private static PropertyDictionary<ProjectPropertyInstance> GetGlobalPropertiesForItem(
ProjectItemInstance projectReference,
PropertyDictionary<ProjectPropertyInstance> requesterGlobalProperties,
bool dynamicPlatformEnabled,
IEnumerable<GlobalPropertiesModifier> globalPropertyModifiers = null)
{
ErrorUtilities.VerifyThrowInternalNull(projectReference, nameof(projectReference));
Expand All @@ -337,7 +338,7 @@ private static PropertyDictionary<ProjectPropertyInstance> GetGlobalPropertiesFo

var globalPropertyParts = globalPropertyModifiers?.Aggregate(defaultParts, (currentProperties, modifier) => modifier(currentProperties, projectReference)) ?? defaultParts;

if (globalPropertyParts.AllEmpty())
if (globalPropertyParts.AllEmpty() && !dynamicPlatformEnabled)
{
return requesterGlobalProperties;
}
Expand Down

0 comments on commit 47a7121

Please sign in to comment.