diff --git a/src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMFileSystem.java b/src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMFileSystem.java index f4a1b80c6..3d6bd56a3 100644 --- a/src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMFileSystem.java +++ b/src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMFileSystem.java @@ -28,6 +28,7 @@ import com.cloudbees.plugins.credentials.common.StandardCredentials; import edu.umd.cs.findbugs.annotations.CheckForNull; import edu.umd.cs.findbugs.annotations.NonNull; +import hudson.AbortException; import hudson.Extension; import hudson.model.Item; import hudson.plugins.git.GitSCM; @@ -35,6 +36,7 @@ import java.io.IOException; import java.io.OutputStream; import java.nio.charset.StandardCharsets; +import java.util.List; import java.util.Locale; import java.util.Objects; @@ -83,9 +85,17 @@ protected GitHubSCMFileSystem(GitHub gitHub, GHRepository repo, String refName, this.repo = repo; if (rev != null) { if (rev.getHead() instanceof PullRequestSCMHead) { - PullRequestSCMHead pr = (PullRequestSCMHead) rev.getHead(); - assert !pr.isMerge(); // TODO see below - this.ref = ((PullRequestSCMRevision) rev).getPullHash(); + PullRequestSCMRevision prRev = (PullRequestSCMRevision) rev; + PullRequestSCMHead pr = (PullRequestSCMHead) prRev.getHead(); + if (pr.isMerge()) { + this.ref = prRev.getMergeHash(); + List parents = repo.getCommit(this.ref).getParentSHA1s(); + if (parents.size() != 2 || !parents.contains(prRev.getBaseHash()) || !parents.contains(prRev.getPullHash())) { + throw new AbortException("Merge commit does not match base and head commits for pull request " + pr.getNumber() + "."); + } + } else { + this.ref = prRev.getPullHash(); + } } else if (rev instanceof AbstractGitSCMSource.SCMRevisionImpl) { this.ref = ((AbstractGitSCMSource.SCMRevisionImpl) rev).getHash(); } else { @@ -271,7 +281,7 @@ public SCMFileSystem build(@NonNull SCMSource source, @NonNull SCMHead head, @Ch refName = "tags/" + head.getName(); } else if (head instanceof PullRequestSCMHead) { PullRequestSCMHead pr = (PullRequestSCMHead) head; - if (!pr.isMerge() && pr.getSourceRepo() != null) { + if (pr.getSourceRepo() != null) { GHUser user = github.getUser(pr.getSourceOwner()); if (user == null) { // 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 } return new GitHubSCMFileSystem( github, repo, - pr.getSourceBranch(), + null, rev); } // we need to release here as we are not throwing an exception or transferring responsibility to FS Connector.release(github); - return null; // TODO support merge revisions somehow + return null; } else { // we need to release here as we are not throwing an exception or transferring responsibility to FS Connector.release(github); diff --git a/src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource.java b/src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource.java index 636f472cf..bb78c2b5c 100644 --- a/src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource.java +++ b/src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource.java @@ -957,6 +957,7 @@ public SCMSourceCriteria.Probe create(@NonNull BranchSCMHead head, branchName = "PR-" + number + "-" + strategy.name().toLowerCase(Locale.ENGLISH); } count++; + ensureDetailedGHPullRequest(pr, listener, github, ghRepository); if (request.process(new PullRequestSCMHead( pr, branchName, strategy == ChangeRequestCheckoutStrategy.MERGE ), @@ -981,19 +982,20 @@ public SCMSourceCriteria.Probe create(@NonNull PullRequestSCMHead head, public SCMRevision create(@NonNull PullRequestSCMHead head, @Nullable Void ignored) throws IOException, InterruptedException { + String prHeadHash = pr.getHead().getSha(); + String baseHash = pr.getBase().getSha(); + String mergeHash = null; + switch (strategy) { case MERGE: request.checkApiRateLimit(); - GHRef mergeRef = ghRepository.getRef( - "heads/" + pr.getBase().getRef() - ); - return new PullRequestSCMRevision(head, - mergeRef.getObject().getSha(), - pr.getHead().getSha()); + baseHash = ghRepository.getRef("heads/" + pr.getBase().getRef()).getObject().getSha(); + mergeHash = pr.getMergeCommitSha(); + break; default: - return new PullRequestSCMRevision(head, pr.getBase().getSha(), - pr.getHead().getSha()); + break; } + return new PullRequestSCMRevision(head, baseHash, prHeadHash, mergeHash); } }, new MergabilityWitness(pr, strategy, listener), @@ -1200,7 +1202,7 @@ protected SCMRevision retrieve(@NonNull String headName, @NonNull TaskListener l int number = Integer.parseInt(prMatcher.group(1)); listener.getLogger().format("Attempting to resolve %s as pull request %d%n", headName, number); try { - GHPullRequest pr = ghRepository.getPullRequest(number); + GHPullRequest pr = getDetailedGHPullRequest(number, listener, github, ghRepository); if (pr != null) { boolean fork = !ghRepository.getOwner().equals(pr.getHead().getUser()); Set strategies; @@ -1253,32 +1255,53 @@ protected SCMRevision retrieve(@NonNull String headName, @NonNull TaskListener l PullRequestSCMHead head = new PullRequestSCMHead( pr, headName, strategy == ChangeRequestCheckoutStrategy.MERGE ); + String prHeadHash = pr.getHead().getSha(); + String baseHash = pr.getBase().getSha(); + String mergeHash = null; + switch (strategy) { case MERGE: Connector.checkApiRateLimit(listener, github); - GHRef mergeRef = ghRepository.getRef( - "heads/" + pr.getBase().getRef() - ); + baseHash = ghRepository.getRef("heads/" + head.getTarget().getName()).getObject().getSha(); + mergeHash = pr.getMergeCommitSha(); + + if (Boolean.FALSE.equals(pr.getMergeable())) { + listener.getLogger().format("Resolved %s as pull request %d: Not mergeable.%n%n", + headName, + number); + return null; + } + List parents = ghRepository.getCommit(mergeHash).getParentSHA1s(); + if (parents.size() != 2 || !parents.contains(baseHash) || !parents.contains(prHeadHash)) { + listener.getLogger().format("Resolved %s as pull request %d: Merge commit does not match base and head.%n%n", + headName, + number); + return null; + + } + listener.getLogger().format( - "Resolved %s as pull request %d at revision %s merged onto %s%n", + "Resolved %s as pull request %d at revision %s merged onto %s as %s%n", headName, number, - pr.getHead().getSha(), - pr.getBase().getSha() + prHeadHash, + baseHash, + mergeHash ); - return new PullRequestSCMRevision(head, - mergeRef.getObject().getSha(), - pr.getHead().getSha()); + break; default: listener.getLogger().format( "Resolved %s as pull request %d at revision %s%n", headName, number, - pr.getHead().getSha() + prHeadHash ); - return new PullRequestSCMRevision(head, pr.getBase().getSha(), - pr.getHead().getSha()); + break; } + return new PullRequestSCMRevision(head, + baseHash, + prHeadHash, + mergeHash); } else { listener.getLogger().format( "Could not resolve %s as pull request %d%n", @@ -1438,21 +1461,32 @@ protected SCMRevision retrieve(SCMHead head, TaskListener listener) throws IOExc Connector.checkApiRateLimit(listener, github); String fullName = repoOwner + "/" + repository; ghRepository = github.getRepository(fullName); + final GHRepository ghRepository = this.ghRepository; repositoryUrl = ghRepository.getHtmlUrl(); if (head instanceof PullRequestSCMHead) { PullRequestSCMHead prhead = (PullRequestSCMHead) head; - int number = prhead.getNumber(); - GHPullRequest pr = ghRepository.getPullRequest(number); - String baseHash; + + GHPullRequest pr = getDetailedGHPullRequest(prhead.getNumber(), listener, github, ghRepository); + String baseHash = pr.getBase().getSha(); + String prHeadHash = pr.getHead().getSha(); + String mergeHash = null; switch (prhead.getCheckoutStrategy()) { case MERGE: - baseHash = ghRepository.getRef("heads/" + prhead.getTarget().getName()).getObject().getSha(); + assert(pr.getMergeable() != null); + if (Boolean.FALSE.equals(pr.getMergeable())) { + throw new AbortException("Pull request " + prhead.getNumber() + " is not mergeable."); + } + baseHash = ghRepository.getRef("heads/" + pr.getBase().getRef()).getObject().getSha(); + mergeHash = pr.getMergeCommitSha(); + List parents = ghRepository.getCommit(mergeHash).getParentSHA1s(); + if (parents.size() != 2 || !parents.contains(baseHash) || !parents.contains(prHeadHash)) { + throw new AbortException("Merge commit does not match base and head commits for pull request " + prhead.getNumber() + "."); + } break; default: - baseHash = pr.getBase().getSha(); break; } - return new PullRequestSCMRevision(prhead, baseHash, pr.getHead().getSha()); + return new PullRequestSCMRevision(prhead, baseHash, prHeadHash, mergeHash); } else if (head instanceof GitHubTagSCMHead) { GitHubTagSCMHead tagHead = (GitHubTagSCMHead) head; GHRef tag = ghRepository.getRef("tags/" + tagHead.getName()); @@ -1475,6 +1509,24 @@ protected SCMRevision retrieve(SCMHead head, TaskListener listener) throws IOExc } } + private GHPullRequest getDetailedGHPullRequest(int number, TaskListener listener, GitHub github, GHRepository ghRepository) throws IOException, InterruptedException { + Connector.checkApiRateLimit(listener, github); + GHPullRequest pr = ghRepository.getPullRequest(number); + ensureDetailedGHPullRequest(pr, listener, github, ghRepository); + return pr; + } + + private void ensureDetailedGHPullRequest(GHPullRequest pr, TaskListener listener, GitHub github, GHRepository ghRepository) throws IOException, InterruptedException { + final long sleep = 1000; + while (pr.getMergeableState() == null) { + listener.getLogger().format( + "Could not determine the mergability of pull request %d. Retrying...%n", + pr.getNumber()); + Thread.sleep(sleep); + Connector.checkApiRateLimit(listener, github); + } + } + @Override public SCM build(SCMHead head, SCMRevision revision) { return new GitHubSCMBuilder(this, head, revision).withTraits(traits).build(); diff --git a/src/main/java/org/jenkinsci/plugins/github_branch_source/PullRequestSCMRevision.java b/src/main/java/org/jenkinsci/plugins/github_branch_source/PullRequestSCMRevision.java index f758c82e7..c9a81f08a 100644 --- a/src/main/java/org/jenkinsci/plugins/github_branch_source/PullRequestSCMRevision.java +++ b/src/main/java/org/jenkinsci/plugins/github_branch_source/PullRequestSCMRevision.java @@ -24,6 +24,7 @@ package org.jenkinsci.plugins.github_branch_source; +import edu.umd.cs.findbugs.annotations.CheckForNull; import edu.umd.cs.findbugs.annotations.NonNull; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import jenkins.plugins.git.AbstractGitSCMSource; @@ -36,16 +37,22 @@ * Revision of a pull request. */ public class PullRequestSCMRevision extends ChangeRequestSCMRevision { - + private static final long serialVersionUID = 1L; private final @NonNull String baseHash; private final @NonNull String pullHash; + private final String mergeHash; public PullRequestSCMRevision(@NonNull PullRequestSCMHead head, @NonNull String baseHash, @NonNull String pullHash) { + this(head, baseHash, pullHash, null); + } + + public PullRequestSCMRevision(@NonNull PullRequestSCMHead head, @NonNull String baseHash, @NonNull String pullHash, String mergeHash) { super(head, new AbstractGitSCMSource.SCMRevisionImpl(head.getTarget(), baseHash)); this.baseHash = baseHash; this.pullHash = pullHash; + this.mergeHash = mergeHash; } @SuppressFBWarnings({"SE_PRIVATE_READ_RESOLVE_NOT_INHERITED", "RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE"}) @@ -81,6 +88,16 @@ public String getPullHash() { return pullHash; } + /** + * The commit hash of the head of the pull request branch. + * + * @return The commit hash of the head of the pull request branch + */ + @CheckForNull + public String getMergeHash() { + return mergeHash; + } + @Override public boolean equivalent(ChangeRequestSCMRevision o) { if (!(o instanceof PullRequestSCMRevision)) { @@ -97,7 +114,7 @@ public int _hashCode() { @Override public String toString() { - return getHead() instanceof PullRequestSCMHead && ((PullRequestSCMHead) getHead()).isMerge() ? pullHash + "+" + baseHash : pullHash; + return getHead() instanceof PullRequestSCMHead && ((PullRequestSCMHead) getHead()).isMerge() ? pullHash + "+" + baseHash + " (" + mergeHash + ")" : pullHash; } }