Skip to content

Add support for Github stacked pull requests behind opt in system property #1550

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.

Sign up for GitHub

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

Open
juliovalcarcel wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Add support for Github stacked pull requests behind opt in system property #1550

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
33740a8
Jenkins #1545 - Adding support for github stacked pull requests with …
juliovalcarcel Aug 11, 2026
9c19946
Jenkins #1545 - Fixing spotbugs + swaping to get the commit by SHA r…
juliovalcarcel Aug 12, 2026
911e7d2
Jenkins #1545 - Fixing spotless
juliovalcarcel Aug 12, 2026
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
24 changes: 22 additions & 2 deletions src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,22 @@

if (h instanceof PullRequestSCMHead) {
PullRequestSCMHead head = (PullRequestSCMHead) h;
if (head.isMerge()) {
// preview: build GitHub's precomputed merge commit directly instead of merging
// locally, when enabled and a usable merge hash is available (issue #1545)
String gitHubMergeHash = null;
if (GitHubSCMSource.preferGitHubMergeCommit && head.isMerge() && r instanceof PullRequestSCMRevision) {

Check warning on line 278 in src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMBuilder.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line 

Line 278 is only partially covered, 2 branches are missing
String mergeHash = ((PullRequestSCMRevision) r).getMergeHash();
if (mergeHash != null && !PullRequestSCMRevision.NOT_MERGEABLE_HASH.equals(mergeHash)) {

Check warning on line 280 in src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMBuilder.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line 

Line 280 is only partially covered, 2 branches are missing
gitHubMergeHash = mergeHash;
}
}
if (gitHubMergeHash != null) {
// fetch refs/pull/<n>/merge so the merge commit is available to check out below;

@jtnord jtnord Aug 11, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Should be able to fetch the commit directly (which should handle all cases?)

(although I recall (perhaps not GH but something else?) sometimes there was an race condition between the merge commit sha being in the metadata and it being available for pulling.)

@juliovalcarcel juliovalcarcel Aug 12, 2026

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Updated and re-tested on my Jenkins instance (Swapped back to PR-Merge from PR-Head) and still worked. Let me know if there is something you want handled for the potential race condition, or if since this is feature gated, leave that for a future issue to resolve?

Also fixed SpotBugs.

// it must use a destination distinct from the pull head ref that the constructor
// already fetches, otherwise git refuses to fetch two sources into one ref
withRefSpec("+refs/pull/" + head.getId() + "/merge:refs/remotes/@{remote}/" + head.getName()
+ "-merge");
} else if (head.isMerge()) {
// add the target branch to ensure that the revision we want to merge is also available
String name = head.getTarget().getName();
String localName = "remotes/" + remoteName() + "/" + name;
Expand Down Expand Up @@ -324,7 +339,12 @@
}
if (r instanceof PullRequestSCMRevision) {
PullRequestSCMRevision rev = (PullRequestSCMRevision) r;
withRevision(new AbstractGitSCMSource.SCMRevisionImpl(head, rev.getPullHash()));
if (GitHubSCMSource.preferGitHubMergeCommit && gitHubMergeHash != null) {

Check warning on line 342 in src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMBuilder.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line 

Line 342 is only partially covered, one branch is missing
// check out github's precomputed merge commit itself
withRevision(new AbstractGitSCMSource.SCMRevisionImpl(head, gitHubMergeHash));
} else {
withRevision(new AbstractGitSCMSource.SCMRevisionImpl(head, rev.getPullHash()));
}
}
}
return super.build();
Expand Down
11 changes: 11 additions & 0 deletions src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
/*
* The MIT License
*
Expand Down Expand Up @@ -180,6 +180,17 @@
private static /* mostly final */ int mergeableStatusRetries = SystemProperties.getInteger(
GitHubSCMSource.class.getName() + ".mergeableStatusRetries", Integer.valueOf(4));

/**
* Preview: for merge-strategy PRs, build GitHub's precomputed merge commit ({@code
* refs/pull/N/merge}) directly instead of reconstructing the merge locally against the base
* branch. Fixes stacked PRs whose base PR is behind the target branch (issue #1545), where the
* derived base commit is a synthetic merge ref unreachable from any branch. Opt-in and subject to
* change; affects all merge-strategy PRs, not only stacked ones.
*/
@SuppressFBWarnings(value = "MS_SHOULD_BE_FINAL", justification = "Non-final for modification from script console")
static /* mostly final */ boolean preferGitHubMergeCommit =
SystemProperties.getBoolean(GitHubSCMSource.class.getName() + ".preferGitHubMergeCommit");

//////////////////////////////////////////////////////////////////////
// Configuration fields
//////////////////////////////////////////////////////////////////////
Expand Down
103 changes: 103 additions & 0 deletions src/test/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMBuilderTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import static jenkins.plugins.git.AbstractGitSCMSource.SpecificRevisionBuildChooser;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.containsInAnyOrder;
import static org.hamcrest.Matchers.containsString;
import static org.hamcrest.Matchers.hasSize;
import static org.hamcrest.Matchers.instanceOf;
import static org.hamcrest.Matchers.is;
Expand All @@ -29,16 +30,20 @@
import hudson.plugins.git.extensions.impl.BuildChooserSetting;
import hudson.util.LogTaskListener;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
import java.util.HashSet;
import java.util.List;
import java.util.logging.Level;
import java.util.logging.Logger;
import jenkins.plugins.git.AbstractGitSCMSource;
import jenkins.plugins.git.GitSCMSourceDefaults;
import jenkins.plugins.git.MergeWithGitSCMExtension;
import jenkins.scm.api.SCMHeadOrigin;
import jenkins.scm.api.mixin.ChangeRequestCheckoutStrategy;
import org.eclipse.jgit.transport.RefSpec;
import org.eclipse.jgit.transport.RemoteConfig;
import org.jenkinsci.plugins.gitclient.GitClient;
import org.jenkinsci.plugins.workflow.multibranch.WorkflowMultiBranchProject;
Expand Down Expand Up @@ -1877,6 +1882,104 @@ public void given__cloud_pullMerge_rev_anon__when__build__then__scmBuilt() throw
assertThat(merge.getBaseHash(), is("deadbeefcafebabedeadbeefcafebabedeadbeef"));
}

// issue #1545 : SHAs taken from a live stacked-PR reproduction (PR whose base is another open PR's
// branch, that base PR being behind the target). baseHash is github's synthetic merge preview of the
// base PR, which is unreachable from any real branch; mergeHash is github's precomputed merge commit.
private static final String STACKED_BASE_HASH = "4546cf2e229fba1330fb9bc9ecd7555127fe6ef4";
private static final String STACKED_PULL_HASH = "263a673b389c5c2f3b6220362aa9c47301b28763";
private static final String STACKED_MERGE_HASH = "e3153159a0f16ffdd3f7b691da6ce73852acf34d";

private static PullRequestSCMHead stackedPullRequestHead() {
return new PullRequestSCMHead(
"PR-2",
"tester",
"test-repo",
"issue-1545-stacked",
2,
new BranchSCMHead("issue-1545-base"),
SCMHeadOrigin.DEFAULT,
ChangeRequestCheckoutStrategy.MERGE);
}

@Test
public void given__cloud_stackedPullMerge_rev__when__preferGitHubMergeCommit__then__mergeCommitCheckedOut()
throws Exception {
createGitHubSCMSourceForTest(false, null);
PullRequestSCMHead head = stackedPullRequestHead();
PullRequestSCMRevision revision =
new PullRequestSCMRevision(head, STACKED_BASE_HASH, STACKED_PULL_HASH, STACKED_MERGE_HASH);
source.setCredentialsId(null);
boolean original = GitHubSCMSource.preferGitHubMergeCommit;
GitHubSCMSource.preferGitHubMergeCommit = true;
try {
GitHubSCMBuilder instance = new GitHubSCMBuilder(source, head, revision);
instance.withGitHubRemote();
GitSCM actual = instance.build();
// github's precomputed merge commit is fetched instead of merging locally
UserRemoteConfig config = actual.getUserRemoteConfigs().get(0);
assertThat(config.getRefspec(), containsString("+refs/pull/2/merge:refs/remotes/origin/PR-2-merge"));
// every fetch refspec must target a distinct destination, or git fetch aborts with
// "Cannot fetch both ... to ..." - the merge ref must not reuse the pull head destination
RemoteConfig origin = actual.getRepositoryByName("origin");
List<String> destinations = new ArrayList<>();
for (RefSpec spec : origin.getFetchRefSpecs()) {
destinations.add(spec.getDestination());
}
assertThat(destinations.size(), is(new HashSet<>(destinations).size()));
// no local merge is performed against the unreachable base hash
assertThat(getExtension(actual, MergeWithGitSCMExtension.class), nullValue());
assertThat(
actual.getExtensions(),
containsInAnyOrder(instanceOf(GitSCMSourceDefaults.class), instanceOf(BuildChooserSetting.class)));
// the merge commit itself is the checked-out revision, not the pull head
BuildChooserSetting chooser = getExtension(actual, BuildChooserSetting.class);
AbstractGitSCMSource.SpecificRevisionBuildChooser revChooser =
(AbstractGitSCMSource.SpecificRevisionBuildChooser) chooser.getBuildChooser();
Collection<Revision> revisions = revChooser.getCandidateRevisions(
false,
"issue-1545-base",
Mockito.mock(GitClient.class),
new LogTaskListener(Logger.getAnonymousLogger(), Level.FINEST),
null,
null);
assertThat(revisions, hasSize(1));
assertThat(revisions.iterator().next().getSha1String(), is(STACKED_MERGE_HASH));
} finally {
GitHubSCMSource.preferGitHubMergeCommit = original;
}
}

@Test
public void given__cloud_stackedPullMerge_rev__when__flagOff__then__localMergeAgainstUnreachableBase()
throws Exception {
createGitHubSCMSourceForTest(false, null);
PullRequestSCMHead head = stackedPullRequestHead();
PullRequestSCMRevision revision =
new PullRequestSCMRevision(head, STACKED_BASE_HASH, STACKED_PULL_HASH, STACKED_MERGE_HASH);
source.setCredentialsId(null);
// flag defaults to off: the merge hash is ignored and the merge is reconstructed locally against the
// unreachable base hash, which is the failure reproduced by issue #1545
GitHubSCMBuilder instance = new GitHubSCMBuilder(source, head, revision);
instance.withGitHubRemote();
GitSCM actual = instance.build();
MergeWithGitSCMExtension merge = getExtension(actual, MergeWithGitSCMExtension.class);
assertThat(merge, notNullValue());
assertThat(merge.getBaseName(), is("remotes/origin/issue-1545-base"));
assertThat(merge.getBaseHash(), is(STACKED_BASE_HASH));
BuildChooserSetting chooser = getExtension(actual, BuildChooserSetting.class);
AbstractGitSCMSource.SpecificRevisionBuildChooser revChooser =
(AbstractGitSCMSource.SpecificRevisionBuildChooser) chooser.getBuildChooser();
Collection<Revision> revisions = revChooser.getCandidateRevisions(
false,
"issue-1545-base",
Mockito.mock(GitClient.class),
new LogTaskListener(Logger.getAnonymousLogger(), Level.FINEST),
null,
null);
assertThat(revisions, hasSize(1));
assertThat(revisions.iterator().next().getSha1String(), is(STACKED_PULL_HASH));
}

@Test
public void given__cloud_pullMerge_rev_userpass__when__build__then__scmBuilt() throws Exception {
createGitHubSCMSourceForTest(false, null);
Expand Down
Loading