Merge pull request #659 from dotnet/fix658

Count merge commits in path filtered height if any parent changed it

Author: AArnott

src/NerdBank.GitVersioning.Tests/VersionOracleTests.cs

@@ -751,6 +751,38 @@ public void GetVersion_PathFilterInTwoDeepSubDirAndVersionBump()
         Assert.Equal(1, this.GetVersionHeight(relativeDirectory));
     }
 
+    [Fact]
+    public void GetVersion_PathFilterPlusMerge()
+    {
+        this.InitializeSourceControl(withInitialCommit: false);
+        this.WriteVersionFile(new VersionOptions
+        {
+            Version = new SemanticVersion("1.0"),
+            PathFilters = new FilterPath[] { new FilterPath(".", string.Empty) },
+        });
+
+        string conflictedFilePath = Path.Combine(this.RepoPath, "foo.txt");
+
+        File.WriteAllText(conflictedFilePath, "foo");
+        Commands.Stage(this.LibGit2Repository, conflictedFilePath);
+        this.LibGit2Repository.Commit("Add foo.txt with foo content.", this.Signer, this.Signer);
+        Branch originalBranch = this.LibGit2Repository.Head;
+
+        Branch topicBranch = this.LibGit2Repository.Branches.Add("topic", "HEAD~1");
+        Commands.Checkout(this.LibGit2Repository, topicBranch);
+        File.WriteAllText(conflictedFilePath, "bar");
+        Commands.Stage(this.LibGit2Repository, conflictedFilePath);
+        this.LibGit2Repository.Commit("Add foo.txt with bar content.", this.Signer, this.Signer);
+
+        Commands.Checkout(this.LibGit2Repository, originalBranch);
+        MergeResult result = this.LibGit2Repository.Merge(topicBranch, this.Signer, new MergeOptions { FileConflictStrategy = CheckoutFileConflictStrategy.Ours });
+        Assert.Equal(MergeStatus.Conflicts, result.Status);
+        Commands.Stage(this.LibGit2Repository, conflictedFilePath);
+        this.LibGit2Repository.Commit("Merge two branches", this.Signer, this.Signer);
+
+        Assert.Equal(3, this.GetVersionHeight());
+    }
+
     [Fact]
     public void GetVersionHeight_ProjectDirectoryDifferentToVersionJsonDirectory()
     {

src/NerdBank.GitVersioning/LibGit2/LibGit2GitExtensions.cs

@@ -433,8 +433,8 @@ bool ContainsRelevantChanges(IEnumerable<TreeEntryChanges> changes) =>
                             : includePaths;
 
                     // If the diff between this commit and any of its parents
-                    // does not touch a path that we care about, don't bump the
-                    // height.
+                    // touches a path that we care about, bump the height.
+                    // A no-parent commit is relevant if it introduces anything in the filtered path.
                     var relevantCommit =
                         commit.Parents.Any()
                             ? commit.Parents.Any(parent => ContainsRelevantChanges(commit.GetRepository().Diff

src/NerdBank.GitVersioning/Managed/ManagedGitExtensions.cs

@@ -169,22 +169,27 @@ bool TryCalculateHeight(GitCommit commit)
 
                 if (pathFilters is not null)
                 {
-                    var relevantCommit = true;
-
-                    foreach (var parentId in commit.Parents)
+                    // If the diff between this commit and any of its parents
+                    // touches a path that we care about, bump the height.
+                    bool relevantCommit = false, anyParents = false;
+                    foreach (GitObjectId parentId in commit.Parents)
                     {
-                        var parent = repository.GetCommit(parentId);
-                        relevantCommit = IsRelevantCommit(repository, commit, parent, pathFilters);
-
-                        // If the diff between this commit and any of its parents
-                        // does not touch a path that we care about, don't bump the
-                        // height.
-                        if (!relevantCommit)
+                        anyParents = true;
+                        GitCommit parent = repository.GetCommit(parentId);
+                        if (IsRelevantCommit(repository, commit, parent, pathFilters))
                         {
+                            // No need to scan further, as a positive match will never turn negative.
+                            relevantCommit = true;
                             break;
                         }
                     }
 
+                    if (!anyParents)
+                    {
+                        // A no-parent commit is relevant if it introduces anything in the filtered path.
+                        relevantCommit = IsRelevantCommit(repository, commit, parent: default(GitCommit), pathFilters);
+                    }
+
                     if (!relevantCommit)
                     {
                         height = 0;
@@ -214,12 +219,12 @@ private static bool IsRelevantCommit(GitRepository repository, GitCommit commit,
             return IsRelevantCommit(
                 repository,
                 repository.GetTree(commit.Tree),
-                repository.GetTree(parent.Tree),
+                parent != default ? repository.GetTree(parent.Tree) : null,
                 relativePath: string.Empty,
                 filters);
         }
 
-        private static bool IsRelevantCommit(GitRepository repository, GitTree tree, GitTree parent, string relativePath, IReadOnlyList<FilterPath> filters)
+        private static bool IsRelevantCommit(GitRepository repository, GitTree tree, GitTree? parent, string relativePath, IReadOnlyList<FilterPath> filters)
         {
             // Walk over all child nodes in the current tree. If a child node was found in the parent,
             // remove it, so that after the iteration the parent contains all nodes which have been
@@ -231,8 +236,9 @@ private static bool IsRelevantCommit(GitRepository repository, GitTree tree, Git
 
                 // If the entry is not present in the parent commit, it was added;
                 // if the Sha does not match, it was modified.
-                if (!parent.Children.TryGetValue(child.Key, out parentEntry)
-                    || parentEntry.Sha != child.Value.Sha)
+                if (parent is null ||
+                    !parent.Children.TryGetValue(child.Key, out parentEntry) ||
+                    parentEntry.Sha != child.Value.Sha)
                 {
                     // Determine whether the change was relevant.
                     var fullPath = $"{relativePath}{entry.Name}";
@@ -264,23 +270,27 @@ private static bool IsRelevantCommit(GitRepository repository, GitTree tree, Git
 
                 if (parentEntry is not null)
                 {
+                    Assumes.NotNull(parent);
                     parent.Children.Remove(child.Key);
                 }
             }
 
             // Inspect removed entries (i.e. present in parent but not in the current tree)
-            foreach (var child in parent.Children)
+            if (parent is not null)
             {
-                // Determine whether the change was relevant.
-                var fullPath = Path.Combine(relativePath, child.Key);
+                foreach (var child in parent.Children)
+                {
+                    // Determine whether the change was relevant.
+                    var fullPath = Path.Combine(relativePath, child.Key);
 
-                bool isRelevant =
-                    filters.Any(f => f.Includes(fullPath, repository.IgnoreCase))
-                    && !filters.Any(f => f.Excludes(fullPath, repository.IgnoreCase));
+                    bool isRelevant =
+                        filters.Any(f => f.Includes(fullPath, repository.IgnoreCase))
+                        && !filters.Any(f => f.Excludes(fullPath, repository.IgnoreCase));
 
-                if (isRelevant)
-                {
-                    return true;
+                    if (isRelevant)
+                    {
+                        return true;
+                    }
                 }
             }