From 3536f7a51b5d6a0e6d753c58207198907f6faf47 Mon Sep 17 00:00:00 2001 From: Forgind Date: Wed, 22 May 2024 19:38:29 -0700 Subject: [PATCH 1/6] Use version comparison function --- .../SdkDirectoryWorkloadManifestProvider.cs | 29 ++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs b/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs index 7cd959f0fd12..ef8635c43fd3 100644 --- a/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs +++ b/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs @@ -170,11 +170,38 @@ public void RefreshWorkloadManifests() if (_workloadSet == null && availableWorkloadSets.Any()) { - var maxWorkloadSetVersion = availableWorkloadSets.Keys.Select(k => new ReleaseVersion(k)).Max()!; + var maxWorkloadSetVersion = availableWorkloadSets.Keys.Aggregate((s1, s2) => VersionCompare(s1, s2) <= 0 ? s1 : s2); _workloadSet = availableWorkloadSets[maxWorkloadSetVersion.ToString()]; } } + private static int VersionCompare(string first, string second) + { + if (first.Equals(second)) + { + return 0; + } + + var firstDash = first.IndexOf('-'); + var secondDash = second.IndexOf('-'); + firstDash = firstDash < 0 ? first.Length : firstDash; + secondDash = secondDash < 0 ? second.Length : secondDash; + + var firstVersion = new Version(first.Substring(0, firstDash)); + var secondVersion = new Version(second.Substring(0, secondDash)); + + var comparison = firstVersion.CompareTo(secondVersion); + if (comparison != 0) + { + return comparison; + } + + var modifiedFirst = "1.1.1" + (firstDash == first.Length ? string.Empty : first.Substring(firstDash)); + var modifiedSecond = "1.1.1" + (secondDash == second.Length ? string.Empty : second.Substring(secondDash)); + + return new ReleaseVersion(modifiedFirst).CompareTo(new ReleaseVersion(modifiedSecond)); + } + void ThrowExceptionIfManifestsNotAvailable() { if (_exceptionToThrow != null) From 20d66140a5b09d4e20f4b09f09c91d5a4e7d0820 Mon Sep 17 00:00:00 2001 From: Forgind Date: Thu, 23 May 2024 08:58:02 -0700 Subject: [PATCH 2/6] Flip sign --- .../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 ef8635c43fd3..a73377f4771e 100644 --- a/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs +++ b/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs @@ -170,7 +170,7 @@ public void RefreshWorkloadManifests() if (_workloadSet == null && availableWorkloadSets.Any()) { - var maxWorkloadSetVersion = availableWorkloadSets.Keys.Aggregate((s1, s2) => VersionCompare(s1, s2) <= 0 ? s1 : s2); + var maxWorkloadSetVersion = availableWorkloadSets.Keys.Aggregate((s1, s2) => VersionCompare(s1, s2) >= 0 ? s1 : s2); _workloadSet = availableWorkloadSets[maxWorkloadSetVersion.ToString()]; } } From 4fe96b3e4c1f2131f5bdefbd9402e31fce996a44 Mon Sep 17 00:00:00 2001 From: Forgind Date: Thu, 23 May 2024 09:10:44 -0700 Subject: [PATCH 3/6] Add tests --- ...dkDirectoryWorkloadManifestProviderTests.cs | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/Tests/Microsoft.NET.Sdk.WorkloadManifestReader.Tests/SdkDirectoryWorkloadManifestProviderTests.cs b/src/Tests/Microsoft.NET.Sdk.WorkloadManifestReader.Tests/SdkDirectoryWorkloadManifestProviderTests.cs index d67946ceffea..280bc6d52796 100644 --- a/src/Tests/Microsoft.NET.Sdk.WorkloadManifestReader.Tests/SdkDirectoryWorkloadManifestProviderTests.cs +++ b/src/Tests/Microsoft.NET.Sdk.WorkloadManifestReader.Tests/SdkDirectoryWorkloadManifestProviderTests.cs @@ -135,23 +135,29 @@ var sdkDirectoryWorkloadManifestProvider .BeEmpty(); } - [Fact] - public void ItReturnsLatestManifestVersion() + [Theory] + [InlineData("11.0.1", "11.0.2", "11.0.2-rc.1", "11.0.2")] + [InlineData("8.0.200", "8.0.201", "8.0.105", "8.0.201")] + [InlineData("8.0.203.1", "8.0.203", "8.0.200-rc.1", "8.0.203.1")] + [InlineData("9.0.100-preview.2", "9.0.100-preview.2.3.4", "9.0.100-preview.2.4.3", "9.0.100-preview.2.4.3")] + [InlineData("8.0.201.1-preview", "8.0.201.1-rc.1", "8.0.201.1-rc.2", "8.0.201.1-rc.2")] + [InlineData("8.0.200-servicing.23015", "8.0.200-preview.7.30301", "8.0.200-servicing.23201", "8.0.200-servicing.23201")] + public void ItReturnsLatestManifestVersion(string first, string second, string third, string answer) { Initialize(); CreateMockManifest(_manifestRoot, "5.0.100-preview.5", "ios", "11.0.3", true); - CreateMockManifest(_manifestRoot, "5.0.100", "ios", "11.0.1", true); - CreateMockManifest(_manifestRoot, "5.0.100", "ios", "11.0.2", true); - CreateMockManifest(_manifestRoot, "5.0.100", "ios", "11.0.2-rc.1", true); + CreateMockManifest(_manifestRoot, "5.0.100", "ios", first, true); + CreateMockManifest(_manifestRoot, "5.0.100", "ios", second, true); + CreateMockManifest(_manifestRoot, "5.0.100", "ios", third, true); var sdkDirectoryWorkloadManifestProvider = new SdkDirectoryWorkloadManifestProvider(sdkRootPath: _fakeDotnetRootDirectory, sdkVersion: "5.0.100", userProfileDir: null, globalJsonPath: null); GetManifestContents(sdkDirectoryWorkloadManifestProvider) .Should() - .BeEquivalentTo("ios: 11.0.2/5.0.100"); + .BeEquivalentTo($"ios: {answer}/5.0.100"); } [Fact] From 15731de5cdfe10f41b77451278aa6d41164efe70 Mon Sep 17 00:00:00 2001 From: Forgind <12969783+Forgind@users.noreply.github.com> Date: Fri, 24 May 2024 08:29:08 -0700 Subject: [PATCH 4/6] Fix other spot --- .../SdkDirectoryWorkloadManifestProvider.cs | 20 ++++++++----------- 1 file changed, 8 insertions(+), 12 deletions(-) diff --git a/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs b/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs index a73377f4771e..167197fcf94d 100644 --- a/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs +++ b/src/Resolvers/Microsoft.NET.Sdk.WorkloadManifestReader/SdkDirectoryWorkloadManifestProvider.cs @@ -267,10 +267,10 @@ void AddManifest(string manifestId, string manifestDirectory, string featureBand void ProbeDirectory(string manifestDirectory, string featureBand) { - (string? id, string? finalManifestDirectory, ReleaseVersion? version) = ResolveManifestDirectory(manifestDirectory); + (string? id, string? finalManifestDirectory, string? version) = ResolveManifestDirectory(manifestDirectory); if (id != null && finalManifestDirectory != null) { - AddManifest(id, finalManifestDirectory, featureBand, version?.ToString() ?? Path.GetFileName(manifestDirectory)); + AddManifest(id, finalManifestDirectory, featureBand, version ?? Path.GetFileName(manifestDirectory)); } } @@ -375,7 +375,7 @@ void ProbeDirectory(string manifestDirectory, string featureBand) /// Given a folder that may directly include a WorkloadManifest.json file, or may have the workload manifests in version subfolders, choose the directory /// with the latest workload manifest. /// - private (string? id, string? manifestDirectory, ReleaseVersion? version) ResolveManifestDirectory(string manifestDirectory) + private (string? id, string? manifestDirectory, string? version) ResolveManifestDirectory(string manifestDirectory) { string manifestId = Path.GetFileName(manifestDirectory); if (_outdatedManifestIds.Contains(manifestId) || @@ -388,18 +388,14 @@ void ProbeDirectory(string manifestDirectory, string featureBand) .Where(dir => File.Exists(Path.Combine(dir, "WorkloadManifest.json"))) .Select(dir => { - ReleaseVersion? releaseVersion = null; - ReleaseVersion.TryParse(Path.GetFileName(dir), out releaseVersion); - return (directory: dir, version: releaseVersion); - }) - .Where(t => t.version != null) - .OrderByDescending(t => t.version) - .ToList(); + return (directory: dir, version: Path.GetFileName(dir)); + }); // Assume that if there are any versioned subfolders, they are higher manifest versions than a workload manifest directly in the specified folder, if it exists if (manifestVersionDirectories.Any()) { - return (manifestId, manifestVersionDirectories.First().directory, manifestVersionDirectories.First().version); + var maxVersionDirectory = manifestVersionDirectories.Aggregate((d1, d2) => VersionCompare(d1.version, d2.version) > 0 ? d1 : d2); + return (manifestId, maxVersionDirectory.directory, maxVersionDirectory.version); } else if (File.Exists(Path.Combine(manifestDirectory, "WorkloadManifest.json"))) { @@ -407,7 +403,7 @@ void ProbeDirectory(string manifestDirectory, string featureBand) try { var manifestContents = WorkloadManifestReader.ReadWorkloadManifest(manifestId, File.OpenRead(manifestPath), manifestPath); - return (manifestId, manifestDirectory, new ReleaseVersion(manifestContents.Version)); + return (manifestId, manifestDirectory, manifestContents.Version); } catch { } From 0b482b655373c7f81b11d04b319abf916ab8cfa1 Mon Sep 17 00:00:00 2001 From: Forgind <12969783+Forgind@users.noreply.github.com> Date: Fri, 24 May 2024 10:59:27 -0700 Subject: [PATCH 5/6] 8.0.200.1 is not an invalid version --- ...dkDirectoryWorkloadManifestProviderTests.cs | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/src/Tests/Microsoft.NET.Sdk.WorkloadManifestReader.Tests/SdkDirectoryWorkloadManifestProviderTests.cs b/src/Tests/Microsoft.NET.Sdk.WorkloadManifestReader.Tests/SdkDirectoryWorkloadManifestProviderTests.cs index 280bc6d52796..0cee7327c71b 100644 --- a/src/Tests/Microsoft.NET.Sdk.WorkloadManifestReader.Tests/SdkDirectoryWorkloadManifestProviderTests.cs +++ b/src/Tests/Microsoft.NET.Sdk.WorkloadManifestReader.Tests/SdkDirectoryWorkloadManifestProviderTests.cs @@ -385,24 +385,6 @@ var sdkDirectoryWorkloadManifestProvider .BeEquivalentTo("ios: 11.0.2/8.0.100", "android: 33.0.2/8.0.100", "maui: 15.0.1-rc.456/8.0.200-rc.2"); } - [Fact] - public void ItThrowsIfWorkloadSetHasInvalidVersion() - { - Initialize("8.0.200"); - - CreateMockManifest(_manifestRoot, "8.0.100", "ios", "11.0.1", true); - CreateMockManifest(_manifestRoot, "8.0.100", "ios", "11.0.2", true); - CreateMockManifest(_manifestRoot, "8.0.200", "ios", "12.0.1", true); - - CreateMockWorkloadSet(_manifestRoot, "8.0.200", "8.0.200.1", """ - { - "ios": "11.0.2/8.0.100" - } - """); - - Assert.Throws(() => new SdkDirectoryWorkloadManifestProvider(sdkRootPath: _fakeDotnetRootDirectory, sdkVersion: "8.0.200", userProfileDir: null, globalJsonPath: null)); - } - [Fact] public void ItThrowsIfManifestFromWorkloadSetIsNotFound() { From 9a01f1e2183b0d2124a4b0d9a92490939e054288 Mon Sep 17 00:00:00 2001 From: Forgind <12969783+Forgind@users.noreply.github.com> Date: Fri, 24 May 2024 13:06:30 -0700 Subject: [PATCH 6/6] disambiguate folders --- .../SdkDirectoryWorkloadManifestProviderTests.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Tests/Microsoft.NET.Sdk.WorkloadManifestReader.Tests/SdkDirectoryWorkloadManifestProviderTests.cs b/src/Tests/Microsoft.NET.Sdk.WorkloadManifestReader.Tests/SdkDirectoryWorkloadManifestProviderTests.cs index 0cee7327c71b..3316a7d0a3ea 100644 --- a/src/Tests/Microsoft.NET.Sdk.WorkloadManifestReader.Tests/SdkDirectoryWorkloadManifestProviderTests.cs +++ b/src/Tests/Microsoft.NET.Sdk.WorkloadManifestReader.Tests/SdkDirectoryWorkloadManifestProviderTests.cs @@ -144,7 +144,7 @@ var sdkDirectoryWorkloadManifestProvider [InlineData("8.0.200-servicing.23015", "8.0.200-preview.7.30301", "8.0.200-servicing.23201", "8.0.200-servicing.23201")] public void ItReturnsLatestManifestVersion(string first, string second, string third, string answer) { - Initialize(); + Initialize(identifier: answer); CreateMockManifest(_manifestRoot, "5.0.100-preview.5", "ios", "11.0.3", true);