Fix manifest ID returned for side-by-side workload manifests

dotnet · Jul 7, 2023 · 29f515d · 29f515d
1 parent c6f8911
commit 29f515d
Show file tree

Hide file tree

Showing 9 changed files with 52 additions and 42 deletions.
diff --git a/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/CachingWorkloadResolver.cs b/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/CachingWorkloadResolver.cs
@@ -139,7 +139,7 @@ private static ResolutionResult Resolve(string sdkReferenceName, IWorkloadManife
             else if (sdkReferenceName.Equals("Microsoft.NET.SDK.WorkloadManifestTargetsLocator", StringComparison.OrdinalIgnoreCase))
             {
                 List<string> workloadManifestPaths = new List<string>();
-                foreach (var manifestDirectory in manifestProvider.GetManifestDirectories())
+                foreach (var manifestDirectory in manifestProvider.GetManifests().Select(m => m.ManifestDirectory))
                 {
                     var workloadManifestTargetPath = Path.Combine(manifestDirectory, "WorkloadManifest.targets");
                     if (File.Exists(workloadManifestTargetPath))

diff --git a/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/IWorkloadManifestProvider.cs b/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/IWorkloadManifestProvider.cs
@@ -15,8 +15,6 @@ public interface IWorkloadManifestProvider
     {
         IEnumerable<ReadableWorkloadManifest> GetManifests();
 
-        IEnumerable<string> GetManifestDirectories();
-
         string GetSdkFeatureBand();
     }
 }
diff --git a/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/ReadableWorkloadManifest.cs b/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/ReadableWorkloadManifest.cs
@@ -14,17 +14,21 @@ namespace Microsoft.NET.Sdk.WorkloadManifestReader
     public class ReadableWorkloadManifest
     {
         public string ManifestId { get; }
+
+        public string ManifestDirectory { get; }
+
         public string ManifestPath { get; }
 
         readonly Func<Stream> _openManifestStreamFunc;
 
 
         readonly Func<Stream?> _openLocalizationStream;
 
-        public ReadableWorkloadManifest(string manifestId, string manifestPath, Func<Stream> openManifestStreamFunc, Func<Stream?> openLocalizationStream)
+        public ReadableWorkloadManifest(string manifestId, string manifestDirectory, string manifestPath, Func<Stream> openManifestStreamFunc, Func<Stream?> openLocalizationStream)
         {
             ManifestId = manifestId;
             ManifestPath = manifestPath;
+            ManifestDirectory = manifestDirectory;
             _openManifestStreamFunc = openManifestStreamFunc;
             _openLocalizationStream = openLocalizationStream;
         }

diff --git a/...esolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs b/...esolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs
@@ -127,22 +127,6 @@ internal SdkDirectoryWorkloadManifestProvider(string sdkRootPath, string sdkVers
         }
 
         public IEnumerable<ReadableWorkloadManifest> GetManifests()
-        {
-            foreach (var workloadManifestDirectory in GetManifestDirectories())
-            {
-                var workloadManifestPath = Path.Combine(workloadManifestDirectory, "WorkloadManifest.json");
-                var id = Path.GetFileName(workloadManifestDirectory);
-
-                yield return new(
-                    id,
-                    workloadManifestPath,
-                    () => File.OpenRead(workloadManifestPath),
-                    () => WorkloadManifestReader.TryOpenLocalizationCatalogForManifest(workloadManifestPath)
-                );
-            }
-        }
-
-        public IEnumerable<string> GetManifestDirectories()
         {
             //  Scan manifest directories
             var manifestIdsToDirectories = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);
@@ -246,7 +230,19 @@ void ProbeDirectory(string manifestDirectory)
                     return int.MaxValue;
                 })
                 .ThenBy(kvp => kvp.Key, StringComparer.OrdinalIgnoreCase)
-                .Select(kvp => kvp.Value)
+                .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));
+                })
                 .ToList();
         }
 

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

diff --git a/src/Tests/Microsoft.NET.Sdk.WorkloadManifestReader.Tests/FakeManifestProvider.cs b/src/Tests/Microsoft.NET.Sdk.WorkloadManifestReader.Tests/FakeManifestProvider.cs
@@ -34,6 +34,7 @@ public IEnumerable<ReadableWorkloadManifest> GetManifests()
             {
                 yield return new(
                     Path.GetFileNameWithoutExtension(filePath.manifest),
+                    Path.GetDirectoryName(filePath.manifest)!,
                     filePath.manifest,
                     () => new FileStream(filePath.manifest, FileMode.Open, FileAccess.Read),
                     () => filePath.localizationCatalog != null ? new FileStream(filePath.localizationCatalog, FileMode.Open, FileAccess.Read) : null
@@ -54,6 +55,7 @@ internal class InMemoryFakeManifestProvider : IWorkloadManifestProvider, IEnumer
         public IEnumerable<ReadableWorkloadManifest> GetManifests()
             => _manifests.Select(m => new ReadableWorkloadManifest(
                 m.id,
+                $@"C:\fake\{m.id}",
                 $@"C:\fake\{m.id}\WorkloadManifest.json",
                 (Func<Stream>)(() => new MemoryStream(m.content)),
                 (Func<Stream?>)(() => null)

diff --git a/...crosoft.NET.Sdk.WorkloadManifestReader.Tests/SdkDirectoryWorkloadManifestProviderTests.cs b/...crosoft.NET.Sdk.WorkloadManifestReader.Tests/SdkDirectoryWorkloadManifestProviderTests.cs
@@ -925,19 +925,19 @@ public void ItShouldReturnManifestsFromTestHook()
 
             //  Manifest in test hook directory
             Directory.CreateDirectory(Path.Combine(additionalManifestDirectory, sdkVersion, "Android"));
-            File.WriteAllText(Path.Combine(additionalManifestDirectory, sdkVersion, "Android", "WorkloadManifest.json"), "AndroidContent");
+            File.WriteAllText(Path.Combine(additionalManifestDirectory, sdkVersion, "Android", "WorkloadManifest.json"), "Android: AndroidContent");
 
             //  Manifest in default directory
             Directory.CreateDirectory(Path.Combine(_manifestVersionBandDirectory, "iOS"));
-            File.WriteAllText(Path.Combine(_manifestVersionBandDirectory, "iOS", "WorkloadManifest.json"), "iOSContent");
+            File.WriteAllText(Path.Combine(_manifestVersionBandDirectory, "iOS", "WorkloadManifest.json"), "iOS: iOSContent");
 
 
             var sdkDirectoryWorkloadManifestProvider
                 = new SdkDirectoryWorkloadManifestProvider(sdkRootPath: _fakeDotnetRootDirectory, sdkVersion: sdkVersion, environmentMock.GetEnvironmentVariable, userProfileDir: null);
 
             GetManifestContents(sdkDirectoryWorkloadManifestProvider)
                 .Should()
-                .BeEquivalentTo("AndroidContent", "iOSContent");
+                .BeEquivalentTo("Android: AndroidContent", "iOS: iOSContent");
         }
 
         [Fact]
@@ -955,18 +955,18 @@ public void ManifestFromTestHookShouldOverrideDefault()
 
             //  Manifest in test hook directory
             Directory.CreateDirectory(Path.Combine(additionalManifestDirectory, sdkVersion, "Android"));
-            File.WriteAllText(Path.Combine(additionalManifestDirectory, sdkVersion, "Android", "WorkloadManifest.json"), "OverridingAndroidContent");
+            File.WriteAllText(Path.Combine(additionalManifestDirectory, sdkVersion, "Android", "WorkloadManifest.json"), "Android: OverridingAndroidContent");
 
             //  Manifest in default directory
             Directory.CreateDirectory(Path.Combine(_manifestVersionBandDirectory, "Android"));
-            File.WriteAllText(Path.Combine(_manifestVersionBandDirectory, "Android", "WorkloadManifest.json"), "OverriddenAndroidContent");
+            File.WriteAllText(Path.Combine(_manifestVersionBandDirectory, "Android", "WorkloadManifest.json"), "Android: OverriddenAndroidContent");
 
             var sdkDirectoryWorkloadManifestProvider
                 = new SdkDirectoryWorkloadManifestProvider(sdkRootPath: _fakeDotnetRootDirectory, sdkVersion: sdkVersion, environmentMock.GetEnvironmentVariable, userProfileDir: null);
 
             GetManifestContents(sdkDirectoryWorkloadManifestProvider)
                 .Should()
-                .BeEquivalentTo("OverridingAndroidContent");
+                .BeEquivalentTo("Android: OverridingAndroidContent");
 
         }
 
@@ -988,28 +988,28 @@ public void ItSupportsMultipleTestHookFolders()
 
             //  Manifests in default directory
             Directory.CreateDirectory(Path.Combine(_manifestVersionBandDirectory, "iOS"));
-            File.WriteAllText(Path.Combine(_manifestVersionBandDirectory, "iOS", "WorkloadManifest.json"), "iOSContent");
+            File.WriteAllText(Path.Combine(_manifestVersionBandDirectory, "iOS", "WorkloadManifest.json"), "iOS: iOSContent");
 
             Directory.CreateDirectory(Path.Combine(_manifestVersionBandDirectory, "Android"));
-            File.WriteAllText(Path.Combine(_manifestVersionBandDirectory, "Android", "WorkloadManifest.json"), "DefaultAndroidContent");
+            File.WriteAllText(Path.Combine(_manifestVersionBandDirectory, "Android", "WorkloadManifest.json"), "Android: DefaultAndroidContent");
 
             //  Manifests in first additional directory
             Directory.CreateDirectory(Path.Combine(additionalManifestDirectory1, sdkVersion, "Android"));
-            File.WriteAllText(Path.Combine(additionalManifestDirectory1, sdkVersion, "Android", "WorkloadManifest.json"), "AndroidContent1");
+            File.WriteAllText(Path.Combine(additionalManifestDirectory1, sdkVersion, "Android", "WorkloadManifest.json"), "Android: AndroidContent1");
 
             //  Manifests in second additional directory
             Directory.CreateDirectory(Path.Combine(additionalManifestDirectory2, sdkVersion, "Android"));
-            File.WriteAllText(Path.Combine(additionalManifestDirectory2, sdkVersion, "Android", "WorkloadManifest.json"), "AndroidContent2");
+            File.WriteAllText(Path.Combine(additionalManifestDirectory2, sdkVersion, "Android", "WorkloadManifest.json"), "Android: AndroidContent2");
 
             Directory.CreateDirectory(Path.Combine(additionalManifestDirectory2, sdkVersion, "Test"));
-            File.WriteAllText(Path.Combine(additionalManifestDirectory2, sdkVersion, "Test", "WorkloadManifest.json"), "TestContent2");
+            File.WriteAllText(Path.Combine(additionalManifestDirectory2, sdkVersion, "Test", "WorkloadManifest.json"), "Test: TestContent2");
 
             var sdkDirectoryWorkloadManifestProvider
                 = new SdkDirectoryWorkloadManifestProvider(sdkRootPath: _fakeDotnetRootDirectory, sdkVersion: sdkVersion, environmentMock.GetEnvironmentVariable, userProfileDir: null);
 
             GetManifestContents(sdkDirectoryWorkloadManifestProvider)
                 .Should()
-                .BeEquivalentTo("AndroidContent1", "iOSContent", "TestContent2");
+                .BeEquivalentTo("Android: AndroidContent1", "iOS: iOSContent", "Test: TestContent2");
 
         }
 
@@ -1025,14 +1025,14 @@ public void IfTestHookFolderDoesNotExistItShouldBeIgnored()
 
             //  Manifest in default directory
             Directory.CreateDirectory(Path.Combine(_manifestVersionBandDirectory, "Android"));
-            File.WriteAllText(Path.Combine(_manifestVersionBandDirectory, "Android", "WorkloadManifest.json"), "AndroidContent");
+            File.WriteAllText(Path.Combine(_manifestVersionBandDirectory, "Android", "WorkloadManifest.json"), "Android: AndroidContent");
 
             var sdkDirectoryWorkloadManifestProvider
                 = new SdkDirectoryWorkloadManifestProvider(sdkRootPath: _fakeDotnetRootDirectory, sdkVersion: "5.0.100", environmentMock.GetEnvironmentVariable, userProfileDir: null);
 
             GetManifestContents(sdkDirectoryWorkloadManifestProvider)
                 .Should()
-                .BeEquivalentTo("AndroidContent");
+                .BeEquivalentTo("Android: AndroidContent");
 
         }
 
@@ -1042,16 +1042,16 @@ public void ItShouldIgnoreOutdatedManifestIds()
             Initialize();
 
             Directory.CreateDirectory(Path.Combine(_manifestVersionBandDirectory, "iOS"));
-            File.WriteAllText(Path.Combine(_manifestVersionBandDirectory, "iOS", "WorkloadManifest.json"), "iOSContent");
+            File.WriteAllText(Path.Combine(_manifestVersionBandDirectory, "iOS", "WorkloadManifest.json"), "iOS: iOSContent");
             Directory.CreateDirectory(Path.Combine(_manifestVersionBandDirectory, "Microsoft.NET.Workload.Android"));
-            File.WriteAllText(Path.Combine(_manifestVersionBandDirectory, "Microsoft.NET.Workload.Android", "WorkloadManifest.json"), "iOSContent");
+            File.WriteAllText(Path.Combine(_manifestVersionBandDirectory, "Microsoft.NET.Workload.Android", "WorkloadManifest.json"), "Microsoft.NET.Workload.Android: AndroidContent");
 
             var sdkDirectoryWorkloadManifestProvider
                 = new SdkDirectoryWorkloadManifestProvider(sdkRootPath: _fakeDotnetRootDirectory, sdkVersion: "5.0.100", userProfileDir: null, globalJsonPath: null);
 
             GetManifestContents(sdkDirectoryWorkloadManifestProvider)
                 .Should()
-                .BeEquivalentTo("iOSContent");
+                .BeEquivalentTo("iOS: iOSContent");
         }
 
         [Fact]
@@ -1231,7 +1231,7 @@ public void ItShouldIgnoreManifestsNotFoundInFallback()
             var manifestDirectory6 = Path.Combine(fakeDotnetRootDirectory, "sdk-manifests", "6.0.100");
             Directory.CreateDirectory(manifestDirectory6);
             Directory.CreateDirectory(Path.Combine(manifestDirectory6, "iOS"));
-            File.WriteAllText(Path.Combine(manifestDirectory6, "iOS", "WorkloadManifest.json"), "iOS-6.0.100");
+            File.WriteAllText(Path.Combine(manifestDirectory6, "iOS", "WorkloadManifest.json"), "iOS: iOS-6.0.100");
 
             var knownWorkloadsFilePath = Path.Combine(fakeDotnetRootDirectory, "sdk", "6.0.100", "KnownWorkloadManifests.txt");
             Directory.CreateDirectory(Path.GetDirectoryName(knownWorkloadsFilePath)!);
@@ -1242,13 +1242,21 @@ var sdkDirectoryWorkloadManifestProvider
 
             GetManifestContents(sdkDirectoryWorkloadManifestProvider)
                 .Should()
-                .BeEquivalentTo("iOS-6.0.100");
+                .BeEquivalentTo("iOS: iOS-6.0.100");
 
         }
 
         private IEnumerable<string> GetManifestContents(SdkDirectoryWorkloadManifestProvider manifestProvider)
         {
-            return manifestProvider.GetManifests().Select(manifest => new StreamReader(manifest.OpenManifestStream()).ReadToEnd());
+            return manifestProvider.GetManifests().Select(manifest =>
+                {
+                    var contents = new StreamReader(manifest.OpenManifestStream()).ReadToEnd();
+
+                    string manifestId = contents.Split(':')[0];
+                    manifest.ManifestId.Should().Be(manifestId);
+
+                    return contents;
+                });
         }
 
         private class EnvironmentMock

diff --git a/src/Tests/Microsoft.NET.Sdk.WorkloadManifestReader.Tests/WorkloadPackGroupTests.cs b/src/Tests/Microsoft.NET.Sdk.WorkloadManifestReader.Tests/WorkloadPackGroupTests.cs
@@ -28,7 +28,7 @@ public void TestGetManifestDirectories()
         {
             var manifestProvider = CreateManifestProvider();
 
-            var manifestDirectories = manifestProvider.GetManifestDirectories();
+            var manifestDirectories = manifestProvider.GetManifests().Select(m => m.ManifestDirectory);
             foreach (var manifestDirectory in manifestDirectories)
             {
                 Log.WriteLine(manifestDirectory);

diff --git a/src/Tests/dotnet-workload-install.Tests/MockManifestProvider.cs b/src/Tests/dotnet-workload-install.Tests/MockManifestProvider.cs
@@ -40,6 +40,7 @@ public IEnumerable<ReadableWorkloadManifest> GetManifests()
                 {
                     yield return new(
                         id,
+                        Path.GetDirectoryName(path),
                         path,
                         () => File.OpenRead(path),
                         () => WorkloadManifestReader.TryOpenLocalizationCatalogForManifest(path)