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.

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

Adding accepted SDK name match pattern to SDK manifests. #7597

Merged
merged 20 commits into from
May 25, 2022
Merged
Show file tree
Hide file tree
Changes from 14 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
22 changes: 22 additions & 0 deletions documentation/specs/sdk-resolvers-algorithm.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
## SDK Resolution Algorithm
In 17.3 under ChangeWave 17.4 the sdk resolution algorithm is changed.

### Reason for change
Previously (before ChangeWave 17.4) all SDK resolvers were loaded and then ordered by priority. The resolvers are tried one after one until one of them succeeds. In order to decrease the number of assemblies to be load we change the behavior in 17.3 under ChangeWave 17.4.

### New SDK Resolution Algorithm
Under ChangeWave 17.4 all the resolvers divides into two groups:
- Specific resolvers, i.e. resolvers with specified name pattern
- General resolvers, i.e. resolvers without specified name pattern

The resolving algorithm works in two passes.
- On the first pass all the specific resolvers that match the given sdk name would be loaded (if needed), ordered by priority and tried one after one.
- If the sdk is not found, on the second pass all general resolvers would be loaded (if needed), ordered by priority and tried one after one.

By default the resolvers are general. To make all the resolvers from some dll specific, in the corresponding manifest (xml file) one need to specify the `NamePattern` using C# regex format:
```
<SdkResolver>
<Path>MySdkResolver.dll</Path>
<NamePattern>MySdk.*</NamePattern>
AR-May marked this conversation as resolved.
Show resolved Hide resolved
</SdkResolver>
```
1 change: 1 addition & 0 deletions documentation/wiki/ChangeWaves.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ A wave of features is set to "rotate out" (i.e. become standard functionality) t
- [Respect deps.json when loading assemblies](https://github.com/dotnet/msbuild/pull/7520)
- [Remove opt in for new schema for CombineTargetFrameworkInfoProperties](https://github.com/dotnet/msbuild/pull/6928)
- [Consider `Platform` as default during Platform Negotiation](https://github.com/dotnet/msbuild/pull/7511)
- [Adding accepted SDK name match pattern to SDK manifests](https://github.com/dotnet/msbuild/pull/7597)

### 17.0
- [Scheduler should honor BuildParameters.DisableInprocNode](https://github.com/dotnet/msbuild/pull/6400)
Expand Down
37 changes: 32 additions & 5 deletions src/Build.UnitTests/BackEnd/SdkResolverLoader_Tests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ public void AssertDefaultLoaderReturnsDefaultResolvers()
{
var loader = new SdkResolverLoader();

var resolvers = loader.LoadResolvers(_loggingContext, new MockElementLocation("file"));
var resolvers = loader.LoadAllResolvers(_loggingContext, new MockElementLocation("file"));

resolvers.Select(i => i.GetType().FullName).ShouldBe(new [] { typeof(DefaultSdkResolver).FullName });

Expand Down Expand Up @@ -106,7 +106,7 @@ public void VerifyThrowsWhenResolverFailsToLoad()

InvalidProjectFileException exception = Should.Throw<InvalidProjectFileException>(() =>
{
sdkResolverLoader.LoadResolvers(_loggingContext, ElementLocation.EmptyLocation);
sdkResolverLoader.LoadAllResolvers(_loggingContext, ElementLocation.EmptyLocation);
});

exception.Message.ShouldBe($"The SDK resolver type \"{nameof(MockSdkResolverThatDoesNotLoad)}\" failed to load. A8BB8B3131D3475D881ACD3AF8D75BD6");
Expand Down Expand Up @@ -138,7 +138,7 @@ public void VerifyThrowsWhenResolverHasNoPublicConstructor()

InvalidProjectFileException exception = Should.Throw<InvalidProjectFileException>(() =>
{
sdkResolverLoader.LoadResolvers(_loggingContext, ElementLocation.EmptyLocation);
sdkResolverLoader.LoadAllResolvers(_loggingContext, ElementLocation.EmptyLocation);
});

exception.Message.ShouldStartWith($"The SDK resolver type \"{nameof(MockSdkResolverNoPublicConstructor)}\" failed to load.");
Expand Down Expand Up @@ -169,7 +169,7 @@ public void VerifyWarningLoggedWhenResolverAssemblyCannotBeLoaded()

InvalidProjectFileException exception = Should.Throw<InvalidProjectFileException>(() =>
{
sdkResolverLoader.LoadResolvers(_loggingContext, ElementLocation.EmptyLocation);
sdkResolverLoader.LoadAllResolvers(_loggingContext, ElementLocation.EmptyLocation);
});

exception.Message.ShouldBe($"The SDK resolver assembly \"{assemblyPath}\" could not be loaded. {expectedMessage}");
Expand Down Expand Up @@ -207,6 +207,33 @@ public void SdkResolverLoaderReadsManifestFile()
}
}

[Fact]
public void SdkResolverLoaderReadsManifestFileWithResolvableSdkPattern()
{
using (var env = TestEnvironment.Create(_output))
{
var root = env.CreateFolder().Path;
var resolverPath = Path.Combine(root, "MyTestResolver");
var resolverManifest = Path.Combine(resolverPath, "MyTestResolver.xml");

var assemblyToLoad = env.CreateFile(".dll").Path;

Directory.CreateDirectory(resolverPath);
File.WriteAllText(resolverManifest, $@"
<SdkResolver>
<ResolvableSdkPattern>1&lt;.*</ResolvableSdkPattern>
<Path>{assemblyToLoad}</Path>
</SdkResolver>");

SdkResolverLoader loader = new SdkResolverLoader();
var resolversManifestsFound = loader.FindPotentialSdkResolversManifests(root, new MockElementLocation("file"));

resolversManifestsFound.Count.ShouldBe(1);
resolversManifestsFound.First().Path.ShouldBe(assemblyToLoad);
resolversManifestsFound.First().ResolvableSdkRegex.ToString().ShouldBe("1<.*");
}
}

[Fact]
public void SdkResolverLoaderErrorsWithInvalidManifestFile()
{
Expand Down Expand Up @@ -287,7 +314,7 @@ public void SdkResolverLoaderHonorsIncludeDefaultEnvVar()
resolvers.Add(new MockSdkResolverWithAssemblyPath(resolverPath));
}
};
IList<SdkResolverBase> resolvers = loader.LoadResolvers(_loggingContext, new MockElementLocation("file"));
IList<SdkResolverBase> resolvers = loader.LoadAllResolvers(_loggingContext, new MockElementLocation("file"));

resolvers.Count.ShouldBe(0);
}
Expand Down
180 changes: 168 additions & 12 deletions src/Build.UnitTests/BackEnd/SdkResolverService_Tests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
using SdkResultBase = Microsoft.Build.Framework.SdkResult;
using SdkResultFactoryBase = Microsoft.Build.Framework.SdkResultFactory;
using SdkResultImpl = Microsoft.Build.BackEnd.SdkResolution.SdkResult;
using Microsoft.Build.Shared;
using System.Text.RegularExpressions;

#nullable disable

Expand All @@ -39,9 +41,10 @@ public SdkResolverService_Tests()
}

[Fact]
// Scenario: Sdk is not resolved.
public void AssertAllResolverErrorsLoggedWhenSdkNotResolved()
{
SdkResolverService.Instance.InitializeForTests(new MockLoaderStrategy());
SdkResolverService.Instance.InitializeForTests(new MockLoaderStrategy(includeResolversWithPatterns: true));

SdkReference sdk = new SdkReference("notfound", "referencedVersion", "minimumVersion");

Expand All @@ -56,8 +59,10 @@ public void AssertAllResolverErrorsLoggedWhenSdkNotResolved()

_logger.BuildMessageEvents.Select(i => i.Message).ShouldContain("MockSdkResolver1 running");
_logger.BuildMessageEvents.Select(i => i.Message).ShouldContain("MockSdkResolver2 running");
_logger.Errors.Select(i => i.Message).ShouldBe(new [] { "ERROR1", "ERROR2" });
_logger.Warnings.Select(i => i.Message).ShouldBe(new[] { "WARNING2" });
_logger.BuildMessageEvents.Select(i => i.Message).ShouldNotContain("MockSdkResolverWithResolvableSdkPattern1 running");
_logger.BuildMessageEvents.Select(i => i.Message).ShouldContain("MockSdkResolverWithResolvableSdkPattern2 running");
_logger.Errors.Select(i => i.Message).ShouldBe(new [] { "ERROR4", "ERROR1", "ERROR2" });
_logger.Warnings.Select(i => i.Message).ShouldBe(new[] { "WARNING4", "WARNING2" });
}

[Fact]
Expand Down Expand Up @@ -99,7 +104,27 @@ public void AssertResolverThrows()
e.Sdk.Name.ShouldBe("1sdkName");
}


[Fact]
// Scenario: MockSdkResolverWithResolvableSdkPattern2 is a specific resolver (i.e. resolver with pattern)
// and it successfully resolves sdk.
public void AssertSecondResolverWithPatternCanResolve()
{
SdkResolverService.Instance.InitializeForTests(new MockLoaderStrategy(includeResolversWithPatterns: true));

SdkReference sdk = new SdkReference("2sdkName", "referencedVersion", "minimumVersion");

var result = SdkResolverService.Instance.ResolveSdk(BuildEventContext.InvalidSubmissionId, sdk, _loggingContext, new MockElementLocation("file"), "sln", "projectPath", interactive: false, isRunningInVisualStudio: false);

result.Path.ShouldBe("resolverpathwithresolvablesdkpattern2");
_logger.BuildMessageEvents.Select(i => i.Message).ShouldContain("MockSdkResolverWithResolvableSdkPattern2 running");
_logger.BuildMessageEvents.Select(i => i.Message).ShouldNotContain("MockSdkResolver1 running");
_logger.BuildMessageEvents.Select(i => i.Message).ShouldNotContain("MockSdkResolver2 running");
}

[Fact]
// Scenario: MockSdkResolverWithResolvableSdkPattern1 is a specific resolver, it is loaded but did not resolve sdk.
// MockSdkResolver1 is a general resolver (i.e. resolver without pattern), it resolves sdk on a fallback.
public void AssertFirstResolverCanResolve()
{
SdkResolverService.Instance.InitializeForTests(new MockLoaderStrategy());
Expand All @@ -110,6 +135,46 @@ public void AssertFirstResolverCanResolve()

result.Path.ShouldBe("resolverpath1");
_logger.BuildMessageEvents.Select(i => i.Message).ShouldContain("MockSdkResolver1 running");
_logger.BuildMessageEvents.Select(i => i.Message).ShouldNotContain("MockSdkResolverWithResolvableSdkPattern1 running");
}

[Fact]
// Scenario: MockSdkResolver1 has higher priority than MockSdkResolverWithResolvableSdkPattern1 and resolves sdk.
public void AssertFirstResolverWithPatternCantResolveChangeWave17_4()
{
using (TestEnvironment env = TestEnvironment.Create())
{
ChangeWaves.ResetStateForTests();
env.SetEnvironmentVariable("MSBUILDDISABLEFEATURESFROMVERSION", ChangeWaves.Wave17_4.ToString());
BuildEnvironmentHelper.ResetInstance_ForUnitTestsOnly();

SdkResolverService.Instance.InitializeForTests(new MockLoaderStrategy(includeResolversWithPatterns: true));

SdkReference sdk = new SdkReference("1sdkName", "referencedVersion", "minimumVersion");

var result = SdkResolverService.Instance.ResolveSdk(BuildEventContext.InvalidSubmissionId, sdk, _loggingContext, new MockElementLocation("file"), "sln", "projectPath", interactive: false, isRunningInVisualStudio: false);

result.Path.ShouldBe("resolverpath1");
_logger.BuildMessageEvents.Select(i => i.Message).ShouldContain("MockSdkResolver1 running");
_logger.BuildMessageEvents.Select(i => i.Message).ShouldNotContain("MockSdkResolverWithResolvableSdkPattern1 running");
ChangeWaves.ResetStateForTests();
}
}

[Fact]
// Scenario: MockSdkResolver1 has higher priority than MockSdkResolverWithResolvableSdkPattern1 but MockSdkResolverWithResolvableSdkPattern1 resolves sdk,
// becuase MockSdkResolver1 is general and MockSdkResolverWithResolvableSdkPattern1 is specific.
public void AssertFirstResolverWithPatternCanResolve()
{
SdkResolverService.Instance.InitializeForTests(new MockLoaderStrategy(includeResolversWithPatterns: true));

SdkReference sdk = new SdkReference("11sdkName", "referencedVersion", "minimumVersion");

var result = SdkResolverService.Instance.ResolveSdk(BuildEventContext.InvalidSubmissionId, sdk, _loggingContext, new MockElementLocation("file"), "sln", "projectPath", interactive: false, isRunningInVisualStudio: false);

result.Path.ShouldBe("resolverpathwithresolvablesdkpattern1");
_logger.BuildMessageEvents.Select(i => i.Message).ShouldContain("MockSdkResolverWithResolvableSdkPattern1 running");
_logger.BuildMessageEvents.Select(i => i.Message).ShouldNotContain("MockSdkResolver1 running");
}

[Fact]
Expand Down Expand Up @@ -539,29 +604,86 @@ public void IsRunningInVisualStudioIsSetForResolverContext()

private class MockLoaderStrategy : SdkResolverLoader
{
private readonly bool _includeErrorResolver;
private List<SdkResolver> _resolvers;
private List<(string ResolvableSdkPattern, SdkResolver Resolver)> _resolversWithPatterns;


public MockLoaderStrategy(bool includeErrorResolver = false)
public MockLoaderStrategy(bool includeErrorResolver = false, bool includeResolversWithPatterns = false) : this()
{
_includeErrorResolver = includeErrorResolver;
if (includeErrorResolver)
{
_resolvers.Add(new MockSdkResolverThrows());
}

if (includeResolversWithPatterns)
{
_resolversWithPatterns.Add(("1.*", new MockSdkResolverWithResolvableSdkPattern1()));
_resolversWithPatterns.Add((".*", new MockSdkResolverWithResolvableSdkPattern2()));
}
}

internal override IList<SdkResolver> LoadResolvers(LoggingContext loggingContext, ElementLocation location)
private MockLoaderStrategy()
{
List<SdkResolver> resolvers = new List<SdkResolver>
_resolvers = new List<SdkResolver>
{
new MockSdkResolver1(),
new MockSdkResolver2(),
new MockResolverReturnsNull(),
new MockSdkResolverWithState()
};

if (_includeErrorResolver)
_resolversWithPatterns = new List<(string ResolvableSdkPattern, SdkResolver Resolver)>();
}

internal override IList<SdkResolver> LoadAllResolvers(LoggingContext loggingContext, ElementLocation location)
{
return _resolvers.OrderBy(i => i.Priority).ToList();
}

internal override IList<SdkResolverManifest> GetResolversManifests(LoggingContext loggingContext,
ElementLocation location)
{
var manifests = new List<SdkResolverManifest>();
foreach(SdkResolver resolver in _resolvers)
{
SdkResolverManifest sdkResolverManifest = new SdkResolverManifest(resolver.Name, null, null);
manifests.Add(sdkResolverManifest);
}
foreach ((string ResolvableSdkPattern, SdkResolver Resolver) pair in _resolversWithPatterns)
{
SdkResolverManifest sdkResolverManifest = new SdkResolverManifest(
pair.Resolver.Name,
null,
new Regex(pair.ResolvableSdkPattern, RegexOptions.Compiled | RegexOptions.CultureInvariant, TimeSpan.FromMilliseconds(500))
);
manifests.Add(sdkResolverManifest);
}
return manifests;
}

protected internal override IList<SdkResolver> LoadResolversFromManifest(SdkResolverManifest manifest, LoggingContext loggingContext, ElementLocation location)
{
var resolvers = new List<SdkResolver>();
foreach (var resolver in _resolvers)
{
if (resolver.Name == manifest.Name)
{
resolvers.Add(resolver);
}
}
foreach (var pair in _resolversWithPatterns)
{
resolvers.Add(new MockSdkResolverThrows());
if (pair.Resolver.Name == manifest.Name)
{
resolvers.Add(pair.Resolver);
}
}
return resolvers.OrderBy(t => t.Priority).ToList();
}

return resolvers.OrderBy(i => i.Priority).ToList();
internal override IList<SdkResolver> LoadDefaultResolvers(LoggingContext loggingContext, ElementLocation location)
{
return new List<SdkResolver>();
}
}

Expand All @@ -587,7 +709,7 @@ public override SdkResultBase Resolve(SdkReference sdk, SdkResolverContextBase r
if (sdk.Name.StartsWith("1"))
return factory.IndicateSuccess("resolverpath1", "version1");

return factory.IndicateFailure(new[] {"ERROR1"});
return factory.IndicateFailure(new[] { "ERROR1" });
}
}

Expand All @@ -608,6 +730,40 @@ public override SdkResultBase Resolve(SdkReference sdk, SdkResolverContextBase r
}
}

private class MockSdkResolverWithResolvableSdkPattern1 : SdkResolver
{
public override string Name => nameof(MockSdkResolverWithResolvableSdkPattern1);

public override int Priority => 2;

public override SdkResultBase Resolve(SdkReference sdk, SdkResolverContextBase resolverContext, SdkResultFactoryBase factory)
{
resolverContext.Logger.LogMessage("MockSdkResolverWithResolvableSdkPattern1 running", MessageImportance.Normal);

if (sdk.Name.StartsWith("11"))
return factory.IndicateSuccess("resolverpathwithresolvablesdkpattern1", "version3");

return factory.IndicateFailure(new[] { "ERROR3" });
}
}

private class MockSdkResolverWithResolvableSdkPattern2 : SdkResolver
{
public override string Name => nameof(MockSdkResolverWithResolvableSdkPattern2);

public override int Priority => 0;

public override SdkResultBase Resolve(SdkReference sdk, SdkResolverContextBase resolverContext, SdkResultFactoryBase factory)
{
resolverContext.Logger.LogMessage("MockSdkResolverWithResolvableSdkPattern2 running", MessageImportance.Normal);

if (sdk.Name.StartsWith("2"))
return factory.IndicateSuccess("resolverpathwithresolvablesdkpattern2", "version4", new[] { "WARNING4" });

return factory.IndicateFailure(new[] { "ERROR4" }, new[] { "WARNING4" });
}
}

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