Resolve SDKs using an ISdkResolverService (#2847)

A singleton instance handles all resolution. A service hosted in the main node handles build related evaluation on the main node and a service hosted in the out-of-proc node sends requests to the main node for processing.

A cache exists on the main node so that requests are only resolved once per build. The out-of-proc node also caches responses so that an SDK is only resolved once.

Non-build evaluations are not cached and could be slow since there is no way to know if two evaluations are related.

Address the first step in #2803

* Log a warning if different versions of the same SDK are specified
* Log a warning if the SDK version specified does not match what was resolved
* Improve Microsoft.Build.UnitTests.Preprocessor.Preprocessor_Tests.SdkImportsAreInPreprocessedOutput by using TestEnvironment to mock SDK resolution
* Add escape hatch to disable caching the result on the main node `MSBUILDDISABLESDKCACHE`

src/Build.UnitTests/BackEnd/MockHost.cs

@@ -10,6 +10,8 @@
 using Microsoft.Build.UnitTests.BackEnd;
 using System;
 using System.Threading;
+using Microsoft.Build.BackEnd.SdkResolution;
+using Microsoft.Build.Engine.UnitTests.BackEnd;
 using Microsoft.Build.Evaluation;
 using Microsoft.Build.Execution;
 using LegacyThreadingData = Microsoft.Build.Execution.LegacyThreadingData;
@@ -61,6 +63,8 @@ internal class MockHost : MockLoggingService, IBuildComponentHost, IBuildCompone
         /// </summary>
         private LegacyThreadingData _legacyThreadingData;
 
+        private ISdkResolverService _sdkResolverService;
+
         #region SystemParameterFields
 
         #endregion;
@@ -101,6 +105,9 @@ public MockHost(BuildParameters buildParameters)
 
             _targetBuilder = new TestTargetBuilder();
             ((IBuildComponent)_targetBuilder).InitializeComponent(this);
+
+            _sdkResolverService = new MockSdkResolverService();
+            ((IBuildComponent)_sdkResolverService).InitializeComponent(this);
         }
 
         /// <summary>
@@ -180,6 +187,9 @@ public IBuildComponent GetComponent(BuildComponentType type)
                 case BuildComponentType.RequestBuilder:
                     return (IBuildComponent)_requestBuilder;
 
+                case BuildComponentType.SdkResolverService:
+                    return (IBuildComponent)_sdkResolverService;
+
                 default:
                     throw new ArgumentException("Unexpected type " + type);
             }
@@ -225,4 +235,4 @@ public void ShutdownComponent()
 
         #endregion
     }
-}
+}

src/Build.UnitTests/BackEnd/MockSdkResolverService.cs

@@ -0,0 +1,31 @@
+using Microsoft.Build.BackEnd;
+using Microsoft.Build.BackEnd.Logging;
+using Microsoft.Build.BackEnd.SdkResolution;
+using Microsoft.Build.Construction;
+using Microsoft.Build.Framework;
+using System;
+
+namespace Microsoft.Build.Engine.UnitTests.BackEnd
+{
+    internal class MockSdkResolverService : IBuildComponent, ISdkResolverService
+    {
+        public Action<INodePacket> SendPacket { get; }
+
+        public void ClearCache(int submissionId)
+        {
+        }
+
+        public string ResolveSdk(int submissionId, SdkReference sdk, LoggingContext loggingContext, ElementLocation sdkReferenceLocation, string solutionPath, string projectPath)
+        {
+            return null;
+        }
+
+        public void InitializeComponent(IBuildComponentHost host)
+        {
+        }
+
+        public void ShutdownComponent()
+        {
+        }
+    }
+}

src/Build.UnitTests/BackEnd/SdkResolverLoader_Tests.cs

@@ -1,6 +1,7 @@
 using System.IO;
 using System.Text;
 using Microsoft.Build.BackEnd;
+using Microsoft.Build.BackEnd.SdkResolution;
 using Microsoft.Build.Framework;
 using Microsoft.Build.Shared;
 using Microsoft.Build.UnitTests;

src/Build.UnitTests/BackEnd/SdkResolution_Tests.cs → src/Build.UnitTests/BackEnd/SdkResolverService_Tests.cs

@@ -1,39 +1,75 @@
-using System;
-using Microsoft.Build.BackEnd;
-using Microsoft.Build.BackEnd.Logging;
+using Microsoft.Build.BackEnd.Logging;
+using Microsoft.Build.BackEnd.SdkResolution;
 using Microsoft.Build.Construction;
 using Microsoft.Build.Framework;
 using Microsoft.Build.UnitTests;
 using Microsoft.Build.UnitTests.BackEnd;
+using System;
 using System.Collections.Generic;
 using System.Linq;
 using System.Text;
+using Shouldly;
 using Xunit;
+using SdkResolverContextBase = Microsoft.Build.Framework.SdkResolverContext;
+using SdkResultBase = Microsoft.Build.Framework.SdkResult;
+using SdkResultFactoryBase = Microsoft.Build.Framework.SdkResultFactory;
 
 namespace Microsoft.Build.Engine.UnitTests.BackEnd
 {
-    public class SdkResolution_Tests
+    public class SdkResolverService_Tests
     {
         private readonly StringBuilder _log;
         private readonly MockLoggingContext _loggingContext;
 
-        public SdkResolution_Tests()
+        public SdkResolverService_Tests()
         {
             _log = new StringBuilder();
 
-            var logger = new MockLoggingService(message => _log.AppendLine(message));
-            var bec = new BuildEventContext(0, 0, 0, 0, 0);
+            MockLoggingService logger = new MockLoggingService(message => _log.AppendLine(message));
+            BuildEventContext bec = new BuildEventContext(0, 0, 0, 0, 0);
 
             _loggingContext = new MockLoggingContext(logger, bec);
         }
 
+        [Fact]
+        public void AssertAllResolverErrorsLoggedWhenSdkNotResolved()
+        {
+            SdkResolverService.Instance.InitializeForTests(new MockLoaderStrategy());
+
+            SdkReference sdk = new SdkReference("notfound", "referencedVersion", "minimumVersion");
+
+            string result = SdkResolverService.Instance.ResolveSdk(BuildEventContext.InvalidSubmissionId, sdk, _loggingContext, new MockElementLocation("file"), "sln", "projectPath");
+
+            string logResult = _log.ToString();
+            Assert.Null(result);
+            Assert.Contains("MockSdkResolver1 running", logResult);
+            Assert.Contains("MockSdkResolver2 running", logResult);
+            Assert.Contains("ERROR1", logResult);
+            Assert.Contains("ERROR2", logResult);
+            Assert.Contains("WARNING2", logResult);
+        }
+
+        [Fact]
+        public void AssertErrorLoggedWhenResolverThrows()
+        {
+            SdkResolverService.Instance.InitializeForTests(new MockLoaderStrategy(includeErrorResolver: true));
+
+            SdkReference sdk = new SdkReference("1sdkName", "referencedVersion", "minimumVersion");
+
+            string result = SdkResolverService.Instance.ResolveSdk(BuildEventContext.InvalidSubmissionId, sdk, _loggingContext, new MockElementLocation("file"), "sln", "projectPath");
+
+            Assert.Equal("resolverpath1", result);
+            Assert.Contains("EXMESSAGE", _log.ToString());
+        }
+
         [Fact]
         public void AssertFirstResolverCanResolve()
         {
-            var sdk = new SdkReference("1sdkName", "referencedVersion", "minimumVersion");
-
-            SdkResolution resolution = new SdkResolution(new MockLoaderStrategy());
-            var result = resolution.GetSdkPath(sdk, _loggingContext, new MockElementLocation("file"), "sln", "projectPath");
+            SdkResolverService.Instance.InitializeForTests(new MockLoaderStrategy());
+
+            SdkReference sdk = new SdkReference("1sdkName", "referencedVersion", "minimumVersion");
+
+            string result = SdkResolverService.Instance.ResolveSdk(BuildEventContext.InvalidSubmissionId, sdk, _loggingContext, new MockElementLocation("file"), "sln", "projectPath");
 
             Assert.Equal("resolverpath1", result);
             Assert.Equal("MockSdkResolver1 running", _log.ToString().Trim());
@@ -42,17 +78,15 @@ public void AssertFirstResolverCanResolve()
         [Fact]
         public void AssertFirstResolverErrorsSupressedWhenResolved()
         {
-            // 2sdkName will cause MockSdkResolver1 to fail with an error reason. The error will not 
+            SdkResolverService.Instance.InitializeForTests(new MockLoaderStrategy());
+
+            // 2sdkName will cause MockSdkResolver1 to fail with an error reason. The error will not
             // be logged because MockSdkResolver2 will succeed.
-            var log = new StringBuilder();
-            var sdk = new SdkReference("2sdkName", "referencedVersion", "minimumVersion");
-            var logger = new MockLoggingService(message => log.AppendLine(message));
-            var bec = new BuildEventContext(0, 0, 0, 0, 0);
+            SdkReference sdk = new SdkReference("2sdkName", "referencedVersion", "minimumVersion");
 
-            SdkResolution resolution = new SdkResolution(new MockLoaderStrategy());
-            var result = resolution.GetSdkPath(sdk, _loggingContext, new MockElementLocation("file"), "sln", "projectPath");
+            string result = SdkResolverService.Instance.ResolveSdk(BuildEventContext.InvalidSubmissionId, sdk, _loggingContext, new MockElementLocation("file"), "sln", "projectPath");
 
-            var logResult = _log.ToString();
+            string logResult = _log.ToString();
             Assert.Equal("resolverpath2", result);
 
             // Both resolvers should run, and no ERROR string.
@@ -64,39 +98,21 @@ public void AssertFirstResolverErrorsSupressedWhenResolved()
             Assert.DoesNotContain("ERROR", logResult);
         }
 
-        [Fact]
-        public void AssertAllResolverErrorsLoggedWhenSdkNotResolved()
+        [Theory]
+        [InlineData(null, "1.0", true)]
+        [InlineData("1.0", "1.0", true)]
+        [InlineData("1.0-preview", "1.0-PrEvIeW", true)]
+        [InlineData("1.0", "1.0.0", false)]
+        [InlineData("1.0", "1.0.0.0", false)]
+        [InlineData("1.0.0", "1.0.0.0", false)]
+        [InlineData("1.2.0.0", "1.0.0.0", false)]
+        [InlineData("1.2.3.0", "1.2.0.0", false)]
+        [InlineData("1.2.3.4", "1.2.3.0", false)]
+        public void IsReferenceSameVersionTests(string version1, string version2, bool expected)
         {
-            var log = new StringBuilder();
-            var sdk = new SdkReference("notfound", "referencedVersion", "minimumVersion");
-            var logger = new MockLoggingService(message => log.AppendLine(message));
-            var bec = new BuildEventContext(0, 0, 0, 0, 0);
-
-            SdkResolution resolution = new SdkResolution(new MockLoaderStrategy());
-            var result = resolution.GetSdkPath(sdk, _loggingContext, new MockElementLocation("file"), "sln", "projectPath");
+            SdkReference sdk = new SdkReference("Microsoft.NET.Sdk", version1, null);
 
-            var logResult = _log.ToString();
-            Assert.Null(result);
-            Assert.Contains("MockSdkResolver1 running", logResult);
-            Assert.Contains("MockSdkResolver2 running", logResult);
-            Assert.Contains("ERROR1", logResult);
-            Assert.Contains("ERROR2", logResult);
-            Assert.Contains("WARNING2", logResult);
-        }
-
-        [Fact]
-        public void AssertErrorLoggedWhenResolverThrows()
-        {
-            var log = new StringBuilder();
-            var sdk = new SdkReference("1sdkName", "referencedVersion", "minimumVersion");
-            var logger = new MockLoggingService(message => log.AppendLine(message));
-            var bec = new BuildEventContext(0, 0, 0, 0, 0);
-
-            SdkResolution resolution = new SdkResolution(new MockLoaderStrategy(true));
-            var result = resolution.GetSdkPath(sdk, _loggingContext, new MockElementLocation("file"), "sln", "projectPath");
-
-            Assert.Equal("resolverpath1", result);
-            Assert.Contains("EXMESSAGE", _log.ToString());
+            SdkResolverService.IsReferenceSameVersion(sdk, version2).ShouldBe(expected);
         }
 
         private class MockLoaderStrategy : SdkResolverLoader
@@ -126,13 +142,22 @@ internal override IList<SdkResolver> LoadResolvers(LoggingContext loggingContext
             }
         }
 
+        private class MockResolverReturnsNull : SdkResolver
+        {
+            public override string Name => nameof(MockResolverReturnsNull);
+
+            public override int Priority => -1;
+
+            public override SdkResultBase Resolve(SdkReference sdkReference, SdkResolverContextBase resolverContext, SdkResultFactoryBase factory) => null;
+        }
+
         private class MockSdkResolver1 : SdkResolver
         {
             public override string Name => nameof(MockSdkResolver1);
 
             public override int Priority => 1;
 
-            public override SdkResult Resolve(SdkReference sdk, SdkResolverContext resolverContext, SdkResultFactory factory)
+            public override SdkResultBase Resolve(SdkReference sdk, SdkResolverContextBase resolverContext, SdkResultFactoryBase factory)
             {
                 resolverContext.Logger.LogMessage("MockSdkResolver1 running", MessageImportance.Normal);
 
@@ -149,14 +174,14 @@ private class MockSdkResolver2 : SdkResolver
 
             public override int Priority => 2;
 
-            public override SdkResult Resolve(SdkReference sdk, SdkResolverContext resolverContext, SdkResultFactory factory)
+            public override SdkResultBase Resolve(SdkReference sdk, SdkResolverContextBase resolverContext, SdkResultFactoryBase factory)
             {
                 resolverContext.Logger.LogMessage("MockSdkResolver2 running", MessageImportance.Normal);
 
                 if (sdk.Name.StartsWith("2"))
                     return factory.IndicateSuccess("resolverpath2", "version2", new[] {"WARNING2"});
 
-                return factory.IndicateFailure(new[] { "ERROR2" }, new[] { "WARNING2" });
+                return factory.IndicateFailure(new[] {"ERROR2"}, new[] {"WARNING2"});
             }
         }
 
@@ -165,21 +190,12 @@ private class MockSdkResolverThrows : SdkResolver
             public override string Name => nameof(MockSdkResolverThrows);
             public override int Priority => 0;
 
-            public override SdkResult Resolve(SdkReference sdk, SdkResolverContext resolverContext, SdkResultFactory factory)
+            public override SdkResultBase Resolve(SdkReference sdk, SdkResolverContextBase resolverContext, SdkResultFactoryBase factory)
             {
                 resolverContext.Logger.LogMessage("MockSdkResolverThrows running", MessageImportance.Normal);
 
                 throw new ArithmeticException("EXMESSAGE");
             }
         }
-
-        private class MockResolverReturnsNull : SdkResolver
-        {
-            public override string Name => nameof(MockResolverReturnsNull);
-
-            public override int Priority => -1;
-
-            public override SdkResult Resolve(SdkReference sdkReference, SdkResolverContext resolverContext, SdkResultFactory factory) => null;
-        }
     }
 }

src/Build.UnitTests/BackEnd/TargetBuilder_Tests.cs

@@ -26,6 +26,8 @@
 using NodeLoggingContext = Microsoft.Build.BackEnd.Logging.NodeLoggingContext;
 using LegacyThreadingData = Microsoft.Build.Execution.LegacyThreadingData;
 using System.Threading.Tasks;
+using Microsoft.Build.BackEnd.SdkResolution;
+using Microsoft.Build.Engine.UnitTests.BackEnd;
 using Xunit;
 
 namespace Microsoft.Build.UnitTests.BackEnd
@@ -1533,6 +1535,8 @@ private class MockHost : MockLoggingService, IBuildComponentHost, IBuildComponen
             /// </summary>
             private LegacyThreadingData _legacyThreadingData;
 
+            private ISdkResolverService _sdkResolverService;
+
             /// <summary>
             /// Constructor
             /// </summary>
@@ -1557,6 +1561,9 @@ public MockHost()
 
                 _targetBuilder = new TargetBuilder();
                 ((IBuildComponent)_targetBuilder).InitializeComponent(this);
+
+                _sdkResolverService = new MockSdkResolverService();
+                ((IBuildComponent)_sdkResolverService).InitializeComponent(this);
             }
 
             /// <summary>
@@ -1631,6 +1638,9 @@ public IBuildComponent GetComponent(BuildComponentType type)
                     case BuildComponentType.TargetBuilder:
                         return (IBuildComponent)_targetBuilder;
 
+                    case BuildComponentType.SdkResolverService:
+                        return (IBuildComponent)_sdkResolverService;
+
                     default:
                         throw new ArgumentException("Unexpected type " + type);
                 }

src/Build.UnitTests/BackEnd/TargetEntry_Tests.cs

@@ -14,7 +14,9 @@
 using System.Xml;
 using Microsoft.Build.BackEnd;
 using Microsoft.Build.BackEnd.Logging;
+using Microsoft.Build.BackEnd.SdkResolution;
 using Microsoft.Build.Collections;
+using Microsoft.Build.Engine.UnitTests.BackEnd;
 using Microsoft.Build.Evaluation;
 using Microsoft.Build.Execution;
 using Microsoft.Build.Framework;
@@ -1224,6 +1226,8 @@ private class MockHost : MockLoggingService, IBuildComponentHost, IBuildComponen
             /// </summary>
             private LegacyThreadingData _legacyThreadingData;
 
+            private ISdkResolverService _sdkResolverService;
+
             /// <summary>
             /// Constructor
             /// </summary>
@@ -1245,6 +1249,9 @@ public MockHost()
 
                 _taskBuilder = new MockTaskBuilder();
                 ((IBuildComponent)_taskBuilder).InitializeComponent(this);
+
+                _sdkResolverService = new MockSdkResolverService();
+                ((IBuildComponent)_sdkResolverService).InitializeComponent(this);
             }
 
             /// <summary>
@@ -1316,6 +1323,9 @@ public IBuildComponent GetComponent(BuildComponentType type)
                     case BuildComponentType.TaskBuilder:
                         return (IBuildComponent)_taskBuilder;
 
+                    case BuildComponentType.SdkResolverService:
+                        return (IBuildComponent)_sdkResolverService;
+
                     default:
                         throw new ArgumentException("Unexpected type " + type);
                 }