Skip to content

Commit 0fb6201

Browse files
committed
Correctly report manifest feature band for side-by-side manifests in WorkloadResolver.GetInstalledManifests
1 parent 4acb569 commit 0fb6201

File tree

11 files changed

+180
-65
lines changed

11 files changed

+180
-65
lines changed

src/Cli/dotnet/commands/dotnet-workload/WorkloadCommandParser.cs

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -54,14 +54,12 @@ internal static void ShowWorkloadsInfo(ParseResult parseResult = null, IWorkload
5454
return;
5555
}
5656

57+
var manifestInfoDict = workloadInfoHelper.WorkloadResolver.GetInstalledManifests().ToDictionary(info => info.Id, StringComparer.OrdinalIgnoreCase);
5758

5859
foreach (var workload in installedWorkloads.AsEnumerable())
5960
{
6061
var workloadManifest = workloadInfoHelper.WorkloadResolver.GetManifestFromWorkload(new WorkloadId(workload.Key));
61-
var workloadFeatureBand = new WorkloadManifestInfo(
62-
workloadManifest.Id,
63-
workloadManifest.Version,
64-
Path.GetDirectoryName(workloadManifest.ManifestPath)!).ManifestFeatureBand;
62+
var workloadFeatureBand = manifestInfoDict[workloadManifest.Id].ManifestFeatureBand;
6563

6664
const int align = 10;
6765
const string separator = " ";

src/Cli/dotnet/commands/dotnet-workload/list/WorkloadListCommand.cs

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
// Licensed to the .NET Foundation under one or more agreements.
22
// The .NET Foundation licenses this file to you under the MIT license.
33

4+
using System;
45
using System.Collections.Generic;
56
using System.CommandLine;
67
using System.CommandLine.Parsing;
@@ -81,15 +82,17 @@ public override int Execute()
8182
}
8283
else
8384
{
85+
var manifestInfoDict = _workloadListHelper.WorkloadResolver.GetInstalledManifests().ToDictionary(info => info.Id, StringComparer.OrdinalIgnoreCase);
86+
8487
InstalledWorkloadsCollection installedWorkloads = _workloadListHelper.AddInstalledVsWorkloads(installedList);
8588
Reporter.WriteLine();
8689
PrintableTable<KeyValuePair<string, string>> table = new();
8790
table.AddColumn(InformationStrings.WorkloadIdColumn, workload => workload.Key);
8891
table.AddColumn(InformationStrings.WorkloadManfiestVersionColumn, workload =>
8992
{
9093
var m = _workloadListHelper.WorkloadResolver.GetManifestFromWorkload(new WorkloadId(workload.Key));
91-
return m.Version + "/" +
92-
new WorkloadManifestInfo(m.Id, m.Version, Path.GetDirectoryName(m.ManifestPath)!).ManifestFeatureBand;
94+
var manifestInfo = manifestInfoDict[m.Id];
95+
return m.Version + "/" + manifestInfo.ManifestFeatureBand;
9396
});
9497
table.AddColumn(InformationStrings.WorkloadSourceColumn, workload => workload.Value);
9598

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,16 +18,19 @@ public class ReadableWorkloadManifest
1818

1919
public string ManifestPath { get; }
2020

21+
public string ManifestFeatureBand { get; }
22+
2123
readonly Func<Stream> _openManifestStreamFunc;
2224

2325

2426
readonly Func<Stream?> _openLocalizationStream;
2527

26-
public ReadableWorkloadManifest(string manifestId, string manifestDirectory, string manifestPath, Func<Stream> openManifestStreamFunc, Func<Stream?> openLocalizationStream)
28+
public ReadableWorkloadManifest(string manifestId, string manifestDirectory, string manifestPath, string manifestFeatureBand, Func<Stream> openManifestStreamFunc, Func<Stream?> openLocalizationStream)
2729
{
2830
ManifestId = manifestId;
2931
ManifestPath = manifestPath;
3032
ManifestDirectory = manifestDirectory;
33+
ManifestFeatureBand = manifestFeatureBand;
3134
_openManifestStreamFunc = openManifestStreamFunc;
3235
_openLocalizationStream = openLocalizationStream;
3336
}

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

Lines changed: 36 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -128,14 +128,29 @@ internal SdkDirectoryWorkloadManifestProvider(string sdkRootPath, string sdkVers
128128
public IEnumerable<ReadableWorkloadManifest> GetManifests()
129129
{
130130
// Scan manifest directories
131-
var manifestIdsToDirectories = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);
131+
var manifestIdsToManifests = new Dictionary<string, ReadableWorkloadManifest>(StringComparer.OrdinalIgnoreCase);
132132

133-
void ProbeDirectory(string manifestDirectory)
133+
void AddManifest(string manifestId, string manifestDirectory, string featureBand)
134+
{
135+
var workloadManifestPath = Path.Combine(manifestDirectory, "WorkloadManifest.json");
136+
137+
var readableManifest = new ReadableWorkloadManifest(
138+
manifestId,
139+
manifestDirectory,
140+
workloadManifestPath,
141+
featureBand,
142+
() => File.OpenRead(workloadManifestPath),
143+
() => WorkloadManifestReader.TryOpenLocalizationCatalogForManifest(workloadManifestPath));
144+
145+
manifestIdsToManifests[manifestId] = readableManifest;
146+
}
147+
148+
void ProbeDirectory(string manifestDirectory, string featureBand)
134149
{
135150
(string? id, string? finalManifestDirectory) = ResolveManifestDirectory(manifestDirectory);
136151
if (id != null && finalManifestDirectory != null)
137152
{
138-
manifestIdsToDirectories.Add(id, finalManifestDirectory);
153+
AddManifest(id, finalManifestDirectory, featureBand);
139154
}
140155
}
141156

@@ -147,7 +162,7 @@ void ProbeDirectory(string manifestDirectory)
147162
{
148163
foreach (var workloadManifestDirectory in Directory.EnumerateDirectories(manifestVersionBandDirectory))
149164
{
150-
ProbeDirectory(workloadManifestDirectory);
165+
ProbeDirectory(workloadManifestDirectory, _sdkVersionBand.ToString());
151166
}
152167
}
153168
}
@@ -169,7 +184,7 @@ void ProbeDirectory(string manifestDirectory)
169184

170185
foreach (var workloadManifestDirectory in directoriesWithManifests.Values)
171186
{
172-
ProbeDirectory(workloadManifestDirectory);
187+
ProbeDirectory(workloadManifestDirectory, _sdkVersionBand.ToString());
173188
}
174189
}
175190

@@ -184,7 +199,8 @@ void ProbeDirectory(string manifestDirectory)
184199
{
185200
throw new FileNotFoundException(string.Format(Strings.ManifestFromWorkloadSetNotFound, manifestSpecifier.ToString(), _workloadSet.Version));
186201
}
187-
manifestIdsToDirectories[kvp.Key.ToString()] = manifestDirectory;
202+
AddManifest(manifestSpecifier.Id.ToString(), manifestDirectory, manifestSpecifier.FeatureBand.ToString());
203+
188204
}
189205
}
190206

@@ -199,26 +215,26 @@ void ProbeDirectory(string manifestDirectory)
199215
{
200216
throw new FileNotFoundException(string.Format(Strings.ManifestFromInstallStateNotFound, manifestSpecifier.ToString(), _installStateFilePath));
201217
}
202-
manifestIdsToDirectories[kvp.Key.ToString()] = manifestDirectory;
218+
AddManifest(manifestSpecifier.Id.ToString(), manifestDirectory, manifestSpecifier.FeatureBand.ToString());
203219
}
204220
}
205221

206-
if (_knownManifestIdsAndOrder != null && _knownManifestIdsAndOrder.Keys.Any(id => !manifestIdsToDirectories.ContainsKey(id)))
222+
if (_knownManifestIdsAndOrder != null && _knownManifestIdsAndOrder.Keys.Any(id => !manifestIdsToManifests.ContainsKey(id)))
207223
{
208-
var missingManifestIds = _knownManifestIdsAndOrder.Keys.Where(id => !manifestIdsToDirectories.ContainsKey(id));
224+
var missingManifestIds = _knownManifestIdsAndOrder.Keys.Where(id => !manifestIdsToManifests.ContainsKey(id));
209225
foreach (var missingManifestId in missingManifestIds)
210226
{
211-
var manifestDir = FallbackForMissingManifest(missingManifestId);
227+
var (manifestDir, featureBand) = FallbackForMissingManifest(missingManifestId);
212228
if (!string.IsNullOrEmpty(manifestDir))
213229
{
214-
manifestIdsToDirectories.Add(missingManifestId, manifestDir);
230+
AddManifest(missingManifestId, manifestDir, featureBand);
215231
}
216232
}
217233
}
218234

219235
// 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.
220236
// Then the rest of the manifests (if any) will be returned in (ordinal case-insensitive) alphabetical order.
221-
return manifestIdsToDirectories
237+
return manifestIdsToManifests
222238
.OrderBy(kvp =>
223239
{
224240
if (_knownManifestIdsAndOrder != null &&
@@ -229,19 +245,7 @@ void ProbeDirectory(string manifestDirectory)
229245
return int.MaxValue;
230246
})
231247
.ThenBy(kvp => kvp.Key, StringComparer.OrdinalIgnoreCase)
232-
.Select(kvp =>
233-
{
234-
var manifestId = kvp.Key;
235-
var manifestDirectory = kvp.Value;
236-
var workloadManifestPath = Path.Combine(manifestDirectory, "WorkloadManifest.json");
237-
238-
return new ReadableWorkloadManifest(
239-
manifestId,
240-
manifestDirectory,
241-
workloadManifestPath,
242-
() => File.OpenRead(workloadManifestPath),
243-
() => WorkloadManifestReader.TryOpenLocalizationCatalogForManifest(workloadManifestPath));
244-
})
248+
.Select(kvp => kvp.Value)
245249
.ToList();
246250
}
247251

@@ -282,21 +286,21 @@ void ProbeDirectory(string manifestDirectory)
282286
return (null, null);
283287
}
284288

285-
private string FallbackForMissingManifest(string manifestId)
289+
private (string manifestDirectory, string manifestFeatureBand) FallbackForMissingManifest(string manifestId)
286290
{
287291
// Only use the last manifest root (usually the dotnet folder itself) for fallback
288292
var sdkManifestPath = _manifestRoots.Last();
289293
if (!Directory.Exists(sdkManifestPath))
290294
{
291-
return string.Empty;
295+
return (string.Empty, string.Empty);
292296
}
293297

294298
var candidateFeatureBands = Directory.GetDirectories(sdkManifestPath)
295299
.Select(dir => Path.GetFileName(dir))
296300
.Select(featureBand => new SdkFeatureBand(featureBand))
297301
.Where(featureBand => featureBand < _sdkVersionBand || _sdkVersionBand.ToStringWithoutPrerelease().Equals(featureBand.ToString(), StringComparison.Ordinal));
298302

299-
var matchingManifestFatureBandsAndResolvedManifestDirectories = candidateFeatureBands
303+
var matchingManifestFeatureBandsAndResolvedManifestDirectories = candidateFeatureBands
300304
// Calculate path to <FeatureBand>\<ManifestID>
301305
.Select(featureBand => (featureBand, manifestDirectory: Path.Combine(sdkManifestPath, featureBand.ToString(), manifestId)))
302306
// Filter out directories that don't exist
@@ -307,14 +311,15 @@ private string FallbackForMissingManifest(string manifestId)
307311
.Where(t => t.res.id != null && t.res.manifestDirectory != null)
308312
.ToList();
309313

310-
if (matchingManifestFatureBandsAndResolvedManifestDirectories.Any())
314+
if (matchingManifestFeatureBandsAndResolvedManifestDirectories.Any())
311315
{
312-
return matchingManifestFatureBandsAndResolvedManifestDirectories.OrderByDescending(t => t.featureBand).First().res.manifestDirectory!;
316+
var selectedFeatureBandAndManifestDirectory = matchingManifestFeatureBandsAndResolvedManifestDirectories.OrderByDescending(t => t.featureBand).First();
317+
return (selectedFeatureBandAndManifestDirectory.res.manifestDirectory!, selectedFeatureBandAndManifestDirectory.featureBand.ToString());
313318
}
314319
else
315320
{
316321
// Manifest does not exist
317-
return string.Empty;
322+
return (string.Empty, string.Empty);
318323
}
319324
}
320325

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ public IEnumerable<ReadableWorkloadManifest>
3737
manifestId,
3838
workloadManifestDirectory,
3939
workloadManifestPath,
40+
_sdkVersionBand,
4041
() => File.OpenRead(workloadManifestPath),
4142
() => WorkloadManifestReader.TryOpenLocalizationCatalogForManifest(workloadManifestPath)
4243
);

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,12 +12,12 @@ namespace Microsoft.NET.Sdk.WorkloadManifestReader
1212
{
1313
public class WorkloadManifestInfo
1414
{
15-
public WorkloadManifestInfo(string id, string version, string manifestDirectory)
15+
public WorkloadManifestInfo(string id, string version, string manifestDirectory, string manifestFeatureBand)
1616
{
1717
Id = id;
1818
Version = version;
1919
ManifestDirectory = manifestDirectory;
20-
ManifestFeatureBand = Path.GetFileName(Path.GetDirectoryName(manifestDirectory))!;
20+
ManifestFeatureBand = manifestFeatureBand;
2121
}
2222

2323
public string Id { get; }

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

Lines changed: 17 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ namespace Microsoft.NET.Sdk.WorkloadManifestReader
2121
/// </remarks>
2222
public class WorkloadResolver : IWorkloadResolver
2323
{
24-
private readonly Dictionary<string, WorkloadManifest> _manifests = new(StringComparer.OrdinalIgnoreCase);
24+
private readonly Dictionary<string, (WorkloadManifest manifest, WorkloadManifestInfo info)> _manifests = new(StringComparer.OrdinalIgnoreCase);
2525
private readonly Dictionary<WorkloadId, (WorkloadDefinition workload, WorkloadManifest manifest)> _workloads = new();
2626
private readonly Dictionary<WorkloadPackId, (WorkloadPack pack, WorkloadManifest manifest)> _packs = new();
2727
private IWorkloadManifestProvider? _manifestProvider;
@@ -118,9 +118,10 @@ private void LoadManifestsFromProvider(IWorkloadManifestProvider manifestProvide
118118
using (Stream? localizationStream = readableManifest.OpenLocalizationStream())
119119
{
120120
var manifest = WorkloadManifestReader.ReadWorkloadManifest(readableManifest.ManifestId, manifestStream, localizationStream, readableManifest.ManifestPath);
121-
if (!_manifests.TryAdd(readableManifest.ManifestId, manifest))
121+
var manifestInfo = new WorkloadManifestInfo(manifest.Id, manifest.Version, readableManifest.ManifestDirectory, readableManifest.ManifestFeatureBand);
122+
if (!_manifests.TryAdd(readableManifest.ManifestId, (manifest, manifestInfo)))
122123
{
123-
var existingManifest = _manifests[readableManifest.ManifestId];
124+
var existingManifest = _manifests[readableManifest.ManifestId].manifest;
124125
throw new WorkloadManifestCompositionException(Strings.DuplicateManifestID, manifestProvider.GetType().FullName, readableManifest.ManifestId, readableManifest.ManifestPath, existingManifest.ManifestPath);
125126
}
126127
}
@@ -134,14 +135,15 @@ private void ComposeWorkloadManifests()
134135

135136
Dictionary<WorkloadId, (WorkloadRedirect redirect, WorkloadManifest manifest)>? redirects = null;
136137

137-
foreach (var manifest in _manifests.Values)
138+
foreach (var (manifest, info) in _manifests.Values)
138139
{
139140
if (manifest.DependsOnManifests != null)
140141
{
141142
foreach (var dependency in manifest.DependsOnManifests)
142143
{
143-
if (_manifests.TryGetValue(dependency.Key, out var resolvedDependency))
144+
if (_manifests.TryGetValue(dependency.Key, out var t))
144145
{
146+
var resolvedDependency = t.manifest;
145147
if (FXVersion.Compare(dependency.Value, resolvedDependency.ParsedVersion) > 0)
146148
{
147149
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
711713

712714
private bool IsWorkloadImplicitlyAbstract(WorkloadDefinition workload, WorkloadManifest manifest) => !GetPacksInWorkload(workload, manifest).Any();
713715

714-
public string GetManifestVersion(string manifestId) =>
715-
(_manifests.TryGetValue(manifestId, out WorkloadManifest? value)? value : null)?.Version
716-
?? throw new Exception($"Manifest with id {manifestId} does not exist.");
716+
public string GetManifestVersion(string manifestId)
717+
{
718+
if (_manifests.TryGetValue(manifestId, out var value))
719+
{
720+
return value.manifest.Version;
721+
}
722+
throw new Exception($"Manifest with id {manifestId} does not exist.");
723+
}
724+
717725

718-
public IEnumerable<WorkloadManifestInfo> GetInstalledManifests() => _manifests.Select(m => new WorkloadManifestInfo(m.Value.Id, m.Value.Version, Path.GetDirectoryName(m.Value.ManifestPath)!));
726+
public IEnumerable<WorkloadManifestInfo> GetInstalledManifests() => _manifests.Select(t => t.Value.info);
719727
}
720728

721729
static class DictionaryExtensions

src/Tests/Microsoft.NET.Sdk.WorkloadManifestReader.Tests/FakeManifestProvider.cs

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,6 @@ public FakeManifestProvider(params (string manifest, string? localizationCatalog
2626
_filePaths = filePaths;
2727
}
2828

29-
public IEnumerable<string> GetManifestDirectories() => throw new NotImplementedException();
30-
3129
public IEnumerable<ReadableWorkloadManifest> GetManifests()
3230
{
3331
foreach (var filePath in _filePaths)
@@ -36,6 +34,7 @@ public IEnumerable<ReadableWorkloadManifest> GetManifests()
3634
Path.GetFileNameWithoutExtension(filePath.manifest),
3735
Path.GetDirectoryName(filePath.manifest)!,
3836
filePath.manifest,
37+
"8.0.100",
3938
() => new FileStream(filePath.manifest, FileMode.Open, FileAccess.Read),
4039
() => filePath.localizationCatalog != null ? new FileStream(filePath.localizationCatalog, FileMode.Open, FileAccess.Read) : null
4140
);
@@ -50,13 +49,13 @@ internal class InMemoryFakeManifestProvider : IWorkloadManifestProvider, IEnumer
5049
readonly List<(string id, byte[] content)> _manifests = new List<(string, byte[])>();
5150

5251
public void Add(string id, string content) => _manifests.Add((id, Encoding.UTF8.GetBytes(content)));
53-
public IEnumerable<string> GetManifestDirectories() => throw new NotImplementedException();
5452

5553
public IEnumerable<ReadableWorkloadManifest> GetManifests()
5654
=> _manifests.Select(m => new ReadableWorkloadManifest(
5755
m.id,
5856
$@"C:\fake\{m.id}",
5957
$@"C:\fake\{m.id}\WorkloadManifest.json",
58+
"8.0.100",
6059
(Func<Stream>)(() => new MemoryStream(m.content)),
6160
(Func<Stream?>)(() => null)
6261
));

0 commit comments

Comments
 (0)