Embed solution node into a graph#14407
Conversation
07153ac to
85c9f04
Compare
There was a problem hiding this comment.
Pull request overview
This PR adjusts solution graph builds so they model a .sln as a single synthetic entry/root node, enabling solution-level execution (including before/after.<sln>.targets hooks) while still building solution projects through the static graph scheduler, and ensuring the solution file is captured in graph-mode binlog import collection.
Changes:
- Add a synthetic solution node during graph construction and make it the sole entry/root node for solution graph builds.
- Update graph target inference and BuildManager graph scheduling to recognize/execute the synthetic solution node while keeping per-project node results behavior.
- Extend binary logger import collection and add/adjust unit tests for graph + binlog solution behavior.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Build/Logging/BinaryLogger/BinaryLogger.cs | Adds solution-path import collection during ProjectStarted to embed the .sln in ProjectImports. |
| src/Build/Graph/ProjectGraph.cs | Adjusts target inference to account for entry points that are not buildable project nodes (synthetic solution node). |
| src/Build/Graph/GraphBuilder.cs | Constructs a synthetic solution ProjectGraphNode and wires it to depend on all solution projects. |
| src/Build/Construction/Solution/SolutionProjectGenerator.cs | Conditions solution traversal MSBuild tasks to avoid redispatching projects during graph builds. |
| src/Build/BackEnd/BuildManager/BuildManager.cs | Executes the synthetic solution node in graph scheduling and treats its failure as submission failure while excluding it from per-node results. |
| src/Build.UnitTests/Graph/ProjectGraph_Tests.cs | Updates expectations for solution graphs now having a single .sln entry/root node. |
| src/Build.UnitTests/Graph/GraphLoadedFromSolution_tests.cs | Updates assertions to reflect the synthetic .sln entry/root node model. |
| src/Build.UnitTests/BinaryLogger_Tests.cs | Adds a test asserting graph solution builds embed the solution file into ProjectImports zip. |
| src/Build.UnitTests/BackEnd/BuildManager_Tests.cs | Adds a test covering synthetic solution node execution and solution hook execution in graph mode. |
| if (projectArgs.GlobalProperties?.TryGetValue("SolutionPath", out string solutionPath) == true && | ||
| !string.IsNullOrEmpty(solutionPath)) | ||
| { | ||
| projectImportsCollector.AddFile(EscapingUtilities.UnescapeAll(solutionPath)); | ||
| } |
| string projectMatchCondition = Strings.WeakIntern($"'%(ProjectReference.Identity)' == '{GetMetaprojectName(project)}'"); | ||
| string combinedCondition = $"({projectMatchCondition}) and {NotGraphBuildCondition}"; | ||
| ProjectTaskInstance task = target.AddTask("MSBuild", combinedCondition, String.Empty); |
| if (Solution != null && !projectNodeSet.Contains(entryPointNode)) | ||
| { | ||
| return ["Build"]; | ||
| } |
| if (syntheticSolutionNodeResult?.OverallResult == BuildResultCode.Failure) | ||
| { | ||
| graphBuildResult.Exception = syntheticSolutionNodeResult.Exception | ||
| ?? new InvalidOperationException("Synthetic solution graph node execution failed."); | ||
| } |
| _logger.AssertLogContains("AfterSolutionHookRan"); | ||
| } |
There was a problem hiding this comment.
I think this would catch my big problem . . .
There was a problem hiding this comment.
Repro: build a tiny solution with graph mode + binlog and verify solution presence/order in
with-graph.binlog
& <local-msbuild> .\tiny-graph.sln -graph -bl:with-graph.binlog
Try this again, please, I think it shows a problem (that the projects aren't actually building):
❯ dotnet msbuild -graph -bl
info NETSDK1057: You are using a preview version of .NET. See: https://aka.ms/dotnet-support-policy
Console1 net11.0 succeeded (1.3s) → Console1\bin\Debug\net11.0\Console1.dll
Lib1 net11.0 succeeded (0.2s) → Lib1\bin\Debug\net11.0\Lib1.dll
Build succeeded in 3.2s
❯ S:\msbuild\artifacts\bin\bootstrap\core\dotnet.exe msbuild -graph -bl
Build succeeded in 0.0s| if (projectArgs.GlobalProperties?.TryGetValue("SolutionPath", out string solutionPath) == true && | ||
| !string.IsNullOrEmpty(solutionPath)) | ||
| { | ||
| projectImportsCollector.AddFile(EscapingUtilities.UnescapeAll(solutionPath)); |
There was a problem hiding this comment.
Hm--this works, but I don't like it. Can you trace why the sln is getting into the non-graph binlog (my guess is as an actual ProjectStartedEventArgs path) and replicate that for the graph path?
| private static void AddReferencesBuildTask(ProjectTargetInstance target, string targetToBuild, string outputItem) | ||
| { | ||
| ProjectTaskInstance task = target.AddTask("MSBuild", String.Empty, String.Empty); | ||
| ProjectTaskInstance task = target.AddTask("MSBuild", NotGraphBuildCondition, String.Empty); |
There was a problem hiding this comment.
Question I'd like you to look at deeply for onboarding: should this be a condition on the target, or on the tasks within? The relevant reference doc is https://learn.microsoft.com/visualstudio/msbuild/target-build-order.
| private readonly ProjectGraph.ProjectInstanceFactoryFunc _projectInstanceFactory; | ||
| private readonly ProjectGraphMode _graphMode; | ||
| private IReadOnlyDictionary<string, IReadOnlyCollection<string>> _solutionDependencies; | ||
| private ImmutableDictionary<string, string> _solutionGlobalProperties; |
There was a problem hiding this comment.
I don't think I understand why these need to be preserved at the graph level, can you expand on that (or avoid it?)?
| syntheticSolutionInstance = new ProjectInstance( | ||
| syntheticRootElement, | ||
| solutionGlobalProperties, | ||
| toolsVersion: null, | ||
| _projectCollection); |
There was a problem hiding this comment.
Can you get the instance out of the SolutionProjectGenerator instead of reconstructing one here?
| _logger.AssertLogContains("AfterSolutionHookRan"); | ||
| } |
There was a problem hiding this comment.
I think this would catch my big problem . . .
Fixes #8170 #10657
Context
Graph builds for solutions did not consistently model/execute a solution node as the single entrypoint, and graph-mode binlog behavior around the solution-level execution path needed to be aligned.
Changes Made
IsGraphBuildtask conditions so hooks/imports run without rebuilding already-built projects.Testing
.\build.cmdwith-graph.binlog& <local-msbuild> .\tiny-graph.sln -graph -bl:with-graph.binlogNotes