Microsoft.NETCore.Platforms: support adding rids with '-' in the base…

… part. (#84413) * Microsoft.NETCore.Platforms: support adding rids with '-' in the base part. Currently when trying to add a rid like 'linux-musl-x64' the rid is not understood to be base = 'linux-musl', arch = 'x64'. Instead the parser considers a potential optional qualifier. This causes the rid to be parsed as base = 'linux', arch = 'musl', and qualifier = 'x64'. We know the rids being added won't have a qualifier. If we take this into account while parsing, we can parse the rid correctly. * Update src/libraries/Microsoft.NETCore.Platforms/src/RuntimeGroupCollection.cs Co-authored-by: Eric StJohn <[email protected]> --------- Co-authored-by: Eric StJohn <[email protected]>
dotnet · Apr 6, 2023 · 238e3d3 · 238e3d3
1 parent 8e26f2b
commit 238e3d3
Show file tree

Hide file tree

Showing 3 changed files with 48 additions and 16 deletions.
diff --git a/src/libraries/Microsoft.NETCore.Platforms/src/RID.cs b/src/libraries/Microsoft.NETCore.Platforms/src/RID.cs
@@ -56,7 +56,7 @@ private enum RIDPart : int
             Max = Qualifier
         }
 
-        public static RID Parse(string runtimeIdentifier)
+        public static RID Parse(string runtimeIdentifier, bool noQualifier)
         {
             string[] parts = new string[(int)RIDPart.Max + 1];
             bool omitVersionDelimiter = true;
@@ -90,6 +90,15 @@ public static RID Parse(string runtimeIdentifier)
                         // version might be omitted
                         else if (current == ArchitectureDelimiter)
                         {
+                            // The qualifier delimiter and architecture delimiter are the same.
+                            // When there is no qualifier, there will be one delimiter past the base part
+                            // for the architecture.
+                            // So if we see another delimiter later in the string (for the architecture),
+                            // extend the base part instead of starting the architecture part.
+                            if (noQualifier && runtimeIdentifier.IndexOf(ArchitectureDelimiter, i + 1) != -1)
+                            {
+                                break;
+                            }
                             // ensure there's no version later in the string
                             if (runtimeIdentifier.IndexOf(VersionDelimiter, i) != -1)
                             {

diff --git a/src/libraries/Microsoft.NETCore.Platforms/src/RuntimeGroupCollection.cs b/src/libraries/Microsoft.NETCore.Platforms/src/RuntimeGroupCollection.cs
@@ -34,7 +34,8 @@ public RuntimeGroupCollection(ICollection<RuntimeGroup> runtimeGroups)
         /// <param name="parent"></param>
         public void AddRuntimeIdentifier(string runtimeIdentifier, string parent)
         {
-            RID rid = RID.Parse(runtimeIdentifier);
+            // don't parse qualifier since we don't use them and they are ambiguous with `-` in base RID
+            RID rid = RID.Parse(runtimeIdentifier, noQualifier: true);
 
             AddRuntimeIdentifier(rid, parent);
         }

diff --git a/src/libraries/Microsoft.NETCore.Platforms/tests/RidTests.cs b/src/libraries/Microsoft.NETCore.Platforms/tests/RidTests.cs
@@ -10,35 +10,57 @@ public class RidTests
     {
         public static IEnumerable<object[]> ValidRIDData()
         {
-            yield return new object[] { "win10-x64", new RID() { BaseRID = "win", OmitVersionDelimiter = true, Version = new RuntimeVersion("10"), Architecture = "x64" } };
-            yield return new object[] { "win10", new RID() { BaseRID = "win", OmitVersionDelimiter = true, Version = new RuntimeVersion("10")} };
-            yield return new object[] { "linux", new RID() { BaseRID = "linux" } };
-            yield return new object[] { "linux-x64", new RID() { BaseRID = "linux", Architecture = "x64" } };
-            yield return new object[] { "linux-x64", new RID() { BaseRID = "linux", Architecture = "x64" } };
-            yield return new object[] { "debian.10-x64", new RID() { BaseRID = "debian", Version = new RuntimeVersion("10"), Architecture = "x64" } };
-            yield return new object[] { "linuxmint.19.2-x64", new RID() { BaseRID = "linuxmint", Version = new RuntimeVersion("19.2"), Architecture = "x64" } };
-            yield return new object[] { "ubuntu.14.04-x64", new RID() { BaseRID = "ubuntu", Version = new RuntimeVersion("14.04"), Architecture = "x64" } };
-            yield return new object[] { "foo-bar.42-arm", new RID() { BaseRID = "foo-bar", Version = new RuntimeVersion("42"), Architecture = "arm" } };
-            yield return new object[] { "foo-bar-arm", new RID() { BaseRID = "foo", Architecture = "bar", Qualifier = "arm" } };  // demonstrates ambiguity, avoid using `-` in base
-            yield return new object[] { "linux-musl-x64", new RID() { BaseRID = "linux", Architecture = "musl", Qualifier = "x64" } };  // yes, we already have ambiguous RIDs
+            yield return new object[] { "win10-x64", new RID() { BaseRID = "win", OmitVersionDelimiter = true, Version = new RuntimeVersion("10"), Architecture = "x64" }, null };
+            yield return new object[] { "win10", new RID() { BaseRID = "win", OmitVersionDelimiter = true, Version = new RuntimeVersion("10")}, null };
+            yield return new object[] { "linux", new RID() { BaseRID = "linux" }, null };
+            yield return new object[] { "linux-x64", new RID() { BaseRID = "linux", Architecture = "x64" }, null };
+            yield return new object[] { "linux-x64", new RID() { BaseRID = "linux", Architecture = "x64" }, null };
+            yield return new object[] { "debian.10-x64", new RID() { BaseRID = "debian", Version = new RuntimeVersion("10"), Architecture = "x64" }, null };
+            yield return new object[] { "linuxmint.19.2-x64", new RID() { BaseRID = "linuxmint", Version = new RuntimeVersion("19.2"), Architecture = "x64" }, null };
+            yield return new object[] { "ubuntu.14.04-x64", new RID() { BaseRID = "ubuntu", Version = new RuntimeVersion("14.04"), Architecture = "x64" }, null };
+            yield return new object[] { "foo-bar.42-arm", new RID() { BaseRID = "foo-bar", Version = new RuntimeVersion("42"), Architecture = "arm" }, null };
+            yield return new object[] { "foo-bar-arm", new RID() { BaseRID = "foo", Architecture = "bar", Qualifier = "arm" },       // demonstrates ambiguity, avoid using `-` in base
+                                                       new RID() { BaseRID = "foo-bar", Architecture = "arm" } };
+            yield return new object[] { "linux-musl-x64", new RID() { BaseRID = "linux", Architecture = "musl", Qualifier = "x64" }, // yes, we already have ambiguous RIDs
+                                                       new RID() { BaseRID = "linux-musl", Architecture = "x64" } };
         }
 
         [Theory]
         [MemberData(nameof(ValidRIDData))]
-        internal void ParseCorrectly(string input, RID expected)
+        internal void ParseCorrectly(string input, RID expected, RID? expectedNoQualifier)
         {
-            RID actual = RID.Parse(input);
+            _ = expectedNoQualifier; // unused
+
+            RID actual = RID.Parse(input, noQualifier: false);
 
             Assert.Equal(expected, actual);
         }
 
         [Theory]
         [MemberData(nameof(ValidRIDData))]
-        internal void ToStringAsExpected(string expected, RID rid)
+        internal void ParseCorrectlyNoQualifier(string input, RID expected, RID? expectedNoQualifier)
+        {
+            expectedNoQualifier ??= expected;
+
+            RID actual = RID.Parse(input, noQualifier: true);
+
+            Assert.Equal(expectedNoQualifier, actual);
+        }
+
+        [Theory]
+        [MemberData(nameof(ValidRIDData))]
+        internal void ToStringAsExpected(string expected, RID rid, RID? expectedNoQualifierRid)
         {
             string actual = rid.ToString();
 
             Assert.Equal(expected, actual);
+
+            if (expectedNoQualifierRid is not null)
+            {
+                actual = expectedNoQualifierRid.ToString();
+
+                Assert.Equal(expected, actual);
+            }
         }
     }
 }