Skip to content

Commit

Permalink
Add new TargetBuiltReasons (#10092)
Browse files Browse the repository at this point in the history
Fixes #9467

Context
When building targets we only specify a reason when it has direct control over the flow. So some targets have no built reason at the end of the build and can get very confusing on large builds.

Changes Made
Added options for TargetBuiltReason: Initial target, entry target, default target.
These are added to TargetSpecification so they can be referenced in different points of the build.
  • Loading branch information
maridematte authored Jun 28, 2024
1 parent 671434e commit a9c95c7
Show file tree
Hide file tree
Showing 9 changed files with 178 additions and 100 deletions.
10 changes: 5 additions & 5 deletions src/Build.UnitTests/BackEnd/RequestBuilder_Tests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -356,16 +356,16 @@ internal void SetNewBuildRequests(FullyQualifiedBuildRequest[] requests)

#region ITargetBuilder Members

public Task<BuildResult> BuildTargets(ProjectLoggingContext loggingContext, BuildRequestEntry entry, IRequestBuilderCallback callback, string[] targets, Lookup baseLookup, CancellationToken cancellationToken)
public Task<BuildResult> BuildTargets(ProjectLoggingContext loggingContext, BuildRequestEntry entry, IRequestBuilderCallback callback, (string name, TargetBuiltReason reason)[] targets, Lookup baseLookup, CancellationToken cancellationToken)
{
_requestBuilderCallback = callback;

if (cancellationToken.WaitHandle.WaitOne(1500))
{
BuildResult result = new BuildResult(entry.Request);
foreach (string target in targets)
foreach ((string name, TargetBuiltReason reason) target in targets)
{
result.AddResultsForTarget(target, BuildResultUtilities.GetEmptyFailingTargetResult());
result.AddResultsForTarget(target.name, BuildResultUtilities.GetEmptyFailingTargetResult());
}
return Task<BuildResult>.FromResult(result);
}
Expand All @@ -388,9 +388,9 @@ public Task<BuildResult> BuildTargets(ProjectLoggingContext loggingContext, Buil
if (cancellationToken.WaitHandle.WaitOne(1500))
{
BuildResult result = new BuildResult(entry.Request);
foreach (string target in targets)
foreach ((string name, TargetBuiltReason reason) target in targets)
{
result.AddResultsForTarget(target, BuildResultUtilities.GetEmptyFailingTargetResult());
result.AddResultsForTarget(target.name, BuildResultUtilities.GetEmptyFailingTargetResult());
}
return Task<BuildResult>.FromResult(result);
}
Expand Down
170 changes: 101 additions & 69 deletions src/Build.UnitTests/BackEnd/TargetBuilder_Tests.cs

Large diffs are not rendered by default.

8 changes: 4 additions & 4 deletions src/Build/BackEnd/Components/Logging/NodeLoggingContext.cs
Original file line number Diff line number Diff line change
Expand Up @@ -90,16 +90,16 @@ internal void LogRequestHandledFromCache(BuildRequest request, BuildRequestConfi

// When pulling a request from the cache, we want to make sure we log a target skipped event for any targets which
// were used to build the request including default and initial targets.
foreach (string target in configuration.GetTargetsUsedToBuildRequest(request))
foreach ((string name, TargetBuiltReason reason) target in configuration.GetTargetsUsedToBuildRequest(request))
{
var targetResult = result[target];
var targetResult = result[target.name];
bool isFailure = targetResult.ResultCode == TargetResultCode.Failure;

var skippedTargetEventArgs = new TargetSkippedEventArgs(message: null)
{
BuildEventContext = projectLoggingContext.BuildEventContext,
TargetName = target,
BuildReason = TargetBuiltReason.None,
TargetName = target.name,
BuildReason = target.reason,
SkipReason = isFailure ? TargetSkipReason.PreviouslyBuiltUnsuccessfully : TargetSkipReason.PreviouslyBuiltSuccessfully,
OriginallySucceeded = !isFailure,
OriginalBuildEventContext = (targetResult as TargetResult)?.OriginalBuildEventContext
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
using System.Threading;
using System.Threading.Tasks;
using Microsoft.Build.BackEnd.Logging;
using Microsoft.Build.Framework;
using BuildResult = Microsoft.Build.Execution.BuildResult;

#nullable disable
Expand All @@ -25,6 +26,6 @@ internal interface ITargetBuilder
/// <param name="baseLookup">The Lookup containing all current items and properties for this target.</param>
/// <param name="cancellationToken">The cancellation token used to cancel processing of targets.</param>
/// <returns>A Task representing the work to be done.</returns>
Task<BuildResult> BuildTargets(ProjectLoggingContext projectLoggingContext, BuildRequestEntry entry, IRequestBuilderCallback callback, string[] targets, Lookup baseLookup, CancellationToken cancellationToken);
Task<BuildResult> BuildTargets(ProjectLoggingContext projectLoggingContext, BuildRequestEntry entry, IRequestBuilderCallback callback, (string name, TargetBuiltReason reason)[] targets, Lookup baseLookup, CancellationToken cancellationToken);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -1177,7 +1177,7 @@ private async Task<BuildResult> BuildProject()
_requestEntry.Request.BuildEventContext = _projectLoggingContext.BuildEventContext;

// Determine the set of targets we need to build
string[] allTargets = _requestEntry.RequestConfiguration
(string name, TargetBuiltReason reason)[] allTargets = _requestEntry.RequestConfiguration
.GetTargetsUsedToBuildRequest(_requestEntry.Request).ToArray();

ProjectErrorUtilities.VerifyThrowInvalidProject(allTargets.Length > 0,
Expand Down
29 changes: 17 additions & 12 deletions src/Build/BackEnd/Components/RequestBuilder/TargetBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ internal class TargetBuilder : ITargetBuilder, ITargetBuilderCallback, IBuildCom
/// <param name="baseLookup">The Lookup containing all current items and properties for this target.</param>
/// <param name="cancellationToken">The <see cref="CancellationToken"/> to use when building the targets.</param>
/// <returns>The target's outputs and result codes</returns>
public async Task<BuildResult> BuildTargets(ProjectLoggingContext loggingContext, BuildRequestEntry entry, IRequestBuilderCallback callback, string[] targetNames, Lookup baseLookup, CancellationToken cancellationToken)
public async Task<BuildResult> BuildTargets(ProjectLoggingContext loggingContext, BuildRequestEntry entry, IRequestBuilderCallback callback, (string name, TargetBuiltReason reason)[] targetNames, Lookup baseLookup, CancellationToken cancellationToken)
{
ErrorUtilities.VerifyThrowArgumentNull(loggingContext, "projectLoggingContext");
ErrorUtilities.VerifyThrowArgumentNull(entry, nameof(entry));
Expand Down Expand Up @@ -143,17 +143,17 @@ public async Task<BuildResult> BuildTargets(ProjectLoggingContext loggingContext

List<TargetSpecification> targets = new List<TargetSpecification>(targetNames.Length);

foreach (string targetName in targetNames)
foreach ((string name, TargetBuiltReason reason) targetName in targetNames)
{
var targetExists = _projectInstance.Targets.TryGetValue(targetName, out ProjectTargetInstance targetInstance);
var targetExists = _projectInstance.Targets.TryGetValue(targetName.name, out ProjectTargetInstance targetInstance);
if (!targetExists && entry.Request.BuildRequestDataFlags.HasFlag(BuildRequestDataFlags.SkipNonexistentTargets))
{
_projectLoggingContext.LogComment(Framework.MessageImportance.Low,
"TargetSkippedWhenSkipNonexistentTargets", targetName);
"TargetSkippedWhenSkipNonexistentTargets", targetName.name);
}
else
{
targets.Add(new TargetSpecification(targetName, targetExists ? targetInstance.Location : _projectInstance.ProjectFileLocation));
targets.Add(new TargetSpecification(targetName.name, targetExists ? targetInstance.Location : _projectInstance.ProjectFileLocation, targetName.reason));
}
}

Expand Down Expand Up @@ -185,7 +185,7 @@ public async Task<BuildResult> BuildTargets(ProjectLoggingContext loggingContext

// Gather up outputs for the requested targets and return those. All of our information should be in the base lookup now.
ComputeAfterTargetFailures(targetNames);
BuildResult resultsToReport = new BuildResult(_buildResult, targetNames);
BuildResult resultsToReport = new BuildResult(_buildResult, targetNames.Select(target => target.name).ToArray());

// Return after-build project state if requested.
if (_requestEntry.Request.BuildRequestDataFlags.HasFlag(BuildRequestDataFlags.ProvideProjectStateAfterBuild))
Expand Down Expand Up @@ -735,9 +735,14 @@ private async Task<bool> PushTargets(IList<TargetSpecification> targets, TargetE
}
}

// The buildReason argument for this function can be BeforeTargets or AfterTargets, we don't want to override the reason when adding a new entry
// If the reason is None, it means it does not depend on another target. So we can use the target's BuiltReason.
TargetBuiltReason entryReason = buildReason == TargetBuiltReason.None ? targetSpecification._targetBuiltReason : buildReason;

// Add to the list of targets to push. We don't actually put it on the stack here because we could run into a circular dependency
// during this loop, in which case the target stack would be out of whack.
TargetEntry newEntry = new TargetEntry(_requestEntry, this as ITargetBuilderCallback, targetSpecification, baseLookup, parentTargetEntry, buildReason, _componentHost, _projectLoggingContext, stopProcessingOnCompletion);
TargetEntry newEntry = new TargetEntry(_requestEntry, this as ITargetBuilderCallback, targetSpecification, baseLookup, parentTargetEntry, entryReason, _componentHost, _projectLoggingContext, stopProcessingOnCompletion);

newEntry.ErrorTarget = addAsErrorTarget;
targetsToPush.Add(newEntry);
stopProcessingOnCompletion = false; // The first target on the stack (the last one to be run) always inherits the stopProcessing flag.
Expand Down Expand Up @@ -772,15 +777,15 @@ private async Task<bool> CompleteOutstandingActiveRequests(string targetName)
return false;
}

private void ComputeAfterTargetFailures(string[] targetNames)
private void ComputeAfterTargetFailures((string name, TargetBuiltReason reason)[] targetNames)
{
foreach (string targetName in targetNames)
foreach ((string name, TargetBuiltReason reason) targetName in targetNames)
{
if (_buildResult.ResultsByTarget.TryGetValue(targetName, out TargetResult targetBuildResult))
if (_buildResult.ResultsByTarget.TryGetValue(targetName.name, out TargetResult targetBuildResult))
{
// Queue of targets waiting to be processed, seeded with the specific target for which we're computing AfterTargetsHaveFailed.
var targetsToCheckForAfterTargets = new Queue<string>();
targetsToCheckForAfterTargets.Enqueue(targetName);
targetsToCheckForAfterTargets.Enqueue(targetName.name);

// Set of targets already processed, to break cycles of AfterTargets.
// Initialized lazily when needed below.
Expand All @@ -804,7 +809,7 @@ private void ComputeAfterTargetFailures(string[] targetNames)

targetsChecked ??= new HashSet<string>(MSBuildNameIgnoreCaseComparer.Default)
{
targetName
targetName.name
};

// If we haven't seen this target yet, add it to the list to check.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Diagnostics;
using Microsoft.Build.Framework;
using Microsoft.Build.Shared;
using ElementLocation = Microsoft.Build.Construction.ElementLocation;

Expand All @@ -18,18 +19,22 @@ internal class TargetSpecification : ITranslatable
private string _targetName;
private ElementLocation _referenceLocation;

internal TargetBuiltReason _targetBuiltReason;

/// <summary>
/// Construct a target specification.
/// </summary>
/// <param name="targetName">The name of the target</param>
/// <param name="referenceLocation">The location from which it was referred.</param>
internal TargetSpecification(string targetName, ElementLocation referenceLocation)
/// <param name="targetBuiltReason">Reason the target is being built</param>
internal TargetSpecification(string targetName, ElementLocation referenceLocation, TargetBuiltReason targetBuiltReason = TargetBuiltReason.None)
{
ErrorUtilities.VerifyThrowArgumentLength(targetName, nameof(targetName));
ErrorUtilities.VerifyThrowArgumentNull(referenceLocation, nameof(referenceLocation));

this._targetName = targetName;
this._referenceLocation = referenceLocation;
this._targetBuiltReason = targetBuiltReason;
}

private TargetSpecification()
Expand All @@ -41,6 +46,8 @@ private TargetSpecification()
/// </summary>
public string TargetName => _targetName;

public TargetBuiltReason TargetBuiltReason => _targetBuiltReason;

/// <summary>
/// Gets or sets the reference location
/// </summary>
Expand Down
30 changes: 24 additions & 6 deletions src/Build/BackEnd/Shared/BuildRequestConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -762,7 +762,7 @@ public void RetrieveFromCache()
/// </summary>
/// <param name="request">The request </param>
/// <returns>An array of t</returns>
public List<string> GetTargetsUsedToBuildRequest(BuildRequest request)
public List<(string name, TargetBuiltReason reason)> GetTargetsUsedToBuildRequest(BuildRequest request)
{
ErrorUtilities.VerifyThrow(request.ConfigurationId == ConfigurationId, "Request does not match configuration.");
ErrorUtilities.VerifyThrow(_projectInitialTargets != null, "Initial targets have not been set.");
Expand All @@ -775,13 +775,31 @@ public List<string> GetTargetsUsedToBuildRequest(BuildRequest request)
"Targets must be same as proxy targets");
}

List<string> initialTargets = _projectInitialTargets;
List<string> nonInitialTargets = (request.Targets.Count == 0) ? _projectDefaultTargets : request.Targets;
bool hasInitialTargets = request.Targets.Count == 0 ? false : true;

var allTargets = new List<string>(initialTargets.Count + nonInitialTargets.Count);
List<(string name, TargetBuiltReason reason)> allTargets = new(
_projectInitialTargets.Count +
(hasInitialTargets ? _projectDefaultTargets.Count : request.Targets.Count));

allTargets.AddRange(initialTargets);
allTargets.AddRange(nonInitialTargets);
foreach (var target in _projectInitialTargets)
{
allTargets.Add((target, TargetBuiltReason.InitialTargets));
}

if (hasInitialTargets)
{
foreach (var target in request.Targets)
{
allTargets.Add((target, TargetBuiltReason.EntryTargets));
}
}
else
{
foreach (var target in _projectDefaultTargets)
{
allTargets.Add((target, TargetBuiltReason.DefaultTargets));
}
}

return allTargets;
}
Expand Down
17 changes: 16 additions & 1 deletion src/Framework/TargetBuiltReason.cs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,21 @@ public enum TargetBuiltReason
/// <summary>
/// The target was part of the parent's AfterTargets list.
/// </summary>
AfterTargets
AfterTargets,

/// <summary>
/// The target was defined as an initial target of the project.
/// </summary>
InitialTargets,

/// <summary>
/// The target was one of the default targets of the project.
/// </summary>
DefaultTargets,

/// <summary>
/// The target was one of the targets explicitly called to be built.
/// </summary>
EntryTargets,
}
}

0 comments on commit a9c95c7

Please sign in to comment.