diff --git a/src/Build.UnitTests/BackEnd/SdkResolverService_Tests.cs b/src/Build.UnitTests/BackEnd/SdkResolverService_Tests.cs index 831ec4a4091..d15bd7d203d 100644 --- a/src/Build.UnitTests/BackEnd/SdkResolverService_Tests.cs +++ b/src/Build.UnitTests/BackEnd/SdkResolverService_Tests.cs @@ -24,7 +24,7 @@ namespace Microsoft.Build.Engine.UnitTests.BackEnd { - public class SdkResolverService_Tests + public class SdkResolverService_Tests : IDisposable { private readonly MockLogger _logger; private readonly LoggingContext _loggingContext; @@ -40,6 +40,11 @@ public SdkResolverService_Tests() new BuildEventContext(0, 0, BuildEventContext.InvalidProjectContextId, 0, 0)); } + public void Dispose() + { + SdkResolverService.Instance.InitializeForTests(); + } + [Fact] // Scenario: Sdk is not resolved. public void AssertAllResolverErrorsLoggedWhenSdkNotResolved() @@ -252,6 +257,29 @@ public void AssertResolverStateNotPreserved() SdkResolverService.Instance.ResolveSdk(submissionId, sdk, _loggingContext, new MockElementLocation("file"), "sln", "projectPath", interactive: false, isRunningInVisualStudio: false, failOnUnresolvedSdk: true).Path.ShouldBe("resolverpath"); } + [Fact] + public void AssertResolversLoadedIfDefaultResolverSucceeds() + { + const int submissionId = BuildEventContext.InvalidSubmissionId; + + MockLoaderStrategy mockLoaderStrategy = new MockLoaderStrategy(includeDefaultResolver: true); + SdkResolverService.Instance.InitializeForTests(mockLoaderStrategy); + + SdkReference sdk = new SdkReference("notfound", "1.0", "minimumVersion"); + + SdkResolverService.Instance.ResolveSdk(submissionId, sdk, _loggingContext, new MockElementLocation("file"), "sln", "projectPath", interactive: false, isRunningInVisualStudio: false, failOnUnresolvedSdk: true).Path.ShouldBe("defaultpath"); + +#if NETCOREAPP + // On Core, we check the default resolver *first*, so regular resolvers are not loaded. + mockLoaderStrategy.ResolversHaveBeenLoaded.ShouldBeFalse(); + mockLoaderStrategy.ManifestsHaveBeenLoaded.ShouldBeFalse(); +#else + // On Framework, the default resolver is a fallback, so regular resolvers will have been loaded. + mockLoaderStrategy.ResolversHaveBeenLoaded.ShouldBeTrue(); + mockLoaderStrategy.ManifestsHaveBeenLoaded.ShouldBeTrue(); +#endif + } + [Theory] [InlineData(null, "1.0", true)] [InlineData("1.0", "1.0", true)] @@ -622,10 +650,13 @@ public void IsRunningInVisualStudioIsSetForResolverContext() private sealed class MockLoaderStrategy : SdkResolverLoader { private List _resolvers; + private List _defaultResolvers; private List<(string ResolvableSdkPattern, SdkResolver Resolver)> _resolversWithPatterns; + public bool ResolversHaveBeenLoaded { get; private set; } = false; + public bool ManifestsHaveBeenLoaded { get; private set; } = false; - public MockLoaderStrategy(bool includeErrorResolver = false, bool includeResolversWithPatterns = false) : this() + public MockLoaderStrategy(bool includeErrorResolver = false, bool includeResolversWithPatterns = false, bool includeDefaultResolver = false) : this() { if (includeErrorResolver) { @@ -637,6 +668,11 @@ public MockLoaderStrategy(bool includeErrorResolver = false, bool includeResolve _resolversWithPatterns.Add(("1.*", new MockSdkResolverWithResolvableSdkPattern1())); _resolversWithPatterns.Add((".*", new MockSdkResolverWithResolvableSdkPattern2())); } + + if (includeDefaultResolver) + { + _defaultResolvers.Add(new MockSdkResolverDefault()); + } } private MockLoaderStrategy() @@ -649,16 +685,22 @@ private MockLoaderStrategy() new MockSdkResolverWithState() }; + _defaultResolvers = new List(); + _resolversWithPatterns = new List<(string ResolvableSdkPattern, SdkResolver Resolver)>(); } internal override IReadOnlyList LoadAllResolvers(ElementLocation location) { + ResolversHaveBeenLoaded = true; + return _resolvers.OrderBy(i => i.Priority).ToList(); } internal override IReadOnlyList GetResolversManifests(ElementLocation location) { + ManifestsHaveBeenLoaded = true; + var manifests = new List(); foreach (SdkResolver resolver in _resolvers) { @@ -678,6 +720,8 @@ internal override IReadOnlyList GetResolversManifests(Eleme protected internal override IReadOnlyList LoadResolversFromManifest(SdkResolverManifest manifest, ElementLocation location) { + ResolversHaveBeenLoaded = true; + var resolvers = new List(); foreach (var resolver in _resolvers) { @@ -698,7 +742,7 @@ protected internal override IReadOnlyList LoadResolversFromManifest internal override IReadOnlyList GetDefaultResolvers() { - return new List(); + return _defaultResolvers; } } @@ -824,5 +868,18 @@ public override SdkResultBase Resolve(SdkReference sdk, SdkResolverContextBase r throw new ArithmeticException("EXMESSAGE"); } } + + private sealed class MockSdkResolverDefault : SdkResolver + { + public override string Name => nameof(MockSdkResolverDefault); + public override int Priority => 9999; + + public override SdkResultBase Resolve(SdkReference sdk, SdkResolverContextBase resolverContext, SdkResultFactoryBase factory) + { + resolverContext.Logger.LogMessage("MockSdkResolverDefault running", MessageImportance.Normal); + + return factory.IndicateSuccess("defaultpath", string.Empty); + } + } } } diff --git a/src/Build.UnitTests/Evaluation/ProjectSdkImplicitImport_Tests.cs b/src/Build.UnitTests/Evaluation/ProjectSdkImplicitImport_Tests.cs index 7f026071d8c..3b46c282d24 100644 --- a/src/Build.UnitTests/Evaluation/ProjectSdkImplicitImport_Tests.cs +++ b/src/Build.UnitTests/Evaluation/ProjectSdkImplicitImport_Tests.cs @@ -624,6 +624,9 @@ internal void SdkPropertiesAreExpanded(SdkPropertiesAreExpandedCase data) { _env.SetEnvironmentVariable("MSBuildSDKsPath", _testSdkRoot); _env.SetEnvironmentVariable("MSBUILD_SDKREFERENCE_PROPERTY_EXPANSION_MODE", data.Mode.ToString()); + _env.SetEnvironmentVariable("MSBUILDINCLUDEDEFAULTSDKRESOLVER", "false"); + + Build.BackEnd.SdkResolution.CachingSdkResolverLoader.ResetStateForTests(); File.WriteAllText(_sdkPropsPath, _sdkPropsContent); File.WriteAllText(_sdkTargetsPath, _sdkTargetsContent); @@ -804,6 +807,7 @@ public override SdkResult Resolve(SdkReference sdk, SdkResolverContext resolverC public void Dispose() { _env.Dispose(); + Build.BackEnd.SdkResolution.CachingSdkResolverLoader.ResetStateForTests(); } private void VerifyPropertyFromImplicitImport(Project project, string propertyName, string expectedContainingProjectPath, string expectedValue) diff --git a/src/Build/BackEnd/Components/SdkResolution/CachingSdkResolverLoader.cs b/src/Build/BackEnd/Components/SdkResolution/CachingSdkResolverLoader.cs index 4c765982f88..3e2536873a7 100644 --- a/src/Build/BackEnd/Components/SdkResolution/CachingSdkResolverLoader.cs +++ b/src/Build/BackEnd/Components/SdkResolution/CachingSdkResolverLoader.cs @@ -57,6 +57,15 @@ public CachingSdkResolverLoader() _defaultResolvers = base.GetDefaultResolvers(); } + /// + /// Resets the cached state, intended for tests only. + /// + internal static void ResetStateForTests() + { + // Re-create the singleton to pick up environmental changes. + Instance = new CachingSdkResolverLoader(); + } + #region SdkResolverLoader overrides /// @@ -87,5 +96,6 @@ protected internal override IReadOnlyList LoadResolversFromManifest } #endregion + } } diff --git a/src/Build/BackEnd/Components/SdkResolution/SdkResolverService.cs b/src/Build/BackEnd/Components/SdkResolution/SdkResolverService.cs index 79c899b4dc2..56ff6299a87 100644 --- a/src/Build/BackEnd/Components/SdkResolution/SdkResolverService.cs +++ b/src/Build/BackEnd/Components/SdkResolution/SdkResolverService.cs @@ -118,6 +118,53 @@ public virtual void ClearCaches() /// public virtual SdkResult ResolveSdk(int submissionId, SdkReference sdk, LoggingContext loggingContext, ElementLocation sdkReferenceLocation, string solutionPath, string projectPath, bool interactive, bool isRunningInVisualStudio, bool failOnUnresolvedSdk) { + // If we are running in .NET core, we ask the built-in default resolver first. + // - It is a perf optimization (no need to discover and load any of the plug-in assemblies to resolve an "in-box" Sdk). + // - It brings `dotnet build` to parity with `MSBuild.exe` functionally, as the Framework build of Microsoft.DotNet.MSBuildSdkResolver + // contains the same logic and it is the first resolver in priority order. + // + // In an attempt to avoid confusion, this text uses "SDK" to refer to the installation unit, e.g. "C:\Program Files\dotnet\sdk\8.0.100", + // and "Sdk" to refer to the set of imports for targeting a specific project type, e.g. "Microsoft.NET.Sdk.Web". + // + // Here's the flow on Framework (`MSBuild.exe`): + // 1. Microsoft.DotNet.MSBuildSdkResolver is loaded and asked to resolve the Sdk required by the project. + // 1.1. It resolves the SDK (as in installation directory) using machine-wide state and global.json. + // 1.2. It checks the Sdks subdirectory of the SDK installation directory for a matching in-box Sdk. + // 1.3. If no match, checks installed workloads. + // 2. If no match so far, Microsoft.Build.NuGetSdkResolver is loaded and asked to resolve the Sdk. + // 3. If no match still, DefaultSdkResolver checks the Sdks subdirectory of the Visual Studio\MSBuild directory. + // + // Here's the flow on Core (`dotnet build`): + // 1. DefaultSdkResolver checks the Sdks subdirectory of our SDK installation. Note that the work of resolving the + // SDK version using machine-wide state and global.json (step 1.1. in `MSBuild.exe` above) has already been done + // by the `dotnet` muxer. We know which SDK (capital letters) we are in, so the in-box Sdk lookup is trivial. + // 2. If no match, Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver is loaded and asked to resolve the Sdk required by the project. + // 2.1. It checks installed workloads. + // 3. If no match still, Microsoft.Build.NuGetSdkResolver is loaded and asked to resolve the Sdk. + // + // Overall, while Sdk resolvers look like a general plug-in system, there are good reasons why some of the logic is hard-coded. + // It's not really meant to be modified outside of very special/internal scenarios. +#if NETCOREAPP + if (ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_10)) + { + if (TryResolveSdkUsingSpecifiedResolvers( + _sdkResolverLoader.GetDefaultResolvers(), + BuildEventContext.InvalidSubmissionId, // disables GetResolverState/SetResolverState + sdk, + loggingContext, + sdkReferenceLocation, + solutionPath, + projectPath, + interactive, + isRunningInVisualStudio, + out SdkResult sdkResult, + out _, + out _)) + { + return sdkResult; + } + } +#endif if (ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_4)) { return ResolveSdkUsingResolversWithPatternsFirst(submissionId, sdk, loggingContext, sdkReferenceLocation, solutionPath, projectPath, interactive, isRunningInVisualStudio, failOnUnresolvedSdk); @@ -398,6 +445,10 @@ internal void InitializeForTests(SdkResolverLoader resolverLoader = null, IReadO { _sdkResolverLoader = resolverLoader; } + else + { + _sdkResolverLoader = CachingSdkResolverLoader.Instance; + } _specificResolversManifestsRegistry = null; _generalResolversManifestsRegistry = null; @@ -486,15 +537,20 @@ private void RegisterResolversManifests(ElementLocation location) _manifestToResolvers = new Dictionary>(); - // Load and add the manifest for the default resolvers, located directly in this dll. - IReadOnlyList defaultResolvers = _sdkResolverLoader.GetDefaultResolvers(); SdkResolverManifest sdkDefaultResolversManifest = null; - if (defaultResolvers.Count > 0) +#if NETCOREAPP + if (!ChangeWaves.AreFeaturesEnabled(ChangeWaves.Wave17_10)) +#endif { - MSBuildEventSource.Log.SdkResolverServiceLoadResolversStart(); - sdkDefaultResolversManifest = new SdkResolverManifest(DisplayName: "DefaultResolversManifest", Path: null, ResolvableSdkRegex: null); - _manifestToResolvers[sdkDefaultResolversManifest] = defaultResolvers; - MSBuildEventSource.Log.SdkResolverServiceLoadResolversStop(sdkDefaultResolversManifest.DisplayName ?? string.Empty, defaultResolvers.Count); + // Load and add the manifest for the default resolvers, located directly in this dll. + IReadOnlyList defaultResolvers = _sdkResolverLoader.GetDefaultResolvers(); + if (defaultResolvers.Count > 0) + { + MSBuildEventSource.Log.SdkResolverServiceLoadResolversStart(); + sdkDefaultResolversManifest = new SdkResolverManifest(DisplayName: "DefaultResolversManifest", Path: null, ResolvableSdkRegex: null); + _manifestToResolvers[sdkDefaultResolversManifest] = defaultResolvers; + MSBuildEventSource.Log.SdkResolverServiceLoadResolversStop(sdkDefaultResolversManifest.DisplayName ?? string.Empty, defaultResolvers.Count); + } } MSBuildEventSource.Log.SdkResolverServiceFindResolversManifestsStop(allResolversManifests.Count); diff --git a/src/Framework/Traits.cs b/src/Framework/Traits.cs index daf68c5c7dc..81885bfd4a3 100644 --- a/src/Framework/Traits.cs +++ b/src/Framework/Traits.cs @@ -316,11 +316,6 @@ public bool? LogPropertiesAndItemsAfterEvaluation /// public readonly bool DisableSdkResolutionCache = Environment.GetEnvironmentVariable("MSBUILDDISABLESDKCACHE") == "1"; - /// - /// Disable the NuGet-based SDK resolver. - /// - public readonly bool DisableNuGetSdkResolver = Environment.GetEnvironmentVariable("MSBUILDDISABLENUGETSDKRESOLVER") == "1"; - /// /// Don't delete TargetPath metadata from associated files found by RAR. ///