Skip to content

Commit

Permalink
Count merge commits in path filtered height if any parent changed it
Browse files Browse the repository at this point in the history
The managed git implementation had it correctly implemented, according to the code comments in the libgit2 implementation. But those code comments did not match the *code* in the libgit2 implementation. As a result, the managed git code did not match.
In this commit I correct the libgit2 impl code comments to describe what the code *actually* does. Then I fix the identical code comment and code in the managed git implementation to match.

Fixes #658
  • Loading branch information
AArnott committed Sep 22, 2021
1 parent 3866cd6 commit 96be5aa
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 25 deletions.
4 changes: 2 additions & 2 deletions src/NerdBank.GitVersioning/LibGit2/LibGit2GitExtensions.cs
Expand Up @@ -433,8 +433,8 @@ bool TryCalculateHeight(Commit commit)
: 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
Expand Down
56 changes: 33 additions & 23 deletions src/NerdBank.GitVersioning/Managed/ManagedGitExtensions.cs
Expand Up @@ -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;
Expand Down Expand Up @@ -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
Expand All @@ -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}";
Expand Down Expand Up @@ -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;
}
}
}

Expand Down

0 comments on commit 96be5aa

Please sign in to comment.