Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
42 changes: 24 additions & 18 deletions src/Build.UnitTests/BackEnd/Scheduler_Tests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,13 @@
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Xml;
using Microsoft.Build.BackEnd;
using Microsoft.Build.Evaluation;
using Microsoft.Build.Execution;
using Microsoft.Build.Experimental.ProjectCache;
using Microsoft.Build.Framework;
using Microsoft.Build.Shared;
using Microsoft.Build.Unittest;
using Shouldly;
using Xunit;
using TaskItem = Microsoft.Build.Execution.ProjectItemInstance.TaskItem;
Expand All @@ -21,6 +19,8 @@

namespace Microsoft.Build.UnitTests.BackEnd
{
using Microsoft.Build.Unittest;

/// <summary>
/// Tests of the scheduler.
/// </summary>
Expand Down Expand Up @@ -544,13 +544,7 @@ public void TestProxyAffinityIsInProc()

CreateConfiguration(1, "foo.csproj");

BuildRequest request1 = CreateBuildRequest(
nodeRequestId: 1,
configId: 1,
targets: new[] { "foo" },
NodeAffinity.Any,
parentRequest: null,
new ProxyTargets(new Dictionary<string, string> { { "foo", "bar" } }));
BuildRequest request1 = CreateProxyBuildRequest(1, 1, new ProxyTargets(new Dictionary<string, string> { { "foo", "bar" } }), null);

BuildRequestBlocker blocker = new BuildRequestBlocker(-1, Array.Empty<string>(), new[] { request1 });
List<ScheduleResponse> response = new List<ScheduleResponse>(_scheduler.ReportRequestBlocked(1, blocker));
Expand Down Expand Up @@ -812,6 +806,8 @@ private BuildRequest CreateBuildRequest(int nodeRequestId, int configId, string[
/// </summary>
private BuildRequest CreateBuildRequest(int nodeRequestId, int configId, string[] targets, NodeAffinity nodeAffinity, BuildRequest parentRequest, ProxyTargets proxyTargets = null)
{
(targets == null ^ proxyTargets == null).ShouldBeTrue();

HostServices hostServices = null;

if (nodeAffinity != NodeAffinity.Any)
Expand All @@ -820,26 +816,36 @@ private BuildRequest CreateBuildRequest(int nodeRequestId, int configId, string[
hostServices.SetNodeAffinity(String.Empty, nodeAffinity);
}

if (proxyTargets != null)
if (targets != null)
{
parentRequest.ShouldBeNull();
return new BuildRequest(
submissionId: 1,
nodeRequestId,
configId,
proxyTargets,
targets.ToList(),
hostServices);
targets,
hostServices,
BuildEventContext.Invalid,
parentRequest);
}

parentRequest.ShouldBeNull();
return new BuildRequest(
submissionId: 1,
nodeRequestId,
configId,
targets,
hostServices,
BuildEventContext.Invalid,
parentRequest);
proxyTargets,
hostServices);
}

private BuildRequest CreateProxyBuildRequest(int nodeRequestId, int configId, ProxyTargets proxyTargets, BuildRequest parentRequest)
{
return CreateBuildRequest(
nodeRequestId,
configId,
null,
NodeAffinity.Any,
parentRequest,
proxyTargets);
}

/// <summary>
Expand Down
54 changes: 1 addition & 53 deletions src/Build.UnitTests/ProjectCache/ProjectCacheTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -507,6 +507,7 @@ public void ProjectCacheByBuildParametersAndBottomUpBuildWorks(GraphCacheRespons
}
}


AssertCacheBuild(graph, testData, mockCache, logger, nodesToBuildResults, targets: null);
}

Expand Down Expand Up @@ -765,59 +766,6 @@ public void RunningProxyBuildsOnOutOfProcNodesShouldIssueWarning(bool disableInp
logger.AssertMessageCount("MSB4274", 1);
}

// A common scenario is to get a request for N targets, but only some of them can be handled by the cache.
// In this case, missing targets should be passed through.
[Fact]
public async Task PartialProxyTargets()
{
const string ProjectContent = """
<Project>
<Target Name="SomeTarget">
<Message Text="SomeTarget running" />
</Target>
<Target Name="ProxyTarget">
<Message Text="ProxyTarget running" />
</Target>
<Target Name="SomeOtherTarget">
<Message Text="SomeOtherTarget running" />
</Target>
</Project>
""";
TransientTestFile project = _env.CreateFile($"project.proj", ProjectContent);

BuildParameters buildParameters = new()
{
ProjectCacheDescriptor = ProjectCacheDescriptor.FromInstance(
new ConfigurableMockCache
{
GetCacheResultImplementation = (_, _, _) =>
{
return Task.FromResult(
CacheResult.IndicateCacheHit(
new ProxyTargets(
new Dictionary<string, string>
{
{ "ProxyTarget", "SomeTarget" },
})));
}
}),
};

MockLogger logger;
using (Helpers.BuildManagerSession buildSession = new(_env, buildParameters))
{
logger = buildSession.Logger;
BuildResult buildResult = await buildSession.BuildProjectFileAsync(project.Path, new[] { "SomeTarget", "SomeOtherTarget" });

buildResult.Exception.ShouldBeNull();
buildResult.ShouldHaveSucceeded();
}

logger.BuildMessageEvents.Select(i => i.Message).ShouldNotContain("SomeTarget running");
logger.BuildMessageEvents.Select(i => i.Message).ShouldContain("ProxyTarget running");
logger.BuildMessageEvents.Select(i => i.Message).ShouldContain("SomeOtherTarget running");
}

private void AssertCacheBuild(
ProjectGraph graph,
GraphCacheResponse testData,
Expand Down
21 changes: 3 additions & 18 deletions src/Build/BackEnd/BuildManager/BuildManager.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1783,30 +1783,15 @@ private static void AddBuildRequestToSubmission(BuildSubmission submission, int

private static void AddProxyBuildRequestToSubmission(
BuildSubmission submission,
BuildRequestConfiguration configuration,
int configurationId,
ProxyTargets proxyTargets,
int projectContextId)
{
IReadOnlyDictionary<string, string> realTargetsToProxyTargets = proxyTargets.RealTargetToProxyTargetMap;

ICollection<string> requestedTargets = submission.BuildRequestData.TargetNames.Count > 0
? submission.BuildRequestData.TargetNames
: configuration.Project.DefaultTargets;
List<string> targets = new(requestedTargets.Count);
foreach (string requestedTarget in requestedTargets)
{
string effectiveTarget = realTargetsToProxyTargets.TryGetValue(requestedTarget, out string proxyTarget)
? proxyTarget
: requestedTarget;
targets.Add(effectiveTarget);
}

submission.BuildRequest = new BuildRequest(
submission.SubmissionId,
BackEnd.BuildRequest.InvalidNodeRequestId,
configuration.ConfigurationId,
configurationId,
proxyTargets,
targets,
submission.BuildRequestData.HostServices,
submission.BuildRequestData.Flags,
submission.BuildRequestData.RequestedProjectState,
Expand Down Expand Up @@ -2309,7 +2294,7 @@ void HandleCacheResult()
{
// Setup submission.BuildRequest with proxy targets. The proxy request is built on the inproc node (to avoid
// ProjectInstance serialization). The proxy target results are used as results for the real targets.
AddProxyBuildRequestToSubmission(submission, configuration, cacheResult.ProxyTargets, projectContextId);
AddProxyBuildRequestToSubmission(submission, configuration.ConfigurationId, cacheResult.ProxyTargets, projectContextId);
IssueBuildRequestForBuildSubmission(submission, configuration, allowMainThreadBuild: false);
}
else if (cacheResult.ResultType == CacheResultType.CacheHit && cacheResult.BuildResult != null)
Expand Down
17 changes: 0 additions & 17 deletions src/Build/BackEnd/Components/ProjectCache/ProxyTargets.cs
Original file line number Diff line number Diff line change
Expand Up @@ -27,23 +27,6 @@ public class ProxyTargets : ITranslatable
/// </summary>
public IReadOnlyDictionary<string, string> ProxyTargetToRealTargetMap => _proxyTargetToRealTargetMap;

internal IReadOnlyDictionary<string, string> RealTargetToProxyTargetMap
{
get
{
// The ProxyTargetToRealTargetMap is "backwards" from how most users would want to use it and doesn't provide as much flexibility as it could if reversed.
// Unfortunately this is part of a public API so cannot easily change at this point.
Dictionary<string, string> realTargetsToProxyTargets = new(ProxyTargetToRealTargetMap.Count, StringComparer.OrdinalIgnoreCase);
foreach (KeyValuePair<string, string> kvp in ProxyTargetToRealTargetMap)
{
// In the case of multiple proxy targets pointing to the same real target, the last one wins. Another awkwardness of ProxyTargetToRealTargetMap being "backwards".
realTargetsToProxyTargets[kvp.Value] = kvp.Key;
}

return realTargetsToProxyTargets;
}
}

private ProxyTargets()
{
}
Expand Down
4 changes: 1 addition & 3 deletions src/Build/BackEnd/Shared/BuildRequest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,6 @@ private BuildRequest(
/// <param name="nodeRequestId">The id of the node issuing the request</param>
/// <param name="configurationId">The configuration id to use.</param>
/// <param name="proxyTargets"><see cref="ProxyTargets"/></param>
/// <param name="targets">The set of targets to execute</param>
/// <param name="hostServices">Host services if any. May be null.</param>
/// <param name="buildRequestDataFlags">Additional flags for the request.</param>
/// <param name="requestedProjectState">Filter for desired build results.</param>
Expand All @@ -138,15 +137,14 @@ public BuildRequest(
int nodeRequestId,
int configurationId,
ProxyTargets proxyTargets,
List<string> targets,
HostServices hostServices,
BuildRequestDataFlags buildRequestDataFlags = BuildRequestDataFlags.None,
RequestedProjectState requestedProjectState = null,
int projectContextId = BuildEventContext.InvalidProjectContextId)
: this(submissionId, nodeRequestId, configurationId, hostServices, buildRequestDataFlags, requestedProjectState, projectContextId)
{
_proxyTargets = proxyTargets;
_targets = targets;
_targets = proxyTargets.ProxyTargetToRealTargetMap.Keys.ToList();

// Only root requests can have proxy targets.
_parentGlobalRequestId = InvalidGlobalRequestId;
Expand Down
7 changes: 7 additions & 0 deletions src/Build/BackEnd/Shared/BuildRequestConfiguration.cs
Original file line number Diff line number Diff line change
Expand Up @@ -745,6 +745,13 @@ public List<string> GetTargetsUsedToBuildRequest(BuildRequest request)
ErrorUtilities.VerifyThrow(_projectInitialTargets != null, "Initial targets have not been set.");
ErrorUtilities.VerifyThrow(_projectDefaultTargets != null, "Default targets have not been set.");

if (request.ProxyTargets != null)
{
ErrorUtilities.VerifyThrow(
CollectionHelpers.SetEquivalent(request.Targets, request.ProxyTargets.ProxyTargetToRealTargetMap.Keys),
"Targets must be same as proxy targets");
}

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

Expand Down