Propagate solution configuration information to project cache plugins (…

…dotnet#6738)

Context
Visual Studio has solution configurations and project configurations. A configuration is a (Configuration, Platform) tuple.
A solution configuration defines a specific project configuration for each project in the solution. It's like a mapping from a solution configuration to a list of project configurations for each project.
At any moment a single solution configuration is active in VS.
When doing a real build in VS, the active solution configuration is propagated as global properties on each build request.

Up until now I didn't have a clear understanding of this, therefore the project cache setup has a bug when building under VS: it does not propagate the solution configuration information to the plugin instance.
This means that if the user selects a non-default solution configuration (or if the solution is more complicated and the default project configuration assignment is not straightforward), there will be a mismatch between the build requests issued by VS (reflecting the non-default solution configuration) and the state of the plugin (reflecting the default solution configuration).
The mismatches end up as build failures because the plugin is unable to match an incoming build request to an existing quickbuild build node (build node graph is created from MSBuild's static graph)

Changes Made
Unfortunately MSBuild does not know what the active VS solution configuration is. So in this PR we parse the project configuration for each project from an XML that VS sets on each build request as a global property (however horrible the idea of that global property is, without it we could not have enabled the build-under-VS scenario). Previously, msbuild used the .sln file as the single entry-point to the plugin. Starting with this PR it creates one entry point for each project in the parsed XML, with its particular project configuration set as global properties.
This makes the static graph inside the plugin instance match the solution configuration.
These changes are in the VS workaround part of the code. Ideally, if VS passed in the project cache graph descriptor, it would have enough information to just use a single .sln based entrypoint and set the solution configuration on it. Maybe, one day ...

The PR also removes TargetFramework from the entry points. VS sets an empty TargetFramework="" global properties on outerbuilds (because why not). If we don't remove it it leaks into the graph entry-points, which propagates TF="" to all nodes in the graph, messing them up.

Testing
Unit tests and manual testing.

Notes
The meat of the implementation is here: https://github.com/dotnet/msbuild/pull/6738/files#diff-ac47163cf1a4ff2fbd79d56e0cf79ce9636b6a8331dfa89dae3fe8d3547f7fb3R345

src/Build/BackEnd/Components/ProjectCache/ProjectCacheService.cs

@@ -4,18 +4,21 @@
 #nullable enable
 using System;
 using System.Collections.Generic;
+using System.Collections.Immutable;
 using System.Diagnostics;
 using System.Linq;
 using System.Reflection;
 using System.Threading;
 using System.Threading.Tasks;
+using System.Xml;
 using Microsoft.Build.BackEnd;
 using Microsoft.Build.BackEnd.Logging;
 using Microsoft.Build.Construction;
 using Microsoft.Build.Execution;
 using Microsoft.Build.FileSystem;
 using Microsoft.Build.Framework;
 using Microsoft.Build.Graph;
+using Microsoft.Build.Internal;
 using Microsoft.Build.Shared;
 using Microsoft.Build.Shared.FileSystem;
 
@@ -328,6 +331,7 @@ async Task LateInitializePluginForVsWorkaround(CacheRequest request)
             {
                 var (_, configuration) = request;
                 var solutionPath = configuration.Project.GetPropertyValue(SolutionProjectGenerator.SolutionPathPropertyName);
+                var solutionConfigurationXml = configuration.Project.GetPropertyValue(SolutionProjectGenerator.CurrentSolutionConfigurationContents);
 
                 ErrorUtilities.VerifyThrow(
                     solutionPath != null && !string.IsNullOrWhiteSpace(solutionPath) && solutionPath != "*Undefined*",
@@ -337,19 +341,112 @@ async Task LateInitializePluginForVsWorkaround(CacheRequest request)
                     FileSystems.Default.FileExists(solutionPath),
                     $"Solution file does not exist: {solutionPath}");
 
+                ErrorUtilities.VerifyThrow(
+                    string.IsNullOrWhiteSpace(solutionConfigurationXml) is false,
+                    "Expected VS to set a xml with all the solution projects' configurations for the currently building solution configuration.");
+
+                // A solution supports multiple solution configurations (different values for Configuration and Platform).
+                // Each solution configuration generates a different static graph.
+                // Therefore, plugin implementations that rely on creating static graphs in their BeginBuild methods need access to the
+                // currently solution configuration used by VS.
+                //
+                // In this VS workaround, however, we do not have access to VS' solution configuration. The only information we have is a global property
+                // named "CurrentSolutionConfigurationContents" that VS sets on each built project. It does not contain the solution configuration itself, but
+                // instead it contains information on how the solution configuration maps to each project's configuration.
+                //
+                // So instead of using the solution file as the entry point, we parse this VS property and extract graph entry points from it, for every project
+                // mentioned in the "CurrentSolutionConfigurationContents" global property.
+                //
+                // Ideally, when the VS workaround is removed from MSBuild and moved into VS, VS should create ProjectGraphDescriptors with the solution path as
+                // the graph entrypoint file, and the VS solution configuration as the entry point's global properties.
+                var graphEntryPointsFromSolutionConfig = GenerateGraphEntryPointsFromSolutionConfigurationXml(
+                    solutionConfigurationXml,
+                    configuration.Project);
+
                 await BeginBuildAsync(
                     ProjectCacheDescriptor.FromAssemblyPath(
                         _projectCacheDescriptor.PluginAssemblyPath!,
-                        new[]
-                        {
-                            new ProjectGraphEntryPoint(
-                                solutionPath,
-                                configuration.Project.GlobalProperties)
-                        },
+                        graphEntryPointsFromSolutionConfig,
                         projectGraph: null,
                         _projectCacheDescriptor.PluginSettings));
             }
 
+            static IReadOnlyCollection<ProjectGraphEntryPoint> GenerateGraphEntryPointsFromSolutionConfigurationXml(
+                string solutionConfigurationXml,
+                ProjectInstance project
+            )
+            {
+                // TODO: fix code clone for parsing CurrentSolutionConfiguration xml: https://github.com/dotnet/msbuild/issues/6751
+                var doc = new XmlDocument();
+                doc.LoadXml(solutionConfigurationXml);
+
+                var root = doc.DocumentElement!;
+                var projectConfigurationNodes = root.GetElementsByTagName("ProjectConfiguration");
+
+                ErrorUtilities.VerifyThrow(projectConfigurationNodes.Count > 0, "Expected at least one project in solution");
+
+                var definingProjectPath = project.FullPath;
+                var graphEntryPoints = new List<ProjectGraphEntryPoint>(projectConfigurationNodes.Count);
+
+                var templateGlobalProperties = new Dictionary<string, string>(project.GlobalProperties, StringComparer.OrdinalIgnoreCase);
+                RemoveProjectSpecificGlobalProperties(templateGlobalProperties, project);
+
+                foreach (XmlNode node in projectConfigurationNodes)
+                {
+                    ErrorUtilities.VerifyThrowInternalNull(node.Attributes, nameof(node.Attributes));
+
+                    var buildProjectInSolution = node.Attributes!["BuildProjectInSolution"];
+                    if (buildProjectInSolution is not null &&
+                        string.IsNullOrWhiteSpace(buildProjectInSolution.Value) is false &&
+                        bool.TryParse(buildProjectInSolution.Value, out var buildProject) &&
+                        buildProject is false)
+                    {
+                        continue;
+                    }
+
+                    ErrorUtilities.VerifyThrow(
+                        node.ChildNodes.OfType<XmlElement>().FirstOrDefault(e => e.Name == "ProjectDependency") is null,
+                        "Project cache service does not support solution only dependencies when running under Visual Studio.");
+
+                    var projectPathAttribute = node.Attributes!["AbsolutePath"];
+                    ErrorUtilities.VerifyThrow(projectPathAttribute is not null, "Expected VS to set the project path on each ProjectConfiguration element.");
+
+                    var projectPath = projectPathAttribute!.Value;
+
+                    var (configuration, platform) = SolutionFile.ParseConfigurationName(node.InnerText, definingProjectPath, 0, solutionConfigurationXml);
+
+                    // Take the defining project global properties and override the configuration and platform.
+                    // It's sufficient to only set Configuration and Platform.
+                    // But we send everything to maximize the plugins' potential to quickly workaround potential MSBuild issues while the MSBuild fixes flow into VS.
+                    var globalProperties = new Dictionary<string, string>(templateGlobalProperties, StringComparer.OrdinalIgnoreCase)
+                    {
+                        ["Configuration"] = configuration,
+                        ["Platform"] = platform
+                    };
+
+                    graphEntryPoints.Add(new ProjectGraphEntryPoint(projectPath, globalProperties));
+                }
+
+                return graphEntryPoints;
+
+                // If any project specific property is set, it will propagate down the project graph and force all nodes to that property's specific side effects, which is incorrect.
+                void RemoveProjectSpecificGlobalProperties(Dictionary<string, string> globalProperties, ProjectInstance project)
+                {
+                    // InnerBuildPropertyName is TargetFramework for the managed sdk.
+                    var innerBuildPropertyName = ProjectInterpretation.GetInnerBuildPropertyName(project);
+
+                    IEnumerable<string> projectSpecificPropertyNames = new []{innerBuildPropertyName, "Configuration", "Platform", "TargetPlatform", "OutputType"};
+
+                    foreach (var propertyName in projectSpecificPropertyNames)
+                    {
+                        if (!string.IsNullOrWhiteSpace(propertyName) && globalProperties.ContainsKey(propertyName))
+                        {
+                            globalProperties.Remove(propertyName);
+                        }
+                    }
+                }
+            }
+
             static bool MSBuildStringIsTrue(string msbuildString) =>
                 ConversionUtilities.ConvertStringToBool(msbuildString, nullOrWhitespaceIsFalse: true);
         }
@@ -366,7 +463,7 @@ private async Task<CacheResult> GetCacheResultAsync(BuildRequestData buildReques
                     return CacheResult.IndicateNonCacheHit(CacheResultType.CacheNotApplicable);
                 }
             }
-			
+
             ErrorUtilities.VerifyThrowInternalNull(buildRequest.ProjectInstance, nameof(buildRequest.ProjectInstance));
 
             var queryDescription = $"{buildRequest.ProjectFullPath}" +

src/Build/Construction/Solution/SolutionConfigurationInSolution.cs

@@ -14,7 +14,7 @@ public sealed class SolutionConfigurationInSolution
         /// </summary>
         internal const char ConfigurationPlatformSeparator = '|';
 
-        internal static readonly char[] ConfigurationPlatformSeparatorArray = new char[] { '|' };
+        internal static readonly char[] ConfigurationPlatformSeparatorArray = { '|' };
 
         /// <summary>
         /// Constructor

src/Build/Construction/Solution/SolutionFile.cs

@@ -1384,7 +1384,6 @@ internal void ParseNestedProjects()
         internal void ParseSolutionConfigurations()
         {
             var nameValueSeparators = '=';
-            var configPlatformSeparators = new[] { SolutionConfigurationInSolution.ConfigurationPlatformSeparator };
 
             do
             {
@@ -1419,15 +1418,26 @@ internal void ParseSolutionConfigurations()
                 ProjectFileErrorUtilities.VerifyThrowInvalidProjectFile(fullConfigurationName == configurationNames[1].Trim(), "SubCategoryForSolutionParsingErrors",
                     new BuildEventFileInfo(FullPath, _currentLineNumber, 0), "SolutionParseInvalidSolutionConfigurationEntry", str);
 
-                string[] configurationPlatformParts = fullConfigurationName.Split(configPlatformSeparators);
+                var (configuration, platform) = ParseConfigurationName(fullConfigurationName, FullPath, _currentLineNumber, str);
 
-                ProjectFileErrorUtilities.VerifyThrowInvalidProjectFile(configurationPlatformParts.Length == 2, "SubCategoryForSolutionParsingErrors",
-                    new BuildEventFileInfo(FullPath, _currentLineNumber, 0), "SolutionParseInvalidSolutionConfigurationEntry", str);
-
-                _solutionConfigurations.Add(new SolutionConfigurationInSolution(configurationPlatformParts[0], configurationPlatformParts[1]));
+                _solutionConfigurations.Add(new SolutionConfigurationInSolution(configuration, platform));
             } while (true);
         }
 
+        internal static (string Configuration, string Platform) ParseConfigurationName(string fullConfigurationName, string projectPath, int lineNumber, string containingString)
+        {
+            string[] configurationPlatformParts = fullConfigurationName.Split(SolutionConfigurationInSolution.ConfigurationPlatformSeparatorArray);
+
+            ProjectFileErrorUtilities.VerifyThrowInvalidProjectFile(
+                configurationPlatformParts.Length == 2,
+                "SubCategoryForSolutionParsingErrors",
+                new BuildEventFileInfo(projectPath, lineNumber, 0),
+                "SolutionParseInvalidSolutionConfigurationEntry",
+                containingString);
+
+            return (configurationPlatformParts[0], configurationPlatformParts[1]);
+        }
+
         /// <summary>
         /// Read project configurations in solution configurations section.
         /// </summary>

src/Build/Construction/Solution/SolutionProjectGenerator.cs

@@ -49,6 +49,11 @@ internal class SolutionProjectGenerator
         private const string WebProjectOverrideFolder = "_PublishedWebsites";
 #endif // FEATURE_ASPNET_COMPILER
 
+        /// <summary>
+        /// Property set by VS when building projects. It's an XML containing the project configurations for ALL projects in the solution for the currently selected solution configuration.
+        /// </summary>
+        internal const string CurrentSolutionConfigurationContents = nameof(CurrentSolutionConfigurationContents);
+
         /// <summary>
         /// The set of properties all projects in the solution should be built with
         /// </summary>
@@ -230,6 +235,7 @@ internal static void AddPropertyGroupForSolutionConfiguration(ProjectRootElement
             };
             using (XmlWriter xw = XmlWriter.Create(solutionConfigurationContents, settings))
             {
+                // TODO: fix code clone for parsing CurrentSolutionConfiguration xml: https://github.com/dotnet/msbuild/issues/6751
                 xw.WriteStartElement("SolutionConfiguration");
 
                 // add a project configuration entry for each project in the solution

src/Build/Graph/ProjectInterpretation.cs

@@ -100,17 +100,17 @@ public IEnumerable<ReferenceInfo> GetReferences(ProjectInstance requesterInstanc
             }
         }
 
-        private static string GetInnerBuildPropertyValue(ProjectInstance project)
+        internal static string GetInnerBuildPropertyValue(ProjectInstance project)
         {
             return project.GetPropertyValue(GetInnerBuildPropertyName(project));
         }
 
-        private static string GetInnerBuildPropertyName(ProjectInstance project)
+        internal static string GetInnerBuildPropertyName(ProjectInstance project)
         {
             return project.GetPropertyValue(PropertyNames.InnerBuildProperty);
         }
 
-        private static string GetInnerBuildPropertyValues(ProjectInstance project)
+        internal static string GetInnerBuildPropertyValues(ProjectInstance project)
         {
             return project.GetPropertyValue(project.GetPropertyValue(PropertyNames.InnerBuildPropertyValues));
         }

src/Samples/ProjectCachePlugin/AssemblyMockCache.cs

@@ -2,12 +2,13 @@
 // Licensed under the MIT license. See LICENSE file in the project root for full license information.
 
 using System;
-using System.Diagnostics;
+using System.Linq;
 using System.Threading;
 using System.Threading.Tasks;
 using Microsoft.Build.Execution;
 using Microsoft.Build.Experimental.ProjectCache;
 using Microsoft.Build.Framework;
+using Microsoft.Build.Graph;
 using Shouldly;
 
 namespace MockCacheFromAssembly
@@ -23,6 +24,15 @@ public override Task BeginBuildAsync(CacheContext context, PluginLoggerBase logg
         {
             logger.LogMessage($"{nameof(AssemblyMockCache)}: BeginBuildAsync", MessageImportance.High);
 
+            foreach (var ep in context.GraphEntryPoints ?? Enumerable.Empty<ProjectGraphEntryPoint>())
+            {
+                var globalPropertyString = ep.GlobalProperties is not null
+                    ? string.Join("\n\t", ep.GlobalProperties.Select(gp => $"{gp.Key}:{gp.Value}"))
+                    : string.Empty;
+
+                logger.LogMessage($"EntryPoint: {ep.ProjectFile} \n(\n\t{globalPropertyString}\n)");
+            }
+
             ErrorFrom(nameof(BeginBuildAsync), logger);
 
             return Task.CompletedTask;

src/Shared/UnitTests/ObjectModelHelpers.cs

@@ -155,6 +155,40 @@ internal static void AssertItemEvaluationFromGenericItemEvaluator(Func<string, P
             }
         }
 
+        internal static void ShouldHaveSucceeded(this BuildResult result)
+        {
+            result.OverallResult.ShouldBe(
+                BuildResultCode.Success,
+                customMessage: result.Exception is not null ? result.Exception.ToString() : string.Empty);
+        }
+
+        internal static void ShouldHaveSucceeded(this GraphBuildResult result)
+        {
+            result.OverallResult.ShouldBe(
+                BuildResultCode.Success,
+                customMessage: result.Exception is not null ? result.Exception.ToString() : string.Empty);
+        }
+
+        internal static void ShouldHaveFailed(this BuildResult result, string exceptionMessageSubstring = null)
+        {
+            result.OverallResult.ShouldBe(BuildResultCode.Failure);
+
+            if (exceptionMessageSubstring != null)
+            {
+                result.Exception.Message.ShouldContain(exceptionMessageSubstring);
+            }
+        }
+
+        internal static void ShouldHaveFailed(this GraphBuildResult result, string exceptionMessageSubstring = null)
+        {
+            result.OverallResult.ShouldBe(BuildResultCode.Failure);
+
+            if (exceptionMessageSubstring != null)
+            {
+                result.Exception.Message.ShouldContain(exceptionMessageSubstring);
+            }
+        }
+
         internal static string NormalizeSlashes(string path)
         {
             return path.Replace('/', Path.DirectorySeparatorChar).Replace('\\', Path.DirectorySeparatorChar);

src/Tasks/ResolveProjectBase.cs

@@ -121,6 +121,7 @@ internal bool VerifyProjectReferenceItems(ITaskItem[] references, bool treatAsEr
         /// </summary>
         internal void CacheProjectElementsFromXml(string xmlString)
         {
+            // TODO: fix code clone for parsing CurrentSolutionConfiguration xml: https://github.com/dotnet/msbuild/issues/6751
             XmlDocument doc = null;
 
             if (!string.IsNullOrEmpty(xmlString))