Skip to content

More caching of merge bases #1112

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.

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

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
99 changes: 70 additions & 29 deletions src/GitVersionCore/GitRepoMetadataProvider.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2,24 +2,25 @@
using LibGit2Sharp;
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;

namespace GitVersion
{
public class GitRepoMetadataProvider
{
private Dictionary<Branch, List<BranchCommit>> mergeBaseCommitsCache;
private Dictionary<Tuple<Branch, Branch>, MergeBaseData> mergeBaseCache;
private Dictionary<MergeBaseKey, Commit> mergeBaseCache;
private Dictionary<Branch, List<SemanticVersion>> semanticVersionTagsOnBranchCache;
private IRepository Repository { get; set; }
const string missingTipFormat = "{0} has no tip. Please see http://example.com/docs for information on how to fix this.";

public GitRepoMetadataProvider(IRepository repository)
{
mergeBaseCache = new Dictionary<Tuple<Branch, Branch>, MergeBaseData>();
mergeBaseCache = new Dictionary<MergeBaseKey, Commit>();
mergeBaseCommitsCache = new Dictionary<Branch, List<BranchCommit>>();
semanticVersionTagsOnBranchCache = new Dictionary<Branch, List<SemanticVersion>>();
this.Repository = repository;
Repository = repository;
}

public IEnumerable<SemanticVersion> GetVersionTagsOnBranch(Branch branch, string tagPrefixRegex)
Expand All @@ -32,9 +33,9 @@ public IEnumerable<SemanticVersion> GetVersionTagsOnBranch(Branch branch, string

using (Logger.IndentLog(string.Format("Getting version tags from branch '{0}'.", branch.CanonicalName)))
{
var tags = this.Repository.Tags.Select(t => t).ToList();
var tags = Repository.Tags.Select(t => t).ToList();

var versionTags = this.Repository.Commits.QueryBy(new CommitFilter
var versionTags = Repository.Commits.QueryBy(new CommitFilter
{
IncludeReachableFrom = branch.Tip
})
Expand Down Expand Up @@ -86,7 +87,7 @@ public IEnumerable<Branch> GetBranchesContainingCommit([NotNull] Commit commit,
{
Logger.WriteInfo(string.Format("Searching for commits reachable from '{0}'.", branch.FriendlyName));

var commits = this.Repository.Commits.QueryBy(new CommitFilter
var commits = Repository.Commits.QueryBy(new CommitFilter
{
IncludeReachableFrom = branch
}).Where(c => c.Sha == commit.Sha);
Expand All @@ -105,17 +106,26 @@ public IEnumerable<Branch> GetBranchesContainingCommit([NotNull] Commit commit,

/// <summary>
/// Find the merge base of the two branches, i.e. the best common ancestor of the two branches' tips.
/// Note that a local branch and a remote branch tracked by it (or when untracked, has the same name) are considered the same.
/// </summary>
public Commit FindMergeBase(Branch branch, Branch otherBranch)
{
var key = Tuple.Create(branch, otherBranch);
if (branch.IsSameBranch(otherBranch))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should ensure the tips are also pointing at the same place.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JakeGinnivan Actually here it might not be the case.

Consider a local and its remote (tracked) branch the same, with the state of the local branch preferred.

Normally for the merge base it will make no difference if the local or remote branch is not up to date. That is only the case if the local and/or the remote branch was rebased. With this change (actually the algorithm in IsSameBranch()), the state (and thus merge base) of the local branch is always chosen.

{
Logger.WriteDebug(string.Format(
"The branches '{0}' and '{1}' are considered equal.",
branch.FriendlyName, otherBranch.FriendlyName));
return null;
}

if (mergeBaseCache.ContainsKey(key))
Commit mergeBase;
var key = new MergeBaseKey(branch, otherBranch);
if (mergeBaseCache.TryGetValue(key, out mergeBase))
{
Logger.WriteDebug(string.Format(
"Cache hit for merge base between '{0}' and '{1}'.",
branch.FriendlyName, otherBranch.FriendlyName));
return mergeBaseCache[key].MergeBase;
return mergeBase;
}

using (Logger.IndentLog(string.Format("Finding merge base between '{0}' and '{1}'.", branch.FriendlyName, otherBranch.FriendlyName)))
Expand All @@ -128,34 +138,34 @@ public Commit FindMergeBase(Branch branch, Branch otherBranch)
commitToFindCommonBase = otherBranch.Tip.Parents.First();
}

var findMergeBase = this.Repository.ObjectDatabase.FindMergeBase(commit, commitToFindCommonBase);
if (findMergeBase != null)
mergeBase = Repository.ObjectDatabase.FindMergeBase(commit, commitToFindCommonBase);
if (mergeBase != null)
{
Logger.WriteInfo(string.Format("Found merge base of {0}", findMergeBase.Sha));
Logger.WriteInfo(string.Format("Found merge base of {0}", mergeBase.Sha));
// We do not want to include merge base commits which got forward merged into the other branch
Commit mergeBaseAsForwardMerge;
do
{
// Now make sure that the merge base is not a forward merge
mergeBaseAsForwardMerge = otherBranch.Commits
.SkipWhile(c => c != commitToFindCommonBase)
.TakeWhile(c => c != findMergeBase)
.LastOrDefault(c => c.Parents.Contains(findMergeBase));
.TakeWhile(c => c != mergeBase)
.LastOrDefault(c => c.Parents.Contains(mergeBase));

if (mergeBaseAsForwardMerge != null)
{
commitToFindCommonBase = mergeBaseAsForwardMerge.Parents.First();
findMergeBase = this.Repository.ObjectDatabase.FindMergeBase(commit, commitToFindCommonBase);
mergeBase = this.Repository.ObjectDatabase.FindMergeBase(commit, commitToFindCommonBase);

Logger.WriteInfo(string.Format("Merge base was due to a forward merge, next merge base is {0}", findMergeBase));
Logger.WriteInfo(string.Format("Merge base was due to a forward merge, next merge base is {0}", mergeBase));
}
} while (mergeBaseAsForwardMerge != null);
}

// Store in cache.
mergeBaseCache.Add(key, new MergeBaseData(branch, otherBranch, this.Repository, findMergeBase));
mergeBaseCache.Add(key, mergeBase);

return findMergeBase;
return mergeBase;
}
}

Expand Down Expand Up @@ -192,7 +202,9 @@ List<BranchCommit> GetMergeCommitsForBranch(Branch branch)
return mergeBaseCommitsCache[branch];
}

var branchMergeBases = Repository.Branches.Select(otherBranch =>
// Since local and remote branches are considered equal, make sure that local branches are considered first.
var branchesSortedLocalFirst = Repository.Branches.Where(b => !b.IsRemote).Concat(Repository.Branches.Where(b => b.IsRemote));
var branchMergeBases = branchesSortedLocalFirst.Select(otherBranch =>
{
if (otherBranch.Tip == null)
{
Expand All @@ -208,20 +220,49 @@ List<BranchCommit> GetMergeCommitsForBranch(Branch branch)
return branchMergeBases;
}

private class MergeBaseData
/// <summary>
/// The key for the merge base data.
/// Note that the merge base of two branches is symmetric,
/// i.e. the merge base of 'branchA' and 'branchB' is the same as 'branchB' and 'branchA'.
/// </summary>
[DebuggerDisplay("A: {BranchA.CanonicalName}; B: {BranchB.CanonicalName}")]
private struct MergeBaseKey : IEquatable<MergeBaseKey>
{
public Branch Branch { get; private set; }
public Branch OtherBranch { get; private set; }
public IRepository Repository { get; private set; }
private Branch BranchA { get; set; }
private Branch BranchB { get; set; }

public MergeBaseKey(Branch branchA, Branch branchB) : this()
{
BranchA = branchA;
BranchB = branchB;
}

public bool Equals(MergeBaseKey other)
{
return (BranchA.IsSameBranch(other.BranchA) && BranchB.IsSameBranch(other.BranchB)) ||
(BranchB.IsSameBranch(other.BranchA) && BranchA.IsSameBranch(other.BranchB));
}

public override bool Equals(object obj)
{
if (ReferenceEquals(null, obj))
return false;
return obj is MergeBaseKey && Equals((MergeBaseKey)obj);
}

public Commit MergeBase { get; private set; }
public override int GetHashCode()
{
return BranchA.GetComparisonBranchName().GetHashCode() ^ BranchB.GetComparisonBranchName().GetHashCode();
}

public static bool operator ==(MergeBaseKey left, MergeBaseKey right)
{
return left.Equals(right);
}

public MergeBaseData(Branch branch, Branch otherBranch, IRepository repository, Commit mergeBase)
public static bool operator !=(MergeBaseKey left, MergeBaseKey right)
{
Branch = branch;
OtherBranch = otherBranch;
Repository = repository;
MergeBase = mergeBase;
return !left.Equals(right);
}
}
}
Expand Down
41 changes: 31 additions & 10 deletions src/GitVersionCore/LibGitExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,40 @@ public static DateTimeOffset When(this Commit commit)
}

/// <summary>
/// Checks if the two branch objects refer to the same branch (have the same friendly name).
/// Checks if the two branch objects refer to the same branch.
/// If the two branches are the local and (tracking) remote, they are also considered the same.
/// </summary>
public static bool IsSameBranch(this Branch branch, Branch otherBranch)
{
// For each branch, fixup the friendly name if the branch is remote.
var otherBranchFriendlyName = otherBranch.IsRemote ?
otherBranch.FriendlyName.Substring(otherBranch.FriendlyName.IndexOf("/", StringComparison.Ordinal) + 1) :
otherBranch.FriendlyName;
var branchFriendlyName = branch.IsRemote ?
branch.FriendlyName.Substring(branch.FriendlyName.IndexOf("/", StringComparison.Ordinal) + 1) :
branch.FriendlyName;

return otherBranchFriendlyName == branchFriendlyName;
return branch.GetComparisonBranchName() == otherBranch.GetComparisonBranchName();
}

/// <summary>
/// For comparison, find the "best" branch name,
/// either the name of the remote (tracked) branch, or the local branch name.
/// </summary>
public static string GetComparisonBranchName(this Branch branch)
{
// There are several possibilities of the state of a branch:
// 1. local branch, tracks a remote branch with same name
// 2. local branch, tracks a remote branch with *different* name
// 3. local branch, without a remote branch
// 4. remote branch, for which a local tracking branch exists
// 5. remote branch, for which no local tracking branch exists
// branch.UpstreamBranchCanonicalName - Cases 1,2,4,5: 'refs/heads/[remote-branch-name]'; Case 3: null
// branch.FriendlyName - Cases 1-3: '[local-branch-name]'; Cases 4,5: '{branch.RemoteName}/[remote-branch-name]'
//
// We want the the branch name itself, and the remote name should win over the local name,
// since the local and remote version of the branch should be the same.
// Thus, we'll use UpstreamBranchCanonicalName, stripping the /refs/heads/ prefix.
// If that is null, we use FriendlyName instead.
var upstreamName = branch.UpstreamBranchCanonicalName;
if (upstreamName == null)
{
return branch.FriendlyName;
}

return upstreamName.Substring("refs/heads/".Length);
}

/// <summary>
Expand Down