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

* Add NamePattern field to sdk resolver manifest. * Add changewave 17.3 and move the new sdk-resolving algorithm under it. * Refactor resolver finding. * Add new resolving algo. * Fix tests. * Fix resolver algo. * Address comments. * Add comments. * Add spec for the new sdk resolvers algo. * Add PR link to changewaves wiki; adjust a unit test. * Address comments. * Update src/Build/BackEnd/Components/SdkResolution/SdkResolverLoader.cs Co-authored-by: Rainer Sigwald <[email protected]> * Fix MSBuildToolsDirectoryRoot when BuildEnvironmentMode is None. * Prefer manifest file over assembly. * Address some feedback - 1. * Address some feedback - 2. * Update event-source.md * Update my exceptions. * address comments Co-authored-by: Rainer Sigwald <[email protected]>
dotnet · May 25, 2022 · b8d947f · b8d947f
1 parent 5c5e583
commit b8d947f
Show file tree

Hide file tree

Showing 10 changed files with 656 additions and 57 deletions.
diff --git a/documentation/specs/event-source.md b/documentation/specs/event-source.md
@@ -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. |

diff --git a/documentation/specs/sdk-resolvers-algorithm.md b/documentation/specs/sdk-resolvers-algorithm.md
@@ -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)`.
diff --git a/documentation/wiki/ChangeWaves.md b/documentation/wiki/ChangeWaves.md
@@ -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)

diff --git a/src/Build.UnitTests/BackEnd/SdkResolverLoader_Tests.cs b/src/Build.UnitTests/BackEnd/SdkResolverLoader_Tests.cs
@@ -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 });
 
@@ -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>
@@ -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");
@@ -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.");
@@ -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}");
@@ -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()
         {
@@ -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);
                 }