Skip to content

Commit 23adbc8

Browse files
uhafneruhafner
authored
Merge pull request #530 from fo-code/git-merge-commits
Store merge commits for calculating the Git delta
2 parents 208bbc2 + 18f1072 commit 23adbc8

File tree

8 files changed

+492
-79
lines changed

8 files changed

+492
-79
lines changed

‎plugin/src/main/java/io/jenkins/plugins/forensics/git/delta/DeltaRepositoryCallback.java‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -89,7 +89,7 @@ private RemoteResultWrapper<Delta> calculateDelta(final Repository repository) t
8989
ByteArrayOutputStream diffStream = new ByteArrayOutputStream();
9090

9191
FilteredLog log = new FilteredLog("Errors from Git Delta:");
92-
log.logInfo("Start scanning for differences between commits...");
92+
log.logInfo("-> Start scanning for differences between commits...");
9393

9494
try (DiffFormatter diffFormatter = new DiffFormatter(diffStream)) {
9595
diffFormatter.setDiffComparator(RawTextComparator.WS_IGNORE_ALL);
@@ -100,7 +100,7 @@ private RemoteResultWrapper<Delta> calculateDelta(final Repository repository) t
100100
final List<DiffEntry> diffEntries = diffFormatter.scan(referenceCommit, currentCommit);
101101
final Map<String, FileChanges> fileChangesMap = new HashMap<>();
102102

103-
log.logInfo("%d files contain changes", diffEntries.size());
103+
log.logInfo("-> %d files contain changes", diffEntries.size());
104104

105105
for (DiffEntry diffEntry : diffEntries) {
106106
FileEditType fileEditType = getFileEditType(diffEntry.getChangeType());
@@ -110,14 +110,14 @@ private RemoteResultWrapper<Delta> calculateDelta(final Repository repository) t
110110
fileChangesMap.put(fileId, fileChanges);
111111
}
112112

113-
log.logInfo("Creating the Git diff file");
113+
log.logInfo("-> Creating the Git diff file");
114114
String diffFile = new String(diffStream.toByteArray(), StandardCharsets.UTF_8);
115115

116116
GitDelta delta = new GitDelta(currentCommitId, referenceCommitId, fileChangesMap, diffFile);
117117
RemoteResultWrapper<Delta> wrapper = new RemoteResultWrapper<>(delta, "Errors from Git Delta:");
118118
wrapper.merge(log);
119119

120-
log.logInfo("Git code delta successfully calculated");
120+
log.logInfo("-> Git code delta successfully calculated");
121121

122122
return wrapper;
123123
}

‎plugin/src/main/java/io/jenkins/plugins/forensics/git/delta/GitDeltaCalculator.java‎

Lines changed: 21 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -46,11 +46,11 @@ public Optional<Delta> calculateDelta(final Run<?, ?> build, final Run<?, ?> ref
4646
Optional<GitCommitsRecord> buildCommits = GitCommitsRecord.findRecordForScm(build, scmKeyFilter);
4747
Optional<GitCommitsRecord> referenceCommits = GitCommitsRecord.findRecordForScm(referenceBuild, scmKeyFilter);
4848
if (buildCommits.isPresent() && referenceCommits.isPresent()) {
49-
String currentCommit = buildCommits.get().getLatestCommit();
50-
String referenceCommit = referenceCommits.get().getLatestCommit();
49+
String currentCommit = getLatestCommit(build.getFullDisplayName(), buildCommits.get(), log);
50+
String referenceCommit = getLatestCommit(referenceBuild.getFullDisplayName(), referenceCommits.get(), log);
5151
if (!currentCommit.isEmpty() && !referenceCommit.isEmpty()) {
5252
log.logInfo(
53-
"Invoking Git delta calculator for determining the made changes between the commits with the IDs %s and %s",
53+
"-> Invoking Git delta calculator for determining the made changes between the commits with the IDs '%s' and '%s'",
5454
currentCommit, referenceCommit);
5555
try {
5656
RemoteResultWrapper<Delta> wrapped = git.withRepository(
@@ -67,4 +67,22 @@ public Optional<Delta> calculateDelta(final Run<?, ?> build, final Run<?, ?> ref
6767
log.logError(EMPTY_COMMIT_ERROR);
6868
return Optional.empty();
6969
}
70+
71+
/**
72+
* Returns the latest commit of the {@link GitCommitsRecord commits record} of a Git repository.
73+
*
74+
* @param buildName
75+
* The name of the build the commits record corresponds to
76+
* @param record
77+
* The commits record
78+
* @param log
79+
* The log
80+
*
81+
* @return the latest commit
82+
*/
83+
private String getLatestCommit(final String buildName, final GitCommitsRecord record, final FilteredLog log) {
84+
String latestCommitId = record.getLatestCommit();
85+
log.logInfo("-> Using commit '%s' as latest commit for build '%s'", latestCommitId, buildName);
86+
return latestCommitId;
87+
}
7088
}

‎plugin/src/main/java/io/jenkins/plugins/forensics/git/reference/BuildCommits.java‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ class BuildCommits implements Serializable {
2525

2626
private ObjectId head = ObjectId.zeroId();
2727
private ObjectId target = ObjectId.zeroId();
28+
private ObjectId merge = ObjectId.zeroId();
2829

2930
BuildCommits(final String previousBuildCommit) {
3031
this.previousBuildCommit = previousBuildCommit;
@@ -46,6 +47,23 @@ ObjectId getTarget() {
4647
return target;
4748
}
4849

50+
public ObjectId getMerge() {
51+
return merge;
52+
}
53+
54+
public void setMerge(final ObjectId merge) {
55+
this.merge = merge;
56+
}
57+
58+
/**
59+
* Checks whether the commit record contains a merge commit.
60+
*
61+
* @return {@code true} if a merge commit exists
62+
*/
63+
public boolean hasMerge() {
64+
return !merge.equals(ObjectId.zeroId());
65+
}
66+
4967
List<String> getCommits() {
5068
return commits;
5169
}
@@ -75,4 +93,17 @@ String getLatestCommit() {
7593
}
7694
return commits.get(0);
7795
}
96+
97+
/**
98+
* Returns a merge commit if existent or the latest commit if not. In case that there is a merge commit, it is the
99+
* head of the commits of this record.
100+
*
101+
* @return the found commit
102+
*/
103+
String getMergeOrLatestCommit() {
104+
if (getMerge().equals(ObjectId.zeroId())) {
105+
return getLatestCommit();
106+
}
107+
return getMerge().name();
108+
}
78109
}

‎plugin/src/main/java/io/jenkins/plugins/forensics/git/reference/GitCheckoutListener.java‎

Lines changed: 12 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,6 @@
1515
import hudson.model.listeners.SCMListener;
1616
import hudson.scm.SCM;
1717
import hudson.scm.SCMRevisionState;
18-
import jenkins.scm.api.SCMRevision;
19-
import jenkins.scm.api.SCMRevisionAction;
20-
import jenkins.scm.api.mixin.ChangeRequestSCMRevision;
2118

2219
import io.jenkins.plugins.forensics.git.util.GitCommitDecoratorFactory;
2320
import io.jenkins.plugins.forensics.git.util.GitCommitTextDecorator;
@@ -100,46 +97,32 @@ private String getLatestCommitOfPreviousBuild(final Run<?, ?> build, final Strin
10097

10198
private GitCommitsRecord recordNewCommits(final Run<?, ?> build, final GitRepositoryValidator gitRepository,
10299
final FilteredLog logger, final String latestCommit) {
103-
BuildCommits commits = recordCommitsSincePreviousBuild(latestCommit, isMerge(build), gitRepository, logger);
100+
BuildCommits commits = recordCommitsSincePreviousBuild(latestCommit, gitRepository, logger);
101+
String calculatedLatestCommit = commits.getMergeOrLatestCommit();
104102

105-
CommitDecorator commitDecorator
106-
= GitCommitDecoratorFactory.findCommitDecorator(gitRepository.getScm(), logger);
107103
String id = gitRepository.getId();
108104
if (commits.isEmpty()) {
109105
logger.logInfo("-> No new commits found");
110-
111-
return new GitCommitsRecord(build, id, logger, commits, commitDecorator.asLink(latestCommit));
106+
}
107+
else if (commits.size() == 1) {
108+
logger.logInfo("-> Recorded one new commit", commits.size());
112109
}
113110
else {
114-
if (commits.size() == 1) {
115-
logger.logInfo("-> Recorded one new commit", commits.size());
116-
}
117-
else {
118-
logger.logInfo("-> Recorded %d new commits", commits.size());
119-
}
120-
return new GitCommitsRecord(build, id, logger, commits, commitDecorator.asLink(commits.getLatestCommit()));
111+
logger.logInfo("-> Recorded %d new commits", commits.size());
121112
}
122-
}
123-
124-
private boolean isMerge(final Run<?, ?> build) {
125-
// FIXME: see https://issues.jenkins.io/browse/JENKINS-66480?focusedCommentId=412857
126-
SCMRevisionAction scmRevision = build.getAction(SCMRevisionAction.class);
127-
if (scmRevision == null) {
128-
return false;
113+
if (commits.hasMerge()) {
114+
logger.logInfo("-> The latest commit '%s' is a merge commit", calculatedLatestCommit);
129115
}
130116

131-
SCMRevision revision = scmRevision.getRevision();
132-
if (revision instanceof ChangeRequestSCMRevision) {
133-
return ((ChangeRequestSCMRevision<?>) revision).isMerge();
134-
}
135-
return false;
117+
CommitDecorator commitDecorator = GitCommitDecoratorFactory.findCommitDecorator(gitRepository.getScm(), logger);
118+
return new GitCommitsRecord(build, id, logger, commits, commitDecorator.asLink(calculatedLatestCommit));
136119
}
137120

138121
private BuildCommits recordCommitsSincePreviousBuild(final String latestCommitName,
139-
final boolean isMergeCommit, final GitRepositoryValidator gitRepository, final FilteredLog logger) {
122+
final GitRepositoryValidator gitRepository, final FilteredLog logger) {
140123
try {
141124
RemoteResultWrapper<BuildCommits> resultWrapper = gitRepository.createClient()
142-
.withRepository(new GitCommitsCollector(latestCommitName, isMergeCommit));
125+
.withRepository(new GitCommitsCollector(latestCommitName));
143126
logger.merge(resultWrapper);
144127

145128
return resultWrapper.getResult();

‎plugin/src/main/java/io/jenkins/plugins/forensics/git/reference/GitCommitsCollector.java‎

Lines changed: 22 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -29,13 +29,10 @@ class GitCommitsCollector extends AbstractRepositoryCallback<RemoteResultWrapper
2929
private static final int MAX_COMMITS = 200; // TODO: should the number of recorded commits be configurable?
3030

3131
private final String latestRecordedCommit;
32-
private final boolean isMergeCommit;
3332

34-
GitCommitsCollector(final String latestRecordedCommit, final boolean isMergeCommit) {
33+
GitCommitsCollector(final String latestRecordedCommit) {
3534
super();
36-
3735
this.latestRecordedCommit = latestRecordedCommit;
38-
this.isMergeCommit = isMergeCommit;
3936
}
4037

4138
@Override
@@ -63,35 +60,30 @@ public RemoteResultWrapper<BuildCommits> invoke(final Repository repository, fin
6360
private void findHeadCommit(final Repository repository, final BuildCommits commits, final FilteredLog logger)
6461
throws IOException {
6562
RevCommit head = getHead(repository);
66-
if (isMergeCommit) {
67-
RevCommit[] parents = head.getParents();
68-
if (parents.length < 1) {
69-
logger.logInfo("-> No parent commits found - detected the first commit in the branch");
70-
logger.logInfo("-> Using head commit '%s' as starting point",
71-
DECORATOR.asText(head));
72-
commits.setHead(head);
73-
}
74-
else if (parents.length == 1) {
75-
logger.logInfo("-> Single parent commit found - branch is already descendant of target branch head");
76-
logger.logInfo("-> Using head commit '%s' as starting point",
77-
DECORATOR.asText(head));
78-
commits.setHead(head);
79-
}
80-
else {
81-
logger.logInfo("-> Multiple parent commits found - skipping commits of local merge '%s'",
82-
DECORATOR.asText(head));
83-
logger.logInfo("-> Using parent commit '%s' of local merge as starting point",
84-
DECORATOR.asText(parents[0]));
85-
logger.logInfo("-> Storing target branch head '%s' (second parent of local merge) ",
86-
DECORATOR.asText(parents[1]));
87-
commits.setHead(parents[0]);
88-
commits.setTarget(parents[1]);
89-
}
63+
RevCommit[] parents = head.getParents();
64+
if (parents.length < 1) {
65+
logger.logInfo("-> No parent commits found - detected the first commit in the branch");
66+
logger.logInfo("-> Using head commit '%s' as starting point",
67+
DECORATOR.asText(head));
68+
commits.setHead(head);
9069
}
91-
else {
92-
logger.logInfo("-> Using head commit '%s' as starting point", DECORATOR.asText(head));
70+
else if (parents.length == 1) {
71+
logger.logInfo("-> Single parent commit found - branch is already descendant of target branch head");
72+
logger.logInfo("-> Using head commit '%s' as starting point",
73+
DECORATOR.asText(head));
9374
commits.setHead(head);
9475
}
76+
else {
77+
logger.logInfo("-> Multiple parent commits found - storing latest commit of local merge '%s'",
78+
DECORATOR.asText(head));
79+
logger.logInfo("-> Using parent commit '%s' of local merge as starting point",
80+
DECORATOR.asText(parents[0]));
81+
logger.logInfo("-> Storing target branch head '%s' (second parent of local merge) ",
82+
DECORATOR.asText(parents[1]));
83+
commits.setHead(parents[0]);
84+
commits.setTarget(parents[1]);
85+
commits.setMerge(head);
86+
}
9587
}
9688

9789
private RevCommit getHead(final Repository repository) throws IOException {

‎plugin/src/main/java/io/jenkins/plugins/forensics/git/reference/GitCommitsRecord.java‎

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ public GitCommitsRecord(final Run<?, ?> owner, final String scmKey, final Filter
8484
this.scmKey = scmKey;
8585
this.infoMessages = new ArrayList<>(logger.getInfoMessages());
8686
this.errorMessages = new ArrayList<>(logger.getErrorMessages());
87-
this.latestCommit = commits.getLatestCommit();
87+
this.latestCommit = commits.getMergeOrLatestCommit();
8888
this.latestCommitLink = latestCommitLink;
8989
this.commits = new ArrayList<>(commits.getCommits());
9090
this.recordingType = commits.getRecordingType();
@@ -157,6 +157,29 @@ public List<String> getCommits() {
157157
return commits;
158158
}
159159

160+
/**
161+
* Returns all recorded new {@link #commits} including the commits which belong to a merge, if it is part of this
162+
* commit record. These affected merge commits consist of the
163+
* {@link #targetParentCommit latest commit of the target branch} as well as the latest merge commit itself.
164+
* The order is as follows:
165+
* <ol>
166+
* <li>Latest merge commit</li>
167+
* <li>Latest target branch commit</li>
168+
* <li>Recorded new commits</li>
169+
* </ol>
170+
*
171+
* @return the hash codes of the found commits
172+
*/
173+
public List<String> getCommitsWithMerge() {
174+
List<String> foundCommits = new ArrayList<>(getCommits());
175+
if (foundCommits.contains(latestCommit) || ObjectId.zeroId().name().equals(targetParentCommit)) {
176+
return foundCommits; // there is no merge commit
177+
}
178+
foundCommits.add(0, latestCommit);
179+
foundCommits.add(1, targetParentCommit);
180+
return foundCommits;
181+
}
182+
160183
/**
161184
* Returns {@code true} if the specified commit is part of the commits.
162185
*
@@ -244,11 +267,13 @@ public String toString() {
244267
Run<?, ?> build = referenceCommits.owner;
245268
for (; targetCommits.size() < maxCommits && build != null;
246269
build = build.getPreviousBuild()) {
270+
if (owner.getExternalizableId().equals(build.getExternalizableId())) {
271+
continue; // skip the identical build when searching for a reference
272+
}
247273
List<String> additionalCommits = getCommitsForRepository(build);
248274
logger.logInfo("-> adding %d commits from build '%s' of reference job (last one: '%s')",
249275
additionalCommits.size(), build.getDisplayName(),
250276
getHeadCommitOf(additionalCommits, textDecorator));
251-
252277
if (!skipUnknownCommits || branchCommits.containsAll(additionalCommits)) {
253278
targetCommits.addAll(additionalCommits);
254279
Optional<String> referencePoint = branchCommits.stream().filter(targetCommits::contains).findFirst();
@@ -290,7 +315,7 @@ private List<String> collectBranchCommits(final int maxCommits) {
290315

291316
private List<String> getCommitsForRepository(final Run<?, ?> run) {
292317
return findRecordForScm(run, getScmKey())
293-
.map(GitCommitsRecord::getCommits)
318+
.map(GitCommitsRecord::getCommitsWithMerge)
294319
.orElse(Collections.emptyList());
295320
}
296321
}

0 commit comments

Comments
 (0)