Skip to content

Commit 5c934f8

Browse files
authored
Add Build Submission Started event (#10424)
Fixes #10145 Context It is currently not possible to check for global variables during build execution. To have access to that data it needs to be passed on events to the processing nodes. None of the events we had previously were good matches to pass along this information, so it was decided to create one for build submission started, as this point in the build we have all properties loaded with the correct value, but its not too late to make use of them. Changes Made Added a new BuildSubmissionStartedEventArgs based on BuildStatusEventArgs and added it to the event handlers. A copy of the enum BuildRequestDataFlags was added to Microsoft.Build.Framework.
1 parent fc175f6 commit 5c934f8

17 files changed

+439
-87
lines changed

src/Build.UnitTests/BackEnd/BuildManager_Tests.cs

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4280,13 +4280,16 @@ public void GraphBuildShouldBeAbleToConstructGraphButSkipBuild()
42804280

42814281
using (var buildSession = new Helpers.BuildManagerSession(_env))
42824282
{
4283-
var graphResult = buildSession.BuildGraphSubmission(
4284-
new GraphBuildRequestData(
4285-
projectGraphEntryPoints: new[] { new ProjectGraphEntryPoint(graph.GraphRoots.First().ProjectInstance.FullPath) },
4283+
var requestData = new GraphBuildRequestData(
4284+
projectGraphEntryPoints: new[] { new ProjectGraphEntryPoint(
4285+
graph.GraphRoots.First().ProjectInstance.FullPath,
4286+
new Dictionary<string, string>() { {"property1", "value1" } }) },
42864287
targetsToBuild: Array.Empty<string>(),
42874288
hostServices: null,
42884289
flags: BuildRequestDataFlags.None,
4289-
graphBuildOptions: new GraphBuildOptions { Build = false }));
4290+
graphBuildOptions: new GraphBuildOptions { Build = false });
4291+
4292+
var graphResult = buildSession.BuildGraphSubmission(requestData);
42904293

42914294
graphResult.OverallResult.ShouldBe(BuildResultCode.Success);
42924295
logger = buildSession.Logger;

src/Build.UnitTests/BackEnd/NodePackets_Tests.cs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
using System.Linq;
88
using FluentAssertions;
99
using Microsoft.Build.BackEnd;
10+
using Microsoft.Build.Execution;
1011
using Microsoft.Build.Framework;
1112
using Microsoft.Build.Shared;
1213
using Xunit;
@@ -75,6 +76,7 @@ public void VerifyEventType()
7576
UninitializedPropertyReadEventArgs uninitializedPropertyRead = new("prop", "message", "help", "sender", MessageImportance.Normal);
7677
EnvironmentVariableReadEventArgs environmentVariableRead = new("env", "message", "file", 0, 0);
7778
GeneratedFileUsedEventArgs generatedFileUsed = new GeneratedFileUsedEventArgs("path", "some content");
79+
BuildSubmissionStartedEventArgs buildSubmissionStarted = new(new Dictionary<string, string> { { "Value1", "Value2" } }, ["Path1"], ["TargetName"], BuildRequestDataFlags.ReplaceExistingProjectInstance, 123);
7880

7981
VerifyLoggingPacket(buildFinished, LoggingEventType.BuildFinishedEvent);
8082
VerifyLoggingPacket(buildStarted, LoggingEventType.BuildStartedEvent);
@@ -108,6 +110,7 @@ public void VerifyEventType()
108110
VerifyLoggingPacket(uninitializedPropertyRead, LoggingEventType.UninitializedPropertyRead);
109111
VerifyLoggingPacket(environmentVariableRead, LoggingEventType.EnvironmentVariableReadEvent);
110112
VerifyLoggingPacket(generatedFileUsed, LoggingEventType.GeneratedFileUsedEvent);
113+
VerifyLoggingPacket(buildSubmissionStarted, LoggingEventType.BuildSubmissionStartedEvent);
111114
}
112115

113116
private static BuildEventContext CreateBuildEventContext()

src/Build.UnitTests/BuildEventArgsSerialization_Tests.cs

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,45 @@ public void RoundtripBuildFinishedEventArgs()
9595
e => e.Succeeded.ToString());
9696
}
9797

98+
[Fact]
99+
public void RoundtripBuildSubmissionStartedEventArgs()
100+
{
101+
var globalVariables = new Dictionary<string, string>
102+
{
103+
{"Variable1", "Value1" },
104+
{"Variable2", "" },
105+
{"Variable3", null },
106+
};
107+
var entryPointProjects = new List<string>()
108+
{
109+
"project1",
110+
"project2",
111+
"",
112+
};
113+
var targetNames = new List<string>()
114+
{
115+
"target1",
116+
"target2",
117+
"",
118+
};
119+
var flag = Execution.BuildRequestDataFlags.FailOnUnresolvedSdk;
120+
var submissionId = 1234;
121+
122+
BuildSubmissionStartedEventArgs args = new(
123+
globalVariables,
124+
entryPointProjects,
125+
targetNames,
126+
flag,
127+
submissionId);
128+
129+
Roundtrip<BuildSubmissionStartedEventArgs>(args,
130+
e => TranslationHelpers.GetPropertiesString(e.GlobalProperties),
131+
e => TranslationHelpers.GetPropertiesString(e.EntryProjectsFullPath),
132+
e => TranslationHelpers.GetPropertiesString(e.TargetNames),
133+
e => e.Flags.ToString(),
134+
e => e.SubmissionId.ToString());
135+
}
136+
98137
[Fact]
99138
public void RoundtripProjectStartedEventArgs()
100139
{

src/Build.UnitTests/Graph/ResultCacheBasedBuilds_Tests.cs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ public void InvalidCacheFilesShouldLogError(byte[] cacheContents)
7070
result.OverallResult.ShouldBe(BuildResultCode.Failure);
7171

7272
_logger.FullLog.ShouldContain("MSB4256:");
73-
_logger.AllBuildEvents.Count.ShouldBe(5);
73+
_logger.AllBuildEvents.Count.ShouldBe(6);
7474
_logger.ErrorCount.ShouldBe(1);
7575
}
7676

@@ -566,7 +566,7 @@ public void NonExistingInputResultsCacheShouldLogError()
566566

567567
result.OverallResult.ShouldBe(BuildResultCode.Failure);
568568

569-
_logger.AllBuildEvents.Count.ShouldBe(5);
569+
_logger.AllBuildEvents.Count.ShouldBe(6);
570570
_logger.Errors.First().Message.ShouldContain("MSB4255:");
571571
_logger.Errors.First().Message.ShouldContain("FileDoesNotExist1");
572572
_logger.Errors.First().Message.ShouldContain("FileDoesNotExist2");

src/Build/AssemblyInfo.cs

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -38,3 +38,5 @@
3838

3939
[assembly: Dependency("BuildXL.Utilities.Core", LoadHint.Sometimes)]
4040
[assembly: Dependency("BuildXL.Processes", LoadHint.Sometimes)]
41+
42+
[assembly: TypeForwardedTo(typeof(Microsoft.Build.Execution.BuildRequestDataFlags))]

src/Build/BackEnd/BuildManager/BuildManager.cs

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1375,10 +1375,24 @@ internal void ExecuteSubmission<TRequestData, TResultData>(
13751375
where TRequestData : BuildRequestDataBase
13761376
where TResultData : BuildResultBase
13771377
{
1378-
// TODO: here we should add BuildRequestStarted https://github.com/dotnet/msbuild/issues/10145
1379-
// BuildEventContext buildEventContext = new BuildEventContext(submission.SubmissionId, 1, BuildEventContext.InvalidProjectInstanceId, BuildEventContext.InvalidProjectContextId, BuildEventContext.InvalidTargetId, BuildEventContext.InvalidTaskId);
1380-
// ((IBuildComponentHost)this).LoggingService.LogBuildEvent()
1381-
1378+
// For the current submission we only know the SubmissionId and that it happened on scheduler node - all other BuildEventContext dimensions are unknown now.
1379+
BuildEventContext buildEventContext = new BuildEventContext(
1380+
submission.SubmissionId,
1381+
nodeId: 1,
1382+
BuildEventContext.InvalidProjectInstanceId,
1383+
BuildEventContext.InvalidProjectContextId,
1384+
BuildEventContext.InvalidTargetId,
1385+
BuildEventContext.InvalidTaskId);
1386+
1387+
BuildSubmissionStartedEventArgs submissionStartedEvent = new(
1388+
submission.BuildRequestDataBase.GlobalPropertiesLookup,
1389+
submission.BuildRequestDataBase.EntryProjectsFullPath,
1390+
submission.BuildRequestDataBase.TargetNames,
1391+
submission.BuildRequestDataBase.Flags,
1392+
submission.SubmissionId);
1393+
submissionStartedEvent.BuildEventContext = buildEventContext;
1394+
1395+
((IBuildComponentHost)this).LoggingService.LogBuildEvent(submissionStartedEvent);
13821396

13831397
if (submission is BuildSubmission buildSubmission)
13841398
{

src/Build/BackEnd/BuildManager/BuildRequestData.cs

Lines changed: 2 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -6,88 +6,15 @@
66
using System.Collections.Generic;
77
using System.Collections.Immutable;
88
using System.Linq;
9+
using System.Runtime.CompilerServices;
910
using Microsoft.Build.Collections;
1011
using Microsoft.Build.Evaluation;
1112
using Microsoft.Build.Experimental.BuildCheck;
13+
using Microsoft.Build.Framework;
1214
using Microsoft.Build.Shared;
1315

1416
namespace Microsoft.Build.Execution
1517
{
16-
/// <summary>
17-
/// Flags providing additional control over the build request
18-
/// </summary>
19-
[Flags]
20-
public enum BuildRequestDataFlags
21-
{
22-
/// <summary>
23-
/// No flags.
24-
/// </summary>
25-
None = 0,
26-
27-
/// <summary>
28-
/// When this flag is present, the existing ProjectInstance in the build will be replaced by this one.
29-
/// </summary>
30-
ReplaceExistingProjectInstance = 1 << 0,
31-
32-
/// <summary>
33-
/// When this flag is present, the <see cref="BuildResult"/> issued in response to this request will
34-
/// include <see cref="BuildResult.ProjectStateAfterBuild"/>.
35-
/// </summary>
36-
ProvideProjectStateAfterBuild = 1 << 1,
37-
38-
/// <summary>
39-
/// When this flag is present and the project has previously been built on a node whose affinity is
40-
/// incompatible with the affinity this request requires, we will ignore the project state (but not
41-
/// target results) that were previously generated.
42-
/// </summary>
43-
/// <remarks>
44-
/// This usually is not desired behavior. It is only provided for those cases where the client
45-
/// knows that the new build request does not depend on project state generated by a previous request. Setting
46-
/// this flag can provide a performance boost in the case of incompatible node affinities, as MSBuild would
47-
/// otherwise have to serialize the project state from one node to another, which may be
48-
/// expensive depending on how much data the project previously generated.
49-
///
50-
/// This flag has no effect on target results, so if a previous request already built a target, the new
51-
/// request will not re-build that target (nor will any of the project state mutations which previously
52-
/// occurred as a consequence of building that target be re-applied.)
53-
/// </remarks>
54-
IgnoreExistingProjectState = 1 << 2,
55-
56-
/// <summary>
57-
/// When this flag is present, caches including the <see cref="ProjectRootElementCacheBase"/> will be cleared
58-
/// after the build request completes. This is used when the build request is known to modify a lot of
59-
/// state such as restoring packages or generating parts of the import graph.
60-
/// </summary>
61-
ClearCachesAfterBuild = 1 << 3,
62-
63-
/// <summary>
64-
/// When this flag is present, the top level target(s) in the build request will be skipped if those targets
65-
/// are not defined in the Project to build. This only applies to this build request (if another target calls
66-
/// the "missing target" at any other point this will still result in an error).
67-
/// </summary>
68-
SkipNonexistentTargets = 1 << 4,
69-
70-
/// <summary>
71-
/// When this flag is present, the <see cref="BuildResult"/> issued in response to this request will
72-
/// include a <see cref="BuildResult.ProjectStateAfterBuild"/> that includes ONLY the
73-
/// explicitly-requested properties, items, and metadata.
74-
/// </summary>
75-
ProvideSubsetOfStateAfterBuild = 1 << 5,
76-
77-
/// <summary>
78-
/// When this flag is present, projects loaded during build will ignore missing imports (<see cref="ProjectLoadSettings.IgnoreMissingImports"/> and <see cref="ProjectLoadSettings.IgnoreInvalidImports"/>).
79-
/// This is especially useful during a restore since some imports might come from packages that haven't been restored yet.
80-
/// </summary>
81-
IgnoreMissingEmptyAndInvalidImports = 1 << 6,
82-
83-
/// <summary>
84-
/// When this flag is present, an unresolved MSBuild project SDK will fail the build. This flag is used to
85-
/// change the <see cref="IgnoreMissingEmptyAndInvalidImports" /> behavior to still fail when an SDK is missing
86-
/// because those are more fatal.
87-
/// </summary>
88-
FailOnUnresolvedSdk = 1 << 7,
89-
}
90-
9118
/// <summary>
9219
/// BuildRequestData encapsulates all the data needed to submit a build request.
9320
/// </summary>

src/Build/Graph/GraphBuildRequestData.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
using System.Linq;
77
using Microsoft.Build.Execution;
88
using Microsoft.Build.Experimental.BuildCheck;
9+
using Microsoft.Build.Framework;
910
using Microsoft.Build.Shared;
1011

1112
namespace Microsoft.Build.Graph

src/Build/Logging/BinaryLogger/BinaryLogRecordKind.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,5 +40,6 @@ public enum BinaryLogRecordKind
4040
TaskParameter,
4141
ResponseFileUsed,
4242
AssemblyLoad,
43+
BuildSubmissionStarted,
4344
}
4445
}

src/Build/Logging/BinaryLogger/BinaryLogger.cs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -75,13 +75,15 @@ public sealed class BinaryLogger : ILogger
7575
// - TaskParameterEventArgs: Added ParameterName and PropertyName properties
7676
// version 22:
7777
// - extend EnvironmentVariableRead with location where environment variable was used.
78+
// version 23:
79+
// - new record kind: BuildSubmissionStartedEventArgs
7880
// This should be never changed.
7981
// The minimum version of the binary log reader that can read log of above version.
8082
internal const int ForwardCompatibilityMinimalVersion = 18;
8183

8284
// The current version of the binary log representation.
8385
// Changes with each update of the binary log format.
84-
internal const int FileFormatVersion = 22;
86+
internal const int FileFormatVersion = 23;
8587

8688
// The minimum version of the binary log reader that can read log of above version.
8789
// This should be changed only when the binary log format is changed in a way that would prevent it from being

0 commit comments

Comments
 (0)