From 8de4e036bb37830417ed9c92ef7f3c5094c45dcd Mon Sep 17 00:00:00 2001 From: Daniel Plaisted Date: Fri, 7 Jul 2023 19:24:16 -0400 Subject: [PATCH] Fix manifest ID returned for side-by-side workload manifests --- .../CachingWorkloadResolver.cs | 2 +- .../IWorkloadManifestProvider.cs | 2 - .../ReadableWorkloadManifest.cs | 6 ++- .../SdkDirectoryWorkloadManifestProvider.cs | 30 +++++------- .../TempDirectoryWorkloadManifestProvider.cs | 1 + .../FakeManifestProvider.cs | 2 + ...kDirectoryWorkloadManifestProviderTests.cs | 48 +++++++++++-------- .../WorkloadPackGroupTests.cs | 2 +- .../MockManifestProvider.cs | 1 + 9 files changed, 52 insertions(+), 42 deletions(-) diff --git a/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/CachingWorkloadResolver.cs b/src/Resolvers/Microsoft.NET.Sdk.WorkloadMSBuildSdkResolver/CachingWorkloadResolver.cs index 126b751316b0..15b57a1a9da0 100644 --- 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 workloadManifestPaths = new List(); - 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 index 9b1073ccce14..85b502f2d1df 100644 --- 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 GetManifests(); - IEnumerable GetManifestDirectories(); - string GetSdkFeatureBand(); } } diff --git a/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/ReadableWorkloadManifest.cs b/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/ReadableWorkloadManifest.cs index c09bb13ac569..5974089df81e 100644 --- a/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/ReadableWorkloadManifest.cs +++ b/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/ReadableWorkloadManifest.cs @@ -13,6 +13,9 @@ namespace Microsoft.NET.Sdk.WorkloadManifestReader public class ReadableWorkloadManifest { public string ManifestId { get; } + + public string ManifestDirectory { get; } + public string ManifestPath { get; } readonly Func _openManifestStreamFunc; @@ -20,10 +23,11 @@ public class ReadableWorkloadManifest readonly Func _openLocalizationStream; - public ReadableWorkloadManifest(string manifestId, string manifestPath, Func openManifestStreamFunc, Func openLocalizationStream) + public ReadableWorkloadManifest(string manifestId, string manifestDirectory, string manifestPath, Func openManifestStreamFunc, Func 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 index 7eb76ccbd879..93c07b1e5fa9 100644 --- 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 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 GetManifestDirectories() { // Scan manifest directories var manifestIdsToDirectories = new Dictionary(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 index ccd1fb7af73c..f5aef412d6ca 100644 --- a/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/TempDirectoryWorkloadManifestProvider.cs +++ b/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/TempDirectoryWorkloadManifestProvider.cs @@ -35,6 +35,7 @@ public IEnumerable 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 index a07d8cae9afd..bd80153d4b50 100644 --- 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 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 GetManifests() => _manifests.Select(m => new ReadableWorkloadManifest( m.id, + $@"C:\fake\{m.id}", $@"C:\fake\{m.id}\WorkloadManifest.json", (Func)(() => new MemoryStream(m.content)), (Func)(() => null) diff --git a/src/Tests/Microsoft.NET.Sdk.WorkloadManifestReader.Tests/SdkDirectoryWorkloadManifestProviderTests.cs b/src/Tests/Microsoft.NET.Sdk.WorkloadManifestReader.Tests/SdkDirectoryWorkloadManifestProviderTests.cs index 9c0ab1bf050d..c62436992ae3 100644 --- a/src/Tests/Microsoft.NET.Sdk.WorkloadManifestReader.Tests/SdkDirectoryWorkloadManifestProviderTests.cs +++ b/src/Tests/Microsoft.NET.Sdk.WorkloadManifestReader.Tests/SdkDirectoryWorkloadManifestProviderTests.cs @@ -925,11 +925,11 @@ 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 @@ -937,7 +937,7 @@ var sdkDirectoryWorkloadManifestProvider 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 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 index 8434b59b2d47..fe6e150cb577 100644 --- 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 index 9f2cc5bbb393..6a3ef2e0197f 100644 --- a/src/Tests/dotnet-workload-install.Tests/MockManifestProvider.cs +++ b/src/Tests/dotnet-workload-install.Tests/MockManifestProvider.cs @@ -40,6 +40,7 @@ public IEnumerable GetManifests() { yield return new( id, + Path.GetDirectoryName(path), path, () => File.OpenRead(path), () => WorkloadManifestReader.TryOpenLocalizationCatalogForManifest(path)