Replace workload TempDirResolver with more generic OverlayResolver

src/Cli/dotnet/commands/dotnet-workload/install/WorkloadInstallCommand.cs

@@ -38,7 +38,6 @@ internal class WorkloadInstallCommand : CommandBase
         private readonly IReadOnlyCollection<string> _workloadIds;
         private readonly IInstaller _workloadInstaller;
         private IWorkloadResolver _workloadResolver;
-        private IWorkloadManifestProvider _workloadManifestProvider;
         private readonly INuGetPackageDownloader _nugetPackageDownloader;
         private readonly IWorkloadManifestUpdater _workloadManifestUpdater;
         private readonly ReleaseVersion _sdkVersion;
@@ -80,8 +79,8 @@ public WorkloadInstallCommand(
             _packageSourceLocation = string.IsNullOrEmpty(configOption) && (sourceOption == null || !sourceOption.Any()) ? null :
                 new PackageSourceLocation(string.IsNullOrEmpty(configOption) ? null : new FilePath(configOption), sourceFeedOverrides: sourceOption);
 
-            _workloadManifestProvider = new SdkDirectoryWorkloadManifestProvider(_dotnetPath, _sdkVersion.ToString());
-            _workloadResolver = workloadResolver ?? WorkloadResolver.Create(_workloadManifestProvider, _dotnetPath, _sdkVersion.ToString());
+            var _sdkWorkloadManifestProvider = new SdkDirectoryWorkloadManifestProvider(_dotnetPath, _sdkVersion.ToString());
+            _workloadResolver = workloadResolver ?? WorkloadResolver.Create(_sdkWorkloadManifestProvider, _dotnetPath, _sdkVersion.ToString());
             var sdkFeatureBand = new SdkFeatureBand(_sdkVersion);
             var tempPackagesDir = new DirectoryPath(Path.Combine(_tempDirPath, "dotnet-sdk-advertising-temp"));
             var restoreActionConfig = _parseResult.ToRestoreActionConfig();
@@ -95,7 +94,7 @@ public WorkloadInstallCommand(
                                      _workloadResolver, _verbosity, _nugetPackageDownloader, _dotnetPath, _tempDirPath,
                                      _packageSourceLocation, restoreActionConfig, elevationRequired: !_printDownloadLinkOnly && string.IsNullOrWhiteSpace(_downloadToCacheOption));
             _userHome = userHome ?? CliFolderPathCalculator.DotnetHomePath;
-            _workloadManifestUpdater = workloadManifestUpdater ?? new WorkloadManifestUpdater(_reporter, _workloadManifestProvider, _workloadResolver, _nugetPackageDownloader, _userHome, _tempDirPath, _packageSourceLocation);
+            _workloadManifestUpdater = workloadManifestUpdater ?? new WorkloadManifestUpdater(_reporter, _sdkWorkloadManifestProvider, _workloadResolver, _nugetPackageDownloader, _userHome, _tempDirPath, _packageSourceLocation);
 
             ValidateWorkloadIdsInput();
         }
@@ -318,8 +317,8 @@ private async Task UseTempManifestsToResolvePacksAsync(DirectoryPath tempPath, b
                 return;
             }
             await _workloadManifestUpdater.ExtractManifestPackagesToTempDirAsync(manifestPackagePaths, tempPath);
-            _workloadManifestProvider = new TempDirectoryWorkloadManifestProvider(tempPath.Value, _sdkVersion.ToString());
-            _workloadResolver = _workloadResolver.CreateTempDirResolver(_workloadManifestProvider, _dotnetPath, _sdkVersion.ToString());
+            var overlayProvider = new TempDirectoryWorkloadManifestProvider(tempPath.Value, _sdkVersion.ToString());
+            _workloadResolver = _workloadResolver.CreateOverlayResolver(overlayProvider);
         }
 
         private async Task DownloadToOfflineCacheAsync(IEnumerable<WorkloadId> workloadIds, DirectoryPath offlineCache, bool skipManifestUpdate, bool includePreviews)
@@ -332,8 +331,8 @@ private async Task DownloadToOfflineCacheAsync(IEnumerable<WorkloadId> workloadI
                 {
                     tempManifestDir = Path.Combine(offlineCache.Value, "temp-manifests");
                     await _workloadManifestUpdater.ExtractManifestPackagesToTempDirAsync(manifestPackagePaths, new DirectoryPath(tempManifestDir));
-                    _workloadManifestProvider = new TempDirectoryWorkloadManifestProvider(tempManifestDir, _sdkVersion.ToString());
-                    _workloadResolver = _workloadResolver.CreateTempDirResolver(_workloadManifestProvider, _dotnetPath, _sdkVersion.ToString());
+                    var overlayProvider = new TempDirectoryWorkloadManifestProvider(tempManifestDir, _sdkVersion.ToString());
+                    _workloadResolver = _workloadResolver.CreateOverlayResolver(overlayProvider);
                 }
                 else
                 {

src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/IWorkloadResolver.cs

@@ -11,7 +11,6 @@ public interface IWorkloadResolver
         IEnumerable<string> GetPacksInWorkload(string workloadId);
         ISet<WorkloadResolver.WorkloadInfo> GetWorkloadSuggestionForMissingPacks(IList<string> packId);
         IEnumerable<WorkloadDefinition> GetAvailableWorkloads();
-        WorkloadResolver CreateTempDirResolver(IWorkloadManifestProvider manifestProvider, string dotnetRootPath, string sdkVersion);
         bool IsWorkloadPlatformCompatible(WorkloadId workloadId);
         string GetManifestVersion(string manifestId);
         IDictionary<string, string> GetInstalledManifests();
@@ -30,6 +29,12 @@ public interface IWorkloadResolver
         /// <summary>
         /// Refresh workload and pack information based on the current installed workload manifest files
         /// </summary>
+        /// <remarks>This is not valid for overlay resolvers</remarks>
         void RefreshWorkloadManifests();
+
+        /// <summary>
+        /// Derives a resolver from this resolver by overlaying a set of updated manifests and recomposing.
+        /// </summary>
+        IWorkloadResolver CreateOverlayResolver(IWorkloadManifestProvider overlayManifestProvider);
     }
 }

src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/WorkloadResolver.cs

@@ -17,9 +17,10 @@ namespace Microsoft.NET.Sdk.WorkloadManifestReader
     /// </remarks>
     public class WorkloadResolver : IWorkloadResolver
     {
+        private readonly Dictionary<string, WorkloadManifest> _manifests = new Dictionary<string, WorkloadManifest>(StringComparer.OrdinalIgnoreCase);
         private readonly Dictionary<WorkloadId, WorkloadDefinition> _workloads = new Dictionary<WorkloadId, WorkloadDefinition>();
         private readonly Dictionary<WorkloadPackId, WorkloadPack> _packs = new Dictionary<WorkloadPackId, WorkloadPack>();
-        private readonly IWorkloadManifestProvider _manifestProvider;
+        private IWorkloadManifestProvider? _manifestProvider;
         private string[] _currentRuntimeIdentifiers;
         private readonly string [] _dotnetRootPaths;
 
@@ -57,59 +58,70 @@ public static WorkloadResolver CreateForTests(IWorkloadManifestProvider manifest
             return new WorkloadResolver(manifestProvider, dotNetRootPaths, currentRuntimeIdentifiers);
         }
 
-        public WorkloadResolver CreateTempDirResolver(IWorkloadManifestProvider manifestProvider, string dotnetRootPath, string sdkVersion)
+        /// <summary>
+        /// Creates a resolver by composing all the manifests from the provider.
+        /// </summary>
+        private WorkloadResolver(IWorkloadManifestProvider manifestProvider, string [] dotnetRootPaths, string [] currentRuntimeIdentifiers)
+            : this (dotnetRootPaths, currentRuntimeIdentifiers)
         {
-            var packRootEnvironmentVariable = Environment.GetEnvironmentVariable("DOTNETSDK_WORKLOAD_PACK_ROOTS");
-            string[] dotnetRootPaths;
-            if (!string.IsNullOrEmpty(packRootEnvironmentVariable))
-            {
-                dotnetRootPaths = packRootEnvironmentVariable.Split(Path.PathSeparator).Append(dotnetRootPath).ToArray();
-            }
-            else
-            {
-                dotnetRootPaths = new[] { dotnetRootPath };
-            }
+            _manifestProvider = manifestProvider;
 
-            return new WorkloadResolver(manifestProvider, dotnetRootPaths, _currentRuntimeIdentifiers);
+            LoadManifestsFromProvider(manifestProvider);
+            ComposeWorkloadManifests();
         }
 
-        private WorkloadResolver(IWorkloadManifestProvider manifestProvider, string [] dotnetRootPaths, string [] currentRuntimeIdentifiers)
+        /// <summary>
+        /// Creates a resolver with no manifests.
+        /// </summary>A
+        private WorkloadResolver(string[] dotnetRootPaths, string[] currentRuntimeIdentifiers)
         {
             _dotnetRootPaths = dotnetRootPaths;
             _currentRuntimeIdentifiers = currentRuntimeIdentifiers;
-            _manifestProvider = manifestProvider;
-
-            RefreshWorkloadManifests();
         }
 
         public void RefreshWorkloadManifests()
         {
-            _workloads.Clear();
-            _packs.Clear();
-
-            var manifests = new Dictionary<string,WorkloadManifest>(StringComparer.OrdinalIgnoreCase);
-
-            foreach ((string manifestId, Func<Stream> openManifestStream) in _manifestProvider.GetManifests())
+            if (_manifestProvider == null)
+            {
+                throw new InvalidOperationException("Resolver was created without provider and cannot be refreshed");
+            }
+            _manifests.Clear();
+            LoadManifestsFromProvider(_manifestProvider);
+            ComposeWorkloadManifests();
+        }
 
+        private void LoadManifestsFromProvider(IWorkloadManifestProvider manifestProvider)
+        {
+            foreach ((string manifestId, Func<Stream> openManifestStream) in manifestProvider.GetManifests())
             {
                 using (var manifestStream = openManifestStream())
                 {
                     var manifest = WorkloadManifestReader.ReadWorkloadManifest(manifestId, manifestStream);
-                    if (manifests.ContainsKey(manifestId))
+                    if (_manifests.ContainsKey(manifestId))
                     {
                         throw new Exception($"Duplicate workload manifest {manifestId}");
                     }
-                    manifests.Add(manifestId, manifest);
+                    if(!string.Equals (manifestId, manifest.Id, StringComparison.OrdinalIgnoreCase))
+                    {
+                        throw new Exception($"Manifest provider {manifestProvider} supplied manifest '{manifestId}' does not match payload id '{manifest.Id}'");
+                    }
+                    _manifests.Add(manifestId, manifest);
                 }
             }
+        }
+
+        private void ComposeWorkloadManifests()
+        {
+            _workloads.Clear();
+            _packs.Clear();
 
-            foreach (var manifest in manifests.Values)
+            foreach (var manifest in _manifests.Values)
             {
                 if (manifest.DependsOnManifests != null)
                 {
                     foreach (var dependency in manifest.DependsOnManifests)
                     {
-                        if (manifests.TryGetValue(dependency.Key, out var resolvedDependency))
+                        if (_manifests.TryGetValue(dependency.Key, out var resolvedDependency))
                         {
                             if (FXVersion.Compare(dependency.Value, resolvedDependency.ParsedVersion) > 0)
                             {
@@ -456,6 +468,25 @@ private bool PackHasChanged(WorkloadPack oldPack, WorkloadPack newPack)
             return false;
         }
 
+        public IWorkloadResolver CreateOverlayResolver(IWorkloadManifestProvider overlayManifestProvider)
+        {
+            // we specifically don't assign the overlayManifestProvider to the new resolver
+            // because it's not possible to refresh an overlay resolver
+            var overlayResolver = new WorkloadResolver(_dotnetRootPaths, _currentRuntimeIdentifiers);
+            overlayResolver.LoadManifestsFromProvider(overlayManifestProvider);
+
+            // after loading the overlay manifests into the new resolver
+            // we add all the manifests from this resolver that are not overlayed
+            foreach (var manifest in _manifests)
+            {
+                overlayResolver._manifests.TryAdd(manifest.Key, manifest.Value);
+            }
+
+            overlayResolver.ComposeWorkloadManifests();
+
+            return overlayResolver;
+        }
+
         public class PackInfo
         {
             public PackInfo(string id, string version, WorkloadPackKind kind, string path, string resolvedPackageId)
@@ -561,4 +592,20 @@ public IDictionary<string, string> GetInstalledManifests()
             return manifests;
         }
     }
+
+#if !NETCOREAPP
+
+    static class DictionaryExtensions
+    {
+        public static bool TryAdd<TKey,TValue>(this Dictionary<TKey, TValue> dictionary, TKey key, TValue value) where TKey : notnull
+        {
+            if (dictionary.ContainsKey(key))
+            {
+                return false;
+            }
+            dictionary.Add(key, value);
+            return true;
+        }
+    }
+#endif
 }

src/Tests/dotnet-workload-search.Tests/MockWorkloadResolver.cs

@@ -26,9 +26,9 @@ public IEnumerable<WorkloadDefinition> GetAvailableWorkloads()
         public ISet<WorkloadResolver.WorkloadInfo> GetWorkloadSuggestionForMissingPacks(IList<string> packId) => throw new NotImplementedException();
         public void RefreshWorkloadManifests() => throw new NotImplementedException();
         public WorkloadResolver.PackInfo TryGetPackInfo(string packId) => throw new NotImplementedException();
-        public WorkloadResolver CreateTempDirResolver(IWorkloadManifestProvider manifestProvider, string dotnetRootPath, string sdkVersion) => throw new NotImplementedException();
         public bool IsWorkloadPlatformCompatible(WorkloadId workloadId) => throw new NotImplementedException();
         public string GetManifestVersion(string manifestId) => throw new NotImplementedException();
         public IDictionary<string, string> GetInstalledManifests() => throw new NotImplementedException();
+        public IWorkloadResolver CreateOverlayResolver(IWorkloadManifestProvider overlayManifestProvider) => throw new NotImplementedException();
     }
 }