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

Allow SDK resolvers to preserve state #2849

Merged
merged 4 commits into from
Jan 9, 2018
+130 −12
Merged

Allow SDK resolvers to preserve state #2849

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b67c97b
Implementation
jeffkl Jan 9, 2018
aa578fb
Unit tests
jeffkl Jan 9, 2018
a2e381b
Update public API
jeffkl Jan 9, 2018
7b1d142
Address comments
jeffkl Jan 9, 2018
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
1 change: 1 addition & 0 deletions ref/net46/Microsoft.Build.Framework/Microsoft.Build.Framework.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,7 @@ protected SdkResolverContext() { }
public virtual System.Version MSBuildVersion { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]protected set { } }
public virtual string ProjectFilePath { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]protected set { } }
public virtual string SolutionFilePath { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]protected set { } }
public virtual object State { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
}
public abstract partial class SdkResult
{
Expand Down
1 change: 1 addition & 0 deletions ref/netstandard1.3/Microsoft.Build.Framework/Microsoft.Build.Framework.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,7 @@ protected SdkResolverContext() { }
public virtual System.Version MSBuildVersion { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]protected set { } }
public virtual string ProjectFilePath { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]protected set { } }
public virtual string SolutionFilePath { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]protected set { } }
public virtual object State { [System.Runtime.CompilerServices.CompilerGeneratedAttribute]get { throw null; } [System.Runtime.CompilerServices.CompilerGeneratedAttribute]set { } }
}
public abstract partial class SdkResult
{
Expand Down
60 changes: 59 additions & 1 deletion src/Build.UnitTests/BackEnd/SdkResolverService_Tests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,38 @@ public void AssertFirstResolverErrorsSupressedWhenResolved()
Assert.DoesNotContain("ERROR", logResult);
}

[Fact]
public void AssertResolverHasStatePreserved()
{
const int submissionId = 5;

SdkResolverService.Instance.InitializeForTests(new MockLoaderStrategy());

SdkReference sdk = new SdkReference("othersdk", "1.0", "minimumVersion");

// First call should not know state
SdkResolverService.Instance.ResolveSdk(submissionId, sdk, _loggingContext, new MockElementLocation("file"), "sln", "projectPath").ShouldBe("resolverpath");

// Second call should have received state
SdkResolverService.Instance.ResolveSdk(submissionId, sdk, _loggingContext, new MockElementLocation("file"), "sln", "projectPath").ShouldBe(MockSdkResolverWithState.Expected);
}

[Fact]
public void AssertResolverStateNotPreserved()
{
const int submissionId = BuildEventContext.InvalidSubmissionId;

SdkResolverService.Instance.InitializeForTests(new MockLoaderStrategy());

SdkReference sdk = new SdkReference("othersdk", "1.0", "minimumVersion");

// First call should not know state
SdkResolverService.Instance.ResolveSdk(submissionId, sdk, _loggingContext, new MockElementLocation("file"), "sln", "projectPath").ShouldBe("resolverpath");

// Second call should have received state
SdkResolverService.Instance.ResolveSdk(submissionId, sdk, _loggingContext, new MockElementLocation("file"), "sln", "projectPath").ShouldBe("resolverpath");
}

[Theory]
[InlineData(null, "1.0", true)]
[InlineData("1.0", "1.0", true)]
Expand Down Expand Up @@ -130,7 +162,8 @@ internal override IList<SdkResolver> LoadResolvers(LoggingContext loggingContext
{
new MockSdkResolver1(),
new MockSdkResolver2(),
new MockResolverReturnsNull()
new MockResolverReturnsNull(),
new MockSdkResolverWithState()
};

if (_includeErrorResolver)
Expand Down Expand Up @@ -185,6 +218,31 @@ public override SdkResultBase Resolve(SdkReference sdk, SdkResolverContextBase r
}
}

private class MockSdkResolverWithState : SdkResolver
{
public const string Expected = "01713226A202458F97D9074168DF2618";

public override string Name => nameof(MockSdkResolverWithState);

public override int Priority => 3;

public override SdkResultBase Resolve(SdkReference sdkReference, SdkResolverContextBase resolverContext, SdkResultFactoryBase factory)
{
if (sdkReference.Name.Equals("notfound"))
{
return null;
}
if (resolverContext.State != null)
{
return factory.IndicateSuccess((string) resolverContext.State, "1.0");
}

resolverContext.State = Expected;

return factory.IndicateSuccess("resolverpath", "1.0");
}
}

private class MockSdkResolverThrows : SdkResolver
{
public override string Name => nameof(MockSdkResolverThrows);
Expand Down
3 changes: 3 additions & 0 deletions src/Build/BackEnd/BuildManager/BuildManager.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1664,6 +1664,9 @@ private void CheckSubmissionCompletenessAndRemove(BuildSubmission submission)

// Clear all cached SDKs for the submission
SdkResolverService.ClearCache(submission.SubmissionId);

// Clear SDK resolver cache for the submission
Copy link
Contributor

@cdmihai cdmihai Jan 9, 2018 •
edited
Loading

Choose a reason for hiding this comment

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

This line (and its comment) seems very similar to the previous one. A code comment on why this is would be nice.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point, and after I took a second look at it, I decided this call should be moved. I've also improved the comment

BackEnd.SdkResolution.SdkResolverService.Instance.ClearCache(submission.SubmissionId);
}

if (_buildSubmissions.Count == 0)
Expand Down
15 changes: 8 additions & 7 deletions src/Build/BackEnd/Components/SdkResolution/MainNodeSdkResolverService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,7 @@ public override void PacketReceived(int node, INodePacket packet)
/// <inheritdoc cref="ISdkResolverService.ResolveSdk"/>
public override string ResolveSdk(int submissionId, SdkReference sdk, LoggingContext loggingContext, ElementLocation sdkReferenceLocation, string solutionPath, string projectPath)
{
SdkResult result = GetSdkResult(submissionId, sdk, loggingContext, sdkReferenceLocation, solutionPath, projectPath);
SdkResult result = GetSdkResultAndCache(submissionId, sdk, loggingContext, sdkReferenceLocation, solutionPath, projectPath);

return result?.Path;
}
Expand All @@ -99,7 +99,7 @@ public override string ResolveSdk(int submissionId, SdkReference sdk, LoggingCon
/// <param name="solutionPath">The full path to the solution, if any, that is being built.</param>
/// <param name="projectPath">The full path to the project that referenced the SDK.</param>
/// <returns>An <see cref="SdkResult"/> containing information about the SDK if one was resolved, otherwise <code>null</code>.</returns>
private SdkResult GetSdkResult(int submissionId, SdkReference sdk, LoggingContext loggingContext, ElementLocation sdkReferenceLocation, string solutionPath, string projectPath)
private SdkResult GetSdkResultAndCache(int submissionId, SdkReference sdk, LoggingContext loggingContext, ElementLocation sdkReferenceLocation, string solutionPath, string projectPath)
{
ErrorUtilities.VerifyThrowInternalNull(sdk, nameof(sdk));
ErrorUtilities.VerifyThrowInternalNull(loggingContext, nameof(loggingContext));
Expand All @@ -109,7 +109,7 @@ private SdkResult GetSdkResult(int submissionId, SdkReference sdk, LoggingContex

if (Traits.Instance.EscapeHatches.DisableSdkResolutionCache)
{
result = GetSdkResult(sdk, loggingContext, sdkReferenceLocation, solutionPath, projectPath);
result = GetSdkResult(submissionId, sdk, loggingContext, sdkReferenceLocation, solutionPath, projectPath);
}
else
{
Expand All @@ -123,7 +123,7 @@ private SdkResult GetSdkResult(int submissionId, SdkReference sdk, LoggingContex
*/
result = cached.GetOrAdd(
sdk.Name,
key => GetSdkResult(sdk, loggingContext, sdkReferenceLocation, solutionPath, projectPath));
key => GetSdkResult(submissionId, sdk, loggingContext, sdkReferenceLocation, solutionPath, projectPath));
}

if (!SdkResolverService.IsReferenceSameVersion(sdk, result.Version))
Expand All @@ -138,15 +138,16 @@ private SdkResult GetSdkResult(int submissionId, SdkReference sdk, LoggingContex
/// <summary>
/// Resolves the specified SDK without caching the result.
/// </summary>
/// <param name="submissionId">The current build submission ID that is resolving an SDK.</param>
/// <param name="sdk">The <see cref="SdkReference"/> containing information about the SDK to resolve.</param>
/// <param name="loggingContext">The <see cref="LoggingContext"/> to use when logging messages during resolution.</param>
/// <param name="sdkReferenceLocation">The <see cref="ElementLocation"/> of the element that referenced the SDK.</param>
/// <param name="solutionPath">The full path to the solution, if any, that is being built.</param>
/// <param name="projectPath">The full path to the project that referenced the SDK.</param>
/// <returns>An <see cref="SdkResult"/> containing information about the SDK if one was resolved, otherwise <code>null</code>.</returns>
private SdkResult GetSdkResult(SdkReference sdk, LoggingContext loggingContext, ElementLocation sdkReferenceLocation, string solutionPath, string projectPath)
private SdkResult GetSdkResult(int submissionId, SdkReference sdk, LoggingContext loggingContext, ElementLocation sdkReferenceLocation, string solutionPath, string projectPath)
{
SdkResult sdkResult = SdkResolverService.Instance.GetSdkResult(sdk, loggingContext, sdkReferenceLocation, solutionPath, projectPath);
SdkResult sdkResult = SdkResolverService.Instance.GetSdkResult(submissionId, sdk, loggingContext, sdkReferenceLocation, solutionPath, projectPath);

if (!SdkResolverService.IsReferenceSameVersion(sdk, sdkResult.Version))
{
Expand Down Expand Up @@ -220,7 +221,7 @@ private void ProcessRequests()
ILoggingService loggingService = Host.GetComponent(BuildComponentType.LoggingService) as ILoggingService;

// This call is usually cached so is very fast but can take longer for a new SDK that is downloaded. Other queued threads for different SDKs will complete sooner and continue on which unblocks evaluations
SdkResult result = GetSdkResult(request.SubmissionId, sdkReference, new EvaluationLoggingContext(loggingService, request.BuildEventContext, request.ProjectPath), request.ElementLocation, request.SolutionPath, request.ProjectPath);
SdkResult result = GetSdkResultAndCache(request.SubmissionId, sdkReference, new EvaluationLoggingContext(loggingService, request.BuildEventContext, request.ProjectPath), request.ElementLocation, request.SolutionPath, request.ProjectPath);

// Create a response
SdkResolverResponse response = new SdkResolverResponse(result.Path, result.Version);
Expand Down
55 changes: 51 additions & 4 deletions src/Build/BackEnd/Components/SdkResolution/SdkResolverService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@
using Microsoft.Build.Framework;
using Microsoft.Build.Shared;
using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using Microsoft.Build.Collections;

namespace Microsoft.Build.BackEnd.SdkResolution
{
Expand All @@ -30,6 +30,11 @@ internal sealed class SdkResolverService : ISdkResolverService
/// </summary>
private readonly object _lockObject = new object();

/// <summary>
/// Stores resolver state by build submission ID.
/// </summary>
private readonly ConcurrentDictionary<int, ConcurrentDictionary<SdkResolver, object>> _resolverStateBySubmission = new ConcurrentDictionary<int, ConcurrentDictionary<SdkResolver, object>>();

/// <summary>
/// Stores the list of SDK resolvers which were loaded.
/// </summary>
Expand Down Expand Up @@ -73,18 +78,22 @@ public static bool IsReferenceSameVersion(SdkReference sdk, string version)
/// <inheritdoc cref="ISdkResolverService.ClearCache"/>
public void ClearCache(int submissionId)
{
ConcurrentDictionary<SdkResolver, object> notused;

_resolverStateBySubmission.TryRemove(submissionId, out notused);
}

/// <summary>
/// Resolves and SDK and gets a result.
/// </summary>
/// <param name="submissionId">The build submission ID that the resolution request is for.</param>
/// <param name="sdk">The <see cref="SdkReference"/> containing information about the referenced SDK.</param>
/// <param name="loggingContext">The <see cref="LoggingContext"/> to use when logging messages during resolution.</param>
/// <param name="sdkReferenceLocation">The <see cref="ElementLocation"/> of the element which referenced the SDK.</param>
/// <param name="solutionPath">The full path to the solution, if any, that is being built.</param>
/// <param name="projectPath">The full path to that referenced the SDK.</param>
/// <returns>An <see cref="SdkResult"/> containing information of the SDK if it could be resolved, otherwise <code>null</code>.</returns>
public SdkResult GetSdkResult(SdkReference sdk, LoggingContext loggingContext, ElementLocation sdkReferenceLocation, string solutionPath, string projectPath)
public SdkResult GetSdkResult(int submissionId, SdkReference sdk, LoggingContext loggingContext, ElementLocation sdkReferenceLocation, string solutionPath, string projectPath)
{
// Lazy initialize the SDK resolvers
if (_resolvers == null)
Expand All @@ -101,12 +110,18 @@ public SdkResult GetSdkResult(SdkReference sdk, LoggingContext loggingContext, E

foreach (SdkResolver sdkResolver in _resolvers)
{
SdkResolverContext context = new SdkResolverContext(buildEngineLogger, projectPath, solutionPath, ProjectCollection.Version);
SdkResolverContext context = new SdkResolverContext(buildEngineLogger, projectPath, solutionPath, ProjectCollection.Version)
{
State = GetResolverState(submissionId, sdkResolver)
};

SdkResultFactory resultFactory = new SdkResultFactory(sdk);
try
{
SdkResult result = (SdkResult) sdkResolver.Resolve(sdk, context, resultFactory);

SetResolverState(submissionId, sdkResolver, context.State);

if (result == null)
{
continue;
Expand Down Expand Up @@ -151,7 +166,7 @@ public SdkResult GetSdkResult(SdkReference sdk, LoggingContext loggingContext, E
/// <inheritdoc cref="ISdkResolverService.ResolveSdk"/>
public string ResolveSdk(int submissionId, SdkReference sdk, LoggingContext loggingContext, ElementLocation sdkReferenceLocation, string solutionPath, string projectPath)
{
SdkResult result = GetSdkResult(sdk, loggingContext, sdkReferenceLocation, solutionPath, projectPath);
SdkResult result = GetSdkResult(submissionId, sdk, loggingContext, sdkReferenceLocation, solutionPath, projectPath);

return result?.Path;
}
Expand Down Expand Up @@ -184,6 +199,27 @@ private static void LogWarnings(LoggingContext loggingContext, ElementLocation l
}
}

private object GetResolverState(int submissionId, SdkResolver resolver)
{
// Do not fetch state for resolution requests that are not associated with a valid build submission ID
if (submissionId != BuildEventContext.InvalidSubmissionId)
{
ConcurrentDictionary<SdkResolver, object> resolverState;

if (_resolverStateBySubmission.TryGetValue(submissionId, out resolverState))
{
object state;

if (resolverState.TryGetValue(resolver, out state))
{
return state;
}
}
}

return null;
}

private void Initialize(LoggingContext loggingContext, ElementLocation location)
{
lock (_lockObject)
Expand All @@ -196,5 +232,16 @@ private void Initialize(LoggingContext loggingContext, ElementLocation location)
_resolvers = _sdkResolverLoader.LoadResolvers(loggingContext, location);
}
}

private void SetResolverState(int submissionId, SdkResolver resolver, object state)
{
// Do not set state for resolution requests that are not associated with a valid build submission ID
if (submissionId != BuildEventContext.InvalidSubmissionId)
{
ConcurrentDictionary<SdkResolver, object> resolverState = _resolverStateBySubmission.GetOrAdd(submissionId, new ConcurrentDictionary<SdkResolver, object>(Environment.ProcessorCount, _resolvers.Count));

resolverState.AddOrUpdate(resolver, state, (sdkResolver, obj) => state);
}
}
}
}
6 changes: 6 additions & 0 deletions src/Framework/Sdk/SdkResolverContext.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,5 +33,11 @@ public abstract class SdkResolverContext
/// </remarks>
/// </summary>
public virtual Version MSBuildVersion { get; protected set; }

/// <summary>
/// Gets or sets any custom state for current build. This allows resolvers to maintain state between resolutions.
/// This property is not thread-safe.
/// </summary>
public virtual object State { get; set; }
}
}