Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Graph: Mimic quirk for sln-based builds with target name containing semicolon #9390

Closed
wants to merge 3 commits into from
Closed

Graph: Mimic quirk for sln-based builds with target name containing semicolon #9390

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4e00b32
Graph: Mimic quirk for sln-based builds with target name containing s…
dfederm Nov 2, 2023
dd0c00f
Removed unused using
dfederm Nov 2, 2023
df7dc89
Avoid unecessary allocation
dfederm Nov 2, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
69 changes: 68 additions & 1 deletion src/Build.UnitTests/Graph/ProjectGraph_Tests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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<ProjectGraphNode, ImmutableList<string>> 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<ProjectGraphNode, ImmutableList<string>> targetLists = projectGraph.GetTargetLists(new[] { "Clean;Build" });
targetLists.Count.ShouldBe(projectGraph.ProjectNodes.Count);
targetLists[GetFirstNodeWithProjectNumber(projectGraph, 1)].ShouldBe(new[] { "Clean", "Build" });
}
}

[Fact]
public void GetTargetListsAggregatesFromMultipleEdges()
{
Expand Down
7 changes: 4 additions & 3 deletions src/Build/Graph/GraphBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@

namespace Microsoft.Build.Graph
{
internal class GraphBuilder
internal sealed class GraphBuilder
{
internal const string SolutionItemReference = "_SolutionReference";

Expand All @@ -38,6 +38,8 @@ internal class GraphBuilder

public GraphEdges Edges { get; private set; }

public bool IsSolution { get; private set; }

private readonly List<ConfigurationMetadata> _entryPointConfigurationMetadata;

private readonly ParallelWorkSet<ConfigurationMetadata, ParsedProject> _graphWorkSet;
Expand Down Expand Up @@ -257,8 +259,7 @@ private static void AddEdgesFromSolution(IReadOnlyDictionary<ConfigurationMetada
string.Join(";", entryPoints.Select(e => e.ProjectFile))));
}

ErrorUtilities.VerifyThrowArgument(entryPoints.Count == 1, "StaticGraphAcceptsSingleSolutionEntryPoint");
dfederm marked this conversation as resolved.
Show resolved Hide resolved

IsSolution = true;
ProjectGraphEntryPoint solutionEntryPoint = entryPoints.Single();
ImmutableDictionary<string, string>.Builder solutionGlobalPropertiesBuilder = ImmutableDictionary.CreateBuilder(
keyComparer: StringComparer.OrdinalIgnoreCase,
Expand Down
35 changes: 28 additions & 7 deletions src/Build/Graph/ProjectGraph.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,11 @@ public delegate ProjectInstance ProjectInstanceFactoryFunc(

private readonly Lazy<IReadOnlyCollection<ProjectGraphNode>> _projectNodesTopologicallySorted;

private GraphBuilder.GraphEdges Edges { get; }
private readonly bool _isSolution;

internal GraphBuilder.GraphEdges TestOnly_Edges => Edges;
private readonly GraphBuilder.GraphEdges _edges;
dfederm marked this conversation as resolved.
Show resolved Hide resolved

internal GraphBuilder.GraphEdges TestOnly_Edges => _edges;

public GraphConstructionMetrics ConstructionMetrics { get; private set; }

Expand Down Expand Up @@ -432,7 +434,8 @@ public ProjectGraph(
EntryPointNodes = graphBuilder.EntryPointNodes;
GraphRoots = graphBuilder.RootNodes;
ProjectNodes = graphBuilder.ProjectNodes;
Edges = graphBuilder.Edges;
_edges = graphBuilder.Edges;
_isSolution = graphBuilder.IsSolution;

_projectNodesTopologicallySorted = new Lazy<IReadOnlyCollection<ProjectGraphNode>>(() => TopologicalSort(GraphRoots, ProjectNodes));

Expand Down Expand Up @@ -472,7 +475,7 @@ GraphConstructionMetrics EndMeasurement()
return new GraphConstructionMetrics(
measurementInfo.Timer.Elapsed,
ProjectNodes.Count,
Edges.Count);
_edges.Count);
}
}

Expand Down Expand Up @@ -598,6 +601,24 @@ public IReadOnlyDictionary<ProjectGraphNode, ImmutableList<string>> GetTargetLis
{
ThrowOnEmptyTargetNames(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<string> newEntryTargets = new(entryProjectTargets.Count);
foreach (string entryTarget in entryProjectTargets)
{
foreach (string s in ExpressionShredder.SplitSemiColonSeparatedList(entryTarget))
{
newEntryTargets.Add(s);
}
}

entryProjectTargets = newEntryTargets;
}
Comment on lines +609 to +620
Copy link
Member

@JanKrivanek JanKrivanek Nov 15, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why recreating the list twice? (SplitSemiColonSeparatedList already creates new)

Suggested change
{
List<string> newEntryTargets = new(entryProjectTargets.Count);
foreach (string entryTarget in entryProjectTargets)
{
foreach (string s in ExpressionShredder.SplitSemiColonSeparatedList(entryTarget))
{
newEntryTargets.Add(s);
}
}
entryProjectTargets = newEntryTargets;
}
{
entryProjectTargets = entryProjectTargets
.SelectMany(target => ExpressionShredder.SplitSemiColonSeparatedList(target))
.ToList();
}


// 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<string>.Empty);

Expand All @@ -606,10 +627,10 @@ public IReadOnlyDictionary<ProjectGraphNode, ImmutableList<string>> GetTargetLis

foreach (ProjectGraphNode entryPointNode in EntryPointNodes)
{
var entryTargets = entryProjectTargets == null || entryProjectTargets.Count == 0
ImmutableList<string> nodeEntryTargets = entryProjectTargets == null || entryProjectTargets.Count == 0
? ImmutableList.CreateRange(entryPointNode.ProjectInstance.DefaultTargets)
: ImmutableList.CreateRange(entryProjectTargets);
var entryEdge = new ProjectGraphBuildRequest(entryPointNode, entryTargets);
var entryEdge = new ProjectGraphBuildRequest(entryPointNode, nodeEntryTargets);
encounteredEdges.Add(entryEdge);
edgesToVisit.Enqueue(entryEdge);
}
Expand Down Expand Up @@ -645,7 +666,7 @@ public IReadOnlyDictionary<ProjectGraphNode, ImmutableList<string>> GetTargetLis
var expandedTargets = ExpandDefaultTargets(
applicableTargets,
referenceNode.ProjectInstance.DefaultTargets,
Edges[(node, referenceNode)]);
_edges[(node, referenceNode)]);

var projectReferenceEdge = new ProjectGraphBuildRequest(
referenceNode,
Expand Down