From 4e00b3286a656083652e8ae78e4d08cd27266bf5 Mon Sep 17 00:00:00 2001 From: David Federman Date: Thu, 2 Nov 2023 09:06:04 -0700 Subject: [PATCH 1/3] Graph: Mimic quirk for sln-based builds with target name containing semicolon --- .../Graph/ProjectGraph_Tests.cs | 69 ++++++++++++++++++- src/Build/Graph/GraphBuilder.cs | 7 +- src/Build/Graph/ProjectGraph.cs | 40 ++++++++--- 3 files changed, 104 insertions(+), 12 deletions(-) diff --git a/src/Build.UnitTests/Graph/ProjectGraph_Tests.cs b/src/Build.UnitTests/Graph/ProjectGraph_Tests.cs index 932c3c8e439..f929d2b9d34 100644 --- a/src/Build.UnitTests/Graph/ProjectGraph_Tests.cs +++ b/src/Build.UnitTests/Graph/ProjectGraph_Tests.cs @@ -760,7 +760,6 @@ public void ConstructGraphWithSolution() EndGlobal """; TransientTestFile slnFile = env.CreateFile(@"Solution.sln", SolutionFileContents); - SolutionFile solutionFile = SolutionFile.Parse(slnFile.Path); ProjectRootElement project1Xml = ProjectRootElement.Create(); @@ -829,6 +828,74 @@ public void ConstructGraphWithSolution() } } + [Fact] + public void GetTargetListsWithSemicolonInTarget() + { + using (var env = TestEnvironment.Create()) + { + TransientTestFile entryProject = CreateProjectFile(env, 1); + + var projectGraph = new ProjectGraph(entryProject.Path); + projectGraph.ProjectNodes.Count.ShouldBe(1); + projectGraph.ProjectNodes.First().ProjectInstance.FullPath.ShouldBe(entryProject.Path); + + // Example: msbuild /graph /t:"Clean;Build". For projects, this does not expand + IReadOnlyDictionary> targetLists = projectGraph.GetTargetLists(new[] { "Clean;Build" }); + targetLists.Count.ShouldBe(projectGraph.ProjectNodes.Count); + targetLists[GetFirstNodeWithProjectNumber(projectGraph, 1)].ShouldBe(new[] { "Clean;Build" }); + } + } + + [Fact] + public void GetTargetListsWithSemicolonInTargetSolution() + { + using (var env = TestEnvironment.Create()) + { + TransientTestFile project = CreateProjectFile(env, 1); + + string solutionFileContents = $$""" + Microsoft Visual Studio Solution File, Format Version 12.00 + # Visual Studio Version 17 + VisualStudioVersion = 17.0.31903.59 + MinimumVisualStudioVersion = 17.0.31903.59 + Project("{FAE04EC0-301F-11D3-BF4B-00C04F79EFBC}") = "Project1", "{{project.Path}}", "{8761499A-7280-43C4-A32F-7F41C47CA6DF}" + EndProject + Global + GlobalSection(SolutionConfigurationPlatforms) = preSolution + Debug|x64 = Debug|x64 + Debug|x86 = Debug|x86 + Release|x64 = Release|x64 + Release|x86 = Release|x86 + EndGlobalSection + GlobalSection(ProjectConfigurationPlatforms) = postSolution + {8761499A-7280-43C4-A32F-7F41C47CA6DF}.Debug|x64.ActiveCfg = Debug|x64 + {8761499A-7280-43C4-A32F-7F41C47CA6DF}.Debug|x64.Build.0 = Debug|x64 + {8761499A-7280-43C4-A32F-7F41C47CA6DF}.Debug|x86.ActiveCfg = Debug|x86 + {8761499A-7280-43C4-A32F-7F41C47CA6DF}.Debug|x86.Build.0 = Debug|x86 + {8761499A-7280-43C4-A32F-7F41C47CA6DF}.Release|x64.ActiveCfg = Release|x64 + {8761499A-7280-43C4-A32F-7F41C47CA6DF}.Release|x64.Build.0 = Release|x64 + {8761499A-7280-43C4-A32F-7F41C47CA6DF}.Release|x86.ActiveCfg = Release|x86 + {8761499A-7280-43C4-A32F-7F41C47CA6DF}.Release|x86.Build.0 = Release|x86 + EndGlobalSection + GlobalSection(SolutionProperties) = preSolution + HideSolutionNode = FALSE + EndGlobalSection + EndGlobal + """; + TransientTestFile slnFile = env.CreateFile(@"Solution.sln", solutionFileContents); + SolutionFile solutionFile = SolutionFile.Parse(slnFile.Path); + + var projectGraph = new ProjectGraph(slnFile.Path); + projectGraph.ProjectNodes.Count.ShouldBe(1); + projectGraph.ProjectNodes.First().ProjectInstance.FullPath.ShouldBe(project.Path); + + // Example: msbuild /graph /t:"Clean;Build". For solutions, this does expand! + IReadOnlyDictionary> targetLists = projectGraph.GetTargetLists(new[] { "Clean;Build" }); + targetLists.Count.ShouldBe(projectGraph.ProjectNodes.Count); + targetLists[GetFirstNodeWithProjectNumber(projectGraph, 1)].ShouldBe(new[] { "Clean", "Build" }); + } + } + [Fact] public void GetTargetListsAggregatesFromMultipleEdges() { diff --git a/src/Build/Graph/GraphBuilder.cs b/src/Build/Graph/GraphBuilder.cs index 58f9af58bdf..929855b1200 100644 --- a/src/Build/Graph/GraphBuilder.cs +++ b/src/Build/Graph/GraphBuilder.cs @@ -21,7 +21,7 @@ namespace Microsoft.Build.Graph { - internal class GraphBuilder + internal sealed class GraphBuilder { internal const string SolutionItemReference = "_SolutionReference"; @@ -38,6 +38,8 @@ internal class GraphBuilder public GraphEdges Edges { get; private set; } + public bool IsSolution { get; private set; } + private readonly List _entryPointConfigurationMetadata; private readonly ParallelWorkSet _graphWorkSet; @@ -257,8 +259,7 @@ private static void AddEdgesFromSolution(IReadOnlyDictionary e.ProjectFile)))); } - ErrorUtilities.VerifyThrowArgument(entryPoints.Count == 1, "StaticGraphAcceptsSingleSolutionEntryPoint"); - + IsSolution = true; ProjectGraphEntryPoint solutionEntryPoint = entryPoints.Single(); ImmutableDictionary.Builder solutionGlobalPropertiesBuilder = ImmutableDictionary.CreateBuilder( keyComparer: StringComparer.OrdinalIgnoreCase, diff --git a/src/Build/Graph/ProjectGraph.cs b/src/Build/Graph/ProjectGraph.cs index 39993e3a4fc..3d7a466a844 100644 --- a/src/Build/Graph/ProjectGraph.cs +++ b/src/Build/Graph/ProjectGraph.cs @@ -8,6 +8,7 @@ using System.Diagnostics; using System.IO; using System.Linq; +using System.Linq.Expressions; using System.Text; using System.Threading; using Microsoft.Build.Evaluation; @@ -56,9 +57,11 @@ public delegate ProjectInstance ProjectInstanceFactoryFunc( private readonly Lazy> _projectNodesTopologicallySorted; - private GraphBuilder.GraphEdges Edges { get; } + private readonly bool _isSolution; - internal GraphBuilder.GraphEdges TestOnly_Edges => Edges; + private readonly GraphBuilder.GraphEdges _edges; + + internal GraphBuilder.GraphEdges TestOnly_Edges => _edges; public GraphConstructionMetrics ConstructionMetrics { get; private set; } @@ -432,7 +435,8 @@ public ProjectGraph( EntryPointNodes = graphBuilder.EntryPointNodes; GraphRoots = graphBuilder.RootNodes; ProjectNodes = graphBuilder.ProjectNodes; - Edges = graphBuilder.Edges; + _edges = graphBuilder.Edges; + _isSolution = graphBuilder.IsSolution; _projectNodesTopologicallySorted = new Lazy>(() => TopologicalSort(GraphRoots, ProjectNodes)); @@ -472,7 +476,7 @@ GraphConstructionMetrics EndMeasurement() return new GraphConstructionMetrics( measurementInfo.Timer.Elapsed, ProjectNodes.Count, - Edges.Count); + _edges.Count); } } @@ -598,6 +602,26 @@ public IReadOnlyDictionary> GetTargetLis { ThrowOnEmptyTargetNames(entryProjectTargets); + List entryTargets = entryProjectTargets == null ? null : new(entryProjectTargets); + + // Solutions have quirky behavior when provided a target with ';' in it, eg "Clean;Build". This can happen if via the command-line the user provides something + // like /t:"Clean;Build". When building a project, the target named "Clean;Build" is executed (which usually doesn't exist, but could). However, for solutions + // the generated metaproject ends up calling the MSBuild task with the provided targets, which ends up splitting the value as if it were [ "Clean", "Build" ]. + // Mimic this flattening behavior for consistency. + if (_isSolution && entryProjectTargets != null && entryProjectTargets.Count != 0) + { + List newEntryTargets = new(entryTargets.Count); + foreach (string entryTarget in entryTargets) + { + foreach (string s in ExpressionShredder.SplitSemiColonSeparatedList(entryTarget)) + { + newEntryTargets.Add(s); + } + } + + entryTargets = newEntryTargets; + } + // Seed the dictionary with empty lists for every node. In this particular case though an empty list means "build nothing" rather than "default targets". var targetLists = ProjectNodes.ToDictionary(node => node, node => ImmutableList.Empty); @@ -606,10 +630,10 @@ public IReadOnlyDictionary> GetTargetLis foreach (ProjectGraphNode entryPointNode in EntryPointNodes) { - var entryTargets = entryProjectTargets == null || entryProjectTargets.Count == 0 + ImmutableList nodeEntryTargets = entryTargets == null || entryTargets.Count == 0 ? ImmutableList.CreateRange(entryPointNode.ProjectInstance.DefaultTargets) - : ImmutableList.CreateRange(entryProjectTargets); - var entryEdge = new ProjectGraphBuildRequest(entryPointNode, entryTargets); + : ImmutableList.CreateRange(entryTargets); + var entryEdge = new ProjectGraphBuildRequest(entryPointNode, nodeEntryTargets); encounteredEdges.Add(entryEdge); edgesToVisit.Enqueue(entryEdge); } @@ -645,7 +669,7 @@ public IReadOnlyDictionary> GetTargetLis var expandedTargets = ExpandDefaultTargets( applicableTargets, referenceNode.ProjectInstance.DefaultTargets, - Edges[(node, referenceNode)]); + _edges[(node, referenceNode)]); var projectReferenceEdge = new ProjectGraphBuildRequest( referenceNode, From dd0c00fe6ab77cc57a2ef790fa2c75511a606116 Mon Sep 17 00:00:00 2001 From: David Federman Date: Thu, 2 Nov 2023 09:11:26 -0700 Subject: [PATCH 2/3] Removed unused using --- src/Build/Graph/ProjectGraph.cs | 1 - 1 file changed, 1 deletion(-) diff --git a/src/Build/Graph/ProjectGraph.cs b/src/Build/Graph/ProjectGraph.cs index 3d7a466a844..c74b99006ae 100644 --- a/src/Build/Graph/ProjectGraph.cs +++ b/src/Build/Graph/ProjectGraph.cs @@ -8,7 +8,6 @@ using System.Diagnostics; using System.IO; using System.Linq; -using System.Linq.Expressions; using System.Text; using System.Threading; using Microsoft.Build.Evaluation; From df7dc891a2581a561c1d1226c063d88b14c9976f Mon Sep 17 00:00:00 2001 From: David Federman Date: Thu, 2 Nov 2023 09:14:34 -0700 Subject: [PATCH 3/3] Avoid unecessary allocation --- src/Build/Graph/ProjectGraph.cs | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/src/Build/Graph/ProjectGraph.cs b/src/Build/Graph/ProjectGraph.cs index c74b99006ae..5aa325ceae9 100644 --- a/src/Build/Graph/ProjectGraph.cs +++ b/src/Build/Graph/ProjectGraph.cs @@ -601,16 +601,14 @@ public IReadOnlyDictionary> GetTargetLis { ThrowOnEmptyTargetNames(entryProjectTargets); - List entryTargets = entryProjectTargets == null ? null : new(entryProjectTargets); - // Solutions have quirky behavior when provided a target with ';' in it, eg "Clean;Build". This can happen if via the command-line the user provides something // like /t:"Clean;Build". When building a project, the target named "Clean;Build" is executed (which usually doesn't exist, but could). However, for solutions // the generated metaproject ends up calling the MSBuild task with the provided targets, which ends up splitting the value as if it were [ "Clean", "Build" ]. // Mimic this flattening behavior for consistency. if (_isSolution && entryProjectTargets != null && entryProjectTargets.Count != 0) { - List newEntryTargets = new(entryTargets.Count); - foreach (string entryTarget in entryTargets) + List newEntryTargets = new(entryProjectTargets.Count); + foreach (string entryTarget in entryProjectTargets) { foreach (string s in ExpressionShredder.SplitSemiColonSeparatedList(entryTarget)) { @@ -618,7 +616,7 @@ public IReadOnlyDictionary> GetTargetLis } } - entryTargets = newEntryTargets; + entryProjectTargets = newEntryTargets; } // Seed the dictionary with empty lists for every node. In this particular case though an empty list means "build nothing" rather than "default targets". @@ -629,9 +627,9 @@ public IReadOnlyDictionary> GetTargetLis foreach (ProjectGraphNode entryPointNode in EntryPointNodes) { - ImmutableList nodeEntryTargets = entryTargets == null || entryTargets.Count == 0 + ImmutableList nodeEntryTargets = entryProjectTargets == null || entryProjectTargets.Count == 0 ? ImmutableList.CreateRange(entryPointNode.ProjectInstance.DefaultTargets) - : ImmutableList.CreateRange(entryTargets); + : ImmutableList.CreateRange(entryProjectTargets); var entryEdge = new ProjectGraphBuildRequest(entryPointNode, nodeEntryTargets); encounteredEdges.Add(entryEdge); edgesToVisit.Enqueue(entryEdge);