Correctly report manifest feature band for side-by-side manifests in …

…WorkloadResolver.GetInstalledManifests
dotnet · Jul 12, 2023 · 7876941 · 7876941
1 parent 29f515d
commit 7876941
Show file tree

Hide file tree

Showing 12 changed files with 190 additions and 71 deletions.
diff --git a/src/Cli/dotnet/commands/dotnet-workload/WorkloadCommandParser.cs b/src/Cli/dotnet/commands/dotnet-workload/WorkloadCommandParser.cs
@@ -1,6 +1,7 @@
 // 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 System;
 using System.Collections.Generic;
 using System.CommandLine;
 using System.CommandLine.Parsing;
@@ -41,14 +42,12 @@ internal static void ShowWorkloadsInfo(IWorkloadInfoHelper workloadInfoHelper =
                 return;
             }
 
+            var manifestInfoDict =  workloadInfoHelper.WorkloadResolver.GetInstalledManifests().ToDictionary(info => info.Id, StringComparer.OrdinalIgnoreCase);
 
             foreach (var workload in installedWorkloads.AsEnumerable())
             {
                 var workloadManifest = workloadInfoHelper.WorkloadResolver.GetManifestFromWorkload(new WorkloadId(workload.Key));
-                var workloadFeatureBand = new WorkloadManifestInfo(
-                    workloadManifest.Id,
-                    workloadManifest.Version,
-                    Path.GetDirectoryName(workloadManifest.ManifestPath)!).ManifestFeatureBand;
+                var workloadFeatureBand = manifestInfoDict[workloadManifest.Id].ManifestFeatureBand;
 
                 const int align = 10;
                 const string separator = "   ";

diff --git a/src/Cli/dotnet/commands/dotnet-workload/list/WorkloadListCommand.cs b/src/Cli/dotnet/commands/dotnet-workload/list/WorkloadListCommand.cs
@@ -1,6 +1,7 @@
 // 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 System;
 using System.Collections.Generic;
 using System.CommandLine;
 using System.CommandLine.Parsing;
@@ -79,15 +80,17 @@ public override int Execute()
             }
             else
             {
+                var manifestInfoDict = _workloadListHelper.WorkloadResolver.GetInstalledManifests().ToDictionary(info => info.Id, StringComparer.OrdinalIgnoreCase);
+
                 InstalledWorkloadsCollection installedWorkloads = _workloadListHelper.AddInstalledVsWorkloads(installedList);
                 Reporter.WriteLine();
                 PrintableTable<KeyValuePair<string, string>> table = new();
                 table.AddColumn(InformationStrings.WorkloadIdColumn, workload => workload.Key);
                 table.AddColumn(InformationStrings.WorkloadManfiestVersionColumn, workload =>
                 {
                     var m = _workloadListHelper.WorkloadResolver.GetManifestFromWorkload(new WorkloadId(workload.Key));
-                    return m.Version + "/" +
-                    new WorkloadManifestInfo(m.Id, m.Version, Path.GetDirectoryName(m.ManifestPath)!).ManifestFeatureBand;
+                    var manifestInfo = manifestInfoDict[m.Id];
+                    return m.Version + "/" + manifestInfo.ManifestFeatureBand;
                 });
                 table.AddColumn(InformationStrings.WorkloadSourceColumn, workload => workload.Value);
 

diff --git a/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/ReadableWorkloadManifest.cs b/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/ReadableWorkloadManifest.cs
@@ -19,16 +19,19 @@ public class ReadableWorkloadManifest
 
         public string ManifestPath { get; }
 
+        public string ManifestFeatureBand { get; }
+
         readonly Func<Stream> _openManifestStreamFunc;
 
 
         readonly Func<Stream?> _openLocalizationStream;
 
-        public ReadableWorkloadManifest(string manifestId, string manifestDirectory, string manifestPath, Func<Stream> openManifestStreamFunc, Func<Stream?> openLocalizationStream)
+        public ReadableWorkloadManifest(string manifestId, string manifestDirectory, string manifestPath, string manifestFeatureBand, Func<Stream> openManifestStreamFunc, Func<Stream?> openLocalizationStream)
         {
             ManifestId = manifestId;
             ManifestPath = manifestPath;
             ManifestDirectory = manifestDirectory;
+            ManifestFeatureBand = manifestFeatureBand;
             _openManifestStreamFunc = openManifestStreamFunc;
             _openLocalizationStream = openLocalizationStream;
         }

diff --git a/...esolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs b/...esolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs
@@ -129,14 +129,29 @@ internal SdkDirectoryWorkloadManifestProvider(string sdkRootPath, string sdkVers
         public IEnumerable<ReadableWorkloadManifest> GetManifests()
         {
             //  Scan manifest directories
-            var manifestIdsToDirectories = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);
+            var manifestIdsToManifests = new Dictionary<string, ReadableWorkloadManifest>(StringComparer.OrdinalIgnoreCase);
 
-            void ProbeDirectory(string manifestDirectory)
+            void AddManifest(string manifestId, string manifestDirectory, string featureBand)
+            {
+                var workloadManifestPath = Path.Combine(manifestDirectory, "WorkloadManifest.json");
+
+                var readableManifest = new ReadableWorkloadManifest(
+                    manifestId,
+                    manifestDirectory,
+                    workloadManifestPath,
+                    featureBand,
+                    () => File.OpenRead(workloadManifestPath),
+                    () => WorkloadManifestReader.TryOpenLocalizationCatalogForManifest(workloadManifestPath));
+
+                manifestIdsToManifests[manifestId] = readableManifest;
+            }
+
+            void ProbeDirectory(string manifestDirectory, string featureBand)
             {
                 (string? id, string? finalManifestDirectory) = ResolveManifestDirectory(manifestDirectory);
                 if (id != null && finalManifestDirectory != null)
                 {
-                    manifestIdsToDirectories.Add(id, finalManifestDirectory);
+                    AddManifest(id, finalManifestDirectory, featureBand);
                 }
             }
 
@@ -148,7 +163,7 @@ void ProbeDirectory(string manifestDirectory)
                 {
                     foreach (var workloadManifestDirectory in Directory.EnumerateDirectories(manifestVersionBandDirectory))
                     {
-                        ProbeDirectory(workloadManifestDirectory);
+                        ProbeDirectory(workloadManifestDirectory, _sdkVersionBand.ToString());
                     }
                 }
             }
@@ -170,7 +185,7 @@ void ProbeDirectory(string manifestDirectory)
 
                 foreach (var workloadManifestDirectory in directoriesWithManifests.Values)
                 {
-                    ProbeDirectory(workloadManifestDirectory);
+                    ProbeDirectory(workloadManifestDirectory, _sdkVersionBand.ToString());
                 }
             }
 
@@ -185,7 +200,8 @@ void ProbeDirectory(string manifestDirectory)
                     {
                         throw new FileNotFoundException(string.Format(Strings.ManifestFromWorkloadSetNotFound, manifestSpecifier.ToString(), _workloadSet.Version));
                     }
-                    manifestIdsToDirectories[kvp.Key.ToString()] = manifestDirectory;
+                    AddManifest(manifestSpecifier.Id.ToString(), manifestDirectory, manifestSpecifier.FeatureBand.ToString());
+
                 }
             }
 
@@ -200,26 +216,26 @@ void ProbeDirectory(string manifestDirectory)
                     {
                         throw new FileNotFoundException(string.Format(Strings.ManifestFromInstallStateNotFound, manifestSpecifier.ToString(), _installStateFilePath));
                     }
-                    manifestIdsToDirectories[kvp.Key.ToString()] = manifestDirectory;
+                    AddManifest(manifestSpecifier.Id.ToString(), manifestDirectory, manifestSpecifier.FeatureBand.ToString());
                 }
             }
 
-            if (_knownManifestIdsAndOrder != null && _knownManifestIdsAndOrder.Keys.Any(id => !manifestIdsToDirectories.ContainsKey(id)))
+            if (_knownManifestIdsAndOrder != null && _knownManifestIdsAndOrder.Keys.Any(id => !manifestIdsToManifests.ContainsKey(id)))
             {
-                var missingManifestIds = _knownManifestIdsAndOrder.Keys.Where(id => !manifestIdsToDirectories.ContainsKey(id));
+                var missingManifestIds = _knownManifestIdsAndOrder.Keys.Where(id => !manifestIdsToManifests.ContainsKey(id));
                 foreach (var missingManifestId in missingManifestIds)
                 {
-                    var manifestDir = FallbackForMissingManifest(missingManifestId);
+                    var (manifestDir, featureBand) = FallbackForMissingManifest(missingManifestId);
                     if (!string.IsNullOrEmpty(manifestDir))
                     {
-                        manifestIdsToDirectories.Add(missingManifestId, manifestDir);
+                        AddManifest(missingManifestId, manifestDir, featureBand);
                     }
                 }
             }
 
             //  Return manifests in a stable order.  Manifests in the KnownWorkloadManifests.txt file will be first, and in the same order they appear in that file.
             //  Then the rest of the manifests (if any) will be returned in (ordinal case-insensitive) alphabetical order.
-            return manifestIdsToDirectories
+            return manifestIdsToManifests
                 .OrderBy(kvp =>
                 {
                     if (_knownManifestIdsAndOrder != null &&
@@ -230,19 +246,7 @@ void ProbeDirectory(string manifestDirectory)
                     return int.MaxValue;
                 })
                 .ThenBy(kvp => kvp.Key, StringComparer.OrdinalIgnoreCase)
-                .Select(kvp =>
-                {
-                    var manifestId = kvp.Key;
-                    var manifestDirectory = kvp.Value;
-                    var workloadManifestPath = Path.Combine(manifestDirectory, "WorkloadManifest.json");
-
-                    return new ReadableWorkloadManifest(
-                        manifestId,
-                        manifestDirectory,
-                        workloadManifestPath,
-                        () => File.OpenRead(workloadManifestPath),
-                        () => WorkloadManifestReader.TryOpenLocalizationCatalogForManifest(workloadManifestPath));
-                })
+                .Select(kvp => kvp.Value)
                 .ToList();
         }
 
@@ -283,21 +287,21 @@ void ProbeDirectory(string manifestDirectory)
             return (null, null);
         }
 
-        private string FallbackForMissingManifest(string manifestId)
+        private (string manifestDirectory, string manifestFeatureBand) FallbackForMissingManifest(string manifestId)
         {
             //  Only use the last manifest root (usually the dotnet folder itself) for fallback
             var sdkManifestPath = _manifestRoots.Last();
             if (!Directory.Exists(sdkManifestPath))
             {
-                return string.Empty;
+                return (string.Empty, string.Empty);
             }
 
             var candidateFeatureBands = Directory.GetDirectories(sdkManifestPath)
                 .Select(dir => Path.GetFileName(dir))
                 .Select(featureBand => new SdkFeatureBand(featureBand))
                 .Where(featureBand => featureBand < _sdkVersionBand || _sdkVersionBand.ToStringWithoutPrerelease().Equals(featureBand.ToString(), StringComparison.Ordinal));
 
-            var matchingManifestFatureBandsAndResolvedManifestDirectories = candidateFeatureBands
+            var matchingManifestFeatureBandsAndResolvedManifestDirectories = candidateFeatureBands
                 //  Calculate path to <FeatureBand>\<ManifestID>
                 .Select(featureBand => (featureBand, manifestDirectory: Path.Combine(sdkManifestPath, featureBand.ToString(), manifestId)))
                 //  Filter out directories that don't exist
@@ -308,14 +312,15 @@ private string FallbackForMissingManifest(string manifestId)
                 .Where(t => t.res.id != null && t.res.manifestDirectory != null)
                 .ToList();
 
-            if (matchingManifestFatureBandsAndResolvedManifestDirectories.Any())
+            if (matchingManifestFeatureBandsAndResolvedManifestDirectories.Any())
             {
-                return matchingManifestFatureBandsAndResolvedManifestDirectories.OrderByDescending(t => t.featureBand).First().res.manifestDirectory!;
+                var selectedFeatureBandAndManifestDirectory = matchingManifestFeatureBandsAndResolvedManifestDirectories.OrderByDescending(t => t.featureBand).First();
+                return (selectedFeatureBandAndManifestDirectory.res.manifestDirectory!, selectedFeatureBandAndManifestDirectory.featureBand.ToString());
             }
             else
             {
                 // Manifest does not exist
-                return string.Empty;
+                return (string.Empty, string.Empty);
             }
         }
 

diff --git a/...solvers/Microsoft.NET.Sdk.WorkloadManifestReader/TempDirectoryWorkloadManifestProvider.cs b/...solvers/Microsoft.NET.Sdk.WorkloadManifestReader/TempDirectoryWorkloadManifestProvider.cs
@@ -37,6 +37,7 @@ public IEnumerable<ReadableWorkloadManifest>
                     manifestId,
                     workloadManifestDirectory,
                     workloadManifestPath,
+                    _sdkVersionBand,
                     () => File.OpenRead(workloadManifestPath),
                     () => WorkloadManifestReader.TryOpenLocalizationCatalogForManifest(workloadManifestPath)
                 );

diff --git a/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/WorkloadManifestInfo.cs b/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/WorkloadManifestInfo.cs
@@ -13,12 +13,12 @@ namespace Microsoft.NET.Sdk.WorkloadManifestReader
 {
     public class WorkloadManifestInfo
     {
-        public WorkloadManifestInfo(string id, string version, string manifestDirectory)
+        public WorkloadManifestInfo(string id, string version, string manifestDirectory, string manifestFeatureBand)
         {
             Id = id;
             Version = version;
             ManifestDirectory = manifestDirectory;
-            ManifestFeatureBand = Path.GetFileName(Path.GetDirectoryName(manifestDirectory))!;
+            ManifestFeatureBand = manifestFeatureBand;
         }
 
         public string Id { get; }

diff --git a/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/WorkloadResolver.cs b/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/WorkloadResolver.cs
@@ -21,7 +21,7 @@ namespace Microsoft.NET.Sdk.WorkloadManifestReader
     /// </remarks>
     public class WorkloadResolver : IWorkloadResolver
     {
-        private readonly Dictionary<string, WorkloadManifest> _manifests = new(StringComparer.OrdinalIgnoreCase);
+        private readonly Dictionary<string, (WorkloadManifest manifest, WorkloadManifestInfo info)> _manifests = new(StringComparer.OrdinalIgnoreCase);
         private readonly Dictionary<WorkloadId, (WorkloadDefinition workload, WorkloadManifest manifest)> _workloads = new();
         private readonly Dictionary<WorkloadPackId, (WorkloadPack pack, WorkloadManifest manifest)> _packs = new();
         private IWorkloadManifestProvider? _manifestProvider;
@@ -118,9 +118,10 @@ private void LoadManifestsFromProvider(IWorkloadManifestProvider manifestProvide
                 using (Stream? localizationStream = readableManifest.OpenLocalizationStream())
                 {
                     var manifest = WorkloadManifestReader.ReadWorkloadManifest(readableManifest.ManifestId, manifestStream, localizationStream, readableManifest.ManifestPath);
-                    if (!_manifests.TryAdd(readableManifest.ManifestId, manifest))
+                    var manifestInfo = new WorkloadManifestInfo(manifest.Id, manifest.Version, readableManifest.ManifestDirectory, readableManifest.ManifestFeatureBand);
+                    if (!_manifests.TryAdd(readableManifest.ManifestId, (manifest, manifestInfo)))
                     {
-                        var existingManifest = _manifests[readableManifest.ManifestId];
+                        var existingManifest = _manifests[readableManifest.ManifestId].manifest;
                         throw new WorkloadManifestCompositionException(Strings.DuplicateManifestID, manifestProvider.GetType().FullName, readableManifest.ManifestId, readableManifest.ManifestPath, existingManifest.ManifestPath);
                     }
                 }
@@ -134,14 +135,15 @@ private void ComposeWorkloadManifests()
 
             Dictionary<WorkloadId, (WorkloadRedirect redirect, WorkloadManifest manifest)>? redirects = null;
 
-            foreach (var manifest in _manifests.Values)
+            foreach (var (manifest, info) 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 t))
                         {
+                            var resolvedDependency = t.manifest;
                             if (FXVersion.Compare(dependency.Value, resolvedDependency.ParsedVersion) > 0)
                             {
                                 throw new WorkloadManifestCompositionException(Strings.ManifestDependencyVersionTooLow, dependency.Key, resolvedDependency.Version, dependency.Value, manifest.Id, manifest.ManifestPath);
@@ -711,11 +713,17 @@ private bool IsWorkloadPlatformCompatible(WorkloadDefinition workload, WorkloadM
 
         private bool IsWorkloadImplicitlyAbstract(WorkloadDefinition workload, WorkloadManifest manifest) => !GetPacksInWorkload(workload, manifest).Any();
 
-        public string GetManifestVersion(string manifestId) =>
-            (_manifests.TryGetValue(manifestId, out WorkloadManifest? value)? value : null)?.Version
-            ?? throw new Exception($"Manifest with id {manifestId} does not exist.");
+        public string GetManifestVersion(string manifestId)
+        {
+            if (_manifests.TryGetValue(manifestId, out var value))
+            {
+                return value.manifest.Version;
+            }
+            throw new Exception($"Manifest with id {manifestId} does not exist.");
+        }
+
 
-        public IEnumerable<WorkloadManifestInfo> GetInstalledManifests() => _manifests.Select(m => new WorkloadManifestInfo(m.Value.Id, m.Value.Version, Path.GetDirectoryName(m.Value.ManifestPath)!));
+        public IEnumerable<WorkloadManifestInfo> GetInstalledManifests() => _manifests.Select(t => t.Value.info);
     }
 
     static class DictionaryExtensions

diff --git a/src/Tests/Microsoft.NET.Sdk.WorkloadManifestReader.Tests/FakeManifestProvider.cs b/src/Tests/Microsoft.NET.Sdk.WorkloadManifestReader.Tests/FakeManifestProvider.cs
@@ -26,8 +26,6 @@ public FakeManifestProvider(params (string manifest, string? localizationCatalog
             _filePaths = filePaths;
         }
 
-        public IEnumerable<string> GetManifestDirectories() => throw new NotImplementedException();
-
         public IEnumerable<ReadableWorkloadManifest> GetManifests()
         {
             foreach (var filePath in _filePaths)
@@ -36,6 +34,7 @@ public IEnumerable<ReadableWorkloadManifest> GetManifests()
                     Path.GetFileNameWithoutExtension(filePath.manifest),
                     Path.GetDirectoryName(filePath.manifest)!,
                     filePath.manifest,
+                    "8.0.100",
                     () => new FileStream(filePath.manifest, FileMode.Open, FileAccess.Read),
                     () => filePath.localizationCatalog != null ? new FileStream(filePath.localizationCatalog, FileMode.Open, FileAccess.Read) : null
                 );
@@ -50,13 +49,13 @@ internal class InMemoryFakeManifestProvider : IWorkloadManifestProvider, IEnumer
         readonly List<(string id, byte[] content)> _manifests = new List<(string, byte[])>();
 
         public void Add(string id, string content) => _manifests.Add((id, Encoding.UTF8.GetBytes(content)));
-        public IEnumerable<string> GetManifestDirectories() => throw new NotImplementedException();
 
         public IEnumerable<ReadableWorkloadManifest> GetManifests()
             => _manifests.Select(m => new ReadableWorkloadManifest(
                 m.id,
                 $@"C:\fake\{m.id}",
                 $@"C:\fake\{m.id}\WorkloadManifest.json",
+                "8.0.100",
                 (Func<Stream>)(() => new MemoryStream(m.content)),
                 (Func<Stream?>)(() => null)
             ));