Merged PR 713643: The Nuget resolver should always take the smaller m…

…oniker in the case where other compound ones are present.

The nuget resolver sorts the content of a package in order. The assumption is that any non-compound moniker (e.g. 'net6.0') will come before any compound one (e.g. 'net6.0-android31.0). However, the comparison was made over strings across the whole relative path. This means that

lib\net6.0-android31.0\Microsoft.Identity.Client.dll < lib\net6.0\Microsoft.Identity.Client.xml

because 'lib\net6.0-' is less than 'lib/net6.0/' (the character '-' is less than the character '/').

Fix this by making a hierarchical relative path comparison, where each atom in the path is compared separately.

This was reported before for package Microsoft.Identity.Client, but the workaound (pin a qualifier to net5.0/netstandard.2.0) was not actually working for other scenarios that also required bumping the package version.

Related work items: #2047084

Public/Src/FrontEnd/Nuget/NugetAnalyzedPackage.cs

@@ -129,6 +129,7 @@ public sealed class NugetAnalyzedPackage
 
         private readonly Dictionary<string, INugetPackage> m_packagesOnConfig;
         private readonly bool m_doNotEnforceDependencyVersions;
+        private readonly NugetRelativePathComparer m_nugetRelativePathComparer;
 
         /// <nodoc/>
         private NugetAnalyzedPackage(
@@ -151,6 +152,7 @@ private NugetAnalyzedPackage(
             m_dependencies = new List<INugetPackage>();
             DependenciesPerFramework = new MultiValueDictionary<PathAtom, INugetPackage>();
             CredentialProviderPath = credentialProviderPath;
+            m_nugetRelativePathComparer = new NugetRelativePathComparer(m_context.StringTable);
         }
 
         /// <summary>
@@ -189,7 +191,7 @@ private void ParseManagedSemantics()
             var magicNugetMarker = PathAtom.Create(stringTable, "_._");
             var dllExtension = PathAtom.Create(stringTable, ".dll");
 
-            foreach (var relativePath in PackageOnDisk.Contents.OrderBy(path => path.ToString(stringTable)))
+            foreach (var relativePath in PackageOnDisk.Contents.OrderBy(path => path, m_nugetRelativePathComparer))
             {
                 // This is a dll. Check if it is in a lib folder or ref folder.
 

Public/Src/FrontEnd/Nuget/NugetPathComparer.cs

@@ -0,0 +1,62 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+using System;
+using System.Collections.Generic;
+using System.Diagnostics.ContractsLight;
+using BuildXL.Utilities.Core;
+
+namespace BuildXL.FrontEnd.Nuget
+{
+    /// <summary>
+    /// Compares two relative paths in hierarchical order, starting with the atom closer to the root. Each atom is compared as a string, case insensitive.
+    /// </summary>
+    /// <remarks>
+    /// For example, consider these two paths:
+    /// 
+    /// 1- lib/net6.0-android31/Microsoft.Identity.Client.dll
+    /// 2- lib/net6.0/Microsoft.Identity.Client.dll
+    /// 
+    /// A regular string-based comparison would determine 1 &lt; 2, because the prefix string 'lib/net6.0-' is lexicographically smaller than 'lib/net6.0/'. On the other hand
+    /// this comparer will determine that 2 &lt; 1, because the second atom on both paths is the first one that differs (starting from the root) and the string 'net6.0' is less than the string 'net6.0-android31'.
+    /// </remarks>
+    internal class NugetRelativePathComparer : IComparer<RelativePath>
+    {
+        private readonly StringTable m_stringTable;
+
+        /// <nodoc/>
+        public NugetRelativePathComparer(StringTable stringTable) 
+        {
+            Contract.Requires(stringTable != null); 
+            m_stringTable = stringTable;
+        }
+
+        /// <inheritdoc/>
+        public int Compare(RelativePath left, RelativePath right)
+        {
+            Contract.Requires(left.IsValid);
+            Contract.Requires(right.IsValid);
+
+            var leftAtoms = left.GetAtoms();
+            var rightAtoms = right.GetAtoms();
+            // Let's go in order, starting from the atom closer to the root
+            for(var i = 0; i < Math.Min(leftAtoms.Length, rightAtoms.Length); i++)
+            {
+                // Each pair is compared as strings - case insensitive (even on Linux, nuget is case insensitive across the board, so path differing in casing should be
+                // understood as the same path
+                var comparison = StringComparer.OrdinalIgnoreCase.Compare(leftAtoms[i].ToString(m_stringTable), rightAtoms[i].ToString(m_stringTable));
+
+                // If the pair of atoms are different, that determines the comparison of the whole path
+                if (comparison != 0)
+                {
+                    return comparison;
+                }
+            }
+
+            // If all atoms are the same up to the minimum length that is present on both sides,
+            // the one with less atoms is smaller
+            return leftAtoms.Length - rightAtoms.Length;
+
+        }
+    }
+}

Public/Src/FrontEnd/UnitTests/Nuget/NugetRelativePathComparerTests.cs

@@ -0,0 +1,50 @@
+// Copyright (c) Microsoft Corporation.
+// Licensed under the MIT License.
+
+using BuildXL.FrontEnd.Nuget;
+using BuildXL.FrontEnd.Sdk;
+using BuildXL.Utilities.Core;
+using Xunit;
+
+namespace Test.BuildXL.FrontEnd.Nuget
+{
+    public class NugetRelativePathComparerTests
+    {
+        private readonly FrontEndContext m_context;
+
+        public NugetRelativePathComparerTests()
+        {
+            m_context = FrontEndContext.CreateInstanceForTesting();
+        }
+
+        [Theory]
+        [InlineData("same", "same", 0)]
+        [InlineData("same/path","same/path", 0)]
+        [InlineData("same/PATH", "same/path", 0)]
+        [InlineData("short", "shortNot", -1)]
+        [InlineData("prefix/short", "prefix/shortNot", -1)]
+        [InlineData("short/but/longer/path", "shortNot/shorterPath", -1)]
+        [InlineData("lib/net6.0/Microsoft.Identity.Client.dll", "lib/net6.0-android31.0/Microsoft.Identity.Client.dll", -1)]
+        [InlineData("path/with/a/lot/of/atoms", "path/with/a/lot", 1)]
+        public void ValidateComparison(string left, string right, int expectedResult)
+        {
+            var comparer = new NugetRelativePathComparer(m_context.StringTable);
+            var leftPath = RelativePath.Create(m_context.StringTable, left);
+            var rightPath = RelativePath.Create(m_context.StringTable, right);
+
+            var result = comparer.Compare(leftPath, rightPath);
+            switch (expectedResult)
+            {
+                case -1:
+                    Assert.True(result < 0);
+                    break;
+                case 0:
+                    Assert.True(result == 0);
+                    break;
+                case 1:
+                    Assert.True(result > 0);
+                    break;
+            }
+        }
+    }
+}