Skip to content

Commit 497b3c3

Browse files
bitwisemanbitwiseman
authored
Merge pull request #209 from bitwiseman/github-api-poc
[JENKINS-43194] Use merge_commit_sha for master
2 parents 3aa418b + fe37f64 commit 497b3c3

File tree

3 files changed

+114
-35
lines changed

3 files changed

+114
-35
lines changed

‎src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMFileSystem.java

+16-6
Original file line numberDiff line numberDiff line change
@@ -28,13 +28,15 @@
2828
import com.cloudbees.plugins.credentials.common.StandardCredentials;
2929
import edu.umd.cs.findbugs.annotations.CheckForNull;
3030
import edu.umd.cs.findbugs.annotations.NonNull;
31+
import hudson.AbortException;
3132
import hudson.Extension;
3233
import hudson.model.Item;
3334
import hudson.plugins.git.GitSCM;
3435
import hudson.scm.SCM;
3536
import java.io.IOException;
3637
import java.io.OutputStream;
3738
import java.nio.charset.StandardCharsets;
39+
import java.util.List;
3840
import java.util.Locale;
3941
import java.util.Objects;
4042

@@ -83,9 +85,17 @@ protected GitHubSCMFileSystem(GitHub gitHub, GHRepository repo, String refName,
8385
this.repo = repo;
8486
if (rev != null) {
8587
if (rev.getHead() instanceof PullRequestSCMHead) {
86-
PullRequestSCMHead pr = (PullRequestSCMHead) rev.getHead();
87-
assert !pr.isMerge(); // TODO see below
88-
this.ref = ((PullRequestSCMRevision) rev).getPullHash();
88+
PullRequestSCMRevision prRev = (PullRequestSCMRevision) rev;
89+
PullRequestSCMHead pr = (PullRequestSCMHead) prRev.getHead();
90+
if (pr.isMerge()) {
91+
this.ref = prRev.getMergeHash();
92+
List<String> parents = repo.getCommit(this.ref).getParentSHA1s();
93+
if (parents.size() != 2 || !parents.contains(prRev.getBaseHash()) || !parents.contains(prRev.getPullHash())) {
94+
throw new AbortException("Merge commit does not match base and head commits for pull request " + pr.getNumber() + ".");
95+
}
96+
} else {
97+
this.ref = prRev.getPullHash();
98+
}
8999
} else if (rev instanceof AbstractGitSCMSource.SCMRevisionImpl) {
90100
this.ref = ((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash();
91101
} else {
@@ -271,7 +281,7 @@ public SCMFileSystem build(@NonNull SCMSource source, @NonNull SCMHead head, @Ch
271281
refName = "tags/" + head.getName();
272282
} else if (head instanceof PullRequestSCMHead) {
273283
PullRequestSCMHead pr = (PullRequestSCMHead) head;
274-
if (!pr.isMerge() && pr.getSourceRepo() != null) {
284+
if (pr.getSourceRepo() != null) {
275285
GHUser user = github.getUser(pr.getSourceOwner());
276286
if (user == null) {
277287
// we need to release here as we are not throwing an exception or transferring
@@ -288,12 +298,12 @@ public SCMFileSystem build(@NonNull SCMSource source, @NonNull SCMHead head, @Ch
288298
}
289299
return new GitHubSCMFileSystem(
290300
github, repo,
291-
pr.getSourceBranch(),
301+
null,
292302
rev);
293303
}
294304
// we need to release here as we are not throwing an exception or transferring responsibility to FS
295305
Connector.release(github);
296-
return null; // TODO support merge revisions somehow
306+
return null;
297307
} else {
298308
// we need to release here as we are not throwing an exception or transferring responsibility to FS
299309
Connector.release(github);

‎src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource.java

+79-27
Original file line numberDiff line numberDiff line change
@@ -957,6 +957,7 @@ public SCMSourceCriteria.Probe create(@NonNull BranchSCMHead head,
957957
branchName = "PR-" + number + "-" + strategy.name().toLowerCase(Locale.ENGLISH);
958958
}
959959
count++;
960+
ensureDetailedGHPullRequest(pr, listener, github, ghRepository);
960961
if (request.process(new PullRequestSCMHead(
961962
pr, branchName, strategy == ChangeRequestCheckoutStrategy.MERGE
962963
),
@@ -981,19 +982,20 @@ public SCMSourceCriteria.Probe create(@NonNull PullRequestSCMHead head,
981982
public SCMRevision create(@NonNull PullRequestSCMHead head,
982983
@Nullable Void ignored)
983984
throws IOException, InterruptedException {
985+
String prHeadHash = pr.getHead().getSha();
986+
String baseHash = pr.getBase().getSha();
987+
String mergeHash = null;
988+
984989
switch (strategy) {
985990
case MERGE:
986991
request.checkApiRateLimit();
987-
GHRef mergeRef = ghRepository.getRef(
988-
"heads/" + pr.getBase().getRef()
989-
);
990-
return new PullRequestSCMRevision(head,
991-
mergeRef.getObject().getSha(),
992-
pr.getHead().getSha());
992+
baseHash = ghRepository.getRef("heads/" + pr.getBase().getRef()).getObject().getSha();
993+
mergeHash = pr.getMergeCommitSha();
994+
break;
993995
default:
994-
return new PullRequestSCMRevision(head, pr.getBase().getSha(),
995-
pr.getHead().getSha());
996+
break;
996997
}
998+
return new PullRequestSCMRevision(head, baseHash, prHeadHash, mergeHash);
997999
}
9981000
},
9991001
new MergabilityWitness(pr, strategy, listener),
@@ -1200,7 +1202,7 @@ protected SCMRevision retrieve(@NonNull String headName, @NonNull TaskListener l
12001202
int number = Integer.parseInt(prMatcher.group(1));
12011203
listener.getLogger().format("Attempting to resolve %s as pull request %d%n", headName, number);
12021204
try {
1203-
GHPullRequest pr = ghRepository.getPullRequest(number);
1205+
GHPullRequest pr = getDetailedGHPullRequest(number, listener, github, ghRepository);
12041206
if (pr != null) {
12051207
boolean fork = !ghRepository.getOwner().equals(pr.getHead().getUser());
12061208
Set<ChangeRequestCheckoutStrategy> strategies;
@@ -1253,32 +1255,53 @@ protected SCMRevision retrieve(@NonNull String headName, @NonNull TaskListener l
12531255
PullRequestSCMHead head = new PullRequestSCMHead(
12541256
pr, headName, strategy == ChangeRequestCheckoutStrategy.MERGE
12551257
);
1258+
String prHeadHash = pr.getHead().getSha();
1259+
String baseHash = pr.getBase().getSha();
1260+
String mergeHash = null;
1261+
12561262
switch (strategy) {
12571263
case MERGE:
12581264
Connector.checkApiRateLimit(listener, github);
1259-
GHRef mergeRef = ghRepository.getRef(
1260-
"heads/" + pr.getBase().getRef()
1261-
);
1265+
baseHash = ghRepository.getRef("heads/" + head.getTarget().getName()).getObject().getSha();
1266+
mergeHash = pr.getMergeCommitSha();
1267+
1268+
if (Boolean.FALSE.equals(pr.getMergeable())) {
1269+
listener.getLogger().format("Resolved %s as pull request %d: Not mergeable.%n%n",
1270+
headName,
1271+
number);
1272+
return null;
1273+
}
1274+
List<String> parents = ghRepository.getCommit(mergeHash).getParentSHA1s();
1275+
if (parents.size() != 2 || !parents.contains(baseHash) || !parents.contains(prHeadHash)) {
1276+
listener.getLogger().format("Resolved %s as pull request %d: Merge commit does not match base and head.%n%n",
1277+
headName,
1278+
number);
1279+
return null;
1280+
1281+
}
1282+
12621283
listener.getLogger().format(
1263-
"Resolved %s as pull request %d at revision %s merged onto %s%n",
1284+
"Resolved %s as pull request %d at revision %s merged onto %s as %s%n",
12641285
headName,
12651286
number,
1266-
pr.getHead().getSha(),
1267-
pr.getBase().getSha()
1287+
prHeadHash,
1288+
baseHash,
1289+
mergeHash
12681290
);
1269-
return new PullRequestSCMRevision(head,
1270-
mergeRef.getObject().getSha(),
1271-
pr.getHead().getSha());
1291+
break;
12721292
default:
12731293
listener.getLogger().format(
12741294
"Resolved %s as pull request %d at revision %s%n",
12751295
headName,
12761296
number,
1277-
pr.getHead().getSha()
1297+
prHeadHash
12781298
);
1279-
return new PullRequestSCMRevision(head, pr.getBase().getSha(),
1280-
pr.getHead().getSha());
1299+
break;
12811300
}
1301+
return new PullRequestSCMRevision(head,
1302+
baseHash,
1303+
prHeadHash,
1304+
mergeHash);
12821305
} else {
12831306
listener.getLogger().format(
12841307
"Could not resolve %s as pull request %d%n",
@@ -1438,21 +1461,32 @@ protected SCMRevision retrieve(SCMHead head, TaskListener listener) throws IOExc
14381461
Connector.checkApiRateLimit(listener, github);
14391462
String fullName = repoOwner + "/" + repository;
14401463
ghRepository = github.getRepository(fullName);
1464+
final GHRepository ghRepository = this.ghRepository;
14411465
repositoryUrl = ghRepository.getHtmlUrl();
14421466
if (head instanceof PullRequestSCMHead) {
14431467
PullRequestSCMHead prhead = (PullRequestSCMHead) head;
1444-
int number = prhead.getNumber();
1445-
GHPullRequest pr = ghRepository.getPullRequest(number);
1446-
String baseHash;
1468+
1469+
GHPullRequest pr = getDetailedGHPullRequest(prhead.getNumber(), listener, github, ghRepository);
1470+
String baseHash = pr.getBase().getSha();
1471+
String prHeadHash = pr.getHead().getSha();
1472+
String mergeHash = null;
14471473
switch (prhead.getCheckoutStrategy()) {
14481474
case MERGE:
1449-
baseHash = ghRepository.getRef("heads/" + prhead.getTarget().getName()).getObject().getSha();
1475+
assert(pr.getMergeable() != null);
1476+
if (Boolean.FALSE.equals(pr.getMergeable())) {
1477+
throw new AbortException("Pull request " + prhead.getNumber() + " is not mergeable.");
1478+
}
1479+
baseHash = ghRepository.getRef("heads/" + pr.getBase().getRef()).getObject().getSha();
1480+
mergeHash = pr.getMergeCommitSha();
1481+
List<String> parents = ghRepository.getCommit(mergeHash).getParentSHA1s();
1482+
if (parents.size() != 2 || !parents.contains(baseHash) || !parents.contains(prHeadHash)) {
1483+
throw new AbortException("Merge commit does not match base and head commits for pull request " + prhead.getNumber() + ".");
1484+
}
14501485
break;
14511486
default:
1452-
baseHash = pr.getBase().getSha();
14531487
break;
14541488
}
1455-
return new PullRequestSCMRevision(prhead, baseHash, pr.getHead().getSha());
1489+
return new PullRequestSCMRevision(prhead, baseHash, prHeadHash, mergeHash);
14561490
} else if (head instanceof GitHubTagSCMHead) {
14571491
GitHubTagSCMHead tagHead = (GitHubTagSCMHead) head;
14581492
GHRef tag = ghRepository.getRef("tags/" + tagHead.getName());
@@ -1475,6 +1509,24 @@ protected SCMRevision retrieve(SCMHead head, TaskListener listener) throws IOExc
14751509
}
14761510
}
14771511

1512+
private GHPullRequest getDetailedGHPullRequest(int number, TaskListener listener, GitHub github, GHRepository ghRepository) throws IOException, InterruptedException {
1513+
Connector.checkApiRateLimit(listener, github);
1514+
GHPullRequest pr = ghRepository.getPullRequest(number);
1515+
ensureDetailedGHPullRequest(pr, listener, github, ghRepository);
1516+
return pr;
1517+
}
1518+
1519+
private void ensureDetailedGHPullRequest(GHPullRequest pr, TaskListener listener, GitHub github, GHRepository ghRepository) throws IOException, InterruptedException {
1520+
final long sleep = 1000;
1521+
while (pr.getMergeableState() == null) {
1522+
listener.getLogger().format(
1523+
"Could not determine the mergability of pull request %d. Retrying...%n",
1524+
pr.getNumber());
1525+
Thread.sleep(sleep);
1526+
Connector.checkApiRateLimit(listener, github);
1527+
}
1528+
}
1529+
14781530
@Override
14791531
public SCM build(SCMHead head, SCMRevision revision) {
14801532
return new GitHubSCMBuilder(this, head, revision).withTraits(traits).build();

‎src/main/java/org/jenkinsci/plugins/github_branch_source/PullRequestSCMRevision.java

+19-2
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424

2525
package org.jenkinsci.plugins.github_branch_source;
2626

27+
import edu.umd.cs.findbugs.annotations.CheckForNull;
2728
import edu.umd.cs.findbugs.annotations.NonNull;
2829
import edu.umd.cs.findbugs.annotations.SuppressFBWarnings;
2930
import jenkins.plugins.git.AbstractGitSCMSource;
@@ -36,16 +37,22 @@
3637
* Revision of a pull request.
3738
*/
3839
public class PullRequestSCMRevision extends ChangeRequestSCMRevision<PullRequestSCMHead> {
39-
40+
4041
private static final long serialVersionUID = 1L;
4142

4243
private final @NonNull String baseHash;
4344
private final @NonNull String pullHash;
45+
private final String mergeHash;
4446

4547
public PullRequestSCMRevision(@NonNull PullRequestSCMHead head, @NonNull String baseHash, @NonNull String pullHash) {
48+
this(head, baseHash, pullHash, null);
49+
}
50+
51+
public PullRequestSCMRevision(@NonNull PullRequestSCMHead head, @NonNull String baseHash, @NonNull String pullHash, String mergeHash) {
4652
super(head, new AbstractGitSCMSource.SCMRevisionImpl(head.getTarget(), baseHash));
4753
this.baseHash = baseHash;
4854
this.pullHash = pullHash;
55+
this.mergeHash = mergeHash;
4956
}
5057

5158
@SuppressFBWarnings({"SE_PRIVATE_READ_RESOLVE_NOT_INHERITED", "RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE"})
@@ -81,6 +88,16 @@ public String getPullHash() {
8188
return pullHash;
8289
}
8390

91+
/**
92+
* The commit hash of the head of the pull request branch.
93+
*
94+
* @return The commit hash of the head of the pull request branch
95+
*/
96+
@CheckForNull
97+
public String getMergeHash() {
98+
return mergeHash;
99+
}
100+
84101
@Override
85102
public boolean equivalent(ChangeRequestSCMRevision<?> o) {
86103
if (!(o instanceof PullRequestSCMRevision)) {
@@ -97,7 +114,7 @@ public int _hashCode() {
97114

98115
@Override
99116
public String toString() {
100-
return getHead() instanceof PullRequestSCMHead && ((PullRequestSCMHead) getHead()).isMerge() ? pullHash + "+" + baseHash : pullHash;
117+
return getHead() instanceof PullRequestSCMHead && ((PullRequestSCMHead) getHead()).isMerge() ? pullHash + "+" + baseHash + " (" + mergeHash + ")" : pullHash;
101118
}
102119

103120
}

0 commit comments

Comments
 (0)