Merge pull request #8974 from JanKrivanek/proto/TaskRegistryOrdering

Make TaskRegistry tasks ordering deterministic (FIFO)

Author: JanKrivanek

src/Build.UnitTests/TestComparers/TaskRegistryComparers.cs

@@ -20,6 +20,7 @@ internal sealed class TaskRegistryComparer : IEqualityComparer<TaskRegistry>
             public bool Equals(TaskRegistry x, TaskRegistry y)
             {
                 Assert.Equal(x.Toolset, y.Toolset, new ToolsetComparer());
+                Assert.Equal(x.NextRegistrationOrderId, y.NextRegistrationOrderId);
 
                 Helpers.AssertDictionariesEqual(
                     x.TaskRegistrations,

src/Build/Instance/TaskRegistry.cs

@@ -6,7 +6,9 @@
 using System.Collections.ObjectModel;
 using System.Diagnostics;
 using System.IO;
+using System.Linq;
 using System.Reflection;
+using System.Threading;
 using Microsoft.Build.BackEnd;
 using Microsoft.Build.Collections;
 using Microsoft.Build.Construction;
@@ -122,6 +124,11 @@ internal sealed class TaskRegistry : ITranslatable
         /// </summary>
         private static string s_potentialTasksCoreLocation = Path.Combine(BuildEnvironmentHelper.Instance.CurrentMSBuildToolsDirectory, s_tasksCoreFilename);
 
+        /// <summary>
+        /// Monotonically increasing counter for registered tasks.
+        /// </summary>
+        private int _nextRegistrationOrderId = 0;
+
         /// <summary>
         /// Cache of tasks already found using exact matching,
         /// keyed by the task identity requested.
@@ -192,6 +199,12 @@ internal Toolset Toolset
             { return _toolset; }
         }
 
+        /// <summary>
+        /// Access the next registration sequence id.
+        /// FOR UNIT TESTING ONLY.
+        /// </summary>
+        internal int NextRegistrationOrderId => _nextRegistrationOrderId;
+
         /// <summary>
         /// Access list of task registrations.
         /// FOR UNIT TESTING ONLY.
@@ -525,23 +538,11 @@ internal RegisteredTaskRecord GetTaskRegistrationRecord(
                     }
                 }
 
-                Dictionary<RegisteredTaskIdentity, List<RegisteredTaskRecord>> registrations = GetRelevantRegistrations(taskIdentity, exactMatchRequired);
+                IEnumerable<RegisteredTaskRecord> registrations = GetRelevantOrderedRegistrations(taskIdentity, exactMatchRequired);
 
                 // look for the given task name in the registry; if not found, gather all registered task names that partially
                 // match the given name
-                foreach (KeyValuePair<RegisteredTaskIdentity, List<RegisteredTaskRecord>> registration in registrations)
-                {
-                    // if the given task name is longer than the registered task name
-                    // we will use the longer name to help disambiguate between multiple matches
-                    string mostSpecificTaskName = (taskName.Length > registration.Key.Name.Length) ? taskName : registration.Key.Name;
-
-                    taskRecord = GetMatchingRegistration(mostSpecificTaskName, registration.Value, taskProjectFile, taskIdentityParameters, targetLoggingContext, elementLocation);
-
-                    if (taskRecord != null)
-                    {
-                        break;
-                    }
-                }
+                taskRecord = GetMatchingRegistration(taskName, registrations, taskProjectFile, taskIdentityParameters, targetLoggingContext, elementLocation);
             }
 
             // If we didn't find the task but we have a fallback registry in the toolset state, try that one.
@@ -603,36 +604,24 @@ private static bool IsTaskFactoryClass(Type type, object unused)
         /// If no exact match is found, looks for partial matches.
         /// A task name that is not fully qualified may produce several partial matches.
         /// </summary>
-        private Dictionary<RegisteredTaskIdentity, List<RegisteredTaskRecord>> GetRelevantRegistrations(RegisteredTaskIdentity taskIdentity, bool exactMatchRequired)
+        private IEnumerable<RegisteredTaskRecord> GetRelevantOrderedRegistrations(RegisteredTaskIdentity taskIdentity, bool exactMatchRequired)
         {
-            Dictionary<RegisteredTaskIdentity, List<RegisteredTaskRecord>> relevantTaskRegistrations =
-                new Dictionary<RegisteredTaskIdentity, List<RegisteredTaskRecord>>(RegisteredTaskIdentity.RegisteredTaskIdentityComparer.Exact);
-
-            List<RegisteredTaskRecord> taskAssemblies;
-
-            // if we find an exact match
-            if (_taskRegistrations.TryGetValue(taskIdentity, out taskAssemblies))
+            if (_taskRegistrations.TryGetValue(taskIdentity, out List<RegisteredTaskRecord> taskAssemblies))
             {
-                // we're done
-                relevantTaskRegistrations[taskIdentity] = taskAssemblies;
-                return relevantTaskRegistrations;
+                // (records for single key should be ordered by order of registrations - as they are inserted into the list)
+                return taskAssemblies;
             }
 
             if (exactMatchRequired)
             {
-                return relevantTaskRegistrations;
+                return Enumerable.Empty<RegisteredTaskRecord>();
             }
 
             // look through all task declarations for partial matches
-            foreach (KeyValuePair<RegisteredTaskIdentity, List<RegisteredTaskRecord>> taskRegistration in _taskRegistrations)
-            {
-                if (RegisteredTaskIdentity.RegisteredTaskIdentityComparer.IsPartialMatch(taskIdentity, taskRegistration.Key))
-                {
-                    relevantTaskRegistrations[taskRegistration.Key] = taskRegistration.Value;
-                }
-            }
-
-            return relevantTaskRegistrations;
+            return _taskRegistrations
+                .Where(tp => RegisteredTaskIdentity.RegisteredTaskIdentityComparer.IsPartialMatch(taskIdentity, tp.Key))
+                .SelectMany(tp => tp.Value)
+                .OrderBy(r => r.RegistrationOrderId);
         }
 
         // Create another set containing architecture-specific task entries.
@@ -673,7 +662,13 @@ private void RegisterTask(
                 _taskRegistrations[taskIdentity] = registeredTaskEntries;
             }
 
-            RegisteredTaskRecord newRecord = new RegisteredTaskRecord(taskName, assemblyLoadInfo, taskFactory, taskFactoryParameters, inlineTaskRecord);
+            RegisteredTaskRecord newRecord = new RegisteredTaskRecord(
+                taskName,
+                assemblyLoadInfo,
+                taskFactory,
+                taskFactoryParameters,
+                inlineTaskRecord,
+                Interlocked.Increment(ref _nextRegistrationOrderId));
 
             if (overrideTask)
             {
@@ -721,23 +716,21 @@ private static Dictionary<RegisteredTaskIdentity, List<RegisteredTaskRecord>> Cr
         /// </summary>
         private RegisteredTaskRecord GetMatchingRegistration(
             string taskName,
-            List<RegisteredTaskRecord> taskRecords,
+            IEnumerable<RegisteredTaskRecord> taskRecords,
             string taskProjectFile,
             IDictionary<string, string> taskIdentityParameters,
             TargetLoggingContext targetLoggingContext,
             ElementLocation elementLocation)
-        {
-            foreach (RegisteredTaskRecord record in taskRecords)
-            {
-                if (record.CanTaskBeCreatedByFactory(taskName, taskProjectFile, taskIdentityParameters, targetLoggingContext, elementLocation))
-                {
-                    return record;
-                }
-            }
-
-            // Cannot find the task in any of the records
-            return null;
-        }
+            =>
+                taskRecords.FirstOrDefault(r =>
+                    r.CanTaskBeCreatedByFactory(
+                        // if the given task name is longer than the registered task name
+                        // we will use the longer name to help disambiguate between multiple matches
+                        (taskName.Length > r.TaskIdentity.Name.Length) ? taskName : r.TaskIdentity.Name,
+                        taskProjectFile,
+                        taskIdentityParameters,
+                        targetLoggingContext,
+                        elementLocation));
 
         /// <summary>
         /// An object representing the identity of a task -- not just task name, but also
@@ -1115,17 +1108,23 @@ internal class RegisteredTaskRecord : ITranslatable
             /// </summary>
             private ParameterGroupAndTaskElementRecord _parameterGroupAndTaskBody;
 
+            /// <summary>
+            /// The registration order id for this task.  This is used to determine the order in which tasks are registered.
+            /// </summary>
+            private int _registrationOrderId;
+
             /// <summary>
             /// Constructor
             /// </summary>
-            internal RegisteredTaskRecord(string registeredName, AssemblyLoadInfo assemblyLoadInfo, string taskFactory, Dictionary<string, string> taskFactoryParameters, ParameterGroupAndTaskElementRecord inlineTask)
+            internal RegisteredTaskRecord(string registeredName, AssemblyLoadInfo assemblyLoadInfo, string taskFactory, Dictionary<string, string> taskFactoryParameters, ParameterGroupAndTaskElementRecord inlineTask, int registrationOrderId)
             {
                 ErrorUtilities.VerifyThrowArgumentNull(assemblyLoadInfo, "AssemblyLoadInfo");
                 _registeredName = registeredName;
                 _taskFactoryAssemblyLoadInfo = assemblyLoadInfo;
                 _taskFactoryParameters = taskFactoryParameters;
                 _taskIdentity = new RegisteredTaskIdentity(registeredName, taskFactoryParameters);
                 _parameterGroupAndTaskBody = inlineTask;
+                _registrationOrderId = registrationOrderId;
 
                 if (String.IsNullOrEmpty(taskFactory))
                 {
@@ -1206,6 +1205,11 @@ internal ParameterGroupAndTaskElementRecord ParameterGroupAndTaskBody
             /// </summary>
             internal RegisteredTaskIdentity TaskIdentity => _taskIdentity;
 
+            /// <summary>
+            /// The registration order id for this task.  This is used to determine the order in which tasks are registered.
+            /// </summary>
+            internal int RegistrationOrderId => _registrationOrderId;
+
             /// <summary>
             /// Ask the question, whether or not the task name can be created by the task factory.
             /// To answer this question we need to instantiate and initialize the task factory and ask it if it can create the given task name.
@@ -1765,6 +1769,7 @@ public void Translate(ITranslator translator)
                 translator.Translate(ref _taskFactoryAssemblyLoadInfo, AssemblyLoadInfo.FactoryForTranslation);
                 translator.Translate(ref _taskFactory);
                 translator.Translate(ref _parameterGroupAndTaskBody);
+                translator.Translate(ref _registrationOrderId);
 
                 IDictionary<string, string> localParameters = _taskFactoryParameters;
                 translator.TranslateDictionary(ref localParameters, count => CreateTaskFactoryParametersDictionary(count));
@@ -1787,7 +1792,7 @@ internal static RegisteredTaskRecord FactoryForDeserialization(ITranslator trans
         public void Translate(ITranslator translator)
         {
             translator.Translate(ref _toolset, Toolset.FactoryForDeserialization);
-
+            translator.Translate(ref _nextRegistrationOrderId);
             IDictionary<RegisteredTaskIdentity, List<RegisteredTaskRecord>> copy = _taskRegistrations;
             translator.TranslateDictionary(ref copy, TranslateTaskRegistrationKey, TranslateTaskRegistrationValue, count => CreateRegisteredTaskDictionary(count));