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 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
4 changes: 3 additions & 1 deletion documentation/specs/event-source.md
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,9 @@ EventSource is primarily used to profile code. For MSBuild specifically, a major
| ReusableStringBuilderFactoryUnbalanced | Identifies improper usage from multiple threads or buggy code: multiple Gets were called without a Relase. |
| Save | Saves a project to the file system if dirty, creating directories as necessary. |
| SdkResolverResolveSdk | A single SDK resolver is called. |
| SdkResolverServiceInitialize | Initializes SDK resolvers. |
| SdkResolverServiceFindResolversManifests | Find all resolvers manifests. (Only appear under Changewave 17.4.) |
| SdkResolverServiceInitialize | Initializes SDK resolvers. (Only appear before Changewave 17.4.) |
| SdkResolverServiceLoadResolvers | Load resolvers given a resolver manifest. (Only appear under Changewave 17.4.) |
| SdkResolverEvent | An SDK resolver logs an event. |
| Target | Executes a target. |
| TargetUpToDate | Checks whether a particular target needs to run or is up-to-date. |
Expand Down
28 changes: 28 additions & 0 deletions documentation/specs/sdk-resolvers-algorithm.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
## 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 sdk name pattern `ResolvableSdkPattern`
- General resolvers, i.e. resolvers without specified sdk name pattern `ResolvableSdkPattern`

The resolving algorithm works in two passes.
- On the first pass all the specific resolvers that match the given sdk name would be loaded, ordered by priority and tried one after one.
- If the sdk is not found, on the second pass all general resolvers would be loaded, 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 `ResolvableSdkPattern` using C# regex format:
```
<SdkResolver>
<Path>MySdkResolver.dll</Path>
<ResolvableSdkPattern>MySdk.*</ResolvableSdkPattern>
</SdkResolver>
```

Note, that the manifest file, if exists, from ChangeWave 17.4 would have preference over the dll.
The sdk discovery works according to the following algorithm:
- First try locate the manifest file and use it.
- If it is not found, we try to locate the dll in the resolver's folder.
Both xml and dll name should match the following name pattern `...\SdkResolvers\(ResolverName)\(ResolverName).(xml/dll)`.
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
69 changes: 64 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 @@ -88,6 +88,38 @@ public void VerifySdkResolverLoaderFileDiscoveryPattern()
}
}

[Fact]
public void SdkResolverLoaderPrefersManifestFile()
{
var root = FileUtilities.GetTemporaryDirectory();
try
{
var testFolder = Directory.CreateDirectory(Path.Combine(root, "MyTestResolver"));

var wrongResolverDll = Path.Combine(testFolder.FullName, "MyTestResolver.dll");
var resolverManifest = Path.Combine(testFolder.FullName, "MyTestResolver.xml");
var assemblyToLoad = Path.Combine(root, "SomeOtherResolver.dll");

File.WriteAllText(wrongResolverDll, string.Empty);
File.WriteAllText(assemblyToLoad, string.Empty);

File.WriteAllText(resolverManifest, $@"
<SdkResolver>
<Path>{assemblyToLoad}</Path>
</SdkResolver>");

SdkResolverLoader loader = new SdkResolverLoader();
var resolversFound = loader.FindPotentialSdkResolvers(root, new MockElementLocation("file"));

resolversFound.Count.ShouldBe(1);
resolversFound.First().ShouldBe(assemblyToLoad);
}
finally
{
FileUtilities.DeleteDirectoryNoThrow(root, true);
}
}

/// <summary>
/// Verifies that if an SDK resolver throws while creating an instance that a warning is logged.
/// </summary>
Expand All @@ -106,7 +138,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 +170,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 +201,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 +239,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 +346,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
Loading