Skip to content

Commit 5532346

Browse files
asbjornuasbjornu
committed
Refactor RepositoryStore.FindMergeBase
Refactor `RepositoryStore.FindMergeBase` into the `MergeCommitFinder` class to improve readability, provide better stack traces if an exception occurs and provide better protection against `null`.
1 parent 2cdef55 commit 5532346

File tree

3 files changed

+128
-87
lines changed

3 files changed

+128
-87
lines changed

‎src/GitVersion.Core/Core/MergeBaseFinder.cs

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,123 @@
1+
using GitVersion.Common;
2+
using GitVersion.Extensions;
3+
using GitVersion.Logging;
4+
5+
namespace GitVersion;
6+
7+
internal class MergeBaseFinder
8+
{
9+
private readonly ILog log;
10+
private readonly Dictionary<Tuple<IBranch, IBranch>, ICommit> mergeBaseCache = new();
11+
private readonly IGitRepository repository;
12+
private readonly IRepositoryStore repositoryStore;
13+
14+
public MergeBaseFinder(IRepositoryStore repositoryStore, IGitRepository gitRepository, ILog log)
15+
{
16+
this.repositoryStore = repositoryStore.NotNull();
17+
this.repository = gitRepository.NotNull();
18+
this.log = log.NotNull();
19+
}
20+
21+
public ICommit? FindMergeBaseOf(IBranch? first, IBranch? second)
22+
{
23+
first = first.NotNull();
24+
second = second.NotNull();
25+
26+
var key = Tuple.Create(first, second);
27+
28+
if (this.mergeBaseCache.ContainsKey(key))
29+
{
30+
this.log.Debug($"Cache hit for merge base between '{first}' and '{second}'.");
31+
return this.mergeBaseCache[key];
32+
}
33+
34+
using (this.log.IndentLog($"Finding merge base between '{first}' and '{second}'."))
35+
{
36+
// Other branch tip is a forward merge
37+
var commitToFindCommonBase = second?.Tip;
38+
var commit = first.Tip;
39+
40+
if (commit == null)
41+
return null;
42+
43+
if (commitToFindCommonBase?.Parents.Contains(commit) == true)
44+
{
45+
commitToFindCommonBase = commitToFindCommonBase.Parents.First();
46+
}
47+
48+
if (commitToFindCommonBase == null)
49+
return null;
50+
51+
var findMergeBase = FindMergeBase(commit, commitToFindCommonBase);
52+
53+
if (findMergeBase == null)
54+
{
55+
this.log.Info($"No merge base of {first}' and '{second} could be found.");
56+
return null;
57+
}
58+
59+
// Store in cache.
60+
this.mergeBaseCache.Add(key, findMergeBase);
61+
62+
this.log.Info($"Merge base of {first}' and '{second} is {findMergeBase}");
63+
return findMergeBase;
64+
}
65+
}
66+
67+
private ICommit? FindMergeBase(ICommit commit, ICommit commitToFindCommonBase)
68+
{
69+
var findMergeBase = this.repositoryStore.FindMergeBase(commit, commitToFindCommonBase);
70+
if (findMergeBase == null)
71+
return null;
72+
73+
this.log.Info($"Found merge base of {findMergeBase}");
74+
75+
// We do not want to include merge base commits which got forward merged into the other branch
76+
ICommit? forwardMerge;
77+
do
78+
{
79+
// Now make sure that the merge base is not a forward merge
80+
forwardMerge = GetForwardMerge(commitToFindCommonBase, findMergeBase);
81+
82+
if (forwardMerge == null)
83+
continue;
84+
85+
// TODO Fix the logging up in this section
86+
var second = forwardMerge.Parents.First();
87+
this.log.Debug($"Second {second}");
88+
var mergeBase = this.repositoryStore.FindMergeBase(commit, second);
89+
if (mergeBase == null)
90+
{
91+
this.log.Warning("Could not find merge base for " + commit);
92+
}
93+
else
94+
{
95+
this.log.Debug($"New Merge base {mergeBase}");
96+
}
97+
98+
if (Equals(mergeBase, findMergeBase))
99+
{
100+
this.log.Debug("Breaking");
101+
break;
102+
}
103+
104+
findMergeBase = mergeBase;
105+
commitToFindCommonBase = second;
106+
this.log.Info($"Merge base was due to a forward merge, next merge base is {findMergeBase}");
107+
} while (forwardMerge != null);
108+
109+
return findMergeBase;
110+
}
111+
112+
private ICommit? GetForwardMerge(ICommit? commitToFindCommonBase, ICommit? findMergeBase)
113+
{
114+
var filter = new CommitFilter
115+
{
116+
IncludeReachableFrom = commitToFindCommonBase,
117+
ExcludeReachableFrom = findMergeBase
118+
};
119+
var commitCollection = this.repository.Commits.QueryBy(filter);
120+
121+
return commitCollection.FirstOrDefault(c => c.Parents.Contains(findMergeBase));
122+
}
123+
}

‎src/GitVersion.Core/Core/MergeCommitFinder.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ private IEnumerable<BranchCommit> FindMergeBases(IBranch branch)
4848
{
4949
if (sourceBranch.Tip == null)
5050
{
51-
this.log.Warning(string.Format(RepositoryStore.MissingTipFormat, sourceBranch));
51+
this.log.Warning($"{sourceBranch} has no tip.");
5252
continue;
5353
}
5454

‎src/GitVersion.Core/Core/RepositoryStore.cs

Lines changed: 4 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -10,98 +10,25 @@ namespace GitVersion;
1010

1111
public class RepositoryStore : IRepositoryStore
1212
{
13-
internal const string MissingTipFormat = "{0} has no tip. Please see https://example.com/docs for information on how to fix this.";
1413
private readonly IIncrementStrategyFinder incrementStrategyFinder;
1514
private readonly ILog log;
16-
private readonly Dictionary<Tuple<IBranch, IBranch?>, ICommit?> mergeBaseCache = new();
1715
private readonly IGitRepository repository;
1816
private readonly Dictionary<IBranch, List<SemanticVersion>> semanticVersionTagsOnBranchCache = new();
17+
private readonly MergeBaseFinder mergeBaseFinder;
1918

2019
public RepositoryStore(ILog log, IGitRepository repository, IIncrementStrategyFinder incrementStrategyFinder)
2120
{
2221
this.log = log.NotNull();
2322
this.repository = repository.NotNull();
2423
this.incrementStrategyFinder = incrementStrategyFinder.NotNull();
24+
this.mergeBaseFinder = new MergeBaseFinder(this, repository, log);
2525
}
2626

2727
/// <summary>
2828
/// Find the merge base of the two branches, i.e. the best common ancestor of the two branches' tips.
2929
/// </summary>
3030
public ICommit? FindMergeBase(IBranch? branch, IBranch? otherBranch)
31-
{
32-
branch = branch.NotNull();
33-
34-
var key = Tuple.Create(branch, otherBranch);
35-
36-
if (this.mergeBaseCache.ContainsKey(key))
37-
{
38-
this.log.Debug($"Cache hit for merge base between '{branch}' and '{otherBranch}'.");
39-
return this.mergeBaseCache[key];
40-
}
41-
42-
using (this.log.IndentLog($"Finding merge base between '{branch}' and '{otherBranch}'."))
43-
{
44-
// Other branch tip is a forward merge
45-
var commitToFindCommonBase = otherBranch?.Tip;
46-
var commit = branch.Tip;
47-
48-
if (commit == null)
49-
return null;
50-
51-
if (commitToFindCommonBase?.Parents.Contains(commit) == true)
52-
{
53-
commitToFindCommonBase = commitToFindCommonBase.Parents.First();
54-
}
55-
56-
if (commitToFindCommonBase == null)
57-
return null;
58-
59-
var findMergeBase = FindMergeBase(commit, commitToFindCommonBase);
60-
if (findMergeBase != null)
61-
{
62-
this.log.Info($"Found merge base of {findMergeBase}");
63-
// We do not want to include merge base commits which got forward merged into the other branch
64-
ICommit? forwardMerge;
65-
do
66-
{
67-
// Now make sure that the merge base is not a forward merge
68-
forwardMerge = GetForwardMerge(commitToFindCommonBase, findMergeBase);
69-
70-
if (forwardMerge == null)
71-
continue;
72-
73-
// TODO Fix the logging up in this section
74-
var second = forwardMerge.Parents.First();
75-
this.log.Debug($"Second {second}");
76-
var mergeBase = FindMergeBase(commit, second);
77-
if (mergeBase == null)
78-
{
79-
this.log.Warning("Could not find merge base for " + commit);
80-
}
81-
else
82-
{
83-
this.log.Debug($"New Merge base {mergeBase}");
84-
}
85-
86-
if (Equals(mergeBase, findMergeBase))
87-
{
88-
this.log.Debug("Breaking");
89-
break;
90-
}
91-
92-
findMergeBase = mergeBase;
93-
commitToFindCommonBase = second;
94-
this.log.Info($"Merge base was due to a forward merge, next merge base is {findMergeBase}");
95-
} while (forwardMerge != null);
96-
}
97-
98-
// Store in cache.
99-
this.mergeBaseCache.Add(key, findMergeBase);
100-
101-
this.log.Info($"Merge base of {branch}' and '{otherBranch} is {findMergeBase}");
102-
return findMergeBase;
103-
}
104-
}
31+
=> this.mergeBaseFinder.FindMergeBaseOf(branch, otherBranch);
10532

10633
public ICommit? GetCurrentCommit(IBranch currentBranch, string? commitId)
10734
{
@@ -312,7 +239,7 @@ public BranchCommit FindCommitBranchWasBranchedFrom(IBranch? branch, Config conf
312239
{
313240
if (branch.Tip == null)
314241
{
315-
this.log.Warning(string.Format(MissingTipFormat, branch));
242+
this.log.Warning($"{branch} has no tip.");
316243
return BranchCommit.Empty;
317244
}
318245

@@ -463,20 +390,11 @@ private bool BranchMatchesMainlineConfig(INamedReference branch, KeyValuePair<st
463390
return null;
464391
}
465392

466-
467393
private static IEnumerable<ICommit> GetCommitsReacheableFrom(IGitRepository repository, IGitObject commit, IBranch branch)
468394
{
469395
var filter = new CommitFilter { IncludeReachableFrom = branch };
470396
var commitCollection = repository.Commits.QueryBy(filter);
471397

472398
return commitCollection.Where(c => c.Sha == commit.Sha);
473399
}
474-
475-
private ICommit? GetForwardMerge(ICommit? commitToFindCommonBase, ICommit? findMergeBase)
476-
{
477-
var filter = new CommitFilter { IncludeReachableFrom = commitToFindCommonBase, ExcludeReachableFrom = findMergeBase };
478-
var commitCollection = this.repository.Commits.QueryBy(filter);
479-
480-
return commitCollection.FirstOrDefault(c => c.Parents.Contains(findMergeBase));
481-
}
482400
}

0 commit comments

Comments
 (0)