From 45887425dd0fd18e162a4ffdf7b7fa32f69c6f9f Mon Sep 17 00:00:00 2001 From: Daniel Plaisted Date: Thu, 18 Mar 2021 07:29:59 -0700 Subject: [PATCH 01/12] Add result caching to workload MSBuild SDK resolver --- ...Microsoft.DotNet.MSBuildSdkResolver.csproj | 6 +- ....NET.Sdk.WorkloadMSBuildSdkResolver.csproj | 1 + .../WorkloadSdkResolver.cs | 141 +++++++++++++----- 3 files changed, 108 insertions(+), 40 deletions(-) diff --git a/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/Microsoft.DotNet.MSBuildSdkResolver.csproj b/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/Microsoft.DotNet.MSBuildSdkResolver.csproj index 9daf6545fffa..be35e5d2de6e 100644 --- a/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/Microsoft.DotNet.MSBuildSdkResolver.csproj +++ b/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/Microsoft.DotNet.MSBuildSdkResolver.csproj @@ -58,11 +58,6 @@ - - - - - @@ -78,6 +73,7 @@ + diff --git a/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver.csproj b/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver.csproj index 893dc349d368..ce88ecf45f0b 100644 --- a/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver.csproj +++ b/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver.csproj @@ -30,6 +30,7 @@ + diff --git a/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/WorkloadSdkResolver.cs b/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/WorkloadSdkResolver.cs index f373a832868a..3125fd8e961c 100644 --- a/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/WorkloadSdkResolver.cs +++ b/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/WorkloadSdkResolver.cs @@ -5,6 +5,7 @@ using System.IO; using System.Linq; using Microsoft.NET.Sdk.WorkloadManifestReader; +using System.Collections.Immutable; #if NET using Microsoft.DotNet.Cli; @@ -24,15 +25,10 @@ public class WorkloadSdkResolver : SdkResolver private bool _enabled; - private IWorkloadManifestProvider _workloadManifestProvider; - private IWorkloadResolver _workloadResolver; - - #if NETFRAMEWORK private readonly NETCoreSdkResolver _sdkResolver; #endif - public WorkloadSdkResolver() { // Put workload resolution behind a feature flag. @@ -63,31 +59,35 @@ public WorkloadSdkResolver() #endif } - private void InitializeWorkloadResolver(SdkResolverContext context) - { - var dotnetRootPath = GetDotNetRoot(context); + private record CachedState( + string DotNetRootPath, + string SdkVersion, + ImmutableDictionary CachedResults + ); - var sdkDirectory = GetSdkDirectory(context); - // The SDK version is the name of the SDK directory (ie dotnet\sdk\5.0.100) - var sdkVersion = Path.GetFileName(sdkDirectory); + private record ResolutionResult(); - _workloadManifestProvider ??= new SdkDirectoryWorkloadManifestProvider(dotnetRootPath, sdkVersion); - _workloadResolver ??= WorkloadResolver.Create(_workloadManifestProvider, dotnetRootPath, sdkVersion); - } + private record SinglePathResolutionResult( + string Path + ) : ResolutionResult; - public override SdkResult Resolve(SdkReference sdkReference, SdkResolverContext resolverContext, SdkResultFactory factory) - { - if (!_enabled) - { - return null; - } + private record MultiplePathResolutionResult( + IEnumerable Paths + ) : ResolutionResult; - InitializeWorkloadResolver(resolverContext); + private record EmptyResolutionResult( + IDictionary propertiesToAdd, + IDictionary itemsToAdd + ) : ResolutionResult; - if (sdkReference.Name.Equals("Microsoft.NET.SDK.WorkloadAutoImportPropsLocator", StringComparison.OrdinalIgnoreCase)) + private record NullResolutionResult() : ResolutionResult; + + private static ResolutionResult Resolve(string sdkReferenceName, IWorkloadManifestProvider manifestProvider, IWorkloadResolver workloadResolver) + { + if (sdkReferenceName.Equals("Microsoft.NET.SDK.WorkloadAutoImportPropsLocator", StringComparison.OrdinalIgnoreCase)) { List autoImportSdkPaths = new List(); - foreach (var sdkPackInfo in _workloadResolver.GetInstalledWorkloadPacksOfKind(WorkloadPackKind.Sdk)) + foreach (var sdkPackInfo in workloadResolver.GetInstalledWorkloadPacksOfKind(WorkloadPackKind.Sdk)) { string sdkPackSdkFolder = Path.Combine(sdkPackInfo.Path, "Sdk"); string autoImportPath = Path.Combine(sdkPackSdkFolder, "AutoImport.props"); @@ -97,12 +97,12 @@ public override SdkResult Resolve(SdkReference sdkReference, SdkResolverContext } } // Call Distinct() here because with aliased packs, there may be duplicates of the same path - return factory.IndicateSuccess(autoImportSdkPaths.Distinct(), sdkReference.Version); + return new MultiplePathResolutionResult(autoImportSdkPaths.Distinct()); } - else if (sdkReference.Name.Equals("Microsoft.NET.SDK.WorkloadManifestTargetsLocator", StringComparison.OrdinalIgnoreCase)) + else if (sdkReferenceName.Equals("Microsoft.NET.SDK.WorkloadManifestTargetsLocator", StringComparison.OrdinalIgnoreCase)) { List workloadManifestPaths = new List(); - foreach (var manifestDirectory in _workloadManifestProvider.GetManifestDirectories()) + foreach (var manifestDirectory in manifestProvider.GetManifestDirectories()) { var workloadManifestTargetPath = Path.Combine(manifestDirectory, "WorkloadManifest.targets"); if (File.Exists(workloadManifestTargetPath)) @@ -110,36 +110,100 @@ public override SdkResult Resolve(SdkReference sdkReference, SdkResolverContext workloadManifestPaths.Add(manifestDirectory); } } - return factory.IndicateSuccess(workloadManifestPaths, sdkReference.Version); + return new MultiplePathResolutionResult(workloadManifestPaths); } else { - var packInfo = _workloadResolver.TryGetPackInfo(sdkReference.Name); + var packInfo = workloadResolver.TryGetPackInfo(sdkReferenceName); if (packInfo != null) { if (Directory.Exists(packInfo.Path)) { - return factory.IndicateSuccess(Path.Combine(packInfo.Path, "Sdk"), sdkReference.Version); + return new SinglePathResolutionResult(Path.Combine(packInfo.Path, "Sdk")); } else { var itemsToAdd = new Dictionary(); itemsToAdd.Add("MissingWorkloadPack", - new SdkResultItem(sdkReference.Name, + new SdkResultItem(sdkReferenceName, metadata: new Dictionary() { { "Version", packInfo.Version } })); Dictionary propertiesToAdd = new Dictionary(); - return factory.IndicateSuccess(Enumerable.Empty(), - sdkReference.Version, - propertiesToAdd: propertiesToAdd, - itemsToAdd: itemsToAdd); + return new EmptyResolutionResult(propertiesToAdd, itemsToAdd); } } } - return null; + return new NullResolutionResult(); + } + + public override SdkResult Resolve(SdkReference sdkReference, SdkResolverContext resolverContext, SdkResultFactory factory) + { + if (!_enabled) + { + return null; + } + + CachedState cachedState; + + if (resolverContext.State is CachedState resolverContextState) + { + cachedState = resolverContextState; + } + else + { + cachedState = new CachedState(null, null, ImmutableDictionary.Create(StringComparer.OrdinalIgnoreCase)); + } + + ResolutionResult resolutionResult; + + if (!cachedState.CachedResults.TryGetValue(sdkReference.Name, out resolutionResult)) + { + if (cachedState.DotNetRootPath == null || cachedState.SdkVersion == null) + { + var dotnetRootPath = GetDotNetRoot(resolverContext); + + var sdkDirectory = GetSdkDirectory(resolverContext); + // The SDK version is the name of the SDK directory (ie dotnet\sdk\5.0.100) + var sdkVersion = Path.GetFileName(sdkDirectory); + + cachedState = cachedState with + { + DotNetRootPath = dotnetRootPath, + SdkVersion = sdkVersion + }; + + } + + var workloadManifestProvider = new SdkDirectoryWorkloadManifestProvider(cachedState.DotNetRootPath, cachedState.SdkVersion); + var workloadResolver = WorkloadResolver.Create(workloadManifestProvider, cachedState.DotNetRootPath, cachedState.SdkVersion); + + resolutionResult = Resolve(sdkReference.Name, workloadManifestProvider, workloadResolver); + + cachedState = cachedState with + { + CachedResults = cachedState.CachedResults.Add(sdkReference.Name, resolutionResult) + }; + + resolverContext.State = cachedState; + } + + switch (resolutionResult) + { + case SinglePathResolutionResult r: + return factory.IndicateSuccess(r.Path, sdkReference.Version); + case MultiplePathResolutionResult r: + return factory.IndicateSuccess(r.Paths, sdkReference.Version); + case EmptyResolutionResult r: + return factory.IndicateSuccess(Enumerable.Empty(), sdkReference.Version, r.propertiesToAdd, r.itemsToAdd); + case NullResolutionResult: + return null; + } + + throw new InvalidOperationException("Unknown resolutionResult type: " + resolutionResult.GetType()); + } private string GetSdkDirectory(SdkResolverContext context) @@ -166,3 +230,10 @@ private string GetDotNetRoot(SdkResolverContext context) } } } + +#if !NET +namespace System.Runtime.CompilerServices +{ + public class IsExternalInit { } +} +#endif From d32d9f4348a3b1bb9b8c4f885c954da29d84addb Mon Sep 17 00:00:00 2001 From: Daniel Plaisted Date: Thu, 18 Mar 2021 07:30:33 -0700 Subject: [PATCH 02/12] Save PropertiesToAdd in cached results for MSBuild SDK resolver --- .../MSBuildSdkResolver.cs | 6 ++++-- .../Microsoft.DotNet.SdkResolver/NETCoreSdkResolver.cs | 4 ++++ 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/MSBuildSdkResolver.cs b/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/MSBuildSdkResolver.cs index 7b7f4f28ef9e..949e37700dea 100644 --- a/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/MSBuildSdkResolver.cs +++ b/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/MSBuildSdkResolver.cs @@ -19,7 +19,6 @@ namespace Microsoft.DotNet.MSBuildSdkResolver // 2. Nevertheless, in the IDE, project re-evaluation can create new instances for each evaluation. // // As such, all state (instance or static) must be guarded against concurrent access/updates. - // Caches of minimum versions, compatible SDKs are static to benefit multiple IDE evaluations. // VSSettings are also effectively static (singleton instance that can be swapped by tests). public sealed class DotNetMSBuildSdkResolver : SdkResolver @@ -48,6 +47,7 @@ private sealed class CachedResult { public string MSBuildSdksDir; public string NETCoreSdkVersion; + public IDictionary PropertiesToAdd; } public override SdkResult Resolve(SdkReference sdkReference, SdkResolverContext context, SdkResultFactory factory) @@ -62,6 +62,7 @@ public override SdkResult Resolve(SdkReference sdkReference, SdkResolverContext { msbuildSdksDir = priorResult.MSBuildSdksDir; netcoreSdkVersion = priorResult.NETCoreSdkVersion; + propertiesToAdd = priorResult.PropertiesToAdd; } if (msbuildSdksDir == null) @@ -138,7 +139,8 @@ public override SdkResult Resolve(SdkReference sdkReference, SdkResolverContext context.State = new CachedResult { MSBuildSdksDir = msbuildSdksDir, - NETCoreSdkVersion = netcoreSdkVersion + NETCoreSdkVersion = netcoreSdkVersion, + PropertiesToAdd = propertiesToAdd }; string msbuildSdkDir = Path.Combine(msbuildSdksDir, sdkReference.Name, "Sdk"); diff --git a/src/Resolvers/Microsoft.DotNet.SdkResolver/NETCoreSdkResolver.cs b/src/Resolvers/Microsoft.DotNet.SdkResolver/NETCoreSdkResolver.cs index b78457f30de9..229e6b186138 100644 --- a/src/Resolvers/Microsoft.DotNet.SdkResolver/NETCoreSdkResolver.cs +++ b/src/Resolvers/Microsoft.DotNet.SdkResolver/NETCoreSdkResolver.cs @@ -10,11 +10,15 @@ namespace Microsoft.DotNet.DotNetSdkResolver { + + // Thread safety note: + // This class is used by the MSBuild SDK resolvers, which can be called on multiple threads. public class NETCoreSdkResolver { private readonly Func _getEnvironmentVariable; private readonly VSSettings _vsSettings; + // Caches of minimum versions, compatible SDKs are static to benefit multiple IDE evaluations. private static readonly ConcurrentDictionary s_minimumMSBuildVersions = new ConcurrentDictionary(); From 76ac9dd90cf31a7a9dc9fc5390ea9a23989c53f4 Mon Sep 17 00:00:00 2001 From: Daniel Plaisted Date: Thu, 18 Mar 2021 07:58:37 -0700 Subject: [PATCH 03/12] Enable workload resolver by default --- .../WorkloadSdkResolver.cs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/WorkloadSdkResolver.cs b/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/WorkloadSdkResolver.cs index 3125fd8e961c..4073e014c6ef 100644 --- a/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/WorkloadSdkResolver.cs +++ b/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/WorkloadSdkResolver.cs @@ -31,23 +31,23 @@ public class WorkloadSdkResolver : SdkResolver public WorkloadSdkResolver() { - // Put workload resolution behind a feature flag. - _enabled = false; + // Support opt-out for workload resolution + _enabled = true; var envVar = Environment.GetEnvironmentVariable("MSBuildEnableWorkloadResolver"); if (envVar != null) { - if (envVar.Equals("true", StringComparison.OrdinalIgnoreCase)) + if (envVar.Equals("false", StringComparison.OrdinalIgnoreCase)) { - _enabled = true; + _enabled = false; } } if (!_enabled) { - string sentinelPath = Path.Combine(Path.GetDirectoryName(typeof(WorkloadSdkResolver).Assembly.Location), "EnableWorkloadResolver.sentinel"); + string sentinelPath = Path.Combine(Path.GetDirectoryName(typeof(WorkloadSdkResolver).Assembly.Location), "DisableWorkloadResolver.sentinel"); if (File.Exists(sentinelPath)) { - _enabled = true; + _enabled = false; } } From ce483b30fce1c1153cd2b11c534791d734db4092 Mon Sep 17 00:00:00 2001 From: Daniel Plaisted Date: Thu, 18 Mar 2021 10:47:08 -0700 Subject: [PATCH 04/12] Use different output paths for different tests --- .../GivenThatWeWantToPublishAWebApp.cs | 2 +- .../GivenThatWeWantToPublishToClickOnce.cs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/Tests/Microsoft.NET.Publish.Tests/GivenThatWeWantToPublishAWebApp.cs b/src/Tests/Microsoft.NET.Publish.Tests/GivenThatWeWantToPublishAWebApp.cs index 4d710fe839b2..e6010ea1597e 100644 --- a/src/Tests/Microsoft.NET.Publish.Tests/GivenThatWeWantToPublishAWebApp.cs +++ b/src/Tests/Microsoft.NET.Publish.Tests/GivenThatWeWantToPublishAWebApp.cs @@ -167,7 +167,7 @@ public void It_should_publish_framework_dependent_for_2x(string platformLibrary) [InlineData(true, false)] [InlineData(false, true)] [InlineData(true, true)] - public void It_publishes_with_a_publish_profile(bool? selfContained, bool? useAppHost) + public void PublishWebAppWithPublishProfile(bool? selfContained, bool? useAppHost) { var tfm = "netcoreapp2.2"; var rid = EnvironmentInfo.GetCompatibleRid(tfm); diff --git a/src/Tests/Microsoft.NET.Publish.Tests/GivenThatWeWantToPublishToClickOnce.cs b/src/Tests/Microsoft.NET.Publish.Tests/GivenThatWeWantToPublishToClickOnce.cs index f07a3ff59291..d636c843ff16 100644 --- a/src/Tests/Microsoft.NET.Publish.Tests/GivenThatWeWantToPublishToClickOnce.cs +++ b/src/Tests/Microsoft.NET.Publish.Tests/GivenThatWeWantToPublishToClickOnce.cs @@ -26,7 +26,7 @@ public GivenThatWeWantToPublishAClickOnceProject(ITestOutputHelper log) : base(l [FullMSBuildOnlyTheory] [InlineData(false)] [InlineData(true)] - public void It_publishes_with_a_publish_profile(bool? publishSingleFile) + public void PublishClickOnceWithPublishProfile(bool? publishSingleFile) { var tfm = "netcoreapp3.1"; var rid = EnvironmentInfo.GetCompatibleRid(tfm); From a86a56c73d53c261f77843f77f0498de7ceff01a Mon Sep 17 00:00:00 2001 From: Daniel Plaisted Date: Sun, 21 Mar 2021 12:13:02 -0700 Subject: [PATCH 05/12] Add Serilog logging for how SDK Resolvers are called --- Directory.Build.props | 4 ++ .../MSBuildSdkResolver.cs | 38 +++++++++++++++++++ ...Microsoft.DotNet.MSBuildSdkResolver.csproj | 5 +++ 3 files changed, 47 insertions(+) diff --git a/Directory.Build.props b/Directory.Build.props index d849ce76c312..1827fbcb0c06 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -19,6 +19,10 @@ $(DefineConstants);CI_BUILD + + + false + $(DefineConstants);USE_SERILOG diff --git a/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/MSBuildSdkResolver.cs b/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/MSBuildSdkResolver.cs index 949e37700dea..e24080cb62ef 100644 --- a/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/MSBuildSdkResolver.cs +++ b/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/MSBuildSdkResolver.cs @@ -10,6 +10,10 @@ using System.Linq; using System.Reflection; +#if USE_SERILOG +using Serilog; +#endif + #nullable disable namespace Microsoft.DotNet.MSBuildSdkResolver @@ -41,7 +45,30 @@ public DotNetMSBuildSdkResolver(Func getEnvironmentVariable, VSS { _getEnvironmentVariable = getEnvironmentVariable; _netCoreSdkResolver = new NETCoreSdkResolver(getEnvironmentVariable, vsSettings); + +#if USE_SERILOG + _instanceId = System.Threading.Interlocked.Increment(ref _lastInstanceId); +#endif + } + +#if USE_SERILOG + static Serilog.Core.Logger Logger; + + static int _lastInstanceId = 1; + int _instanceId; + + static DotNetMSBuildSdkResolver() + { + Logger = new LoggerConfiguration() + .WriteTo.Seq("http://localhost:5341") + .CreateLogger(); + + AppDomain.CurrentDomain.ProcessExit += (_, _) => + { + Logger.Dispose(); + }; } +#endif private sealed class CachedResult { @@ -52,6 +79,17 @@ private sealed class CachedResult public override SdkResult Resolve(SdkReference sdkReference, SdkResolverContext context, SdkResultFactory factory) { +#if USE_SERILOG + var msbuildSubmissionId = (int?) System.Threading.Thread.GetData(System.Threading.Thread.GetNamedDataSlot("MSBuildSubmissionId")); + Logger + .ForContext("Process", System.Diagnostics.Process.GetCurrentProcess().Id) + .ForContext("Thread", System.Threading.Thread.CurrentThread.ManagedThreadId) + .ForContext("ResolverInstance", _instanceId) + .ForContext("MSBuildSubmissionId", msbuildSubmissionId) + .ForContext("HasCache", context.State != null) + .Information("Resolving SDK {sdkName}", sdkReference.Name); +#endif + string msbuildSdksDir = null; string netcoreSdkVersion = null; IDictionary propertiesToAdd = null; diff --git a/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/Microsoft.DotNet.MSBuildSdkResolver.csproj b/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/Microsoft.DotNet.MSBuildSdkResolver.csproj index be35e5d2de6e..a4fcb4df664c 100644 --- a/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/Microsoft.DotNet.MSBuildSdkResolver.csproj +++ b/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/Microsoft.DotNet.MSBuildSdkResolver.csproj @@ -84,6 +84,11 @@ + + + + + From d0d06dc20754d1261f22c6f300232242587f4890 Mon Sep 17 00:00:00 2001 From: Daniel Plaisted Date: Sun, 21 Mar 2021 18:15:23 -0700 Subject: [PATCH 06/12] Use static cache for workload resolver when called from VS --- .../WorkloadSdkResolver.cs | 100 ++++++++++++------ 1 file changed, 69 insertions(+), 31 deletions(-) diff --git a/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/WorkloadSdkResolver.cs b/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/WorkloadSdkResolver.cs index 4073e014c6ef..b53af83fc564 100644 --- a/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/WorkloadSdkResolver.cs +++ b/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/WorkloadSdkResolver.cs @@ -25,6 +25,18 @@ public class WorkloadSdkResolver : SdkResolver private bool _enabled; + + // MSBuild SDK resolvers can use the SdkResolverContext's State property to cache state across multiple calls. However, + // MSBuild only caches those within the same build (which is tracked via a build submission ID). + // When running in Visual Studio, there are lots of evaluations that aren't associated with with a build, and then + // it looks like each project is evaluated under a separate build submission ID. So the built-in caching support doesn't + // work well when running in Visual Studio. + // Because of this, when running in Visual Studio, we will use our own cache state, which will be stored staticly. In order + // to avoid requiring that the workload manifest reader and resolver classes are fully thread safe, we include a lock object + // in the cache state which we use to ensure that multiple threads don't access it at the same time. We don't expect high + // concurrency in MSBuild SDK Resolver calls, so we don't expect the lock to impact the performance. + private static CachedState _staticCachedState; + #if NETFRAMEWORK private readonly NETCoreSdkResolver _sdkResolver; #endif @@ -59,11 +71,21 @@ public WorkloadSdkResolver() #endif } - private record CachedState( - string DotNetRootPath, - string SdkVersion, - ImmutableDictionary CachedResults - ); + private record CachedState + { + public object LockObject { get; init; } + public string DotnetRootPath { get; init; } + public string SdkVersion { get; init; } + public IWorkloadManifestProvider ManifestProvider { get; init; } + public IWorkloadResolver WorkloadResolver { get; init; } + public ImmutableDictionary CachedResults { get; init; } + + public CachedState() + { + LockObject = new object(); + CachedResults = ImmutableDictionary.Create(StringComparer.OrdinalIgnoreCase); + } + } private record ResolutionResult(); @@ -146,48 +168,64 @@ public override SdkResult Resolve(SdkReference sdkReference, SdkResolverContext return null; } - CachedState cachedState; + CachedState cachedState = null; - if (resolverContext.State is CachedState resolverContextState) + if (resolverContext.IsRunningInVisualStudio) { - cachedState = resolverContextState; + cachedState = _staticCachedState; } else { - cachedState = new CachedState(null, null, ImmutableDictionary.Create(StringComparer.OrdinalIgnoreCase)); + if (resolverContext.State is CachedState resolverContextState) + { + cachedState = resolverContextState; + } } - ResolutionResult resolutionResult; - - if (!cachedState.CachedResults.TryGetValue(sdkReference.Name, out resolutionResult)) + + if (cachedState == null) { - if (cachedState.DotNetRootPath == null || cachedState.SdkVersion == null) + // If we don't have any cached state yet, then find the dotnet directory and SDK version, and create the workload resolver classes + // Note that in Visual Studio, we could end up doing this multiple times if the resolver gets called on multiple threads before any + // state has been cached. + + var dotnetRootPath = GetDotNetRoot(resolverContext); + + var sdkDirectory = GetSdkDirectory(resolverContext); + // The SDK version is the name of the SDK directory (ie dotnet\sdk\5.0.100) + var sdkVersion = Path.GetFileName(sdkDirectory); + + var workloadManifestProvider = new SdkDirectoryWorkloadManifestProvider(dotnetRootPath, sdkVersion); + var workloadResolver = WorkloadResolver.Create(workloadManifestProvider, dotnetRootPath, sdkVersion); + + cachedState = new CachedState() { - var dotnetRootPath = GetDotNetRoot(resolverContext); + DotnetRootPath = dotnetRootPath, + SdkVersion = sdkVersion, + ManifestProvider = workloadManifestProvider, + WorkloadResolver = workloadResolver + }; + } - var sdkDirectory = GetSdkDirectory(resolverContext); - // The SDK version is the name of the SDK directory (ie dotnet\sdk\5.0.100) - var sdkVersion = Path.GetFileName(sdkDirectory); + ResolutionResult resolutionResult; + lock (cachedState.LockObject) + { + if (!cachedState.CachedResults.TryGetValue(sdkReference.Name, out resolutionResult)) + { + resolutionResult = Resolve(sdkReference.Name, cachedState.ManifestProvider, cachedState.WorkloadResolver); cachedState = cachedState with { - DotNetRootPath = dotnetRootPath, - SdkVersion = sdkVersion + CachedResults = cachedState.CachedResults.Add(sdkReference.Name, resolutionResult) }; - } - - var workloadManifestProvider = new SdkDirectoryWorkloadManifestProvider(cachedState.DotNetRootPath, cachedState.SdkVersion); - var workloadResolver = WorkloadResolver.Create(workloadManifestProvider, cachedState.DotNetRootPath, cachedState.SdkVersion); + resolverContext.State = cachedState; - resolutionResult = Resolve(sdkReference.Name, workloadManifestProvider, workloadResolver); - - cachedState = cachedState with - { - CachedResults = cachedState.CachedResults.Add(sdkReference.Name, resolutionResult) - }; - - resolverContext.State = cachedState; + if (resolverContext.IsRunningInVisualStudio) + { + _staticCachedState = cachedState; + } + } } switch (resolutionResult) From aaf7028fdd0a7de765d56c0b6d22f0a48f528117 Mon Sep 17 00:00:00 2001 From: Daniel Plaisted Date: Mon, 22 Mar 2021 19:06:42 -0700 Subject: [PATCH 07/12] Add Serilog logging for Workload SDK resolver --- .../MSBuildSdkResolver.cs | 1 + ....NET.Sdk.WorkloadMSBuildSdkResolver.csproj | 5 ++ .../WorkloadSdkResolver.cs | 85 +++++++++++++++++-- 3 files changed, 82 insertions(+), 9 deletions(-) diff --git a/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/MSBuildSdkResolver.cs b/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/MSBuildSdkResolver.cs index e24080cb62ef..d4490925ca6f 100644 --- a/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/MSBuildSdkResolver.cs +++ b/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/MSBuildSdkResolver.cs @@ -82,6 +82,7 @@ public override SdkResult Resolve(SdkReference sdkReference, SdkResolverContext #if USE_SERILOG var msbuildSubmissionId = (int?) System.Threading.Thread.GetData(System.Threading.Thread.GetNamedDataSlot("MSBuildSubmissionId")); Logger + .ForContext("Resolver", "Sdk") .ForContext("Process", System.Diagnostics.Process.GetCurrentProcess().Id) .ForContext("Thread", System.Threading.Thread.CurrentThread.ManagedThreadId) .ForContext("ResolverInstance", _instanceId) diff --git a/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver.csproj b/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver.csproj index ce88ecf45f0b..6fc3c0b0034e 100644 --- a/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver.csproj +++ b/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver.csproj @@ -33,6 +33,11 @@ + + + + + <_ResolverManifestDir>$(OutputPath)\SdkResolvers\$(AssemblyName)\ diff --git a/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/WorkloadSdkResolver.cs b/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/WorkloadSdkResolver.cs index b53af83fc564..2c6d6c30c7ec 100644 --- a/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/WorkloadSdkResolver.cs +++ b/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/WorkloadSdkResolver.cs @@ -13,6 +13,10 @@ using Microsoft.DotNet.DotNetSdkResolver; #endif +#if USE_SERILOG +using Serilog; +#endif + #nullable disable namespace Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver @@ -69,8 +73,31 @@ public WorkloadSdkResolver() _sdkResolver = new NETCoreSdkResolver(); } #endif + +#if USE_SERILOG + _instanceId = System.Threading.Interlocked.Increment(ref _lastInstanceId); +#endif } +#if USE_SERILOG + static Serilog.Core.Logger Logger; + + static int _lastInstanceId = 1; + int _instanceId; + + static WorkloadSdkResolver() + { + Logger = new LoggerConfiguration() + .WriteTo.Seq("http://localhost:5341") + .CreateLogger(); + + AppDomain.CurrentDomain.ProcessExit += (_, _) => + { + Logger.Dispose(); + }; + } +#endif + private record CachedState { public object LockObject { get; init; } @@ -168,6 +195,10 @@ public override SdkResult Resolve(SdkReference sdkReference, SdkResolverContext return null; } +#if USE_SERILOG + List> logActions = new List>(); +#endif + CachedState cachedState = null; if (resolverContext.IsRunningInVisualStudio) @@ -185,6 +216,10 @@ public override SdkResult Resolve(SdkReference sdkReference, SdkResolverContext if (cachedState == null) { +#if USE_SERILOG + logActions.Add(log => log.Information("Initializing resolver state")); +#endif + // If we don't have any cached state yet, then find the dotnet directory and SDK version, and create the workload resolver classes // Note that in Visual Studio, we could end up doing this multiple times if the resolver gets called on multiple threads before any // state has been cached. @@ -206,12 +241,21 @@ public override SdkResult Resolve(SdkReference sdkReference, SdkResolverContext WorkloadResolver = workloadResolver }; } + else + { +#if USE_SERILOG + logActions.Add(log => log.Information("Using cached resolver state")); +#endif + } ResolutionResult resolutionResult; lock (cachedState.LockObject) { if (!cachedState.CachedResults.TryGetValue(sdkReference.Name, out resolutionResult)) { +#if USE_SERILOG + logActions.Add(log => log.Information("Resolving workload")); +#endif resolutionResult = Resolve(sdkReference.Name, cachedState.ManifestProvider, cachedState.WorkloadResolver); cachedState = cachedState with @@ -226,19 +270,42 @@ public override SdkResult Resolve(SdkReference sdkReference, SdkResolverContext _staticCachedState = cachedState; } } +#if USE_SERILOG + else + { + logActions.Add(log => log.Information("Using cached workload resolution result")); + } +#endif } - switch (resolutionResult) + +#if USE_SERILOG + var msbuildSubmissionId = (int?)System.Threading.Thread.GetData(System.Threading.Thread.GetNamedDataSlot("MSBuildSubmissionId")); + var log = Logger + .ForContext("Resolver", "Sdk") + .ForContext("Process", System.Diagnostics.Process.GetCurrentProcess().Id) + .ForContext("Thread", System.Threading.Thread.CurrentThread.ManagedThreadId) + .ForContext("ResolverInstance", _instanceId) + .ForContext("RunningInVS", resolverContext.IsRunningInVisualStudio) + .ForContext("MSBuildSubmissionId", msbuildSubmissionId); + + foreach (var logAction in logActions) { - case SinglePathResolutionResult r: - return factory.IndicateSuccess(r.Path, sdkReference.Version); - case MultiplePathResolutionResult r: - return factory.IndicateSuccess(r.Paths, sdkReference.Version); - case EmptyResolutionResult r: - return factory.IndicateSuccess(Enumerable.Empty(), sdkReference.Version, r.propertiesToAdd, r.itemsToAdd); - case NullResolutionResult: - return null; + logAction(log); } +#endif + + switch (resolutionResult) + { + case SinglePathResolutionResult r: + return factory.IndicateSuccess(r.Path, sdkReference.Version); + case MultiplePathResolutionResult r: + return factory.IndicateSuccess(r.Paths, sdkReference.Version); + case EmptyResolutionResult r: + return factory.IndicateSuccess(Enumerable.Empty(), sdkReference.Version, r.propertiesToAdd, r.itemsToAdd); + case NullResolutionResult: + return null; + } throw new InvalidOperationException("Unknown resolutionResult type: " + resolutionResult.GetType()); From 4da9b22d506fb7e4c04bc08285587bd3e9e092d9 Mon Sep 17 00:00:00 2001 From: Daniel Plaisted Date: Tue, 23 Mar 2021 15:58:41 -0700 Subject: [PATCH 08/12] For full framework, include workload resolution in MSBuildSdkResolver --- .../MSBuildSdkResolver.cs | 90 +++++- ...Microsoft.DotNet.MSBuildSdkResolver.csproj | 2 +- ....NET.Sdk.WorkloadMSBuildSdkResolver.csproj | 5 +- .../WorkloadPartialResolver.cs | 258 ++++++++++++++++++ .../WorkloadSdkResolver.cs | 221 ++------------- 5 files changed, 356 insertions(+), 220 deletions(-) create mode 100644 src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/WorkloadPartialResolver.cs diff --git a/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/MSBuildSdkResolver.cs b/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/MSBuildSdkResolver.cs index d4490925ca6f..879032552d96 100644 --- a/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/MSBuildSdkResolver.cs +++ b/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/MSBuildSdkResolver.cs @@ -9,6 +9,7 @@ using System.IO; using System.Linq; using System.Reflection; +using Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver; #if USE_SERILOG using Serilog; @@ -35,6 +36,8 @@ public sealed class DotNetMSBuildSdkResolver : SdkResolver private readonly Func _getEnvironmentVariable; private readonly NETCoreSdkResolver _netCoreSdkResolver; + private static WorkloadPartialResolver _staticWorkloadResolver = new WorkloadPartialResolver(); + public DotNetMSBuildSdkResolver() : this(Environment.GetEnvironmentVariable, VSSettings.Ambient) { @@ -70,54 +73,85 @@ static DotNetMSBuildSdkResolver() } #endif - private sealed class CachedResult + private sealed class CachedState { + public string DotnetRoot; public string MSBuildSdksDir; public string NETCoreSdkVersion; public IDictionary PropertiesToAdd; + public WorkloadPartialResolver WorkloadResolver; } public override SdkResult Resolve(SdkReference sdkReference, SdkResolverContext context, SdkResultFactory factory) { #if USE_SERILOG - var msbuildSubmissionId = (int?) System.Threading.Thread.GetData(System.Threading.Thread.GetNamedDataSlot("MSBuildSubmissionId")); - Logger + List> logActions = new List>(); + + var result = Resolve(sdkReference, context, factory, logActions); + + var msbuildSubmissionId = (int?)System.Threading.Thread.GetData(System.Threading.Thread.GetNamedDataSlot("MSBuildSubmissionId")); + var log = Logger .ForContext("Resolver", "Sdk") .ForContext("Process", System.Diagnostics.Process.GetCurrentProcess().Id) .ForContext("Thread", System.Threading.Thread.CurrentThread.ManagedThreadId) .ForContext("ResolverInstance", _instanceId) + .ForContext("RunningInVS", context.IsRunningInVisualStudio) .ForContext("MSBuildSubmissionId", msbuildSubmissionId) .ForContext("HasCache", context.State != null) - .Information("Resolving SDK {sdkName}", sdkReference.Name); + .ForContext("SdkName", sdkReference.Name); + + foreach (var logAction in logActions) + { + logAction(log); + } + + return result; + } + + private SdkResult Resolve(SdkReference sdkReference, SdkResolverContext context, SdkResultFactory factory, List> logActions) + { + + logActions.Add(log => log.Information("Resolving SDK {sdkName}", sdkReference.Name)); #endif + string dotnetRoot = null; string msbuildSdksDir = null; string netcoreSdkVersion = null; IDictionary propertiesToAdd = null; IDictionary itemsToAdd = null; List warnings = null; + WorkloadPartialResolver workloadResolver = null; - if (context.State is CachedResult priorResult) + if (context.State is CachedState priorResult) { + dotnetRoot = priorResult.DotnetRoot; msbuildSdksDir = priorResult.MSBuildSdksDir; netcoreSdkVersion = priorResult.NETCoreSdkVersion; propertiesToAdd = priorResult.PropertiesToAdd; + workloadResolver = priorResult.WorkloadResolver; + } + + if (context.IsRunningInVisualStudio) + { + workloadResolver = _staticWorkloadResolver; } if (msbuildSdksDir == null) { - // These are overrides that are used to force the resolved SDK tasks and targets to come from a given - // base directory and report a given version to msbuild (which may be null if unknown. One key use case - // for this is to test SDK tasks and targets without deploying them inside the .NET Core SDK. - msbuildSdksDir = _getEnvironmentVariable("DOTNET_MSBUILD_SDK_RESOLVER_SDKS_DIR"); - netcoreSdkVersion = _getEnvironmentVariable("DOTNET_MSBUILD_SDK_RESOLVER_SDKS_VER"); + + dotnetRoot = _netCoreSdkResolver.GetDotnetExeDirectory(); + } + + if (workloadResolver == null) + { + workloadResolver = new WorkloadPartialResolver(); } if (msbuildSdksDir == null) { - string dotnetExeDir = _netCoreSdkResolver.GetDotnetExeDirectory(); + dotnetRoot = _netCoreSdkResolver.GetDotnetExeDirectory(); string globalJsonStartDir = Path.GetDirectoryName(context.SolutionFilePath ?? context.ProjectFilePath); - var resolverResult = _netCoreSdkResolver.ResolveNETCoreSdkDirectory(globalJsonStartDir, context.MSBuildVersion, context.IsRunningInVisualStudio, dotnetExeDir); + var resolverResult = _netCoreSdkResolver.ResolveNETCoreSdkDirectory(globalJsonStartDir, context.MSBuildVersion, context.IsRunningInVisualStudio, dotnetRoot); if (resolverResult.ResolvedSdkDirectory == null) { @@ -129,6 +163,20 @@ public override SdkResult Resolve(SdkReference sdkReference, SdkResolverContext msbuildSdksDir = Path.Combine(resolverResult.ResolvedSdkDirectory, "Sdks"); netcoreSdkVersion = new DirectoryInfo(resolverResult.ResolvedSdkDirectory).Name; + // These are overrides that are used to force the resolved SDK tasks and targets to come from a given + // base directory and report a given version to msbuild (which may be null if unknown. One key use case + // for this is to test SDK tasks and targets without deploying them inside the .NET Core SDK. + var msbuildSdksDirFromEnv = _getEnvironmentVariable("DOTNET_MSBUILD_SDK_RESOLVER_SDKS_DIR"); + var netcoreSdkVersionFromEnv = _getEnvironmentVariable("DOTNET_MSBUILD_SDK_RESOLVER_SDKS_VER"); + if (!string.IsNullOrEmpty(msbuildSdksDirFromEnv)) + { + msbuildSdksDir = msbuildSdksDirFromEnv; + } + if (!string.IsNullOrEmpty(netcoreSdkVersionFromEnv)) + { + netcoreSdkVersion = netcoreSdkVersionFromEnv; + } + if (IsNetCoreSDKSmallerThanTheMinimumVersion(netcoreSdkVersion, sdkReference.MinimumVersion)) { return Failure( @@ -175,13 +223,27 @@ public override SdkResult Resolve(SdkReference sdkReference, SdkResolverContext } } - context.State = new CachedResult + context.State = new CachedState { + DotnetRoot = dotnetRoot, MSBuildSdksDir = msbuildSdksDir, NETCoreSdkVersion = netcoreSdkVersion, - PropertiesToAdd = propertiesToAdd + PropertiesToAdd = propertiesToAdd, + WorkloadResolver = workloadResolver }; + // First check if requested SDK resolves to a workload SDK pack + var workloadResult = workloadResolver.Resolve(sdkReference.Name, dotnetRoot, netcoreSdkVersion +#if USE_SERILOG + , logActions: logActions +#endif + ); + + if (workloadResult is not WorkloadPartialResolver.NullResolutionResult) + { + return workloadResult.ToSdkResult(sdkReference, factory); + } + string msbuildSdkDir = Path.Combine(msbuildSdksDir, sdkReference.Name, "Sdk"); if (!Directory.Exists(msbuildSdkDir)) { diff --git a/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/Microsoft.DotNet.MSBuildSdkResolver.csproj b/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/Microsoft.DotNet.MSBuildSdkResolver.csproj index a4fcb4df664c..421eb81417e2 100644 --- a/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/Microsoft.DotNet.MSBuildSdkResolver.csproj +++ b/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/Microsoft.DotNet.MSBuildSdkResolver.csproj @@ -61,7 +61,7 @@ - + diff --git a/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver.csproj b/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver.csproj index 6fc3c0b0034e..8f2fad9f5e12 100644 --- a/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver.csproj +++ b/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver.csproj @@ -1,13 +1,10 @@  - $(SdkTargetFramework);net472 - $(SdkTargetFramework) + $(SdkTargetFramework) true - - diff --git a/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/WorkloadPartialResolver.cs b/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/WorkloadPartialResolver.cs new file mode 100644 index 000000000000..fa93ab9b0c58 --- /dev/null +++ b/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/WorkloadPartialResolver.cs @@ -0,0 +1,258 @@ +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + +using Microsoft.Build.Framework; +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.IO; +using System.Linq; +using Microsoft.NET.Sdk.WorkloadManifestReader; +using System.Collections.Immutable; + +#if NET +using Microsoft.DotNet.Cli; +#else +using Microsoft.DotNet.DotNetSdkResolver; +#endif + +#if USE_SERILOG +using Serilog; +#endif + +#nullable disable + +namespace Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver +{ + + // This class contains workload SDK resolution logic which will be used by both .NET SDK MSBuild and Full Framework / Visual Studio MSBuild. + // + // Keeping this performant in Visual Studio is tricky, as VS performs a lot of evaluations, but they are not linked by an MSBuild "Submission ID", + // so the state caching support provided by MSBuild for SDK Resolvers doesn't really help. Additionally, multiple instances of the SDK resolver + // may be created, and the same instance may be called on multiple threads. So state needs to be cached staticly and be thread-safe. + // + // To keep the state static, the MSBuildSdkResolver keeps a static reference to the WorkloadPartialResolver that is used if the build is inside + // Visual Studio. To keep it thread-safe, the body of the Resolve method is all protected by a lock statement. This avoids having to make + // the classes consumed by the WorkloadPartialResolver (the manifest provider and workload resolver) thread-safe. + // + // A resolver should not over-cache and return out-of-date results. For workloads, the resolution could change due to: + // - Installation, update, or uninstallation of a workload + // - Resolved SDK changes (either due to an SDK installation or uninstallation, or a global.json change) + // For SDK or workload installation actions, we expect to be running under a new process since Visual Studio will have been restarted. + // For global.json changes, the Resolve method takes parameters for the dotnet root and the SDK version. If those values have changed + // from the previous call, the cached state will be thrown out and recreated. + class WorkloadPartialResolver + { + private record CachedState + { + public string DotnetRootPath { get; init; } + public string SdkVersion { get; init; } + public IWorkloadManifestProvider ManifestProvider { get; init; } + public IWorkloadResolver WorkloadResolver { get; init; } + public ImmutableDictionary CachedResults { get; init; } + + public CachedState() + { + CachedResults = ImmutableDictionary.Create(StringComparer.OrdinalIgnoreCase); + } + } + + public object _lockObject { get; } = new object(); + private CachedState _cachedState; + private bool _enabled; + + + public WorkloadPartialResolver() + { + // Support opt-out for workload resolution + _enabled = true; + var envVar = Environment.GetEnvironmentVariable("MSBuildEnableWorkloadResolver"); + if (envVar != null) + { + if (envVar.Equals("false", StringComparison.OrdinalIgnoreCase)) + { + _enabled = false; + } + } + + if (!_enabled) + { + string sentinelPath = Path.Combine(Path.GetDirectoryName(typeof(WorkloadPartialResolver).Assembly.Location), "DisableWorkloadResolver.sentinel"); + if (File.Exists(sentinelPath)) + { + _enabled = false; + } + } + } + + public record ResolutionResult() + { + public SdkResult ToSdkResult(SdkReference sdkReference, SdkResultFactory factory) + { + switch (this) + { + case SinglePathResolutionResult r: + return factory.IndicateSuccess(r.Path, sdkReference.Version); + case MultiplePathResolutionResult r: + return factory.IndicateSuccess(r.Paths, sdkReference.Version); + case EmptyResolutionResult r: + return factory.IndicateSuccess(Enumerable.Empty(), sdkReference.Version, r.propertiesToAdd, r.itemsToAdd); + case NullResolutionResult: + return null; + } + + throw new InvalidOperationException("Unknown resolutionResult type: " + this.GetType()); + } + } + + public record SinglePathResolutionResult( + string Path + ) : ResolutionResult; + + public record MultiplePathResolutionResult( + IEnumerable Paths + ) : ResolutionResult; + + public record EmptyResolutionResult( + IDictionary propertiesToAdd, + IDictionary itemsToAdd + ) : ResolutionResult; + + public record NullResolutionResult() : ResolutionResult; + + private static ResolutionResult Resolve(string sdkReferenceName, IWorkloadManifestProvider manifestProvider, IWorkloadResolver workloadResolver) + { + if (sdkReferenceName.Equals("Microsoft.NET.SDK.WorkloadAutoImportPropsLocator", StringComparison.OrdinalIgnoreCase)) + { + List autoImportSdkPaths = new List(); + foreach (var sdkPackInfo in workloadResolver.GetInstalledWorkloadPacksOfKind(WorkloadPackKind.Sdk)) + { + string sdkPackSdkFolder = Path.Combine(sdkPackInfo.Path, "Sdk"); + string autoImportPath = Path.Combine(sdkPackSdkFolder, "AutoImport.props"); + if (File.Exists(autoImportPath)) + { + autoImportSdkPaths.Add(sdkPackSdkFolder); + } + } + // Call Distinct() here because with aliased packs, there may be duplicates of the same path + return new MultiplePathResolutionResult(autoImportSdkPaths.Distinct()); + } + else if (sdkReferenceName.Equals("Microsoft.NET.SDK.WorkloadManifestTargetsLocator", StringComparison.OrdinalIgnoreCase)) + { + List workloadManifestPaths = new List(); + foreach (var manifestDirectory in manifestProvider.GetManifestDirectories()) + { + var workloadManifestTargetPath = Path.Combine(manifestDirectory, "WorkloadManifest.targets"); + if (File.Exists(workloadManifestTargetPath)) + { + workloadManifestPaths.Add(manifestDirectory); + } + } + return new MultiplePathResolutionResult(workloadManifestPaths); + } + else + { + var packInfo = workloadResolver.TryGetPackInfo(sdkReferenceName); + if (packInfo != null) + { + if (Directory.Exists(packInfo.Path)) + { + return new SinglePathResolutionResult(Path.Combine(packInfo.Path, "Sdk")); + } + else + { + var itemsToAdd = new Dictionary(); + itemsToAdd.Add("MissingWorkloadPack", + new SdkResultItem(sdkReferenceName, + metadata: new Dictionary() + { + { "Version", packInfo.Version } + })); + + Dictionary propertiesToAdd = new Dictionary(); + return new EmptyResolutionResult(propertiesToAdd, itemsToAdd); + } + } + } + return new NullResolutionResult(); + } + + public ResolutionResult Resolve(string sdkReferenceName, string dotnetRootPath, string sdkVersion +#if USE_SERILOG + , List> logActions +#endif + ) + { + if (!_enabled) + { + return new NullResolutionResult(); + } + + ResolutionResult resolutionResult; + + lock (_lockObject) + { + if (_cachedState == null || + _cachedState.DotnetRootPath != dotnetRootPath || + _cachedState.SdkVersion != sdkVersion) + { +#if USE_SERILOG + if (_cachedState == null) + { + logActions.Add(log => log.Information($"Creating initial {nameof(WorkloadPartialResolver)} state")); + } + else + { + logActions.Add(log => log.Information($"Recreating {nameof(WorkloadPartialResolver)} state (resolved SDK changed)")); + } +#endif + + var workloadManifestProvider = new SdkDirectoryWorkloadManifestProvider(dotnetRootPath, sdkVersion); + var workloadResolver = WorkloadResolver.Create(workloadManifestProvider, dotnetRootPath, sdkVersion); + + _cachedState = new CachedState() + { + DotnetRootPath = dotnetRootPath, + SdkVersion = sdkVersion, + ManifestProvider = workloadManifestProvider, + WorkloadResolver = workloadResolver + }; + } + else + { +#if USE_SERILOG + logActions.Add(log => log.Information($"Using cached {nameof(WorkloadPartialResolver)} state")); +#endif + } + + if (!_cachedState.CachedResults.TryGetValue(sdkReferenceName, out resolutionResult)) + { +#if USE_SERILOG + logActions.Add(log => log.Information($"Resolving workload in {nameof(WorkloadPartialResolver)}")); +#endif + resolutionResult = Resolve(sdkReferenceName, _cachedState.ManifestProvider, _cachedState.WorkloadResolver); + + _cachedState = _cachedState with + { + CachedResults = _cachedState.CachedResults.Add(sdkReferenceName, resolutionResult) + }; + } +#if USE_SERILOG + else + { + logActions.Add(log => log.Information($"Using cached workload resolution result in {nameof(WorkloadPartialResolver)}")); + } +#endif + } + + return resolutionResult; + } + } +} + +#if !NET +namespace System.Runtime.CompilerServices +{ + public class IsExternalInit { } +} +#endif diff --git a/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/WorkloadSdkResolver.cs b/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/WorkloadSdkResolver.cs index 2c6d6c30c7ec..e73301e5ee4d 100644 --- a/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/WorkloadSdkResolver.cs +++ b/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/WorkloadSdkResolver.cs @@ -1,4 +1,8 @@ -using Microsoft.Build.Framework; +// Copyright (c) .NET Foundation and contributors. All rights reserved. +// Licensed under the MIT license. See LICENSE file in the project root for full license information. + + +using Microsoft.Build.Framework; using System; using System.Collections.Generic; using System.Diagnostics; @@ -21,59 +25,17 @@ namespace Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver { + + // This SdkResolver is used by the .NET SDK version of MSBuild. Workload resolution logic which + // is shared with Full Framework / Visual Studio MSBuild is in WorkloadPartialResolver. public class WorkloadSdkResolver : SdkResolver { public override string Name => "Microsoft.DotNet.MSBuildWorkloadSdkResolver"; public override int Priority => 4000; - private bool _enabled; - - - // MSBuild SDK resolvers can use the SdkResolverContext's State property to cache state across multiple calls. However, - // MSBuild only caches those within the same build (which is tracked via a build submission ID). - // When running in Visual Studio, there are lots of evaluations that aren't associated with with a build, and then - // it looks like each project is evaluated under a separate build submission ID. So the built-in caching support doesn't - // work well when running in Visual Studio. - // Because of this, when running in Visual Studio, we will use our own cache state, which will be stored staticly. In order - // to avoid requiring that the workload manifest reader and resolver classes are fully thread safe, we include a lock object - // in the cache state which we use to ensure that multiple threads don't access it at the same time. We don't expect high - // concurrency in MSBuild SDK Resolver calls, so we don't expect the lock to impact the performance. - private static CachedState _staticCachedState; - -#if NETFRAMEWORK - private readonly NETCoreSdkResolver _sdkResolver; -#endif - public WorkloadSdkResolver() { - // Support opt-out for workload resolution - _enabled = true; - var envVar = Environment.GetEnvironmentVariable("MSBuildEnableWorkloadResolver"); - if (envVar != null) - { - if (envVar.Equals("false", StringComparison.OrdinalIgnoreCase)) - { - _enabled = false; - } - } - - if (!_enabled) - { - string sentinelPath = Path.Combine(Path.GetDirectoryName(typeof(WorkloadSdkResolver).Assembly.Location), "DisableWorkloadResolver.sentinel"); - if (File.Exists(sentinelPath)) - { - _enabled = false; - } - } - -#if NETFRAMEWORK - if (_enabled) - { - _sdkResolver = new NETCoreSdkResolver(); - } -#endif - #if USE_SERILOG _instanceId = System.Threading.Interlocked.Increment(ref _lastInstanceId); #endif @@ -98,119 +60,25 @@ static WorkloadSdkResolver() } #endif - private record CachedState + private class CachedState { - public object LockObject { get; init; } public string DotnetRootPath { get; init; } public string SdkVersion { get; init; } - public IWorkloadManifestProvider ManifestProvider { get; init; } - public IWorkloadResolver WorkloadResolver { get; init; } - public ImmutableDictionary CachedResults { get; init; } - - public CachedState() - { - LockObject = new object(); - CachedResults = ImmutableDictionary.Create(StringComparer.OrdinalIgnoreCase); - } - } - - private record ResolutionResult(); - private record SinglePathResolutionResult( - string Path - ) : ResolutionResult; - - private record MultiplePathResolutionResult( - IEnumerable Paths - ) : ResolutionResult; - - private record EmptyResolutionResult( - IDictionary propertiesToAdd, - IDictionary itemsToAdd - ) : ResolutionResult; - - private record NullResolutionResult() : ResolutionResult; - - private static ResolutionResult Resolve(string sdkReferenceName, IWorkloadManifestProvider manifestProvider, IWorkloadResolver workloadResolver) - { - if (sdkReferenceName.Equals("Microsoft.NET.SDK.WorkloadAutoImportPropsLocator", StringComparison.OrdinalIgnoreCase)) - { - List autoImportSdkPaths = new List(); - foreach (var sdkPackInfo in workloadResolver.GetInstalledWorkloadPacksOfKind(WorkloadPackKind.Sdk)) - { - string sdkPackSdkFolder = Path.Combine(sdkPackInfo.Path, "Sdk"); - string autoImportPath = Path.Combine(sdkPackSdkFolder, "AutoImport.props"); - if (File.Exists(autoImportPath)) - { - autoImportSdkPaths.Add(sdkPackSdkFolder); - } - } - // Call Distinct() here because with aliased packs, there may be duplicates of the same path - return new MultiplePathResolutionResult(autoImportSdkPaths.Distinct()); - } - else if (sdkReferenceName.Equals("Microsoft.NET.SDK.WorkloadManifestTargetsLocator", StringComparison.OrdinalIgnoreCase)) - { - List workloadManifestPaths = new List(); - foreach (var manifestDirectory in manifestProvider.GetManifestDirectories()) - { - var workloadManifestTargetPath = Path.Combine(manifestDirectory, "WorkloadManifest.targets"); - if (File.Exists(workloadManifestTargetPath)) - { - workloadManifestPaths.Add(manifestDirectory); - } - } - return new MultiplePathResolutionResult(workloadManifestPaths); - } - else - { - var packInfo = workloadResolver.TryGetPackInfo(sdkReferenceName); - if (packInfo != null) - { - if (Directory.Exists(packInfo.Path)) - { - return new SinglePathResolutionResult(Path.Combine(packInfo.Path, "Sdk")); - } - else - { - var itemsToAdd = new Dictionary(); - itemsToAdd.Add("MissingWorkloadPack", - new SdkResultItem(sdkReferenceName, - metadata: new Dictionary() - { - { "Version", packInfo.Version } - })); - - Dictionary propertiesToAdd = new Dictionary(); - return new EmptyResolutionResult(propertiesToAdd, itemsToAdd); - } - } - } - return new NullResolutionResult(); + public WorkloadPartialResolver WorkloadResolver { get; init; } } public override SdkResult Resolve(SdkReference sdkReference, SdkResolverContext resolverContext, SdkResultFactory factory) { - if (!_enabled) - { - return null; - } - #if USE_SERILOG List> logActions = new List>(); #endif CachedState cachedState = null; - if (resolverContext.IsRunningInVisualStudio) - { - cachedState = _staticCachedState; - } - else + if (resolverContext.State is CachedState resolverContextState) { - if (resolverContext.State is CachedState resolverContextState) - { - cachedState = resolverContextState; - } + cachedState = resolverContextState; } @@ -219,27 +87,20 @@ public override SdkResult Resolve(SdkReference sdkReference, SdkResolverContext #if USE_SERILOG logActions.Add(log => log.Information("Initializing resolver state")); #endif - - // If we don't have any cached state yet, then find the dotnet directory and SDK version, and create the workload resolver classes - // Note that in Visual Studio, we could end up doing this multiple times if the resolver gets called on multiple threads before any - // state has been cached. - var dotnetRootPath = GetDotNetRoot(resolverContext); var sdkDirectory = GetSdkDirectory(resolverContext); // The SDK version is the name of the SDK directory (ie dotnet\sdk\5.0.100) var sdkVersion = Path.GetFileName(sdkDirectory); - var workloadManifestProvider = new SdkDirectoryWorkloadManifestProvider(dotnetRootPath, sdkVersion); - var workloadResolver = WorkloadResolver.Create(workloadManifestProvider, dotnetRootPath, sdkVersion); - cachedState = new CachedState() { DotnetRootPath = dotnetRootPath, SdkVersion = sdkVersion, - ManifestProvider = workloadManifestProvider, - WorkloadResolver = workloadResolver + WorkloadResolver = new WorkloadPartialResolver() }; + + resolverContext.State = cachedState; } else { @@ -248,35 +109,11 @@ public override SdkResult Resolve(SdkReference sdkReference, SdkResolverContext #endif } - ResolutionResult resolutionResult; - lock (cachedState.LockObject) - { - if (!cachedState.CachedResults.TryGetValue(sdkReference.Name, out resolutionResult)) - { -#if USE_SERILOG - logActions.Add(log => log.Information("Resolving workload")); -#endif - resolutionResult = Resolve(sdkReference.Name, cachedState.ManifestProvider, cachedState.WorkloadResolver); - - cachedState = cachedState with - { - CachedResults = cachedState.CachedResults.Add(sdkReference.Name, resolutionResult) - }; - - resolverContext.State = cachedState; - - if (resolverContext.IsRunningInVisualStudio) - { - _staticCachedState = cachedState; - } - } + var result = cachedState.WorkloadResolver.Resolve(sdkReference.Name, cachedState.DotnetRootPath, cachedState.SdkVersion #if USE_SERILOG - else - { - logActions.Add(log => log.Information("Using cached workload resolution result")); - } + , logActions #endif - } + ); #if USE_SERILOG @@ -288,26 +125,14 @@ public override SdkResult Resolve(SdkReference sdkReference, SdkResolverContext .ForContext("ResolverInstance", _instanceId) .ForContext("RunningInVS", resolverContext.IsRunningInVisualStudio) .ForContext("MSBuildSubmissionId", msbuildSubmissionId); - + foreach (var logAction in logActions) { logAction(log); } #endif - switch (resolutionResult) - { - case SinglePathResolutionResult r: - return factory.IndicateSuccess(r.Path, sdkReference.Version); - case MultiplePathResolutionResult r: - return factory.IndicateSuccess(r.Paths, sdkReference.Version); - case EmptyResolutionResult r: - return factory.IndicateSuccess(Enumerable.Empty(), sdkReference.Version, r.propertiesToAdd, r.itemsToAdd); - case NullResolutionResult: - return null; - } - - throw new InvalidOperationException("Unknown resolutionResult type: " + resolutionResult.GetType()); + return result.ToSdkResult(sdkReference, factory); } @@ -336,9 +161,3 @@ private string GetDotNetRoot(SdkResolverContext context) } } -#if !NET -namespace System.Runtime.CompilerServices -{ - public class IsExternalInit { } -} -#endif From 0af475b22987109712a98146e07143f9b072a212 Mon Sep 17 00:00:00 2001 From: Daniel Plaisted Date: Wed, 21 Apr 2021 23:11:03 -0700 Subject: [PATCH 09/12] Rename WorkloadPartialResolver to CachingWorkloadResolver --- .../MSBuildSdkResolver.cs | 10 +++++----- ...Microsoft.DotNet.MSBuildSdkResolver.csproj | 2 +- ...Resolver.cs => CachingWorkloadResolver.cs} | 20 +++++++++---------- .../WorkloadSdkResolver.cs | 6 +++--- 4 files changed, 19 insertions(+), 19 deletions(-) rename src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/{WorkloadPartialResolver.cs => CachingWorkloadResolver.cs} (94%) diff --git a/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/MSBuildSdkResolver.cs b/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/MSBuildSdkResolver.cs index 879032552d96..052f1ce939d2 100644 --- a/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/MSBuildSdkResolver.cs +++ b/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/MSBuildSdkResolver.cs @@ -36,7 +36,7 @@ public sealed class DotNetMSBuildSdkResolver : SdkResolver private readonly Func _getEnvironmentVariable; private readonly NETCoreSdkResolver _netCoreSdkResolver; - private static WorkloadPartialResolver _staticWorkloadResolver = new WorkloadPartialResolver(); + private static CachingWorkloadResolver _staticWorkloadResolver = new CachingWorkloadResolver(); public DotNetMSBuildSdkResolver() : this(Environment.GetEnvironmentVariable, VSSettings.Ambient) @@ -79,7 +79,7 @@ private sealed class CachedState public string MSBuildSdksDir; public string NETCoreSdkVersion; public IDictionary PropertiesToAdd; - public WorkloadPartialResolver WorkloadResolver; + public CachingWorkloadResolver WorkloadResolver; } public override SdkResult Resolve(SdkReference sdkReference, SdkResolverContext context, SdkResultFactory factory) @@ -120,7 +120,7 @@ private SdkResult Resolve(SdkReference sdkReference, SdkResolverContext context, IDictionary propertiesToAdd = null; IDictionary itemsToAdd = null; List warnings = null; - WorkloadPartialResolver workloadResolver = null; + CachingWorkloadResolver workloadResolver = null; if (context.State is CachedState priorResult) { @@ -144,7 +144,7 @@ private SdkResult Resolve(SdkReference sdkReference, SdkResolverContext context, if (workloadResolver == null) { - workloadResolver = new WorkloadPartialResolver(); + workloadResolver = new CachingWorkloadResolver(); } if (msbuildSdksDir == null) @@ -239,7 +239,7 @@ private SdkResult Resolve(SdkReference sdkReference, SdkResolverContext context, #endif ); - if (workloadResult is not WorkloadPartialResolver.NullResolutionResult) + if (workloadResult is not CachingWorkloadResolver.NullResolutionResult) { return workloadResult.ToSdkResult(sdkReference, factory); } diff --git a/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/Microsoft.DotNet.MSBuildSdkResolver.csproj b/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/Microsoft.DotNet.MSBuildSdkResolver.csproj index 421eb81417e2..bb06591255eb 100644 --- a/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/Microsoft.DotNet.MSBuildSdkResolver.csproj +++ b/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/Microsoft.DotNet.MSBuildSdkResolver.csproj @@ -61,7 +61,7 @@ - + diff --git a/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/WorkloadPartialResolver.cs b/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/CachingWorkloadResolver.cs similarity index 94% rename from src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/WorkloadPartialResolver.cs rename to src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/CachingWorkloadResolver.cs index fa93ab9b0c58..4269bf3ebcd2 100644 --- a/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/WorkloadPartialResolver.cs +++ b/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/CachingWorkloadResolver.cs @@ -31,9 +31,9 @@ namespace Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver // so the state caching support provided by MSBuild for SDK Resolvers doesn't really help. Additionally, multiple instances of the SDK resolver // may be created, and the same instance may be called on multiple threads. So state needs to be cached staticly and be thread-safe. // - // To keep the state static, the MSBuildSdkResolver keeps a static reference to the WorkloadPartialResolver that is used if the build is inside + // To keep the state static, the MSBuildSdkResolver keeps a static reference to the CachingWorkloadResolver that is used if the build is inside // Visual Studio. To keep it thread-safe, the body of the Resolve method is all protected by a lock statement. This avoids having to make - // the classes consumed by the WorkloadPartialResolver (the manifest provider and workload resolver) thread-safe. + // the classes consumed by the CachingWorkloadResolver (the manifest provider and workload resolver) thread-safe. // // A resolver should not over-cache and return out-of-date results. For workloads, the resolution could change due to: // - Installation, update, or uninstallation of a workload @@ -41,7 +41,7 @@ namespace Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver // For SDK or workload installation actions, we expect to be running under a new process since Visual Studio will have been restarted. // For global.json changes, the Resolve method takes parameters for the dotnet root and the SDK version. If those values have changed // from the previous call, the cached state will be thrown out and recreated. - class WorkloadPartialResolver + class CachingWorkloadResolver { private record CachedState { @@ -62,7 +62,7 @@ public CachedState() private bool _enabled; - public WorkloadPartialResolver() + public CachingWorkloadResolver() { // Support opt-out for workload resolution _enabled = true; @@ -77,7 +77,7 @@ public WorkloadPartialResolver() if (!_enabled) { - string sentinelPath = Path.Combine(Path.GetDirectoryName(typeof(WorkloadPartialResolver).Assembly.Location), "DisableWorkloadResolver.sentinel"); + string sentinelPath = Path.Combine(Path.GetDirectoryName(typeof(CachingWorkloadResolver).Assembly.Location), "DisableWorkloadResolver.sentinel"); if (File.Exists(sentinelPath)) { _enabled = false; @@ -199,11 +199,11 @@ public ResolutionResult Resolve(string sdkReferenceName, string dotnetRootPath, #if USE_SERILOG if (_cachedState == null) { - logActions.Add(log => log.Information($"Creating initial {nameof(WorkloadPartialResolver)} state")); + logActions.Add(log => log.Information($"Creating initial {nameof(CachingWorkloadResolver)} state")); } else { - logActions.Add(log => log.Information($"Recreating {nameof(WorkloadPartialResolver)} state (resolved SDK changed)")); + logActions.Add(log => log.Information($"Recreating {nameof(CachingWorkloadResolver)} state (resolved SDK changed)")); } #endif @@ -221,14 +221,14 @@ public ResolutionResult Resolve(string sdkReferenceName, string dotnetRootPath, else { #if USE_SERILOG - logActions.Add(log => log.Information($"Using cached {nameof(WorkloadPartialResolver)} state")); + logActions.Add(log => log.Information($"Using cached {nameof(CachingWorkloadResolver)} state")); #endif } if (!_cachedState.CachedResults.TryGetValue(sdkReferenceName, out resolutionResult)) { #if USE_SERILOG - logActions.Add(log => log.Information($"Resolving workload in {nameof(WorkloadPartialResolver)}")); + logActions.Add(log => log.Information($"Resolving workload in {nameof(CachingWorkloadResolver)}")); #endif resolutionResult = Resolve(sdkReferenceName, _cachedState.ManifestProvider, _cachedState.WorkloadResolver); @@ -240,7 +240,7 @@ public ResolutionResult Resolve(string sdkReferenceName, string dotnetRootPath, #if USE_SERILOG else { - logActions.Add(log => log.Information($"Using cached workload resolution result in {nameof(WorkloadPartialResolver)}")); + logActions.Add(log => log.Information($"Using cached workload resolution result in {nameof(CachingWorkloadResolver)}")); } #endif } diff --git a/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/WorkloadSdkResolver.cs b/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/WorkloadSdkResolver.cs index e73301e5ee4d..330f2d6565f8 100644 --- a/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/WorkloadSdkResolver.cs +++ b/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/WorkloadSdkResolver.cs @@ -27,7 +27,7 @@ namespace Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver { // This SdkResolver is used by the .NET SDK version of MSBuild. Workload resolution logic which - // is shared with Full Framework / Visual Studio MSBuild is in WorkloadPartialResolver. + // is shared with Full Framework / Visual Studio MSBuild is in CachingWorkloadResolver. public class WorkloadSdkResolver : SdkResolver { public override string Name => "Microsoft.DotNet.MSBuildWorkloadSdkResolver"; @@ -65,7 +65,7 @@ private class CachedState public string DotnetRootPath { get; init; } public string SdkVersion { get; init; } - public WorkloadPartialResolver WorkloadResolver { get; init; } + public CachingWorkloadResolver WorkloadResolver { get; init; } } public override SdkResult Resolve(SdkReference sdkReference, SdkResolverContext resolverContext, SdkResultFactory factory) @@ -97,7 +97,7 @@ public override SdkResult Resolve(SdkReference sdkReference, SdkResolverContext { DotnetRootPath = dotnetRootPath, SdkVersion = sdkVersion, - WorkloadResolver = new WorkloadPartialResolver() + WorkloadResolver = new CachingWorkloadResolver() }; resolverContext.State = cachedState; From 9da93056d3425c1340bf28c446af10049fd1a4a1 Mon Sep 17 00:00:00 2001 From: Daniel Plaisted Date: Thu, 22 Apr 2021 00:28:07 -0700 Subject: [PATCH 10/12] Apply code review feedback --- .../MSBuildSdkResolver.cs | 6 ------ .../CachingWorkloadResolver.cs | 2 ++ 2 files changed, 2 insertions(+), 6 deletions(-) diff --git a/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/MSBuildSdkResolver.cs b/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/MSBuildSdkResolver.cs index 052f1ce939d2..f8e49c38afe5 100644 --- a/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/MSBuildSdkResolver.cs +++ b/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/MSBuildSdkResolver.cs @@ -136,12 +136,6 @@ private SdkResult Resolve(SdkReference sdkReference, SdkResolverContext context, workloadResolver = _staticWorkloadResolver; } - if (msbuildSdksDir == null) - { - - dotnetRoot = _netCoreSdkResolver.GetDotnetExeDirectory(); - } - if (workloadResolver == null) { workloadResolver = new CachingWorkloadResolver(); diff --git a/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/CachingWorkloadResolver.cs b/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/CachingWorkloadResolver.cs index 4269bf3ebcd2..acc4fb78b0ae 100644 --- a/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/CachingWorkloadResolver.cs +++ b/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/CachingWorkloadResolver.cs @@ -250,6 +250,8 @@ public ResolutionResult Resolve(string sdkReferenceName, string dotnetRootPath, } } + +// Add attribute to support init-only properties on .NET Framework #if !NET namespace System.Runtime.CompilerServices { From a1de9fd9bf40bdc40646727de3d0dc791683ec6a Mon Sep 17 00:00:00 2001 From: Daniel Plaisted Date: Thu, 22 Apr 2021 00:43:40 -0700 Subject: [PATCH 11/12] Remove Serilog logging --- Directory.Build.props | 4 -- .../MSBuildSdkResolver.cs | 63 +----------------- ...Microsoft.DotNet.MSBuildSdkResolver.csproj | 5 -- .../CachingWorkloadResolver.cs | 36 +--------- ....NET.Sdk.WorkloadMSBuildSdkResolver.csproj | 5 -- .../WorkloadSdkResolver.cs | 66 +------------------ 6 files changed, 3 insertions(+), 176 deletions(-) diff --git a/Directory.Build.props b/Directory.Build.props index 1827fbcb0c06..d849ce76c312 100644 --- a/Directory.Build.props +++ b/Directory.Build.props @@ -19,10 +19,6 @@ $(DefineConstants);CI_BUILD - - - false - $(DefineConstants);USE_SERILOG diff --git a/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/MSBuildSdkResolver.cs b/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/MSBuildSdkResolver.cs index f8e49c38afe5..630923439c57 100644 --- a/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/MSBuildSdkResolver.cs +++ b/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/MSBuildSdkResolver.cs @@ -11,10 +11,6 @@ using System.Reflection; using Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver; -#if USE_SERILOG -using Serilog; -#endif - #nullable disable namespace Microsoft.DotNet.MSBuildSdkResolver @@ -48,30 +44,7 @@ public DotNetMSBuildSdkResolver(Func getEnvironmentVariable, VSS { _getEnvironmentVariable = getEnvironmentVariable; _netCoreSdkResolver = new NETCoreSdkResolver(getEnvironmentVariable, vsSettings); - -#if USE_SERILOG - _instanceId = System.Threading.Interlocked.Increment(ref _lastInstanceId); -#endif - } - -#if USE_SERILOG - static Serilog.Core.Logger Logger; - - static int _lastInstanceId = 1; - int _instanceId; - - static DotNetMSBuildSdkResolver() - { - Logger = new LoggerConfiguration() - .WriteTo.Seq("http://localhost:5341") - .CreateLogger(); - - AppDomain.CurrentDomain.ProcessExit += (_, _) => - { - Logger.Dispose(); - }; } -#endif private sealed class CachedState { @@ -84,36 +57,6 @@ private sealed class CachedState public override SdkResult Resolve(SdkReference sdkReference, SdkResolverContext context, SdkResultFactory factory) { -#if USE_SERILOG - List> logActions = new List>(); - - var result = Resolve(sdkReference, context, factory, logActions); - - var msbuildSubmissionId = (int?)System.Threading.Thread.GetData(System.Threading.Thread.GetNamedDataSlot("MSBuildSubmissionId")); - var log = Logger - .ForContext("Resolver", "Sdk") - .ForContext("Process", System.Diagnostics.Process.GetCurrentProcess().Id) - .ForContext("Thread", System.Threading.Thread.CurrentThread.ManagedThreadId) - .ForContext("ResolverInstance", _instanceId) - .ForContext("RunningInVS", context.IsRunningInVisualStudio) - .ForContext("MSBuildSubmissionId", msbuildSubmissionId) - .ForContext("HasCache", context.State != null) - .ForContext("SdkName", sdkReference.Name); - - foreach (var logAction in logActions) - { - logAction(log); - } - - return result; - } - - private SdkResult Resolve(SdkReference sdkReference, SdkResolverContext context, SdkResultFactory factory, List> logActions) - { - - logActions.Add(log => log.Information("Resolving SDK {sdkName}", sdkReference.Name)); -#endif - string dotnetRoot = null; string msbuildSdksDir = null; string netcoreSdkVersion = null; @@ -227,11 +170,7 @@ private SdkResult Resolve(SdkReference sdkReference, SdkResolverContext context, }; // First check if requested SDK resolves to a workload SDK pack - var workloadResult = workloadResolver.Resolve(sdkReference.Name, dotnetRoot, netcoreSdkVersion -#if USE_SERILOG - , logActions: logActions -#endif - ); + var workloadResult = workloadResolver.Resolve(sdkReference.Name, dotnetRoot, netcoreSdkVersion); if (workloadResult is not CachingWorkloadResolver.NullResolutionResult) { diff --git a/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/Microsoft.DotNet.MSBuildSdkResolver.csproj b/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/Microsoft.DotNet.MSBuildSdkResolver.csproj index bb06591255eb..5feaf2601132 100644 --- a/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/Microsoft.DotNet.MSBuildSdkResolver.csproj +++ b/src/Resolvers/Microsoft.DotNet.MSBuildSdkResolver/Microsoft.DotNet.MSBuildSdkResolver.csproj @@ -84,11 +84,6 @@ - - - - - diff --git a/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/CachingWorkloadResolver.cs b/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/CachingWorkloadResolver.cs index acc4fb78b0ae..1a174d0f07d6 100644 --- a/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/CachingWorkloadResolver.cs +++ b/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/CachingWorkloadResolver.cs @@ -16,10 +16,6 @@ using Microsoft.DotNet.DotNetSdkResolver; #endif -#if USE_SERILOG -using Serilog; -#endif - #nullable disable namespace Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver @@ -177,11 +173,7 @@ private static ResolutionResult Resolve(string sdkReferenceName, IWorkloadManife return new NullResolutionResult(); } - public ResolutionResult Resolve(string sdkReferenceName, string dotnetRootPath, string sdkVersion -#if USE_SERILOG - , List> logActions -#endif - ) + public ResolutionResult Resolve(string sdkReferenceName, string dotnetRootPath, string sdkVersion) { if (!_enabled) { @@ -196,17 +188,6 @@ public ResolutionResult Resolve(string sdkReferenceName, string dotnetRootPath, _cachedState.DotnetRootPath != dotnetRootPath || _cachedState.SdkVersion != sdkVersion) { -#if USE_SERILOG - if (_cachedState == null) - { - logActions.Add(log => log.Information($"Creating initial {nameof(CachingWorkloadResolver)} state")); - } - else - { - logActions.Add(log => log.Information($"Recreating {nameof(CachingWorkloadResolver)} state (resolved SDK changed)")); - } -#endif - var workloadManifestProvider = new SdkDirectoryWorkloadManifestProvider(dotnetRootPath, sdkVersion); var workloadResolver = WorkloadResolver.Create(workloadManifestProvider, dotnetRootPath, sdkVersion); @@ -218,18 +199,9 @@ public ResolutionResult Resolve(string sdkReferenceName, string dotnetRootPath, WorkloadResolver = workloadResolver }; } - else - { -#if USE_SERILOG - logActions.Add(log => log.Information($"Using cached {nameof(CachingWorkloadResolver)} state")); -#endif - } if (!_cachedState.CachedResults.TryGetValue(sdkReferenceName, out resolutionResult)) { -#if USE_SERILOG - logActions.Add(log => log.Information($"Resolving workload in {nameof(CachingWorkloadResolver)}")); -#endif resolutionResult = Resolve(sdkReferenceName, _cachedState.ManifestProvider, _cachedState.WorkloadResolver); _cachedState = _cachedState with @@ -237,12 +209,6 @@ public ResolutionResult Resolve(string sdkReferenceName, string dotnetRootPath, CachedResults = _cachedState.CachedResults.Add(sdkReferenceName, resolutionResult) }; } -#if USE_SERILOG - else - { - logActions.Add(log => log.Information($"Using cached workload resolution result in {nameof(CachingWorkloadResolver)}")); - } -#endif } return resolutionResult; diff --git a/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver.csproj b/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver.csproj index 8f2fad9f5e12..644fdf2e2eac 100644 --- a/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver.csproj +++ b/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver.csproj @@ -30,11 +30,6 @@ - - - - - <_ResolverManifestDir>$(OutputPath)\SdkResolvers\$(AssemblyName)\ diff --git a/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/WorkloadSdkResolver.cs b/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/WorkloadSdkResolver.cs index 330f2d6565f8..804466f85b22 100644 --- a/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/WorkloadSdkResolver.cs +++ b/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/WorkloadSdkResolver.cs @@ -17,10 +17,6 @@ using Microsoft.DotNet.DotNetSdkResolver; #endif -#if USE_SERILOG -using Serilog; -#endif - #nullable disable namespace Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver @@ -34,32 +30,6 @@ public class WorkloadSdkResolver : SdkResolver public override int Priority => 4000; - public WorkloadSdkResolver() - { -#if USE_SERILOG - _instanceId = System.Threading.Interlocked.Increment(ref _lastInstanceId); -#endif - } - -#if USE_SERILOG - static Serilog.Core.Logger Logger; - - static int _lastInstanceId = 1; - int _instanceId; - - static WorkloadSdkResolver() - { - Logger = new LoggerConfiguration() - .WriteTo.Seq("http://localhost:5341") - .CreateLogger(); - - AppDomain.CurrentDomain.ProcessExit += (_, _) => - { - Logger.Dispose(); - }; - } -#endif - private class CachedState { public string DotnetRootPath { get; init; } @@ -70,10 +40,6 @@ private class CachedState public override SdkResult Resolve(SdkReference sdkReference, SdkResolverContext resolverContext, SdkResultFactory factory) { -#if USE_SERILOG - List> logActions = new List>(); -#endif - CachedState cachedState = null; if (resolverContext.State is CachedState resolverContextState) @@ -84,9 +50,6 @@ public override SdkResult Resolve(SdkReference sdkReference, SdkResolverContext if (cachedState == null) { -#if USE_SERILOG - logActions.Add(log => log.Information("Initializing resolver state")); -#endif var dotnetRootPath = GetDotNetRoot(resolverContext); var sdkDirectory = GetSdkDirectory(resolverContext); @@ -102,38 +65,11 @@ public override SdkResult Resolve(SdkReference sdkReference, SdkResolverContext resolverContext.State = cachedState; } - else - { -#if USE_SERILOG - logActions.Add(log => log.Information("Using cached resolver state")); -#endif - } - var result = cachedState.WorkloadResolver.Resolve(sdkReference.Name, cachedState.DotnetRootPath, cachedState.SdkVersion -#if USE_SERILOG - , logActions -#endif - ); + var result = cachedState.WorkloadResolver.Resolve(sdkReference.Name, cachedState.DotnetRootPath, cachedState.SdkVersion); -#if USE_SERILOG - var msbuildSubmissionId = (int?)System.Threading.Thread.GetData(System.Threading.Thread.GetNamedDataSlot("MSBuildSubmissionId")); - var log = Logger - .ForContext("Resolver", "Sdk") - .ForContext("Process", System.Diagnostics.Process.GetCurrentProcess().Id) - .ForContext("Thread", System.Threading.Thread.CurrentThread.ManagedThreadId) - .ForContext("ResolverInstance", _instanceId) - .ForContext("RunningInVS", resolverContext.IsRunningInVisualStudio) - .ForContext("MSBuildSubmissionId", msbuildSubmissionId); - - foreach (var logAction in logActions) - { - logAction(log); - } -#endif - return result.ToSdkResult(sdkReference, factory); - } private string GetSdkDirectory(SdkResolverContext context) From 77f444cd24e8cb6817c968dbbe6338859cabecda Mon Sep 17 00:00:00 2001 From: Daniel Plaisted Date: Thu, 22 Apr 2021 10:04:14 -0700 Subject: [PATCH 12/12] Apply code review feedback --- .../CachingWorkloadResolver.cs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/CachingWorkloadResolver.cs b/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/CachingWorkloadResolver.cs index 1a174d0f07d6..351b003d4a0e 100644 --- a/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/CachingWorkloadResolver.cs +++ b/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/CachingWorkloadResolver.cs @@ -39,7 +39,7 @@ namespace Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver // from the previous call, the cached state will be thrown out and recreated. class CachingWorkloadResolver { - private record CachedState + private sealed record CachedState { public string DotnetRootPath { get; init; } public string SdkVersion { get; init; } @@ -55,7 +55,7 @@ public CachedState() public object _lockObject { get; } = new object(); private CachedState _cachedState; - private bool _enabled; + private readonly bool _enabled; public CachingWorkloadResolver() @@ -71,7 +71,7 @@ public CachingWorkloadResolver() } } - if (!_enabled) + if (_enabled) { string sentinelPath = Path.Combine(Path.GetDirectoryName(typeof(CachingWorkloadResolver).Assembly.Location), "DisableWorkloadResolver.sentinel"); if (File.Exists(sentinelPath)) @@ -101,20 +101,20 @@ public SdkResult ToSdkResult(SdkReference sdkReference, SdkResultFactory factory } } - public record SinglePathResolutionResult( + public sealed record SinglePathResolutionResult( string Path ) : ResolutionResult; - public record MultiplePathResolutionResult( + public sealed record MultiplePathResolutionResult( IEnumerable Paths ) : ResolutionResult; - public record EmptyResolutionResult( + public sealed record EmptyResolutionResult( IDictionary propertiesToAdd, IDictionary itemsToAdd ) : ResolutionResult; - public record NullResolutionResult() : ResolutionResult; + public sealed record NullResolutionResult() : ResolutionResult; private static ResolutionResult Resolve(string sdkReferenceName, IWorkloadManifestProvider manifestProvider, IWorkloadResolver workloadResolver) {