Skip to content

Commit

Permalink
Removing sln level turn off of setplatform feature
Browse files Browse the repository at this point in the history 
Currently we turn off dynamic platform resolution for a whole solution
if a single project in the solution is assigned a configuration. This is
problematic as some projects are outside of the scope of the solution
but still have certain targets that run on them that are architecture
specific. These projects will build as the wrong architecture because no
configuration is defined and no platform negotiation takes place.

I removed the conditional that turns platform negotiation off on a sln
level. The logic to turn this off on a project level is already in place
through checking is a projectreference has setplatform appended to it.
This will make sure no projects with configurations defined will be
negotiated for as MSbuild adds setplatform metadata to projectreferences
with configurations.
  • Loading branch information
@MIchaelRShea @rainersigwald
MIchaelRShea authored and rainersigwald committed Jul 12, 2023
1 parent 5785ed5 commit 657005a
Show file tree
Hide file tree
Showing 3 changed files with 110 additions and 5 deletions.
99 changes: 98 additions & 1 deletion src/Build.UnitTests/Graph/GetCompatiblePlatformGraph_Tests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ namespace Microsoft.Build.Graph.UnitTests
/// <summary>
/// Performs SetPlatform negotiation for all project references when opted
/// in via the EnableDynamicPlatformResolution property.
///
///
/// The static graph mirrors the negotiation during build to determine plartform for each node.
/// These tests mirror GetCompatiblePlatform_Tests.cs in order to make sure they both are in sync.
/// </summary>
Expand Down Expand Up @@ -351,5 +351,102 @@ public void PlatformIsChosenAsDefault()
GetFirstNodeWithProjectNumber(graph, 2).ProjectInstance.GetPropertyValue("Platform").ShouldBe(GetFirstNodeWithProjectNumber(graph, 1).ProjectInstance.GetPropertyValue("Platform"));
}
}

// Validate configurations are defined in project reference protocol
[Fact]
public void SolutionWithoutAllConfigurations()
{
using (TestEnvironment testEnvironment = TestEnvironment.Create())
{
var firstProjectName = "1";
var secondProjectName = "2";
var thirdProjectName = "3";
TransientTestFolder folder = testEnvironment.CreateFolder(createFolder: true);
TransientTestFolder project1Folder = testEnvironment.CreateFolder(Path.Combine(folder.Path, firstProjectName), createFolder: true);
TransientTestFolder project1SubFolder = testEnvironment.CreateFolder(Path.Combine(project1Folder.Path, firstProjectName), createFolder: true);
TransientTestFile project1 = testEnvironment.CreateFile(project1SubFolder, $"{firstProjectName}.csproj",
@"<Project>
<PropertyGroup>
<EnableDynamicPlatformResolution>true</EnableDynamicPlatformResolution>
<Platform>x64</Platform>
</PropertyGroup>
<ItemGroup>
<ProjectReference Include=""$(MSBuildThisFileDirectory)\..\..\2\2\2.proj"" />
<ProjectReference Include=""$(MSBuildThisFileDirectory)\..\..\3\3\3.proj"" />
</ItemGroup>
</Project>
");

TransientTestFolder project2Folder = testEnvironment.CreateFolder(Path.Combine(folder.Path, secondProjectName), createFolder: true);
TransientTestFolder project2SubFolder = testEnvironment.CreateFolder(Path.Combine(project2Folder.Path, secondProjectName), createFolder: true);
TransientTestFile project2 = testEnvironment.CreateFile(project2SubFolder, $"{secondProjectName}.proj",
@"<Project>
<PropertyGroup>
<EnableDynamicPlatformResolution>true</EnableDynamicPlatformResolution>
<Platforms>AnyCPU;x64</Platforms>
</PropertyGroup>
</Project>
");

TransientTestFolder project3Folder = testEnvironment.CreateFolder(Path.Combine(folder.Path, thirdProjectName), createFolder: true);
TransientTestFolder project3SubFolder = testEnvironment.CreateFolder(Path.Combine(project3Folder.Path, thirdProjectName), createFolder: true);
TransientTestFile project3 = testEnvironment.CreateFile(project3SubFolder, $"{thirdProjectName}.proj",
@"<Project>
<PropertyGroup>
<EnableDynamicPlatformResolution>true</EnableDynamicPlatformResolution>
<Platforms>AnyCPU;x64</Platforms>
</PropertyGroup>
</Project>
");


// Slashes here (and in the .slnf) are hardcoded as backslashes intentionally to support the common case.
TransientTestFile solutionFile = testEnvironment.CreateFile(folder, "SimpleProject.sln",
@"
Microsoft Visual Studio Solution File, Format Version 12.00
# Visual Studio Version 16
VisualStudioVersion = 16.0.29326.124
MinimumVisualStudioVersion = 10.0.40219.1
Project(""{9A19103F-16F7-4668-BE54-9A1E7A4F7556}"") = ""Project1"", ""1\1\1.csproj"", ""{79B5EBA6-5D27-4976-BC31-14422245A59A}""
EndProject
Project(""{9A19103F-16F7-4668-BE54-9A1E7A4F7556}"") = ""2"", ""2\2\2.proj"", ""{8EFCCA22-9D51-4268-90F7-A595E11FCB2D}""
EndProject
Global
GlobalSection(SolutionConfigurationPlatforms) = preSolution
Debug|x64 = Debug|x64
Release|x64 = Release|x64
EndGlobalSection
GlobalSection(ProjectConfigurationPlatforms) = postSolution
{79B5EBA6-5D27-4976-BC31-14422245A59A}.Debug|x64.ActiveCfg = Debug|x64
{79B5EBA6-5D27-4976-BC31-14422245A59A}.Debug|x64.Build.0 = Debug|x64
{79B5EBA6-5D27-4976-BC31-14422245A59A}.Release|x64.ActiveCfg = Release|x64
{79B5EBA6-5D27-4976-BC31-14422245A59A}.Release|x64.Build.0 = Release|x64
{8EFCCA22-9D51-4268-90F7-A595E11FCB2D}.Debug|x64.ActiveCfg = Debug|Any CPU
{8EFCCA22-9D51-4268-90F7-A595E11FCB2D}.Debug|x64.Build.0 = Debug|Any CPU
{8EFCCA22-9D51-4268-90F7-A595E11FCB2D}.Release|x64.ActiveCfg = Release|Any CPU
{8EFCCA22-9D51-4268-90F7-A595E11FCB2D}.Release|x64.Build.0 = Release|Any CPU
EndGlobalSection
GlobalSection(SolutionProperties) = preSolution
HideSolutionNode = FALSE
EndGlobalSection
GlobalSection(ExtensibilityGlobals) = postSolution
SolutionGuid = {DE7234EC-0C4D-4070-B66A-DCF1B4F0CFEF}
EndGlobalSection
EndGlobal
");

ProjectCollection projectCollection = testEnvironment.CreateProjectCollection().Collection;
MockLogger logger = new();
projectCollection.RegisterLogger(logger);
ProjectGraphEntryPoint entryPoint = new(solutionFile.Path, new Dictionary<string, string>());

// We want to make sure negotiation respects configuration if defined but negotiates if not.
ProjectGraph graphFromSolution = new(entryPoint, projectCollection);
logger.AssertNoErrors();
GetFirstNodeWithProjectNumber(graphFromSolution, 2).ProjectInstance.GetPropertyValue("Platform").ShouldBe("AnyCPU", "Project2 should have followed the sln config to AnyCPU");
GetFirstNodeWithProjectNumber(graphFromSolution, 3).ProjectInstance.GetPropertyValue("Platform").ShouldBe("x64", "Project3 isn't in the solution so it should have negotiated to x64 to match Project1");
}
}
}
}
15 changes: 12 additions & 3 deletions src/Build/Graph/ProjectInterpretation.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ private readonly struct TargetSpecification
{
public TargetSpecification(string target, bool skipIfNonexistent)
{
// Verify that if this target is skippable then it equals neither
// Verify that if this target is skippable then it equals neither
// ".default" nor ".projectReferenceTargetsOrDefaultTargets".
ErrorUtilities.VerifyThrow(
!skipIfNonexistent || (!target.Equals(MSBuildConstants.DefaultTargetsMarker)
Expand Down Expand Up @@ -131,6 +131,8 @@ public IEnumerable<ReferenceInfo> GetReferences(ProjectInstance requesterInstanc
allowCollectionReuse: solutionConfiguration == null && !enableDynamicPlatformResolution,
globalPropertiesModifiers);

bool configurationDefined = false;

// Match what AssignProjectConfiguration does to resolve project references.
if (solutionConfiguration != null)
{
Expand All @@ -151,6 +153,8 @@ public IEnumerable<ReferenceInfo> GetReferences(ProjectInstance requesterInstanc
{
referenceGlobalProperties.Remove(PlatformMetadataName);
}

configurationDefined = true;
}
else
{
Expand All @@ -161,11 +165,16 @@ public IEnumerable<ReferenceInfo> GetReferences(ProjectInstance requesterInstanc
referenceGlobalProperties.Remove(ConfigurationMetadataName);
referenceGlobalProperties.Remove(PlatformMetadataName);
}
else
{
configurationDefined = true;
}
}
}

// Note: Dynamic platform resolution is not enabled for sln-based builds.
else if (!projectReferenceItem.HasMetadata(SetPlatformMetadataName) && enableDynamicPlatformResolution)
// Note: Dynamic platform resolution is not enabled for sln-based builds,
// unless the project isn't known to the solution.
if (enableDynamicPlatformResolution && !configurationDefined && !projectReferenceItem.HasMetadata(SetPlatformMetadataName))
{
string requesterPlatform = requesterInstance.GetPropertyValue("Platform");
string requesterPlatformLookupTable = requesterInstance.GetPropertyValue("PlatformLookupTable");
Expand Down
1 change: 0 additions & 1 deletion src/Tasks/Microsoft.Common.CurrentVersion.targets
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1648,7 +1648,6 @@ Copyright (C) Microsoft Corporation. All rights reserved.
Configuration information. See AssignProjectConfiguration -->
<Target Name="_GetProjectReferencePlatformProperties"
Condition="'$(EnableDynamicPlatformResolution)' == 'true'
and '$(CurrentSolutionConfigurationContents)' == ''
and '@(_MSBuildProjectReferenceExistent)' != ''">

<!-- Allow preset SetPlatform to override this operation -->
Expand Down

0 comments on commit 657005a

Please sign in to comment.