Finialize subdirectory filter

Firebolt.Core/Builtins/PruneEmptyFilter.cs

@@ -9,32 +9,50 @@ namespace Firebolt.Core.Builtins
     public class PruneEmptyFilter : IParentFilter
     {
         private readonly bool pruneMerges;
-		private readonly bool pruneMergesAggressive;
+        private readonly bool pruneMergesAggressive;
 
         public PruneEmptyFilter(bool mergePruning, bool aggressiveMergePruning)
         {
-			pruneMerges = mergePruning;
-			pruneMergesAggressive = aggressiveMergePruning;
-		}
-		public async Task<bool> FilterParents(CommitMetadata commit, ParentFilterContext context)
+            pruneMerges = mergePruning;
+            pruneMergesAggressive = aggressiveMergePruning;
+        }
+        public async Task<bool> FilterParents(CommitMetadata commit, ParentFilterContext context)
         {
             // Re-process merges to eliminate branches that don't contribute anything to the tree
             if (commit.Parents.Count > 1 && pruneMergesAggressive)
             {
                 var treeSameTo = commit.Parents.Where(p => commit.Tree.Equals(p.Tree)).FirstOrDefault();
 
-                if (treeSameTo == null)
+                if (treeSameTo != null)
                 {
-                    // No parents were treesame, so there's actual content in this merge. Keep it.
-                    return true;
+                    // eliminate the parents that are not treesame as they contribute nothing
+                    commit.Parents = commit.Parents.Where(p => p.Tree.Equals(treeSameTo.Tree)).Distinct().ToList();
+
+                    // If it's demoted from a merge, it's a pointless commit now
+                    // as it's treesame to its only parent. So dump out early and drop it
+                    if (commit.Parents.Count == 1)
+                    {
+                        return false;
+                    }
                 }
+            }
 
-                // Else, eliminate the parents that are not treesame as they contribute nothing
-                commit.Parents = commit.Parents.Where(p => p.Tree.Equals(treeSameTo.Tree)).Distinct().ToList();
+            if (commit.Parents.Count == 2 && pruneMerges)
+            {
+                // Heuristic to quickly eliminate the common case of a triangle
+                var p1 = commit.Parents[0];
+                var p2 = commit.Parents[1];
 
-                // If it's demoted from a merge, it's a pointless commit now
-                // as it's treesame to its only parent. So dump out early and drop it
-                return commit.Parents.Count != 1;
+                if (p2.Parents.Contains(p1))
+                {
+                    // p1 is redundant as it's reachable from p2
+                    commit.Parents.Remove(p1);
+                }
+                else if (p1.Parents.Contains(p2))
+                {
+                    // p2 is redundant since reachable from p1
+                    commit.Parents.Remove(p2);
+                }
             }
 
             if (commit.Parents.Count > 1 && pruneMerges)
@@ -48,14 +66,14 @@ public async Task<bool> FilterParents(CommitMetadata commit, ParentFilterContext
 
                 using (var proc = await RunProcessAsync(procInfo).ConfigureAwait(false))
                 {
-                    var newParentShas = proc.StandardOutput.ReadToEnd().Split(' ', '\r', '\n').Where(x => !string.IsNullOrWhiteSpace(x)).Select(x => x.Trim()).Distinct();
+                    var newParentShas = proc.StandardOutput.ReadToEnd().Split(' ', '\r', '\n').Where(x => !string.IsNullOrWhiteSpace(x)).Select(x => x.Trim()).Distinct().ToSet();
 
                     if (proc.ExitCode != 0)
                     {
-						throw new Exception("git show-branch failed! " + proc.StandardError);
-					}
+                        throw new Exception("git show-branch failed! " + proc.StandardError.ReadToEnd());
+                    }
 
-                    commit.Parents = newParentShas.Select(context.Repo.Lookup<Commit>).ToList();
+                    commit.Parents = commit.Parents.Where(p => newParentShas.Contains(p.Sha)).ToList();
                 }
             }
 

Firebolt.Core/Builtins/SubdirectoryFilter.cs

@@ -6,33 +6,20 @@
 
 namespace Firebolt.Core.Builtins
 {
-	public class Relocation
-	{
-		public string From { get; }
-		public string To { get; }
-
-		public Relocation(string from, string to)
-		{
-			this.From = from;
-
-			// Special case of empty = move to same place
-			this.To = to == string.Empty ? from : to;
-		}
-	}
 	public class SubdirectoryFilter : ICommitFilter
 	{
-		private List<Relocation> relocations;
+		private IDictionary<string, string> relocations;
 
-		public SubdirectoryFilter(IEnumerable<Relocation> relocations)
+		public SubdirectoryFilter(IDictionary<string, string> relocations)
 		{
-			this.relocations = relocations.ToList();
+			this.relocations = relocations;
 
 			var seen = new HashSet<string>();
-            foreach (var rel in this.relocations)
+            foreach (var rel in this.relocations.Values)
             {
-                if (!seen.Add(rel.To))
+                if (!seen.Add(rel))
                 {
-					throw new Exception("Multiple relocations write to " + rel.To);
+					throw new Exception("Multiple relocations write to " + rel);
 				}
             }
 		}
@@ -43,29 +30,29 @@ public bool FilterCommit(CommitMetadata commit, FilterContext context)
 
 			foreach (var rel in relocations)
 			{
-				if (string.IsNullOrEmpty(rel.From))
-				{
-					// Move whole tree under a subdir
-					throw new NotImplementedException();
-				}
-				else
-				{
-					var subtree = commit.Tree[rel.From];
+                if (rel.Key == null)
+                {
+                    // Move whole tree under a subdir
+                    throw new NotImplementedException();
+                }
+                else
+                {
+                    var subtree = commit.Tree[rel.Key];
 
 					if (subtree != null)
 					{
-						if (rel.To == null)
-						{
-							// Move tree under the root
-							throw new NotImplementedException();
-						}
-						else
-						{
-							newTree.Add(rel.To, subtree);
-						}
-					}
-				}
-			}
+                        if (rel.Key == null)
+                        {
+                            // Move tree under the root
+                            throw new NotImplementedException();
+                        }
+                        else
+                        {
+                            newTree.Add(rel.Value, subtree);
+                        }
+                    }
+                }
+            }
 
 			if (newTree.IsEmpty)
 			{

Firebolt.Core/CommitMetadata.cs

@@ -1,6 +1,7 @@
 using LibGit2Sharp;
 using System;
 using System.Collections.Generic;
+using System.Linq;
 using System.Reflection;
 
 namespace Firebolt.Core
@@ -23,6 +24,7 @@ public TreeMetadata()
         public bool Equals(TreeDefinition other)
         {
             var otherEntries = baseEntriesField.GetValue(other) as Dictionary<string, TreeEntryDefinition>;
+
             return entries.DictionaryEqual(otherEntries);
         }
 

Firebolt.Core/Extensions.cs

@@ -50,11 +50,27 @@ public static IEnumerable<T> ConcatWith<T>(this T obj, IEnumerable<T> @enum)
             }
         }
 
-        public static bool DictionaryEqual<TKey, TValue>(this Dictionary<TKey, TValue> dict1, Dictionary<TKey, TValue> dict2)
-            where TKey: IEquatable<TKey>
+        public static bool DictionaryEqual<TKey, TValue>(this IDictionary<TKey, TValue> dict1, IDictionary<TKey, TValue> dict2)
+            where TKey : IEquatable<TKey>
             where TValue : IEquatable<TValue>
         {
-            return dict1.Count == dict2.Count && !dict1.Except(dict2).Any();
+            if (dict1 == dict2) return true;
+            if ((dict1 == null) || (dict2 == null)) return false;
+            if (dict1.Count != dict2.Count) return false;
+
+            foreach (var kvp in dict1)
+            {
+                if (!dict2.TryGetValue(kvp.Key, out TValue value2))
+                {
+                    return false;
+                }
+
+                if (!kvp.Value.Equals(value2))
+                {
+                    return false;
+                }
+            }
+            return true;
         }
     }
 }