Improve workload composition errors

Manifests have an "informational path" describing where they were
loaded from. The workload resolver includes the manifest IDs and
associated informational path in composition errors, so it's easier
to see what caused the conflict.

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

@@ -33,7 +33,7 @@ internal static ReleaseVersion GetValidatedSdkVersion(string versionOption, stri
                 }
                 try
                 {
-                    foreach ((string manifestId, Func<Stream> openManifestStream) in manifests)
+                    foreach ((string manifestId, string informationalPath, Func<Stream> openManifestStream) in manifests)
                     {
                         using (var manifestStream = openManifestStream())
                         {

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

@@ -13,8 +13,10 @@ namespace Microsoft.NET.Sdk.WorkloadManifestReader
     /// </summary>
     public interface IWorkloadManifestProvider
     {
-        IEnumerable<(string manifestId, Func<Stream> openManifestStream)> GetManifests();
+        IEnumerable<(string manifestId, string? informationalPath, Func<Stream> openManifestStream)> GetManifests();
+
         IEnumerable<string> GetManifestDirectories();
+
         string GetSdkFeatureBand();
     }
 }

src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/Microsoft.NET.Sdk.WorkloadManifestReader.csproj

@@ -8,7 +8,6 @@
     <WarningsAsErrors>true</WarningsAsErrors>
     <StrongNameKeyId>MicrosoftAspNetCore</StrongNameKeyId>
 
-    <LangVersion>8.0</LangVersion>
     <Nullable>Enable</Nullable>
     <IsPackable>true</IsPackable>
 

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

@@ -68,13 +68,13 @@ static int Last2DigitsTo0(int versionBuild)
             }
         }
 
-        public IEnumerable<(string manifestId, Func<Stream> openManifestStream)> GetManifests()
+        public IEnumerable<(string manifestId, string? informationalPath, Func<Stream> openManifestStream)> GetManifests()
         {
             foreach (var workloadManifestDirectory in GetManifestDirectories())
             {
                 var workloadManifest = Path.Combine(workloadManifestDirectory, "WorkloadManifest.json");
                 var id = Path.GetFileName(workloadManifestDirectory);
-                yield return (id, () => File.OpenRead(workloadManifest));
+                yield return (id, workloadManifest, () => File.OpenRead(workloadManifest));
             }
         }
 

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

@@ -18,13 +18,13 @@ public TempDirectoryWorkloadManifestProvider(string manifestsPath, string sdkVer
             _sdkVersionBand = sdkVersion;
         }
 
-        public IEnumerable<(string manifestId, Func<Stream> openManifestStream)> GetManifests()
+        public IEnumerable<(string manifestId, string? informationalPath, Func<Stream> openManifestStream)> GetManifests()
         {
             foreach (var workloadManifestDirectory in GetManifestDirectories())
             {
                 var workloadManifest = Path.Combine(workloadManifestDirectory, "WorkloadManifest.json");
                 var id = Path.GetFileName(workloadManifestDirectory);
-                yield return (id, () => File.OpenRead(workloadManifest));
+                yield return (id, workloadManifest, () => File.OpenRead(workloadManifest));
             }
         }
 

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

@@ -12,11 +12,12 @@ namespace Microsoft.NET.Sdk.WorkloadManifestReader
     /// </summary>
     public class WorkloadManifest
     {
-        internal WorkloadManifest(string id, FXVersion version, string? description, Dictionary<WorkloadId, BaseWorkloadDefinition> workloads, Dictionary<WorkloadPackId, WorkloadPack> packs, Dictionary<string, FXVersion>? dependsOnManifests)
+        internal WorkloadManifest(string id, FXVersion version, string? description, string? informationalPath,  Dictionary<WorkloadId, BaseWorkloadDefinition> workloads, Dictionary<WorkloadPackId, WorkloadPack> packs, Dictionary<string, FXVersion>? dependsOnManifests)
         {
             Id = id;
             ParsedVersion = version;
             Description = description;
+            InformationalPath = informationalPath;
             Workloads = workloads;
             Packs = packs;
             DependsOnManifests = dependsOnManifests;
@@ -44,6 +45,12 @@ internal WorkloadManifest(string id, FXVersion version, string? description, Dic
 
         public string? Description { get; }
 
+        /// <summary>
+        /// A path that indicates where the manifest was loaded from, for diagnostic purposes only.
+        /// Not guaranteed to be set or to be a valid filesystem path.
+        /// </summary>
+        public string? InformationalPath { get; }
+
         public Dictionary<WorkloadId, BaseWorkloadDefinition> Workloads { get; }
         public Dictionary<WorkloadPackId, WorkloadPack> Packs { get; }
     }

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

@@ -14,15 +14,14 @@ namespace Microsoft.NET.Sdk.WorkloadManifestReader
 {
     public partial class WorkloadManifestReader
     {
-
-        public static WorkloadManifest ReadWorkloadManifest(string manifestId, Stream manifestStream)
+        public static WorkloadManifest ReadWorkloadManifest(string manifestId, Stream manifestStream, string? informationalPath = null)
         {
             using var textReader = new StreamReader(manifestStream, System.Text.Encoding.UTF8, true);
             using var jsonReader = new JsonTextReader(textReader);
 
             var reader = new Utf8JsonStreamReader(jsonReader);
 
-            return ReadWorkloadManifest(manifestId, ref reader);
+            return ReadWorkloadManifest(manifestId, informationalPath, ref reader);
         }
         // this is a compat wrapper so the source matches the system.text.json impl
         private ref struct Utf8JsonStreamReader

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

@@ -12,7 +12,7 @@ namespace Microsoft.NET.Sdk.WorkloadManifestReader
 {
     public partial class WorkloadManifestReader
     {
-        public static WorkloadManifest ReadWorkloadManifest(string manifestId, Stream manifestStream)
+        public static WorkloadManifest ReadWorkloadManifest(string manifestId, Stream manifestStream, string? informationalPath = null)
         {
             var readerOptions = new JsonReaderOptions
             {
@@ -22,7 +22,7 @@ public static WorkloadManifest ReadWorkloadManifest(string manifestId, Stream ma
 
             var reader = new Utf8JsonStreamReader(manifestStream, readerOptions);
 
-            return ReadWorkloadManifest(manifestId, ref reader);
+            return ReadWorkloadManifest(manifestId, informationalPath, ref reader);
         }
 
         private ref struct Utf8JsonStreamReader

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

@@ -62,7 +62,7 @@ private static bool ReadBool(ref Utf8JsonStreamReader reader)
         private static void ThrowDuplicateKeyException<T> (ref Utf8JsonStreamReader reader, T key)
             => throw new WorkloadManifestFormatException(Strings.DuplicateKeyAtOffset, key?.ToString() ?? throw new ArgumentNullException (nameof(key)), reader.TokenStartIndex);
 
-        private static WorkloadManifest ReadWorkloadManifest(string id, ref Utf8JsonStreamReader reader)
+        private static WorkloadManifest ReadWorkloadManifest(string id, string? informationalPath, ref Utf8JsonStreamReader reader)
         {
             ConsumeToken(ref reader, JsonTokenType.StartObject);
 
@@ -155,6 +155,7 @@ private static WorkloadManifest ReadWorkloadManifest(string id, ref Utf8JsonStre
                             id,
                             version,
                             description,
+                            informationalPath,
                             workloads ?? new Dictionary<WorkloadId, BaseWorkloadDefinition> (),
                             packs ?? new Dictionary<WorkloadPackId, WorkloadPack> (),
                             dependsOn

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

@@ -92,15 +92,11 @@ public void RefreshWorkloadManifests()
 
         private void LoadManifestsFromProvider(IWorkloadManifestProvider manifestProvider)
         {
-            foreach ((string manifestId, Func<Stream> openManifestStream) in manifestProvider.GetManifests())
+            foreach ((string manifestId, string? informationalPath, Func<Stream> openManifestStream) in manifestProvider.GetManifests())
             {
                 using (var manifestStream = openManifestStream())
                 {
-                    var manifest = WorkloadManifestReader.ReadWorkloadManifest(manifestId, manifestStream);
-                    if(!string.Equals (manifestId, manifest.Id, StringComparison.OrdinalIgnoreCase))
-                    {
-                        throw new WorkloadManifestCompositionException($"Manifest '{manifestId}' from provider {manifestProvider} does not match payload id '{manifest.Id}'");
-                    }
+                    var manifest = WorkloadManifestReader.ReadWorkloadManifest(manifestId, manifestStream, informationalPath);
                     if (!_manifests.TryAdd(manifestId, manifest))
                     {
                         throw new WorkloadManifestCompositionException($"Duplicate manifest '{manifestId}' from provider {manifestProvider}");
@@ -124,12 +120,12 @@ private void ComposeWorkloadManifests()
                         {
                             if (FXVersion.Compare(dependency.Value, resolvedDependency.ParsedVersion) > 0)
                             {
-                                throw new WorkloadManifestCompositionException($"Inconsistency in workload manifest '{manifest.Id}': requires '{dependency.Key}' version at least {dependency.Value} but found {resolvedDependency.Version}");
+                                throw new WorkloadManifestCompositionException($"Inconsistency in workload manifest '{manifest.Id}' ({manifest.InformationalPath}): requires '{dependency.Key}' version at least {dependency.Value} but found {resolvedDependency.Version}");
                             }
                         }
                         else
                         {
-                            throw new WorkloadManifestCompositionException($"Inconsistency in workload manifest '{manifest.Id}': missing dependency '{dependency.Key}'");
+                            throw new WorkloadManifestCompositionException($"Inconsistency in workload manifest '{manifest.Id}' ({manifest.InformationalPath}): missing dependency '{dependency.Key}'");
                         }
                     }
                 }
@@ -145,7 +141,8 @@ private void ComposeWorkloadManifests()
                     {
                         if (!_workloads.TryAdd(workload.Key, ((WorkloadDefinition)workload.Value, manifest)))
                         {
-                            throw new WorkloadManifestCompositionException($"Workload '{workload.Key}' in manifest '{manifest.Id}' conflicts with manifest '{_workloads[workload.Key].manifest.Id}'");
+                            WorkloadManifest conflictingManifest = _workloads[workload.Key].manifest;
+                            throw new WorkloadManifestCompositionException($"Workload '{workload.Key}' in manifest '{manifest.Id}' ({manifest.InformationalPath}) conflicts with manifest '{conflictingManifest.Id}' ({conflictingManifest.InformationalPath})");
                         }
                     }
                 }
@@ -161,7 +158,8 @@ private void ComposeWorkloadManifests()
                         {
                             if (!_workloads.TryAdd(redirect.Id, replacement))
                             {
-                                throw new WorkloadManifestCompositionException($"Workload '{redirect.Id}' in manifest '{manifest.Id}' conflicts with manifest '{_workloads[redirect.Id].manifest.Id}'");
+                                WorkloadManifest conflictingManifest = _workloads[redirect.Id].manifest;
+                                throw new WorkloadManifestCompositionException($"Workload '{redirect.Id}' in manifest '{manifest.Id}' ({manifest.InformationalPath}) conflicts with manifest '{conflictingManifest.Id}' ({conflictingManifest.InformationalPath})");
                             }
                             return true;
                         }
@@ -178,7 +176,8 @@ private void ComposeWorkloadManifests()
                 {
                     if (!_packs.TryAdd(pack.Key, (pack.Value, manifest)))
                     {
-                        throw new WorkloadManifestCompositionException($"Workload pack '{pack.Key}' in manifest '{manifest.Id}' conflicts with manifest '{_packs[pack.Key].manifest.Id}'");
+                        WorkloadManifest conflictingManifest = _packs[pack.Key].manifest;
+                        throw new WorkloadManifestCompositionException($"Workload pack '{pack.Key}' in manifest '{manifest.Id}' ({manifest.InformationalPath}) conflicts with manifest '{conflictingManifest.Id}' ({conflictingManifest.InformationalPath})");
                     }
                 }
             }

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

@@ -21,32 +21,32 @@ public FakeManifestProvider(params string[] filePaths)
             _filePaths = filePaths;
         }
 
-        public IEnumerable<string> GetManifestDirectories() => throw new System.NotImplementedException();
+        public IEnumerable<string> GetManifestDirectories() => throw new NotImplementedException();
 
-        public IEnumerable<(string manifestId, Func<Stream> openManifestStream)> GetManifests()
+        public IEnumerable<(string manifestId, string? informationalPath, Func<Stream> openManifestStream)> GetManifests()
         {
             foreach (var filePath in _filePaths)
             {
-                yield return (Path.GetFileNameWithoutExtension(filePath), () => new FileStream(filePath, FileMode.Open, FileAccess.Read));
+                yield return (Path.GetFileNameWithoutExtension(filePath), filePath,() => new FileStream(filePath, FileMode.Open, FileAccess.Read));
             }
         }
 
-        public string GetSdkFeatureBand() => throw new System.NotImplementedException();
+        public string GetSdkFeatureBand() => throw new NotImplementedException();
     }
 
     internal class InMemoryFakeManifestProvider : IWorkloadManifestProvider, IEnumerable<(string id, string content)>
     {
         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 System.NotImplementedException();
+        public IEnumerable<string> GetManifestDirectories() => throw new NotImplementedException();
 
-        public IEnumerable<(string manifestId, Func<Stream> openManifestStream)> GetManifests()
-            => _manifests.Select(m => (m.id, (Func<Stream>)(() => new MemoryStream(m.content))));
+        public IEnumerable<(string manifestId, string? informationalPath, Func<Stream> openManifestStream)> GetManifests()
+            => _manifests.Select(m => (m.id, (string?)null, (Func<Stream>)(() => new MemoryStream(m.content))));
 
         // these are just so the collection initializer works
-        public IEnumerator<(string id, string content)> GetEnumerator() => throw new System.NotImplementedException();
-        IEnumerator IEnumerable.GetEnumerator() => throw new System.NotImplementedException();
-        public string GetSdkFeatureBand() => throw new System.NotImplementedException();
+        public IEnumerator<(string id, string content)> GetEnumerator() => throw new NotImplementedException();
+        IEnumerator IEnumerable.GetEnumerator() => throw new NotImplementedException();
+        public string GetSdkFeatureBand() => throw new NotImplementedException();
     }
 }

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

@@ -190,7 +190,7 @@ string MakeManifest(string version, params (string id, string version)[] depends
                 {  "AAA", MakeManifest("20.0.0", ("BBB", "5.0.0"), ("CCC", "63.0.0"), ("DDD", "25.0.0")) }
             };
 
-            var missingManifestEx = Assert.Throws<Exception>(() => WorkloadResolver.CreateForTests(missingManifestProvider, new[] { fakeRootPath }));
+            var missingManifestEx = Assert.Throws<WorkloadManifestCompositionException>(() => WorkloadResolver.CreateForTests(missingManifestProvider, new[] { fakeRootPath }));
             Assert.Contains("missing dependency", missingManifestEx.Message);
 
             var inconsistentManifestProvider = new InMemoryFakeManifestProvider
@@ -201,7 +201,7 @@ string MakeManifest(string version, params (string id, string version)[] depends
                 {  "DDD", MakeManifest("30.0.0") },
             };
 
-            var inconsistentManifestEx = Assert.Throws<Exception>(() => WorkloadResolver.CreateForTests(inconsistentManifestProvider, new[] { fakeRootPath }));
+            var inconsistentManifestEx = Assert.Throws<WorkloadManifestCompositionException>(() => WorkloadResolver.CreateForTests(inconsistentManifestProvider, new[] { fakeRootPath }));
             Assert.Contains("Inconsistency in workload manifest", inconsistentManifestEx.Message);
         }
 

src/Tests/dotnet-workload-install.Tests/MockManifestProvider.cs

@@ -11,26 +11,31 @@ namespace ManifestReaderTests
 {
     internal class MockManifestProvider : IWorkloadManifestProvider
     {
-        readonly string[] _filePaths;
+        readonly (string name, string path)[] _manifests;
 
-        public MockManifestProvider(params string[] filePaths)
+        public MockManifestProvider(params string[] manifestPaths)
         {
-            _filePaths = filePaths;
+            _manifests = Array.ConvertAll(manifestPaths, mp => (mp, mp));
+        }
+
+        public MockManifestProvider(params (string name, string path)[] manifests)
+        {
+            _manifests = manifests;
         }
 
         public IEnumerable<string> GetManifestDirectories()
         {
-            foreach (var filePath in _filePaths)
+            foreach ((_, var filePath) in _manifests)
             {
                 yield return Path.GetDirectoryName(filePath);
             }
         }
 
-        public IEnumerable<(string manifestId, Func<Stream> openManifestStream)> GetManifests()
+        public IEnumerable<(string manifestId, string informationalPath, Func<Stream> openManifestStream)> GetManifests()
             {
-                foreach (var filePath in _filePaths)
+                foreach ((var id, var path) in _manifests)
                 {
-                    yield return (filePath, () => new FileStream(filePath, FileMode.Open, FileAccess.Read));
+                    yield return (id, path, () => File.OpenRead(path));
                 }
             }