Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

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

Merged
merged 2 commits into from Sep 23, 2021
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3866cd6
Add test for version height with path filters and merge conflict
AArnott Sep 21, 2021
96be5aa
Count merge commits in path filtered height if any parent changed it
AArnott Sep 21, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
32 changes: 32 additions & 0 deletions src/NerdBank.GitVersioning.Tests/VersionOracleTests.cs
View file
Open in desktop
Expand Up @@ -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()
{
Expand Down
4 changes: 2 additions & 2 deletions src/NerdBank.GitVersioning/LibGit2/LibGit2GitExtensions.cs
View file
Open in desktop
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
View file
Open in desktop
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