From f56fffa72910b83498133d660dc23ce8a3655e90 Mon Sep 17 00:00:00 2001 From: Forgind <12969783+Forgind@users.noreply.github.com> Date: Tue, 9 Jul 2024 14:03:08 -0400 Subject: [PATCH 1/4] Permit loose manifest priority --- .../SdkDirectoryWorkloadManifestProvider.cs | 24 ++++++++----------- ...kDirectoryWorkloadManifestProviderTests.cs | 16 +++++++++++++ 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs b/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs index 51285fdcef14..b9413f832d21 100644 --- a/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs +++ b/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs @@ -177,28 +177,24 @@ bool TryGetWorkloadSet(string workloadSetVersion, out WorkloadSet? workloadSet) } } + _installStateFilePath = Path.Combine(WorkloadInstallType.GetInstallStateFolder(_sdkVersionBand, _sdkRootPath), "default.json"); + var installState = InstallStateContents.FromPath(_installStateFilePath); if (_workloadSet is null) { - var installStateFilePath = Path.Combine(WorkloadInstallType.GetInstallStateFolder(_sdkVersionBand, _sdkRootPath), "default.json"); - if (File.Exists(installStateFilePath)) + if (!string.IsNullOrEmpty(installState.WorkloadVersion)) { - var installState = InstallStateContents.FromPath(installStateFilePath); - if (!string.IsNullOrEmpty(installState.WorkloadVersion)) + if (!TryGetWorkloadSet(installState.WorkloadVersion!, out _workloadSet)) { - if (!TryGetWorkloadSet(installState.WorkloadVersion!, out _workloadSet)) - { - throw new FileNotFoundException(string.Format(Strings.WorkloadVersionFromInstallStateNotFound, installState.WorkloadVersion, installStateFilePath)); - } + throw new FileNotFoundException(string.Format(Strings.WorkloadVersionFromInstallStateNotFound, installState.WorkloadVersion, _installStateFilePath)); } - - // Note: It is possible here to have both a workload set and loose manifests listed in the install state. This might happen if there is a - // third-party workload manifest installed that's not part of the workload set - _manifestsFromInstallState = installState.Manifests is null ? null : WorkloadSet.FromDictionaryForJson(installState.Manifests!, _sdkVersionBand); - _installStateFilePath = installStateFilePath; } + + // Note: It is possible here to have both a workload set and loose manifests listed in the install state. This might happen if there is a + // third-party workload manifest installed that's not part of the workload set + _manifestsFromInstallState = installState.Manifests is null ? null : WorkloadSet.FromDictionaryForJson(installState.Manifests!, _sdkVersionBand); } - if (_workloadSet == null && availableWorkloadSets.Any()) + if (_workloadSet == null && installState.UseWorkloadSets == true && availableWorkloadSets.Any()) { var maxWorkloadSetVersion = availableWorkloadSets.Keys.Aggregate((s1, s2) => VersionCompare(s1, s2) >= 0 ? s1 : s2); _workloadSet = availableWorkloadSets[maxWorkloadSetVersion.ToString()]; diff --git a/test/Microsoft.NET.Sdk.WorkloadManifestReader.Tests/SdkDirectoryWorkloadManifestProviderTests.cs b/test/Microsoft.NET.Sdk.WorkloadManifestReader.Tests/SdkDirectoryWorkloadManifestProviderTests.cs index c97b2590ae7a..c8cc6d126fae 100644 --- a/test/Microsoft.NET.Sdk.WorkloadManifestReader.Tests/SdkDirectoryWorkloadManifestProviderTests.cs +++ b/test/Microsoft.NET.Sdk.WorkloadManifestReader.Tests/SdkDirectoryWorkloadManifestProviderTests.cs @@ -4,6 +4,7 @@ using System.Diagnostics.CodeAnalysis; using System.Runtime.CompilerServices; using Microsoft.DotNet.Cli; +using Microsoft.DotNet.Workloads.Workload; using Microsoft.NET.Sdk.Localization; using Microsoft.NET.Sdk.WorkloadManifestReader; @@ -418,6 +419,13 @@ public void WorkloadSetCanIncludeMultipleJsonFiles() CreateMockManifest(_manifestRoot, "8.0.200", "android", "33.0.2-rc.1", true); CreateMockManifest(_manifestRoot, "8.0.200", "android", "33.0.2", true); + // To prepare the resolver to work with workload sets, we need to specify 'workload sets' in the install state file. + var installStateLocation = WorkloadInstallType.GetInstallStateFolder(new SdkFeatureBand("8.0.200"), Path.GetDirectoryName(_manifestRoot)!); + var installStateFilePath = Path.Combine(installStateLocation, "default.json"); + var installState = InstallStateContents.FromPath(installStateFilePath); + installState.UseWorkloadSets = true; + Directory.CreateDirectory(installStateLocation); + File.WriteAllText(installStateFilePath, installState.ToString()); var workloadSetDirectory = Path.Combine(_manifestRoot, "8.0.200", "workloadsets", "8.0.200"); Directory.CreateDirectory(workloadSetDirectory); @@ -1254,6 +1262,14 @@ private void CreateMockManifest(string manifestRoot, string featureBand, string private void CreateMockWorkloadSet(string manifestRoot, string featureBand, string workloadSetVersion, string workloadSetContents) { + // To prepare the resolver to work with workload sets, we need to specify 'workload sets' in the install state file. + var installStateLocation = WorkloadInstallType.GetInstallStateFolder(new SdkFeatureBand(featureBand), Path.GetDirectoryName(manifestRoot)!); + var installStateFilePath = Path.Combine(installStateLocation, "default.json"); + var installState = InstallStateContents.FromPath(installStateFilePath); + installState.UseWorkloadSets = true; + Directory.CreateDirectory(installStateLocation); + File.WriteAllText(installStateFilePath, installState.ToString()); + var workloadSetDirectory = Path.Combine(manifestRoot, featureBand, "workloadsets", workloadSetVersion); if (!Directory.Exists(workloadSetDirectory)) { From 150844d0d459d0e4204c682bb4c7f6e4f8f2f29d Mon Sep 17 00:00:00 2001 From: Forgind <12969783+Forgind@users.noreply.github.com> Date: Thu, 11 Jul 2024 14:11:49 -0400 Subject: [PATCH 2/4] Add test --- ...kDirectoryWorkloadManifestProviderTests.cs | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/test/Microsoft.NET.Sdk.WorkloadManifestReader.Tests/SdkDirectoryWorkloadManifestProviderTests.cs b/test/Microsoft.NET.Sdk.WorkloadManifestReader.Tests/SdkDirectoryWorkloadManifestProviderTests.cs index c8cc6d126fae..f9de7af860d9 100644 --- a/test/Microsoft.NET.Sdk.WorkloadManifestReader.Tests/SdkDirectoryWorkloadManifestProviderTests.cs +++ b/test/Microsoft.NET.Sdk.WorkloadManifestReader.Tests/SdkDirectoryWorkloadManifestProviderTests.cs @@ -32,6 +32,40 @@ void Initialize(string featureBand = "5.0.100", [CallerMemberName] string? testN Directory.CreateDirectory(_manifestVersionBandDirectory); } + [Theory] + [InlineData(true)] + [InlineData(false)] + public void ItShouldPrioritizeInstallStateOverWorkloadSetUnlessSpecified(bool preferWorkloadSet) + { + Initialize(); + + CreateMockManifest(_manifestRoot, "8.0.400", "ios", "11.0.2", true); + CreateMockManifest(_manifestRoot, "8.0.400", "ios", "11.0.6", true); + CreateMockWorkloadSet(_manifestRoot, "8.0.400", "8.0.400", @" + { + ""ios"": ""11.0.2/8.0.400"" + } + "); + + var installStateLocation = WorkloadInstallType.GetInstallStateFolder(new SdkFeatureBand("8.0.400"), Path.GetDirectoryName(_manifestRoot)!); + var installStateFilePath = Path.Combine(installStateLocation, "default.json"); + var installState = InstallStateContents.FromPath(installStateFilePath); + installState.UseWorkloadSets = preferWorkloadSet; + installState.Manifests = new Dictionary() + { + { "ios", "11.0.6/8.0.400" } + }; + Directory.CreateDirectory(installStateLocation); + File.WriteAllText(installStateFilePath, installState.ToString()); + + var sdkDirectoryWorkloadManifestProvider + = new SdkDirectoryWorkloadManifestProvider(sdkRootPath: _fakeDotnetRootDirectory, sdkVersion: "8.0.400", userProfileDir: null, globalJsonPath: null); + + sdkDirectoryWorkloadManifestProvider.GetManifests().Single().Should().NotBeNull(); + + Directory.Delete(Path.Combine(_manifestRoot, "8.0.400"), recursive: true); + } + [Theory] [InlineData(true)] [InlineData(false)] From 335d5516c81f1a8f6d4f1dba17b08ce23e964a83 Mon Sep 17 00:00:00 2001 From: Forgind <12969783+Forgind@users.noreply.github.com> Date: Thu, 11 Jul 2024 14:19:24 -0400 Subject: [PATCH 3/4] Also prio workload sets if requested --- .../SdkDirectoryWorkloadManifestProvider.cs | 1 + .../SdkDirectoryWorkloadManifestProviderTests.cs | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs b/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs index b9413f832d21..c6afaef3cd7c 100644 --- a/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs +++ b/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs @@ -198,6 +198,7 @@ bool TryGetWorkloadSet(string workloadSetVersion, out WorkloadSet? workloadSet) { var maxWorkloadSetVersion = availableWorkloadSets.Keys.Aggregate((s1, s2) => VersionCompare(s1, s2) >= 0 ? s1 : s2); _workloadSet = availableWorkloadSets[maxWorkloadSetVersion.ToString()]; + _useManifestsFromInstallState = false; } } diff --git a/test/Microsoft.NET.Sdk.WorkloadManifestReader.Tests/SdkDirectoryWorkloadManifestProviderTests.cs b/test/Microsoft.NET.Sdk.WorkloadManifestReader.Tests/SdkDirectoryWorkloadManifestProviderTests.cs index f9de7af860d9..e7676a0a78ea 100644 --- a/test/Microsoft.NET.Sdk.WorkloadManifestReader.Tests/SdkDirectoryWorkloadManifestProviderTests.cs +++ b/test/Microsoft.NET.Sdk.WorkloadManifestReader.Tests/SdkDirectoryWorkloadManifestProviderTests.cs @@ -61,7 +61,7 @@ public void ItShouldPrioritizeInstallStateOverWorkloadSetUnlessSpecified(bool pr var sdkDirectoryWorkloadManifestProvider = new SdkDirectoryWorkloadManifestProvider(sdkRootPath: _fakeDotnetRootDirectory, sdkVersion: "8.0.400", userProfileDir: null, globalJsonPath: null); - sdkDirectoryWorkloadManifestProvider.GetManifests().Single().Should().NotBeNull(); + sdkDirectoryWorkloadManifestProvider.GetManifests().Single().ManifestVersion.Should().Be(preferWorkloadSet ? "11.0.2" : "11.0.6"); Directory.Delete(Path.Combine(_manifestRoot, "8.0.400"), recursive: true); } From d66780b280afd779be6813cf964977be394cca56 Mon Sep 17 00:00:00 2001 From: Forgind <12969783+Forgind@users.noreply.github.com> Date: Thu, 25 Jul 2024 20:43:55 -0400 Subject: [PATCH 4/4] Fix bad merge --- .../SdkDirectoryWorkloadManifestProvider.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs b/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs index e72fddb390fa..4acd8945ff9a 100644 --- a/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs +++ b/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs @@ -198,7 +198,7 @@ bool TryGetWorkloadSet(string workloadSetVersion, out WorkloadSet? workloadSet) _manifestsFromInstallState = installState.Manifests is null ? null : WorkloadSet.FromDictionaryForJson(installState.Manifests!, _sdkVersionBand); } - if (_workloadSet == null && installState.UseWorkloadSets == true && availableWorkloadSets.Any()) + if (workloadSet == null && installState.UseWorkloadSets == true && availableWorkloadSets.Any()) { var maxWorkloadSetVersion = availableWorkloadSets.Keys.Aggregate((s1, s2) => VersionCompare(s1, s2) >= 0 ? s1 : s2); workloadSet = availableWorkloadSets[maxWorkloadSetVersion.ToString()];