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

dsplaisted · dsplaisted · commit 8de4e036bb37 · 2023-07-09T10:19:17.000-04:00
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
@@ -13,17 +13,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/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs b/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs
@@ -126,22 +126,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);
@@ -245,7 +229,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/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/TempDirectoryWorkloadManifestProvider.cs b/src/Resolvers/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/src/Tests/Microsoft.NET.Sdk.WorkloadManifestReader.Tests/SdkDirectoryWorkloadManifestProviderTests.cs b/src/Tests/Microsoft.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
@@ -27,7 +27,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)

Original file line number	Diff line number	Diff line change
`@@ -139,7 +139,7 @@ private static ResolutionResult Resolve(string sdkReferenceName, IWorkloadManife`
`139`	`139`	`else if (sdkReferenceName.Equals("Microsoft.NET.SDK.WorkloadManifestTargetsLocator", StringComparison.OrdinalIgnoreCase))`
`140`	`140`	`{`
`141`	`141`	`List<string> workloadManifestPaths = new List<string>();`
`142`		`- foreach (var manifestDirectory in manifestProvider.GetManifestDirectories())`
	`142`	`+ foreach (var manifestDirectory in manifestProvider.GetManifests().Select(m => m.ManifestDirectory))`
`143`	`143`	`{`
`144`	`144`	`var workloadManifestTargetPath = Path.Combine(manifestDirectory, "WorkloadManifest.targets");`
`145`	`145`	`if (File.Exists(workloadManifestTargetPath))`
Original file line number	Diff line number	Diff line change
`@@ -15,8 +15,6 @@ public interface IWorkloadManifestProvider`
`15`	`15`	`{`
`16`	`16`	`IEnumerable<ReadableWorkloadManifest> GetManifests();`
`17`	`17`
`18`		`- IEnumerable<string> GetManifestDirectories();`
`19`		`-`
`20`	`18`	`string GetSdkFeatureBand();`
`21`	`19`	`}`
`22`	`20`	`}`
Original file line number	Diff line number	Diff line change
`@@ -13,17 +13,21 @@ namespace Microsoft.NET.Sdk.WorkloadManifestReader`
`13`	`13`	`public class ReadableWorkloadManifest`
`14`	`14`	`{`
`15`	`15`	`public string ManifestId { get; }`
	`16`	`+`
	`17`	`+ public string ManifestDirectory { get; }`
	`18`	`+`
`16`	`19`	`public string ManifestPath { get; }`
`17`	`20`
`18`	`21`	`readonly Func<Stream> _openManifestStreamFunc;`
`19`	`22`
`20`	`23`
`21`	`24`	`readonly Func<Stream?> _openLocalizationStream;`
`22`	`25`
`23`		`- public ReadableWorkloadManifest(string manifestId, string manifestPath, Func<Stream> openManifestStreamFunc, Func<Stream?> openLocalizationStream)`
	`26`	`+ public ReadableWorkloadManifest(string manifestId, string manifestDirectory, string manifestPath, Func<Stream> openManifestStreamFunc, Func<Stream?> openLocalizationStream)`
`24`	`27`	`{`
`25`	`28`	`ManifestId = manifestId;`
`26`	`29`	`ManifestPath = manifestPath;`
	`30`	`+ ManifestDirectory = manifestDirectory;`
`27`	`31`	`_openManifestStreamFunc = openManifestStreamFunc;`
`28`	`32`	`_openLocalizationStream = openLocalizationStream;`
`29`	`33`	`}`
Original file line number	Diff line number	Diff line change
`@@ -27,7 +27,7 @@ public void TestGetManifestDirectories()`
`27`	`27`	`{`
`28`	`28`	`var manifestProvider = CreateManifestProvider();`
`29`	`29`
`30`		`- var manifestDirectories = manifestProvider.GetManifestDirectories();`
	`30`	`+ var manifestDirectories = manifestProvider.GetManifests().Select(m => m.ManifestDirectory);`
`31`	`31`	`foreach (var manifestDirectory in manifestDirectories)`
`32`	`32`	`{`
`33`	`33`	`Log.WriteLine(manifestDirectory);`
Original file line number	Diff line number	Diff line change
`@@ -40,6 +40,7 @@ public IEnumerable<ReadableWorkloadManifest> GetManifests()`
`40`	`40`	`{`
`41`	`41`	`yield return new(`
`42`	`42`	`id,`
	`43`	`+ Path.GetDirectoryName(path),`
`43`	`44`	`path,`
`44`	`45`	`() => File.OpenRead(path),`
`45`	`46`	`() => WorkloadManifestReader.TryOpenLocalizationCatalogForManifest(path)`