From 996cd93f9146329c34791b6db4a3e42e6e32c4a8 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Thu, 16 Jun 2016 16:42:36 -0400 Subject: [PATCH 01/24] [JENKINS-33161] Draft of support for customized PR build behavior. --- .../GitHubBuildStatusNotification.java | 36 +- .../GitHubSCMNavigator.java | 1 + .../github_branch_source/GitHubSCMSource.java | 405 ++++++++++++++---- .../PullRequestSCMHead.java | 36 +- .../PullRequestSCMRevision.java | 77 ++++ .../UntrustedPullRequestSCMRevision.java | 12 +- .../GitHubSCMSource/config-detail.jelly | 19 + 7 files changed, 463 insertions(+), 123 deletions(-) create mode 100644 src/main/java/org/jenkinsci/plugins/github_branch_source/PullRequestSCMRevision.java diff --git a/src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubBuildStatusNotification.java b/src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubBuildStatusNotification.java index b3bb3d509..d8b5c1036 100644 --- a/src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubBuildStatusNotification.java +++ b/src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubBuildStatusNotification.java @@ -44,10 +44,8 @@ import jenkins.scm.api.SCMRevisionAction; import jenkins.scm.api.SCMSource; import jenkins.scm.api.SCMSourceOwner; -import org.kohsuke.github.GHCommit; import org.kohsuke.github.GHCommitState; import org.kohsuke.github.GHPullRequest; -import org.kohsuke.github.GHPullRequestCommitDetail; import org.kohsuke.github.GHRepository; import org.kohsuke.github.GitHub; @@ -56,7 +54,6 @@ import java.io.File; import java.io.FileNotFoundException; import java.io.IOException; -import java.util.List; import java.util.logging.Level; import java.util.logging.Logger; @@ -239,35 +236,12 @@ private static GitHubSCMSource getSCMSource(final SCMSourceOwner scmSourceOwner) private static String resolveHeadCommit(GHRepository repo, SCMRevision revision) throws IllegalArgumentException { if (revision instanceof SCMRevisionImpl) { - SCMRevisionImpl rev = (SCMRevisionImpl) revision; - try { - GHCommit commit = repo.getCommit(rev.getHash()); - List parents = commit.getParents(); - // if revision has two parent commits, we have really a MergeCommit - if (parents.size() == 2) { - SCMHead head = revision.getHead(); - // MergeCommit is coming from a pull request - if (head instanceof PullRequestSCMHead) { - GHPullRequest pullRequest = repo.getPullRequest(((PullRequestSCMHead) head).getNumber()); - for (GHPullRequestCommitDetail commitDetail : pullRequest.listCommits()) { - if (commitDetail.getSha().equals(parents.get(0).getSHA1())) { - // Parent commit (HeadCommit) found in PR commit list - return parents.get(0).getSHA1(); - } - } - // First parent commit not found in PR commit list, so returning the second one. - return parents.get(1).getSHA1(); - } else { - return rev.getHash(); - } - } - return rev.getHash(); - } catch (IOException e) { - LOGGER.log(Level.WARNING, null, e); - throw new IllegalArgumentException(e); - } + return ((SCMRevisionImpl) revision).getHash(); + } else if (revision instanceof PullRequestSCMRevision) { + return ((PullRequestSCMRevision) revision).getPullHash(); + } else { + throw new IllegalArgumentException("did not recognize " + revision); } - throw new IllegalArgumentException(); } private GitHubBuildStatusNotification() {} diff --git a/src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMNavigator.java b/src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMNavigator.java index 02fd4fe4d..ec28e26e2 100644 --- a/src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMNavigator.java +++ b/src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMNavigator.java @@ -70,6 +70,7 @@ public class GitHubSCMNavigator extends SCMNavigator { @CheckForNull private String includes; @CheckForNull private String excludes; + // TODO buildXXX to match GitHubSCMSource @DataBoundConstructor public GitHubSCMNavigator(String apiUri, String repoOwner, String scanCredentialsId, String checkoutCredentialsId) { this.repoOwner = repoOwner; 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 e5300934a..2926583c8 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 @@ -57,7 +57,6 @@ import org.kohsuke.github.GHMyself; import org.kohsuke.github.GHOrganization; import org.kohsuke.github.GHPullRequest; -import org.kohsuke.github.GHRef; import org.kohsuke.github.GHRepository; import org.kohsuke.github.GHUser; import org.kohsuke.github.GitHub; @@ -84,10 +83,25 @@ import java.util.logging.Logger; import static hudson.model.Items.XSTREAM2; +import hudson.model.Run; +import hudson.plugins.git.GitException; +import hudson.plugins.git.GitSCM; +import hudson.plugins.git.Revision; +import hudson.plugins.git.extensions.GitSCMExtension; +import hudson.plugins.git.util.MergeRecord; +import hudson.scm.SCM; +import java.util.HashSet; +import org.eclipse.jgit.lib.Constants; +import org.eclipse.jgit.lib.ObjectId; +import org.jenkinsci.plugins.gitclient.CheckoutCommand; +import org.jenkinsci.plugins.gitclient.GitClient; +import org.jenkinsci.plugins.gitclient.MergeCommand; import static org.jenkinsci.plugins.github.config.GitHubServerConfig.GITHUB_URL; public class GitHubSCMSource extends AbstractGitSCMSource { + private static final Logger LOGGER = Logger.getLogger(GitHubSCMSource.class.getName()); + private final String apiUri; /** Credentials for actual clone; may be SSH private key. */ @@ -104,6 +118,19 @@ public class GitHubSCMSource extends AbstractGitSCMSource { private @Nonnull String excludes = DescriptorImpl.defaultExcludes; + /** Whether to build regular origin branches. */ + private @Nonnull Boolean buildOriginBranch = DescriptorImpl.defaultBuildOriginBranch; + /** Whether to build origin branches which happen to also have a PR filed from them (but here we are naming and building as a branch). */ + private @Nonnull Boolean buildOriginBranchWithPR = DescriptorImpl.defaultBuildOriginBranchWithPR; + /** Whether to build PRs filed from the origin, where the build is of the merge with the base branch. */ + private @Nonnull Boolean buildOriginPRMerged = DescriptorImpl.defaultBuildOriginPRMerged; + /** Whether to build PRs filed from the origin, where the build is of the branch head. */ + private @Nonnull Boolean buildOriginPRUnmerged = DescriptorImpl.defaultBuildOriginPRUnmerged; + /** Whether to build PRs filed from a fork, where the build is of the merge with the base branch. */ + private @Nonnull Boolean buildForkPRMerged = DescriptorImpl.defaultBuildForkPRMerged; + /** Whether to build PRs filed from a fork, where the build is of the branch head. */ + private @Nonnull Boolean buildForkPRUnmerged = DescriptorImpl.defaultBuildForkPRUnmerged; + @DataBoundConstructor public GitHubSCMSource(String id, String apiUri, String checkoutCredentialsId, String scanCredentialsId, String repoOwner, String repository) { super(id); @@ -114,6 +141,28 @@ public GitHubSCMSource(String id, String apiUri, String checkoutCredentialsId, S this.checkoutCredentialsId = checkoutCredentialsId; } + private Object readResolve() { + if (buildOriginBranch == null) { + buildOriginBranch = DescriptorImpl.defaultBuildOriginBranch; + } + if (buildOriginBranchWithPR == null) { + buildOriginBranchWithPR = DescriptorImpl.defaultBuildOriginBranchWithPR; + } + if (buildOriginPRMerged == null) { + buildOriginPRMerged = DescriptorImpl.defaultBuildOriginPRMerged; + } + if (buildOriginPRUnmerged == null) { + buildOriginPRUnmerged = DescriptorImpl.defaultBuildOriginPRUnmerged; + } + if (buildForkPRMerged == null) { + buildForkPRMerged = DescriptorImpl.defaultBuildForkPRMerged; + } + if (buildForkPRUnmerged == null) { + buildForkPRUnmerged = DescriptorImpl.defaultBuildForkPRUnmerged; + } + return this; + } + @CheckForNull public String getApiUri() { return apiUri; @@ -161,7 +210,7 @@ public String getRepository() { @Override protected List getRefSpecs() { return new ArrayList(Arrays.asList(new RefSpec("+refs/heads/*:refs/remotes/origin/*"), - new RefSpec("+refs/pull/*/merge:refs/remotes/origin/pr/*"))); + new RefSpec("+refs/pull/*/head:refs/remotes/origin/pr/*"))); } /** @@ -216,6 +265,60 @@ Collections. emptyList()), CredentialsMatchers.allOf( this.excludes = excludes; } + public Boolean getBuildOriginBranch() { + return buildOriginBranch; + } + + @DataBoundSetter + public void setBuildOriginBranch(Boolean buildOriginBranch) { + this.buildOriginBranch = buildOriginBranch; + } + + public Boolean getBuildOriginBranchWithPR() { + return buildOriginBranchWithPR; + } + + @DataBoundSetter + public void setBuildOriginBranchWithPR(Boolean buildOriginBranchWithPR) { + this.buildOriginBranchWithPR = buildOriginBranchWithPR; + } + + public Boolean getBuildOriginPRMerged() { + return buildOriginPRMerged; + } + + @DataBoundSetter + public void setBuildOriginPRMerged(Boolean buildOriginPRMerged) { + this.buildOriginPRMerged = buildOriginPRMerged; + } + + public Boolean getBuildOriginPRUnmerged() { + return buildOriginPRUnmerged; + } + + @DataBoundSetter + public void setBuildOriginPRUnmerged(Boolean buildOriginPRUnmerged) { + this.buildOriginPRUnmerged = buildOriginPRUnmerged; + } + + public Boolean getBuildForkPRMerged() { + return buildForkPRMerged; + } + + @DataBoundSetter + public void setBuildForkPRMerged(Boolean buildForkPRMerged) { + this.buildForkPRMerged = buildForkPRMerged; + } + + public Boolean getBuildForkPRUnmerged() { + return buildForkPRUnmerged; + } + + @DataBoundSetter + public void setBuildForkPRUnmerged(Boolean buildForkPRUnmerged) { + this.buildForkPRUnmerged = buildForkPRUnmerged; + } + @Override public String getRemote() { return getUriResolver().getRepositoryUri(apiUri, repoOwner, repository); @@ -263,73 +366,134 @@ public String getRemote() { private void doRetrieve(SCMHeadObserver observer, TaskListener listener, GHRepository repo) throws IOException, InterruptedException { SCMSourceCriteria criteria = getCriteria(); - listener.getLogger().format("%n Getting remote branches...%n"); - int branches = 0; - for (Map.Entry entry : repo.getBranches().entrySet()) { - final String branchName = entry.getKey(); - if (isExcluded(branchName)) { - continue; - } - listener.getLogger().format("%n Checking branch %s%n", HyperlinkNote.encodeTo(repo.getHtmlUrl().toString() + "/tree/" + branchName, branchName)); - if (criteria != null) { - SCMSourceCriteria.Probe probe = getProbe(branchName, "branch", "refs/heads/" + branchName, repo, listener); - if (criteria.isHead(probe, listener)) { - listener.getLogger().format(" Met criteria%n"); - } else { - listener.getLogger().format(" Does not meet criteria%n"); + Set originBranchesWithPR = new HashSet<>(); + + if (buildOriginPRMerged || buildOriginPRUnmerged || buildForkPRMerged || buildForkPRUnmerged) { + listener.getLogger().format("%n Getting remote pull requests...%n"); + int pullrequests = 0; + for (GHPullRequest ghPullRequest : repo.getPullRequests(GHIssueState.OPEN)) { + int number = ghPullRequest.getNumber(); + listener.getLogger().format("%n Checking pull request %s%n", HyperlinkNote.encodeTo(ghPullRequest.getHtmlUrl().toString(), "#" + number)); + boolean fork = !repo.getOwner().equals(ghPullRequest.getHead().getUser()); + if (fork && !buildForkPRMerged && !buildForkPRUnmerged) { + listener.getLogger().format(" Submitted from fork, skipping%n%n"); continue; } + if (!fork && !buildOriginPRMerged && !buildOriginPRUnmerged) { + listener.getLogger().format(" Submitted from origin repository, skipping%n%n"); + continue; + } + boolean trusted = isTrusted(repo, ghPullRequest); + if (!trusted) { + listener.getLogger().format(" (not from a trusted source)%n"); + } + for (boolean merge : new boolean[] {false, true}) { + String branchName = "PR-" + number; + if (merge && fork) { + if (!buildForkPRMerged) { + continue; // not doing this combination + } + if (buildForkPRUnmerged) { + branchName += "-merged"; // make sure they are distinct + } + } + if (merge && !fork) { + if (!buildOriginPRMerged) { + continue; + } + if (buildForkPRUnmerged) { + branchName += "-merged"; + } + } + if (!merge && fork) { + if (!buildForkPRUnmerged) { + continue; + } + if (buildForkPRMerged) { + branchName += "-unmerged"; + } + } + if (!merge && !fork) { + if (!buildOriginPRUnmerged) { + continue; + } + if (buildOriginPRMerged) { + branchName += "-unmerged"; + } + } + PullRequestSCMHead head = new PullRequestSCMHead(ghPullRequest, branchName, merge, trusted); + if (criteria != null) { + SCMSourceCriteria.Probe probe = getProbe(branchName, "pull request", "refs/pull/" + number + (merge ? "/merge" : "/head"), repo, listener); + if (criteria.isHead(probe, listener)) { + // FYI https://developer.github.com/v3/pulls/#response-1 + Boolean mergeable = ghPullRequest.getMergeable(); + if (Boolean.FALSE.equals(mergeable)) { + if (merge) { + listener.getLogger().format(" Not mergeable, build likely to fail%n"); + } else { + listener.getLogger().format(" Not mergeable, but will be built anyway%n"); + } + } + listener.getLogger().format(" Met criteria%n"); + } else { + listener.getLogger().format(" Does not meet criteria%n"); + continue; + } + } + String baseHash; + if (merge) { + baseHash = repo.getRef("heads/" + ghPullRequest.getBase().getRef()).getObject().getSha(); + } else { + baseHash = ghPullRequest.getBase().getSha(); + } + PullRequestSCMRevision rev = new PullRequestSCMRevision(head, baseHash, ghPullRequest.getHead().getSha()); + observer.observe(head, rev); + if (!observer.isObserving()) { + return; + } + } + pullrequests++; } - SCMHead head = new SCMHead(branchName); - SCMRevision hash = new SCMRevisionImpl(head, entry.getValue().getSHA1()); - observer.observe(head, hash); - if (!observer.isObserving()) { - return; - } - branches++; + listener.getLogger().format("%n %d pull requests were processed%n", pullrequests); } - listener.getLogger().format("%n %d branches were processed%n", branches); - - listener.getLogger().format("%n Getting remote pull requests...%n"); - int pullrequests = 0; - for (GHPullRequest ghPullRequest : repo.getPullRequests(GHIssueState.OPEN)) { - PullRequestSCMHead head = new PullRequestSCMHead(ghPullRequest); - final String branchName = head.getName(); - listener.getLogger().format("%n Checking pull request %s%n", HyperlinkNote.encodeTo(ghPullRequest.getHtmlUrl().toString(), "#" + branchName)); - if (repo.getOwner().equals(ghPullRequest.getHead().getUser())) { - listener.getLogger().format(" Submitted from origin repository, skipping%n%n"); - continue; - } - if (criteria != null) { - SCMSourceCriteria.Probe probe = getProbe(branchName, "pull request", "refs/pull/" + head.getNumber() + "/head", repo, listener); - if (criteria.isHead(probe, listener)) { - // FYI https://developer.github.com/v3/pulls/#response-1 - Boolean mergeable = ghPullRequest.getMergeable(); - if (Boolean.FALSE.equals(mergeable)) { - listener.getLogger().format(" Not mergeable, but it will be included%n"); - } - listener.getLogger().format(" Met criteria%n"); - } else { - listener.getLogger().format(" Does not meet criteria%n"); + + if (buildOriginBranch || buildOriginBranchWithPR) { + listener.getLogger().format("%n Getting remote branches...%n"); + int branches = 0; + for (Map.Entry entry : repo.getBranches().entrySet()) { + final String branchName = entry.getKey(); + if (isExcluded(branchName)) { continue; } + boolean hasPR = originBranchesWithPR.contains(branchName); + if (!hasPR && !buildOriginBranch) { + listener.getLogger().format("%n Skipping branch %s since there is no corresponding PR%n", branchName); + continue; + } + if (hasPR && !buildOriginBranchWithPR) { + listener.getLogger().format("%n Skipping branch %s since there is a corresponding PR%n", branchName); + continue; + } + listener.getLogger().format("%n Checking branch %s%n", HyperlinkNote.encodeTo(repo.getHtmlUrl().toString() + "/tree/" + branchName, branchName)); + if (criteria != null) { + SCMSourceCriteria.Probe probe = getProbe(branchName, "branch", "refs/heads/" + branchName, repo, listener); + if (criteria.isHead(probe, listener)) { + listener.getLogger().format(" Met criteria%n"); + } else { + listener.getLogger().format(" Does not meet criteria%n"); + continue; + } + } + SCMHead head = new SCMHead(branchName); + SCMRevision hash = new SCMRevisionImpl(head, entry.getValue().getSHA1()); + observer.observe(head, hash); + if (!observer.isObserving()) { + return; + } + branches++; } - String trustedBase = trustedReplacement(repo, ghPullRequest); - SCMRevision hash; - if (trustedBase == null) { - hash = new SCMRevisionImpl(head, ghPullRequest.getHead().getSha()); - } else { - listener.getLogger().format(" (not from a trusted source)%n"); - hash = new UntrustedPullRequestSCMRevision(head, ghPullRequest.getHead().getSha(), trustedBase); - } - observer.observe(head, hash); - if (!observer.isObserving()) { - return; - } - pullrequests++; + listener.getLogger().format("%n %d branches were processed%n", branches); } - listener.getLogger().format("%n %d pull requests were processed%n", pullrequests); - } /** @@ -411,28 +575,85 @@ protected SCMRevision retrieve(SCMHead head, TaskListener listener) throws IOExc } protected SCMRevision doRetrieve(SCMHead head, TaskListener listener, GHRepository repo) throws IOException, InterruptedException { - GHRef ref; if (head instanceof PullRequestSCMHead) { - int number = ((PullRequestSCMHead) head).getNumber(); - ref = repo.getRef("pull/" + number + "/merge"); - // getPullRequests makes an extra API call, but we need its current .base.sha - String trustedBase = trustedReplacement(repo, repo.getPullRequest(number)); - if (trustedBase != null) { - return new UntrustedPullRequestSCMRevision(head, ref.getObject().getSha(), trustedBase); + PullRequestSCMHead prhead = (PullRequestSCMHead) head; + int number = prhead.getNumber(); + GHPullRequest pr = repo.getPullRequest(number); + String baseHash; + if (prhead.isMerge()) { + PullRequestAction metadata = prhead.getAction(PullRequestAction.class); + if (metadata == null) { + throw new IOException("Cannot find base branch metadata from " + prhead); + } + baseHash = repo.getRef("heads/" + metadata.getTarget().getName()).getObject().getSha(); + } else { + baseHash = pr.getBase().getSha(); } + return new PullRequestSCMRevision((PullRequestSCMHead) head, baseHash, pr.getHead().getSha()); } else { - ref = repo.getRef("heads/" + head.getName()); + return new SCMRevisionImpl(head, repo.getRef("heads/" + head.getName()).getObject().getSha()); + } + } + + @Override + public SCM build(SCMHead head, SCMRevision revision) { + SCM scm = super.build(head, revision); + if (head instanceof PullRequestSCMHead && ((PullRequestSCMHead) head).isMerge()) { + // Similar to PreBuildMerge, but we cannot use that unmodified: we need to specify the exact base branch hash. + PullRequestAction action = head.getAction(PullRequestAction.class); + String baseName; + if (action != null) { + baseName = action.getTarget().getName(); + } else { + baseName = "master?"; // OK if not found, just informational anyway + } + ((GitSCM) scm).getExtensions().add(new MergeWith(baseName, ((PullRequestSCMRevision) revision).getBaseHash())); + } + return scm; + } + private static class MergeWith extends GitSCMExtension { + private final String baseName; + private final String baseHash; + MergeWith(String baseName, String baseHash) { + this.baseName = baseName; + this.baseHash = baseHash; + } + @Override + public Revision decorateRevisionToBuild(GitSCM scm, Run build, GitClient git, TaskListener listener, Revision marked, Revision rev) throws IOException, InterruptedException, GitException { + listener.getLogger().println("Merging " + baseName + " commit " + baseHash + " into PR head commit " + rev.getSha1String()); + try { + MergeCommand cmd = git.merge().setRevisionToMerge(ObjectId.fromString(baseHash)); + for (GitSCMExtension ext : scm.getExtensions()) { + // By default we do a regular merge, allowing it to fast-forward. + ext.decorateMergeCommand(scm, build, git, listener, cmd); + } + cmd.execute(); + } catch (GitException x) { + // Try to revert merge conflict markers. + // TODO IGitAPI offers a reset(hard) method yet GitClient does not. Why? + CheckoutCommand checkoutCommand = git.checkout().ref(rev.getSha1String()); + for (GitSCMExtension ext : scm.getExtensions()) { + ext.decorateCheckoutCommand(scm, build, git, listener, checkoutCommand); + } + checkoutCommand.execute(); + throw x; + } + build.addAction(new MergeRecord(baseName, baseHash)); + return new Revision(git.revParse(Constants.HEAD)); } - return new SCMRevisionImpl(head, ref.getObject().getSha()); } @Override public SCMRevision getTrustedRevision(SCMRevision revision, TaskListener listener) throws IOException, InterruptedException { - if (revision instanceof UntrustedPullRequestSCMRevision) { - PullRequestSCMHead head = (PullRequestSCMHead) revision.getHead(); - UntrustedPullRequestSCMRevision rev = (UntrustedPullRequestSCMRevision) revision; - listener.getLogger().println("Loading trusted files from target branch at " + rev.baseHash + " rather than " + rev.getHash()); - return new SCMRevisionImpl(head, rev.baseHash); + if (revision instanceof PullRequestSCMRevision && !((PullRequestSCMHead) revision.getHead()).isTrusted()) { + PullRequestAction metadata = revision.getHead().getAction(PullRequestAction.class); + if (metadata != null) { + PullRequestSCMRevision rev = (PullRequestSCMRevision) revision; + listener.getLogger().println("Loading trusted files from target branch at " + rev.getBaseHash() + " rather than " + rev.getPullHash()); + return new SCMRevisionImpl(metadata.getTarget(), rev.getBaseHash()); + } else { + throw new IOException("Cannot find base branch metadata from " + revision.getHead()); + } } return revision; } @@ -447,16 +668,12 @@ public SCMRevision getTrustedRevision(SCMRevision revision, TaskListener listene * TODO since the GitHub API wrapper currently supports neither, we list all collaborator names and check for membership, * paying the performance penalty without the benefit of the accuracy. * @param ghPullRequest a PR - * @return the base revision, for an untrusted PR; null for a trusted PR + * @return true if this is a trusted PR * @see PR metadata * @see base revision oddity */ - private @CheckForNull String trustedReplacement(@Nonnull GHRepository repo, @Nonnull GHPullRequest ghPullRequest) throws IOException { - if (repo.getCollaboratorNames().contains(ghPullRequest.getUser().getLogin())) { - return null; - } else { - return ghPullRequest.getBase().getSha(); - } + private boolean isTrusted(@Nonnull GHRepository repo, @Nonnull GHPullRequest ghPullRequest) throws IOException { + return repo.getCollaboratorNames().contains(ghPullRequest.getUser().getLogin()); } @Extension public static class DescriptorImpl extends SCMSourceDescriptor { @@ -467,6 +684,12 @@ public SCMRevision getTrustedRevision(SCMRevision revision, TaskListener listene public static final String defaultExcludes = ""; public static final String ANONYMOUS = "ANONYMOUS"; public static final String SAME = "SAME"; + public static final boolean defaultBuildOriginBranch = true; + public static final boolean defaultBuildOriginBranchWithPR = true; + public static final boolean defaultBuildOriginPRMerged = false; + public static final boolean defaultBuildOriginPRUnmerged = false; + public static final boolean defaultBuildForkPRMerged = true; + public static final boolean defaultBuildForkPRUnmerged = false; @Initializer(before = InitMilestone.PLUGINS_STARTED) public static void addAliases() { @@ -512,6 +735,24 @@ public FormValidation doCheckScanCredentialsId(@AncestorInPath SCMSourceOwner co } } + @Restricted(NoExternalUse.class) + public FormValidation doCheckBuildOriginBranch/* arbitrary which one we pick for web method name */( + @QueryParameter boolean buildOriginBranch, + @QueryParameter boolean buildOriginBranchWithPR, + @QueryParameter boolean buildOriginPRMerged, + @QueryParameter boolean buildOriginPRUnmerged, + @QueryParameter boolean buildForkPRMerged, + @QueryParameter boolean buildForkPRUnmerged + ) { + if (!buildOriginBranch && !buildOriginBranchWithPR && !buildOriginPRMerged && !buildOriginPRUnmerged && !buildForkPRMerged && !buildForkPRUnmerged) { + return FormValidation.warning("You need to build something!"); + } + if (buildOriginBranchWithPR && buildOriginPRUnmerged) { + return FormValidation.warning("Redundant to build an origin PR both as a branch and as an unmerged PR"); + } + return FormValidation.ok(); + } + public ListBoxModel doFillApiUriItems() { ListBoxModel result = new ListBoxModel(); result.add("GitHub", ""); diff --git a/src/main/java/org/jenkinsci/plugins/github_branch_source/PullRequestSCMHead.java b/src/main/java/org/jenkinsci/plugins/github_branch_source/PullRequestSCMHead.java index 0950e1248..eee2b9af3 100644 --- a/src/main/java/org/jenkinsci/plugins/github_branch_source/PullRequestSCMHead.java +++ b/src/main/java/org/jenkinsci/plugins/github_branch_source/PullRequestSCMHead.java @@ -32,27 +32,51 @@ /** * Head corresponding to a pull request. - * Named like {@code PR-123}. + * Named like {@code PR-123} or {@code PR-123-merged} or {@code PR-123-unmerged}. */ public final class PullRequestSCMHead extends SCMHead { - private static final String PR_BRANCH_PREFIX = "PR-"; - private static final long serialVersionUID = 1; private final PullRequestAction metadata; + private Boolean merge; + private final boolean trusted; - PullRequestSCMHead(GHPullRequest pr) { - super(PR_BRANCH_PREFIX + pr.getNumber()); + PullRequestSCMHead(GHPullRequest pr, String name, boolean merge, boolean trusted) { + super(name); metadata = new PullRequestAction(pr); + this.merge = merge; + this.trusted = trusted; } public int getNumber() { if (metadata != null) { return Integer.parseInt(metadata.getId()); } else { // settings compatibility - return Integer.parseInt(getName().substring(PR_BRANCH_PREFIX.length())); + return Integer.parseInt(getName().substring("PR-".length())); + } + } + + private Object readResolve() { + if (merge == null) { + merge = true; } + return this; + } + + /** + * Whether we intend to build the merge of the PR head with the base branch. + * + */ + public boolean isMerge() { + return merge; + } + + /** + * Whether this PR was observed to have come from a trusted author. + */ + public boolean isTrusted() { + return trusted; } @Override 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 new file mode 100644 index 000000000..388b5002f --- /dev/null +++ b/src/main/java/org/jenkinsci/plugins/github_branch_source/PullRequestSCMRevision.java @@ -0,0 +1,77 @@ +/* + * The MIT License + * + * Copyright 2016 CloudBees, Inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +package org.jenkinsci.plugins.github_branch_source; + +import edu.umd.cs.findbugs.annotations.NonNull; +import jenkins.scm.api.SCMHead; +import jenkins.scm.api.SCMRevision; + +/** + * Revision of a pull request. + */ +class PullRequestSCMRevision extends SCMRevision { + + private static final long serialVersionUID = 1L; + + private final @NonNull String baseHash; + private final @NonNull String pullHash; + + PullRequestSCMRevision(@NonNull PullRequestSCMHead head, @NonNull String baseHash, @NonNull String pullHash) { + super(head); + this.baseHash = baseHash; + this.pullHash = pullHash; + } + + /** + * The commit hash of the base branch we are tracking. + * If {@link PullRequestSCMHead#isMerge}, this would be the current head of the base branch. + * Otherwise it would be the PR’s {@code .base.sha}, the common ancestor of the PR branch and the base branch. + */ + public @NonNull String getBaseHash() { + return baseHash; + } + + /** + * The commit hash of the head of the pull request branch. + */ + public @NonNull String getPullHash() { + return pullHash; + } + + @Override + public boolean equals(Object o) { + if (!(o instanceof PullRequestSCMRevision)) { + return false; + } + PullRequestSCMRevision other = (PullRequestSCMRevision) o; + return getHead().equals(other.getHead()) && baseHash.equals(other.baseHash) && pullHash.equals(other.pullHash); + } + + @Override + public int hashCode() { + return pullHash.hashCode(); + } + +} diff --git a/src/main/java/org/jenkinsci/plugins/github_branch_source/UntrustedPullRequestSCMRevision.java b/src/main/java/org/jenkinsci/plugins/github_branch_source/UntrustedPullRequestSCMRevision.java index ebb4d821c..40da45bc3 100644 --- a/src/main/java/org/jenkinsci/plugins/github_branch_source/UntrustedPullRequestSCMRevision.java +++ b/src/main/java/org/jenkinsci/plugins/github_branch_source/UntrustedPullRequestSCMRevision.java @@ -27,14 +27,14 @@ import jenkins.plugins.git.AbstractGitSCMSource; import jenkins.scm.api.SCMHead; -/** - * Revision of a pull request which should load sensitive files from the base branch. - */ +@Deprecated class UntrustedPullRequestSCMRevision extends AbstractGitSCMSource.SCMRevisionImpl { + private static final long serialVersionUID = -6961458604178249880L; + final String baseHash; - UntrustedPullRequestSCMRevision(SCMHead head, String hash, String baseHash) { + private UntrustedPullRequestSCMRevision(SCMHead head, String hash, String baseHash) { super(head, hash); this.baseHash = baseHash; } @@ -49,4 +49,8 @@ public int hashCode() { return super.hashCode(); // good enough } + private Object readResolve() { + return new PullRequestSCMRevision((PullRequestSCMHead) getHead(), baseHash, getHash()); + } + } diff --git a/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/config-detail.jelly b/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/config-detail.jelly index 7f7b0209a..1f247ad78 100644 --- a/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/config-detail.jelly +++ b/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/config-detail.jelly @@ -22,5 +22,24 @@ + + + + + + + + + + + + + + + + + + + \ No newline at end of file From ce5f94195a16474693216f443fa03d51427f13b1 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Thu, 16 Jun 2016 16:52:40 -0400 Subject: [PATCH 02/24] Oops, forgot to record entries in originBranchesWithPR. --- .../plugins/github_branch_source/GitHubSCMSource.java | 3 +++ 1 file changed, 3 insertions(+) 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 2926583c8..992db4227 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 @@ -383,6 +383,9 @@ private void doRetrieve(SCMHeadObserver observer, TaskListener listener, GHRepos listener.getLogger().format(" Submitted from origin repository, skipping%n%n"); continue; } + if (!fork) { + originBranchesWithPR.add(ghPullRequest.getHead().getRef()); + } boolean trusted = isTrusted(repo, ghPullRequest); if (!trusted) { listener.getLogger().format(" (not from a trusted source)%n"); From 63a600621e0230c5571c8ffa6fa9871baad30a89 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Thu, 16 Jun 2016 16:56:22 -0400 Subject: [PATCH 03/24] Warning about another anomalous combination. --- .../plugins/github_branch_source/GitHubSCMSource.java | 4 ++++ 1 file changed, 4 insertions(+) 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 992db4227..9458596e4 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 @@ -753,6 +753,10 @@ public FormValidation doCheckScanCredentialsId(@AncestorInPath SCMSourceOwner co if (buildOriginBranchWithPR && buildOriginPRUnmerged) { return FormValidation.warning("Redundant to build an origin PR both as a branch and as an unmerged PR"); } + if (buildOriginBranch && !buildOriginBranchWithPR && !buildOriginPRMerged && !buildOriginPRUnmerged && !buildForkPRMerged && !buildForkPRUnmerged) { + // TODO in principle we could make doRetrieve populate originBranchesWithPR without actually including any PRs, but it would be more work and probably never wanted anyway. + return FormValidation.warning("If you are not building any PRs, all origin branches will be built."); + } return FormValidation.ok(); } From ed48654a218ae6bf65076bbd600a91017bfa4d07 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Thu, 16 Jun 2016 17:13:51 -0400 Subject: [PATCH 04/24] Fixing a few warnings and a test failure. --- .../github_branch_source/GitHubSCMSource.java | 18 ++++++++++-------- .../GitHubSCMSource/config-detail.jelly | 9 +++++---- 2 files changed, 15 insertions(+), 12 deletions(-) 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 9458596e4..ef9f71567 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 @@ -31,6 +31,7 @@ import com.cloudbees.plugins.credentials.common.StandardCredentials; import com.cloudbees.plugins.credentials.common.StandardListBoxModel; import com.cloudbees.plugins.credentials.domains.DomainRequirement; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import hudson.AbortException; import hudson.Extension; import hudson.Util; @@ -141,6 +142,7 @@ public GitHubSCMSource(String id, String apiUri, String checkoutCredentialsId, S this.checkoutCredentialsId = checkoutCredentialsId; } + @SuppressFBWarnings(value="RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE", justification="Only non-null after we set them here!") private Object readResolve() { if (buildOriginBranch == null) { buildOriginBranch = DescriptorImpl.defaultBuildOriginBranch; @@ -179,6 +181,7 @@ public String getApiUri() { */ @CheckForNull @Override + @SuppressWarnings("ConvertToStringSwitch") // more cumbersome with null check public String getCredentialsId() { if (DescriptorImpl.ANONYMOUS.equals(checkoutCredentialsId)) { return null; @@ -209,7 +212,7 @@ public String getRepository() { @Override protected List getRefSpecs() { - return new ArrayList(Arrays.asList(new RefSpec("+refs/heads/*:refs/remotes/origin/*"), + return new ArrayList<>(Arrays.asList(new RefSpec("+refs/heads/*:refs/remotes/origin/*"), new RefSpec("+refs/pull/*/head:refs/remotes/origin/pr/*"))); } @@ -513,6 +516,7 @@ private void doRetrieve(SCMHeadObserver observer, TaskListener listener, GHRepos protected SCMSourceCriteria.Probe getProbe(final String branch, final String thing, final String ref, final GHRepository repo, final TaskListener listener) { return new SCMSourceCriteria.Probe() { + private static final long serialVersionUID = 5012552654534124387L; @Override public String name() { return branch; } @@ -681,8 +685,6 @@ private boolean isTrusted(@Nonnull GHRepository repo, @Nonnull GHPullRequest ghP @Extension public static class DescriptorImpl extends SCMSourceDescriptor { - private static final Logger LOGGER = Logger.getLogger(DescriptorImpl.class.getName()); - public static final String defaultIncludes = "*"; public static final String defaultExcludes = ""; public static final String ANONYMOUS = "ANONYMOUS"; @@ -729,7 +731,7 @@ public FormValidation doCheckScanCredentialsId(@AncestorInPath SCMSourceOwner co } } catch (IOException e) { // ignore, never thrown - LOGGER.log(Level.WARNING, "Exception validating credentials " + CredentialsNameProvider.name(credentials) + " on " + apiUri); + LOGGER.log(Level.WARNING, "Exception validating credentials {0} on {1}", new Object[] {CredentialsNameProvider.name(credentials), apiUri}); return FormValidation.error("Exception validating credentials"); } } @@ -803,8 +805,8 @@ public ListBoxModel doFillRepositoryItems(@AncestorInPath SCMSourceOwner context } catch (IllegalStateException e) { LOGGER.log(Level.WARNING, e.getMessage()); } catch (IOException e) { - LOGGER.log(Level.WARNING, "Exception retrieving the repositories of the owner " + repoOwner + - " on " + apiUri + " with credentials " + CredentialsNameProvider.name(credentials)); + LOGGER.log(Level.WARNING, "Exception retrieving the repositories of the owner {0} on {1} with credentials {2}", + new Object[] {repoOwner, apiUri, CredentialsNameProvider.name(credentials)}); } if (myself != null && repoOwner.equalsIgnoreCase(myself.getLogin())) { for (String name : myself.getAllRepositories().keySet()) { @@ -818,7 +820,7 @@ public ListBoxModel doFillRepositoryItems(@AncestorInPath SCMSourceOwner context try { org = github.getOrganization(repoOwner); } catch (FileNotFoundException fnf) { - LOGGER.log(Level.FINE, "There is not any GH Organization named " + repoOwner); + LOGGER.log(Level.FINE, "There is not any GH Organization named {0}", repoOwner); } catch (IOException e) { LOGGER.log(Level.WARNING, e.getMessage()); } @@ -833,7 +835,7 @@ public ListBoxModel doFillRepositoryItems(@AncestorInPath SCMSourceOwner context try { user = github.getUser(repoOwner); } catch (FileNotFoundException fnf) { - LOGGER.log(Level.FINE, "There is not any GH User named " + repoOwner); + LOGGER.log(Level.FINE, "There is not any GH User named {0}", repoOwner); } catch (IOException e) { LOGGER.log(Level.WARNING, e.getMessage()); } diff --git a/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/config-detail.jelly b/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/config-detail.jelly index 1f247ad78..2b6ecf97e 100644 --- a/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/config-detail.jelly +++ b/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/config-detail.jelly @@ -29,16 +29,17 @@ - + + - + - + - + From 6ce16b1ae1cb069ae8b1911eab2d4a20d7a333ce Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Thu, 16 Jun 2016 17:16:27 -0400 Subject: [PATCH 05/24] We are converting this anyway, can strip out more implementation. --- .../UntrustedPullRequestSCMRevision.java | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/github_branch_source/UntrustedPullRequestSCMRevision.java b/src/main/java/org/jenkinsci/plugins/github_branch_source/UntrustedPullRequestSCMRevision.java index 40da45bc3..9d6db26af 100644 --- a/src/main/java/org/jenkinsci/plugins/github_branch_source/UntrustedPullRequestSCMRevision.java +++ b/src/main/java/org/jenkinsci/plugins/github_branch_source/UntrustedPullRequestSCMRevision.java @@ -39,16 +39,6 @@ private UntrustedPullRequestSCMRevision(SCMHead head, String hash, String baseHa this.baseHash = baseHash; } - @Override - public boolean equals(Object o) { - return super.equals(o) && baseHash.equals(((UntrustedPullRequestSCMRevision) o).baseHash); - } - - @Override - public int hashCode() { - return super.hashCode(); // good enough - } - private Object readResolve() { return new PullRequestSCMRevision((PullRequestSCMHead) getHead(), baseHash, getHash()); } From 870125bccab2c846eb91416c0ad7a2c5de819a5f Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Thu, 16 Jun 2016 17:25:35 -0400 Subject: [PATCH 06/24] Comments. --- .../github_branch_source/GitHubSCMSource.java | 16 ++++++++++++---- .../github_branch_source/PullRequestSCMHead.java | 5 ++++- 2 files changed, 16 insertions(+), 5 deletions(-) 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 ef9f71567..c81a1e3e0 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 @@ -89,6 +89,7 @@ import hudson.plugins.git.GitSCM; import hudson.plugins.git.Revision; import hudson.plugins.git.extensions.GitSCMExtension; +import hudson.plugins.git.extensions.impl.PreBuildMerge; import hudson.plugins.git.util.MergeRecord; import hudson.scm.SCM; import java.util.HashSet; @@ -142,6 +143,7 @@ public GitHubSCMSource(String id, String apiUri, String checkoutCredentialsId, S this.checkoutCredentialsId = checkoutCredentialsId; } + /** Use defaults for old settings. */ @SuppressFBWarnings(value="RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE", justification="Only non-null after we set them here!") private Object readResolve() { if (buildOriginBranch == null) { @@ -213,7 +215,8 @@ public String getRepository() { @Override protected List getRefSpecs() { return new ArrayList<>(Arrays.asList(new RefSpec("+refs/heads/*:refs/remotes/origin/*"), - new RefSpec("+refs/pull/*/head:refs/remotes/origin/pr/*"))); + // For PRs we check out the head, then perhaps merge with the base branch. + new RefSpec("+refs/pull/*/head:refs/remotes/origin/pr/*"))); } /** @@ -369,6 +372,7 @@ public String getRemote() { private void doRetrieve(SCMHeadObserver observer, TaskListener listener, GHRepository repo) throws IOException, InterruptedException { SCMSourceCriteria criteria = getCriteria(); + // To implement buildOriginBranch && !buildOriginBranchWithPR we need to first find the pull requests, so we can skip corresponding origin branches later. Awkward. Set originBranchesWithPR = new HashSet<>(); if (buildOriginPRMerged || buildOriginPRUnmerged || buildForkPRMerged || buildForkPRUnmerged) { @@ -402,6 +406,7 @@ private void doRetrieve(SCMHeadObserver observer, TaskListener listener, GHRepos if (buildForkPRUnmerged) { branchName += "-merged"; // make sure they are distinct } + // If we only build merged, or only unmerged, then we use the /PR-\d+/ scheme as before. } if (merge && !fork) { if (!buildOriginPRMerged) { @@ -429,6 +434,8 @@ private void doRetrieve(SCMHeadObserver observer, TaskListener listener, GHRepos } PullRequestSCMHead head = new PullRequestSCMHead(ghPullRequest, branchName, merge, trusted); if (criteria != null) { + // Would be more precise to check whether the merge of the base branch head with the PR branch head contains a given file, etc., + // but this would be a lot more work, and is unlikely to differ from using refs/pull/123/merge: SCMSourceCriteria.Probe probe = getProbe(branchName, "pull request", "refs/pull/" + number + (merge ? "/merge" : "/head"), repo, listener); if (criteria.isHead(probe, listener)) { // FYI https://developer.github.com/v3/pulls/#response-1 @@ -606,7 +613,6 @@ protected SCMRevision doRetrieve(SCMHead head, TaskListener listener, GHReposito public SCM build(SCMHead head, SCMRevision revision) { SCM scm = super.build(head, revision); if (head instanceof PullRequestSCMHead && ((PullRequestSCMHead) head).isMerge()) { - // Similar to PreBuildMerge, but we cannot use that unmodified: we need to specify the exact base branch hash. PullRequestAction action = head.getAction(PullRequestAction.class); String baseName; if (action != null) { @@ -618,6 +624,7 @@ public SCM build(SCMHead head, SCMRevision revision) { } return scm; } + /** Similar to {@link PreBuildMerge}, but we cannot use that unmodified: we need to specify the exact base branch hash. */ private static class MergeWith extends GitSCMExtension { private final String baseName; private final String baseHash; @@ -645,8 +652,8 @@ public Revision decorateRevisionToBuild(GitSCM scm, Run build, GitClient g checkoutCommand.execute(); throw x; } - build.addAction(new MergeRecord(baseName, baseHash)); - return new Revision(git.revParse(Constants.HEAD)); + build.addAction(new MergeRecord(baseName, baseHash)); // does not seem to be used, but just in case + return new Revision(git.revParse(Constants.HEAD)); // note that this ensures Build.revision != Build.marked } } @@ -689,6 +696,7 @@ private boolean isTrusted(@Nonnull GHRepository repo, @Nonnull GHPullRequest ghP public static final String defaultExcludes = ""; public static final String ANONYMOUS = "ANONYMOUS"; public static final String SAME = "SAME"; + // Prior to JENKINS-33161 the unconditional behavior was to build fork PRs plus origin branches, and try to build a merge revision for PRs. public static final boolean defaultBuildOriginBranch = true; public static final boolean defaultBuildOriginBranchWithPR = true; public static final boolean defaultBuildOriginPRMerged = false; diff --git a/src/main/java/org/jenkinsci/plugins/github_branch_source/PullRequestSCMHead.java b/src/main/java/org/jenkinsci/plugins/github_branch_source/PullRequestSCMHead.java index eee2b9af3..ba1c860b9 100644 --- a/src/main/java/org/jenkinsci/plugins/github_branch_source/PullRequestSCMHead.java +++ b/src/main/java/org/jenkinsci/plugins/github_branch_source/PullRequestSCMHead.java @@ -53,14 +53,17 @@ public int getNumber() { if (metadata != null) { return Integer.parseInt(metadata.getId()); } else { // settings compatibility + // if predating PullRequestAction, then also predate -merged/-unmerged suffices return Integer.parseInt(getName().substring("PR-".length())); } } - + + /** Default for old settings. */ private Object readResolve() { if (merge == null) { merge = true; } + // leave trusted at false to be on the safe side return this; } From 32a5de3335e8efd67fe7a5520a325aa7d1f03b08 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Thu, 16 Jun 2016 17:38:37 -0400 Subject: [PATCH 07/24] We need to be sure to feed SpecificRevisionBuildChooser the PR head hash. --- .../github_branch_source/GitHubSCMSource.java | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) 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 c81a1e3e0..889f5225b 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 @@ -611,7 +611,6 @@ protected SCMRevision doRetrieve(SCMHead head, TaskListener listener, GHReposito @Override public SCM build(SCMHead head, SCMRevision revision) { - SCM scm = super.build(head, revision); if (head instanceof PullRequestSCMHead && ((PullRequestSCMHead) head).isMerge()) { PullRequestAction action = head.getAction(PullRequestAction.class); String baseName; @@ -620,11 +619,21 @@ public SCM build(SCMHead head, SCMRevision revision) { } else { baseName = "master?"; // OK if not found, just informational anyway } - ((GitSCM) scm).getExtensions().add(new MergeWith(baseName, ((PullRequestSCMRevision) revision).getBaseHash())); + PullRequestSCMRevision prRev = (PullRequestSCMRevision) revision; + GitSCM scm = (GitSCM) super.build(/* does this matter? */head, new SCMRevisionImpl(/* again probably irrelevant */head, prRev.getPullHash())); + scm.getExtensions().add(new MergeWith(baseName, prRev.getBaseHash())); + return scm; + } else { + return super.build(head, /* casting just as an assertion */(SCMRevisionImpl) revision); } - return scm; } - /** Similar to {@link PreBuildMerge}, but we cannot use that unmodified: we need to specify the exact base branch hash. */ + /** + * Similar to {@link PreBuildMerge}, but we cannot use that unmodified: we need to specify the exact base branch hash. + * It is possible to just ask Git to check out {@code refs/pull/123/merge}, but this has two problems: + * GitHub’s implementation is not all that reliable (for example JENKINS-33237, and time-delayed snapshots); + * and it is subject to a race condition between the {@code baseHash} we think we are merging with and a possibly newer one that was just pushed. + * Performing the merge ourselves is simple enough and ensures that we are building exactly what the {@link PullRequestSCMRevision} represented. + */ private static class MergeWith extends GitSCMExtension { private final String baseName; private final String baseHash; From 7dc37d003ad1840466cd8f8fa7b67b3d68c7296d Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Thu, 16 Jun 2016 17:43:39 -0400 Subject: [PATCH 08/24] Revert "We are converting this anyway, can strip out more implementation." This reverts commit 6ce16b1ae1cb069ae8b1911eab2d4a20d7a333ce. FindBugs is not happy with my simplification! --- .../UntrustedPullRequestSCMRevision.java | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/main/java/org/jenkinsci/plugins/github_branch_source/UntrustedPullRequestSCMRevision.java b/src/main/java/org/jenkinsci/plugins/github_branch_source/UntrustedPullRequestSCMRevision.java index 9d6db26af..40da45bc3 100644 --- a/src/main/java/org/jenkinsci/plugins/github_branch_source/UntrustedPullRequestSCMRevision.java +++ b/src/main/java/org/jenkinsci/plugins/github_branch_source/UntrustedPullRequestSCMRevision.java @@ -39,6 +39,16 @@ private UntrustedPullRequestSCMRevision(SCMHead head, String hash, String baseHa this.baseHash = baseHash; } + @Override + public boolean equals(Object o) { + return super.equals(o) && baseHash.equals(((UntrustedPullRequestSCMRevision) o).baseHash); + } + + @Override + public int hashCode() { + return super.hashCode(); // good enough + } + private Object readResolve() { return new PullRequestSCMRevision((PullRequestSCMHead) getHead(), baseHash, getHash()); } From 56f47c543618f8d99fc86d5340f73dd13712c8de Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Thu, 16 Jun 2016 18:04:19 -0400 Subject: [PATCH 09/24] typo --- .../github_branch_source/GitHubSCMSource/config-detail.jelly | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/config-detail.jelly b/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/config-detail.jelly index 2b6ecf97e..2d5670439 100644 --- a/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/config-detail.jelly +++ b/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/config-detail.jelly @@ -29,7 +29,7 @@ - + From 413f0b00fc846745d04c7f406c40ddb30c09ed37 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 20 Jun 2016 15:54:52 -0400 Subject: [PATCH 10/24] Noting when job names will distinguish kinds of PRs. --- .../plugins/github_branch_source/GitHubSCMSource.java | 3 +++ 1 file changed, 3 insertions(+) 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 889f5225b..e6550fed2 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 @@ -776,6 +776,9 @@ public FormValidation doCheckScanCredentialsId(@AncestorInPath SCMSourceOwner co // TODO in principle we could make doRetrieve populate originBranchesWithPR without actually including any PRs, but it would be more work and probably never wanted anyway. return FormValidation.warning("If you are not building any PRs, all origin branches will be built."); } + if (buildOriginPRMerged && buildOriginPRUnmerged || buildForkPRMerged && buildForkPRUnmerged) { + return FormValidation.ok("Merged vs. unmerged PRs will be distinguished in the job name."); + } return FormValidation.ok(); } From 5a98d2a885aa951087ba329acdc560bc1e6b7062 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 20 Jun 2016 16:59:28 -0400 Subject: [PATCH 11/24] Specify a notification per job, so that we can distinctly see merged and unmerged job results. --- .../GitHubBuildStatusNotification.java | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubBuildStatusNotification.java b/src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubBuildStatusNotification.java index d8b5c1036..d80a54368 100644 --- a/src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubBuildStatusNotification.java +++ b/src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubBuildStatusNotification.java @@ -70,9 +70,9 @@ public class GitHubBuildStatusNotification { private static final Logger LOGGER = Logger.getLogger(GitHubBuildStatusNotification.class.getName()); - private static void createCommitStatus(@Nonnull GHRepository repo, @Nonnull String revision, @Nonnull GHCommitState state, @Nonnull String url, @Nonnull String message) throws IOException { + private static void createCommitStatus(@Nonnull GHRepository repo, @Nonnull String revision, @Nonnull GHCommitState state, @Nonnull String url, @Nonnull String message, @Nonnull Job job) throws IOException { LOGGER.log(Level.FINE, "{0}/commit/{1} {2} from {3}", new Object[] {repo.getHtmlUrl(), revision, state, url}); - repo.createCommitStatus(revision, state, url, message, "Jenkins"); + repo.createCommitStatus(revision, state, url, message, "Jenkins job " + job.getName()); } @SuppressWarnings("deprecation") // Run.getAbsoluteUrl appropriate here @@ -93,17 +93,18 @@ private static void createBuildCommitStatus(Run build, TaskListener listene try { Result result = build.getResult(); String revisionToNotify = resolveHeadCommit(repo, revision); + Job job = build.getParent(); if (Result.SUCCESS.equals(result)) { - createCommitStatus(repo, revisionToNotify, GHCommitState.SUCCESS, url, Messages.GitHubBuildStatusNotification_CommitStatus_Good()); + createCommitStatus(repo, revisionToNotify, GHCommitState.SUCCESS, url, Messages.GitHubBuildStatusNotification_CommitStatus_Good(), job); } else if (Result.UNSTABLE.equals(result)) { - createCommitStatus(repo, revisionToNotify, GHCommitState.FAILURE, url, Messages.GitHubBuildStatusNotification_CommitStatus_Unstable()); + createCommitStatus(repo, revisionToNotify, GHCommitState.FAILURE, url, Messages.GitHubBuildStatusNotification_CommitStatus_Unstable(), job); } else if (Result.FAILURE.equals(result)) { - createCommitStatus(repo, revisionToNotify, GHCommitState.FAILURE, url, Messages.GitHubBuildStatusNotification_CommitStatus_Failure()); + createCommitStatus(repo, revisionToNotify, GHCommitState.FAILURE, url, Messages.GitHubBuildStatusNotification_CommitStatus_Failure(), job); } else if (result != null) { // ABORTED etc. - createCommitStatus(repo, revisionToNotify, GHCommitState.ERROR, url, Messages.GitHubBuildStatusNotification_CommitStatus_Other()); + createCommitStatus(repo, revisionToNotify, GHCommitState.ERROR, url, Messages.GitHubBuildStatusNotification_CommitStatus_Other(), job); } else { ignoreError = true; - createCommitStatus(repo, revisionToNotify, GHCommitState.PENDING, url, Messages.GitHubBuildStatusNotification_CommitStatus_Pending()); + createCommitStatus(repo, revisionToNotify, GHCommitState.PENDING, url, Messages.GitHubBuildStatusNotification_CommitStatus_Pending(), job); } if (result != null) { listener.getLogger().format("%n" + Messages.GitHubBuildStatusNotification_CommitStatusSet() + "%n%n"); @@ -195,7 +196,7 @@ private static GitHubSCMSource getSCMSource(final SCMSourceOwner scmSourceOwner) } // Has not been built yet, so we can only guess that the current PR head is what will be built. // In fact the submitter might push another commit before this build even starts. - createCommitStatus(repo, pr.getHead().getSha(), GHCommitState.PENDING, url, "This pull request is scheduled to be built"); + createCommitStatus(repo, pr.getHead().getSha(), GHCommitState.PENDING, url, "This pull request is scheduled to be built", job); } } catch (FileNotFoundException fnfe) { LOGGER.log(Level.WARNING, "Could not update commit status to PENDING. Valid scan credentials? Valid scopes?"); From cdfe0b30987d3860c540c8542cbecb611e838565 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 20 Jun 2016 16:59:41 -0400 Subject: [PATCH 12/24] Override toString for debugging. --- .../plugins/github_branch_source/PullRequestSCMRevision.java | 5 +++++ 1 file changed, 5 insertions(+) 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 388b5002f..5c88872d2 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 @@ -74,4 +74,9 @@ public int hashCode() { return pullHash.hashCode(); } + @Override + public String toString() { + return getHead() instanceof PullRequestSCMHead && ((PullRequestSCMHead) getHead()).isMerge() ? pullHash + "+" + baseHash : pullHash; + } + } From 7353a0296ef044eb57ad3c9f831c85350589a097 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Mon, 20 Jun 2016 17:00:17 -0400 Subject: [PATCH 13/24] Building merged PRs seems to work now. --- .../github_branch_source/GitHubSCMSource.java | 45 +++++++++++++------ 1 file changed, 31 insertions(+), 14 deletions(-) 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 e6550fed2..f9ffea79d 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 @@ -611,17 +611,27 @@ protected SCMRevision doRetrieve(SCMHead head, TaskListener listener, GHReposito @Override public SCM build(SCMHead head, SCMRevision revision) { - if (head instanceof PullRequestSCMHead && ((PullRequestSCMHead) head).isMerge()) { - PullRequestAction action = head.getAction(PullRequestAction.class); - String baseName; - if (action != null) { - baseName = action.getTarget().getName(); - } else { - baseName = "master?"; // OK if not found, just informational anyway + if (revision == null) { + // TODO will this work sanely for PRs? Branch.scm seems to be used only as a fallback for SCMBinder/SCMVar where they would perhaps better just report an error. + return super.build(head, null); + } else if (head instanceof PullRequestSCMHead) { + if (!(revision instanceof PullRequestSCMRevision)) { + // TODO this seems to happen for PR-6-unmerged in cloudbeers/PR-demo; why? + LOGGER.log(Level.WARNING, "Unexpected revision class {0} for {1}", new Object[] {revision.getClass().getName(), head}); + return super.build(head, revision); } PullRequestSCMRevision prRev = (PullRequestSCMRevision) revision; GitSCM scm = (GitSCM) super.build(/* does this matter? */head, new SCMRevisionImpl(/* again probably irrelevant */head, prRev.getPullHash())); - scm.getExtensions().add(new MergeWith(baseName, prRev.getBaseHash())); + if (((PullRequestSCMHead) head).isMerge()) { + PullRequestAction action = head.getAction(PullRequestAction.class); + String baseName; + if (action != null) { + baseName = action.getTarget().getName(); + } else { + baseName = "master?"; // OK if not found, just informational anyway + } + scm.getExtensions().add(new MergeWith(baseName, prRev.getBaseHash())); + } return scm; } else { return super.build(head, /* casting just as an assertion */(SCMRevisionImpl) revision); @@ -644,6 +654,7 @@ private static class MergeWith extends GitSCMExtension { @Override public Revision decorateRevisionToBuild(GitSCM scm, Run build, GitClient git, TaskListener listener, Revision marked, Revision rev) throws IOException, InterruptedException, GitException { listener.getLogger().println("Merging " + baseName + " commit " + baseHash + " into PR head commit " + rev.getSha1String()); + checkout(scm, build, git, listener, rev); try { MergeCommand cmd = git.merge().setRevisionToMerge(ObjectId.fromString(baseHash)); for (GitSCMExtension ext : scm.getExtensions()) { @@ -654,15 +665,21 @@ public Revision decorateRevisionToBuild(GitSCM scm, Run build, GitClient g } catch (GitException x) { // Try to revert merge conflict markers. // TODO IGitAPI offers a reset(hard) method yet GitClient does not. Why? - CheckoutCommand checkoutCommand = git.checkout().ref(rev.getSha1String()); - for (GitSCMExtension ext : scm.getExtensions()) { - ext.decorateCheckoutCommand(scm, build, git, listener, checkoutCommand); - } - checkoutCommand.execute(); + checkout(scm, build, git, listener, rev); + // TODO would be nicer to throw an AbortException with just the message, but this is actually worse pending https://github.com/jenkinsci/git-client-plugin/pull/208 throw x; } build.addAction(new MergeRecord(baseName, baseHash)); // does not seem to be used, but just in case - return new Revision(git.revParse(Constants.HEAD)); // note that this ensures Build.revision != Build.marked + ObjectId mergeRev = git.revParse(Constants.HEAD); + listener.getLogger().println("Merge succeeded, producing " + mergeRev.name()); + return new Revision(mergeRev); // note that this ensures Build.revision != Build.marked + } + private void checkout(GitSCM scm, Run build, GitClient git, TaskListener listener, Revision rev) throws InterruptedException, IOException, GitException { + CheckoutCommand checkoutCommand = git.checkout().ref(rev.getSha1String()); + for (GitSCMExtension ext : scm.getExtensions()) { + ext.decorateCheckoutCommand(scm, build, git, listener, checkoutCommand); + } + checkoutCommand.execute(); } } From bff827fa25d906178352407eb1ec2f637238222f Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 21 Jun 2016 13:42:14 -0400 Subject: [PATCH 14/24] =?UTF-8?q?@KostyaSha=E2=80=99s=20suggestion=20of=20?= =?UTF-8?q?*-head=20vs.=20*-merge=20job=20naming=20seems=20as=20good=20as?= =?UTF-8?q?=20any.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../github_branch_source/GitHubSCMSource.java | 114 +++++++++--------- .../PullRequestGHEventSubscriber.java | 80 ++++++++++++ .../GitHubSCMSource/config-detail.jelly | 16 +-- 3 files changed, 145 insertions(+), 65 deletions(-) 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 f9ffea79d..d4bd65937 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 @@ -125,13 +125,13 @@ public class GitHubSCMSource extends AbstractGitSCMSource { /** Whether to build origin branches which happen to also have a PR filed from them (but here we are naming and building as a branch). */ private @Nonnull Boolean buildOriginBranchWithPR = DescriptorImpl.defaultBuildOriginBranchWithPR; /** Whether to build PRs filed from the origin, where the build is of the merge with the base branch. */ - private @Nonnull Boolean buildOriginPRMerged = DescriptorImpl.defaultBuildOriginPRMerged; + private @Nonnull Boolean buildOriginPRMerge = DescriptorImpl.defaultBuildOriginPRMerge; /** Whether to build PRs filed from the origin, where the build is of the branch head. */ - private @Nonnull Boolean buildOriginPRUnmerged = DescriptorImpl.defaultBuildOriginPRUnmerged; + private @Nonnull Boolean buildOriginPRHead = DescriptorImpl.defaultBuildOriginPRHead; /** Whether to build PRs filed from a fork, where the build is of the merge with the base branch. */ - private @Nonnull Boolean buildForkPRMerged = DescriptorImpl.defaultBuildForkPRMerged; + private @Nonnull Boolean buildForkPRMerge = DescriptorImpl.defaultBuildForkPRMerge; /** Whether to build PRs filed from a fork, where the build is of the branch head. */ - private @Nonnull Boolean buildForkPRUnmerged = DescriptorImpl.defaultBuildForkPRUnmerged; + private @Nonnull Boolean buildForkPRHead = DescriptorImpl.defaultBuildForkPRHead; @DataBoundConstructor public GitHubSCMSource(String id, String apiUri, String checkoutCredentialsId, String scanCredentialsId, String repoOwner, String repository) { @@ -152,17 +152,17 @@ private Object readResolve() { if (buildOriginBranchWithPR == null) { buildOriginBranchWithPR = DescriptorImpl.defaultBuildOriginBranchWithPR; } - if (buildOriginPRMerged == null) { - buildOriginPRMerged = DescriptorImpl.defaultBuildOriginPRMerged; + if (buildOriginPRMerge == null) { + buildOriginPRMerge = DescriptorImpl.defaultBuildOriginPRMerge; } - if (buildOriginPRUnmerged == null) { - buildOriginPRUnmerged = DescriptorImpl.defaultBuildOriginPRUnmerged; + if (buildOriginPRHead == null) { + buildOriginPRHead = DescriptorImpl.defaultBuildOriginPRHead; } - if (buildForkPRMerged == null) { - buildForkPRMerged = DescriptorImpl.defaultBuildForkPRMerged; + if (buildForkPRMerge == null) { + buildForkPRMerge = DescriptorImpl.defaultBuildForkPRMerge; } - if (buildForkPRUnmerged == null) { - buildForkPRUnmerged = DescriptorImpl.defaultBuildForkPRUnmerged; + if (buildForkPRHead == null) { + buildForkPRHead = DescriptorImpl.defaultBuildForkPRHead; } return this; } @@ -289,40 +289,40 @@ public void setBuildOriginBranchWithPR(Boolean buildOriginBranchWithPR) { this.buildOriginBranchWithPR = buildOriginBranchWithPR; } - public Boolean getBuildOriginPRMerged() { - return buildOriginPRMerged; + public Boolean getBuildOriginPRMerge() { + return buildOriginPRMerge; } @DataBoundSetter - public void setBuildOriginPRMerged(Boolean buildOriginPRMerged) { - this.buildOriginPRMerged = buildOriginPRMerged; + public void setBuildOriginPRMerge(Boolean buildOriginPRMerge) { + this.buildOriginPRMerge = buildOriginPRMerge; } - public Boolean getBuildOriginPRUnmerged() { - return buildOriginPRUnmerged; + public Boolean getBuildOriginPRHead() { + return buildOriginPRHead; } @DataBoundSetter - public void setBuildOriginPRUnmerged(Boolean buildOriginPRUnmerged) { - this.buildOriginPRUnmerged = buildOriginPRUnmerged; + public void setBuildOriginPRHead(Boolean buildOriginPRHead) { + this.buildOriginPRHead = buildOriginPRHead; } - public Boolean getBuildForkPRMerged() { - return buildForkPRMerged; + public Boolean getBuildForkPRMerge() { + return buildForkPRMerge; } @DataBoundSetter - public void setBuildForkPRMerged(Boolean buildForkPRMerged) { - this.buildForkPRMerged = buildForkPRMerged; + public void setBuildForkPRMerge(Boolean buildForkPRMerge) { + this.buildForkPRMerge = buildForkPRMerge; } - public Boolean getBuildForkPRUnmerged() { - return buildForkPRUnmerged; + public Boolean getBuildForkPRHead() { + return buildForkPRHead; } @DataBoundSetter - public void setBuildForkPRUnmerged(Boolean buildForkPRUnmerged) { - this.buildForkPRUnmerged = buildForkPRUnmerged; + public void setBuildForkPRHead(Boolean buildForkPRHead) { + this.buildForkPRHead = buildForkPRHead; } @Override @@ -375,18 +375,18 @@ private void doRetrieve(SCMHeadObserver observer, TaskListener listener, GHRepos // To implement buildOriginBranch && !buildOriginBranchWithPR we need to first find the pull requests, so we can skip corresponding origin branches later. Awkward. Set originBranchesWithPR = new HashSet<>(); - if (buildOriginPRMerged || buildOriginPRUnmerged || buildForkPRMerged || buildForkPRUnmerged) { + if (buildOriginPRMerge || buildOriginPRHead || buildForkPRMerge || buildForkPRHead) { listener.getLogger().format("%n Getting remote pull requests...%n"); int pullrequests = 0; for (GHPullRequest ghPullRequest : repo.getPullRequests(GHIssueState.OPEN)) { int number = ghPullRequest.getNumber(); listener.getLogger().format("%n Checking pull request %s%n", HyperlinkNote.encodeTo(ghPullRequest.getHtmlUrl().toString(), "#" + number)); boolean fork = !repo.getOwner().equals(ghPullRequest.getHead().getUser()); - if (fork && !buildForkPRMerged && !buildForkPRUnmerged) { + if (fork && !buildForkPRMerge && !buildForkPRHead) { listener.getLogger().format(" Submitted from fork, skipping%n%n"); continue; } - if (!fork && !buildOriginPRMerged && !buildOriginPRUnmerged) { + if (!fork && !buildOriginPRMerge && !buildOriginPRHead) { listener.getLogger().format(" Submitted from origin repository, skipping%n%n"); continue; } @@ -400,36 +400,36 @@ private void doRetrieve(SCMHeadObserver observer, TaskListener listener, GHRepos for (boolean merge : new boolean[] {false, true}) { String branchName = "PR-" + number; if (merge && fork) { - if (!buildForkPRMerged) { + if (!buildForkPRMerge) { continue; // not doing this combination } - if (buildForkPRUnmerged) { - branchName += "-merged"; // make sure they are distinct + if (buildForkPRHead) { + branchName += "-merge"; // make sure they are distinct } // If we only build merged, or only unmerged, then we use the /PR-\d+/ scheme as before. } if (merge && !fork) { - if (!buildOriginPRMerged) { + if (!buildOriginPRMerge) { continue; } - if (buildForkPRUnmerged) { - branchName += "-merged"; + if (buildForkPRHead) { + branchName += "-merge"; } } if (!merge && fork) { - if (!buildForkPRUnmerged) { + if (!buildForkPRHead) { continue; } - if (buildForkPRMerged) { - branchName += "-unmerged"; + if (buildForkPRMerge) { + branchName += "-head"; } } if (!merge && !fork) { - if (!buildOriginPRUnmerged) { + if (!buildOriginPRHead) { continue; } - if (buildOriginPRMerged) { - branchName += "-unmerged"; + if (buildOriginPRMerge) { + branchName += "-head"; } } PullRequestSCMHead head = new PullRequestSCMHead(ghPullRequest, branchName, merge, trusted); @@ -689,7 +689,7 @@ public SCMRevision getTrustedRevision(SCMRevision revision, TaskListener listene PullRequestAction metadata = revision.getHead().getAction(PullRequestAction.class); if (metadata != null) { PullRequestSCMRevision rev = (PullRequestSCMRevision) revision; - listener.getLogger().println("Loading trusted files from target branch at " + rev.getBaseHash() + " rather than " + rev.getPullHash()); + listener.getLogger().println("Loading trusted files from base branch " + metadata.getTarget().getName() + " at " + rev.getBaseHash() + " rather than " + rev.getPullHash()); return new SCMRevisionImpl(metadata.getTarget(), rev.getBaseHash()); } else { throw new IOException("Cannot find base branch metadata from " + revision.getHead()); @@ -725,10 +725,10 @@ private boolean isTrusted(@Nonnull GHRepository repo, @Nonnull GHPullRequest ghP // Prior to JENKINS-33161 the unconditional behavior was to build fork PRs plus origin branches, and try to build a merge revision for PRs. public static final boolean defaultBuildOriginBranch = true; public static final boolean defaultBuildOriginBranchWithPR = true; - public static final boolean defaultBuildOriginPRMerged = false; - public static final boolean defaultBuildOriginPRUnmerged = false; - public static final boolean defaultBuildForkPRMerged = true; - public static final boolean defaultBuildForkPRUnmerged = false; + public static final boolean defaultBuildOriginPRMerge = false; + public static final boolean defaultBuildOriginPRHead = false; + public static final boolean defaultBuildForkPRMerge = true; + public static final boolean defaultBuildForkPRHead = false; @Initializer(before = InitMilestone.PLUGINS_STARTED) public static void addAliases() { @@ -778,23 +778,23 @@ public FormValidation doCheckScanCredentialsId(@AncestorInPath SCMSourceOwner co public FormValidation doCheckBuildOriginBranch/* arbitrary which one we pick for web method name */( @QueryParameter boolean buildOriginBranch, @QueryParameter boolean buildOriginBranchWithPR, - @QueryParameter boolean buildOriginPRMerged, - @QueryParameter boolean buildOriginPRUnmerged, - @QueryParameter boolean buildForkPRMerged, - @QueryParameter boolean buildForkPRUnmerged + @QueryParameter boolean buildOriginPRMerge, + @QueryParameter boolean buildOriginPRHead, + @QueryParameter boolean buildForkPRMerge, + @QueryParameter boolean buildForkPRHead ) { - if (!buildOriginBranch && !buildOriginBranchWithPR && !buildOriginPRMerged && !buildOriginPRUnmerged && !buildForkPRMerged && !buildForkPRUnmerged) { + if (!buildOriginBranch && !buildOriginBranchWithPR && !buildOriginPRMerge && !buildOriginPRHead && !buildForkPRMerge && !buildForkPRHead) { return FormValidation.warning("You need to build something!"); } - if (buildOriginBranchWithPR && buildOriginPRUnmerged) { + if (buildOriginBranchWithPR && buildOriginPRHead) { return FormValidation.warning("Redundant to build an origin PR both as a branch and as an unmerged PR"); } - if (buildOriginBranch && !buildOriginBranchWithPR && !buildOriginPRMerged && !buildOriginPRUnmerged && !buildForkPRMerged && !buildForkPRUnmerged) { + if (buildOriginBranch && !buildOriginBranchWithPR && !buildOriginPRMerge && !buildOriginPRHead && !buildForkPRMerge && !buildForkPRHead) { // TODO in principle we could make doRetrieve populate originBranchesWithPR without actually including any PRs, but it would be more work and probably never wanted anyway. return FormValidation.warning("If you are not building any PRs, all origin branches will be built."); } - if (buildOriginPRMerged && buildOriginPRUnmerged || buildForkPRMerged && buildForkPRUnmerged) { - return FormValidation.ok("Merged vs. unmerged PRs will be distinguished in the job name."); + if (buildOriginPRMerge && buildOriginPRHead || buildForkPRMerge && buildForkPRHead) { + return FormValidation.ok("Merged vs. unmerged PRs will be distinguished in the job name (*-merge vs. *-head)."); } return FormValidation.ok(); } diff --git a/src/main/java/org/jenkinsci/plugins/github_branch_source/PullRequestGHEventSubscriber.java b/src/main/java/org/jenkinsci/plugins/github_branch_source/PullRequestGHEventSubscriber.java index 5db9df467..c646a2a5f 100644 --- a/src/main/java/org/jenkinsci/plugins/github_branch_source/PullRequestGHEventSubscriber.java +++ b/src/main/java/org/jenkinsci/plugins/github_branch_source/PullRequestGHEventSubscriber.java @@ -46,6 +46,86 @@ import static com.google.common.collect.Sets.immutableEnumSet; import static org.kohsuke.github.GHEvent.PULL_REQUEST; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; +import static com.google.common.collect.Sets.immutableEnumSet; /** * This subscriber manages {@link org.kohsuke.github.GHEvent} PULL_REQUEST. diff --git a/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/config-detail.jelly b/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/config-detail.jelly index 2d5670439..d6e0cd1b9 100644 --- a/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/config-detail.jelly +++ b/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/config-detail.jelly @@ -30,17 +30,17 @@ - - + + - - + + - - + + - - + + \ No newline at end of file From d067c0c9c3990687c9f0a53cbe8c669a588377ec Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 21 Jun 2016 13:43:22 -0400 Subject: [PATCH 15/24] The getters and setters were intended to be of type boolean, not Boolean; null is only in the field type for the benefit of readResolve. --- .../github_branch_source/GitHubSCMSource.java | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) 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 d4bd65937..af958848c 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 @@ -271,57 +271,57 @@ Collections. emptyList()), CredentialsMatchers.allOf( this.excludes = excludes; } - public Boolean getBuildOriginBranch() { + public boolean getBuildOriginBranch() { return buildOriginBranch; } @DataBoundSetter - public void setBuildOriginBranch(Boolean buildOriginBranch) { + public void setBuildOriginBranch(boolean buildOriginBranch) { this.buildOriginBranch = buildOriginBranch; } - public Boolean getBuildOriginBranchWithPR() { + public boolean getBuildOriginBranchWithPR() { return buildOriginBranchWithPR; } @DataBoundSetter - public void setBuildOriginBranchWithPR(Boolean buildOriginBranchWithPR) { + public void setBuildOriginBranchWithPR(boolean buildOriginBranchWithPR) { this.buildOriginBranchWithPR = buildOriginBranchWithPR; } - public Boolean getBuildOriginPRMerge() { + public boolean getBuildOriginPRMerge() { return buildOriginPRMerge; } @DataBoundSetter - public void setBuildOriginPRMerge(Boolean buildOriginPRMerge) { + public void setBuildOriginPRMerge(boolean buildOriginPRMerge) { this.buildOriginPRMerge = buildOriginPRMerge; } - public Boolean getBuildOriginPRHead() { + public boolean getBuildOriginPRHead() { return buildOriginPRHead; } @DataBoundSetter - public void setBuildOriginPRHead(Boolean buildOriginPRHead) { + public void setBuildOriginPRHead(boolean buildOriginPRHead) { this.buildOriginPRHead = buildOriginPRHead; } - public Boolean getBuildForkPRMerge() { + public boolean getBuildForkPRMerge() { return buildForkPRMerge; } @DataBoundSetter - public void setBuildForkPRMerge(Boolean buildForkPRMerge) { + public void setBuildForkPRMerge(boolean buildForkPRMerge) { this.buildForkPRMerge = buildForkPRMerge; } - public Boolean getBuildForkPRHead() { + public boolean getBuildForkPRHead() { return buildForkPRHead; } @DataBoundSetter - public void setBuildForkPRHead(Boolean buildForkPRHead) { + public void setBuildForkPRHead(boolean buildForkPRHead) { this.buildForkPRHead = buildForkPRHead; } From e003af92eac4381df1d755377c4bb1c101aab972 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 21 Jun 2016 13:45:13 -0400 Subject: [PATCH 16/24] Did my IDE just go nuts? --- .../PullRequestGHEventSubscriber.java | 80 ------------------- 1 file changed, 80 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/github_branch_source/PullRequestGHEventSubscriber.java b/src/main/java/org/jenkinsci/plugins/github_branch_source/PullRequestGHEventSubscriber.java index c646a2a5f..5db9df467 100644 --- a/src/main/java/org/jenkinsci/plugins/github_branch_source/PullRequestGHEventSubscriber.java +++ b/src/main/java/org/jenkinsci/plugins/github_branch_source/PullRequestGHEventSubscriber.java @@ -46,86 +46,6 @@ import static com.google.common.collect.Sets.immutableEnumSet; import static org.kohsuke.github.GHEvent.PULL_REQUEST; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; -import static com.google.common.collect.Sets.immutableEnumSet; /** * This subscriber manages {@link org.kohsuke.github.GHEvent} PULL_REQUEST. From c1feec53be51dc27173e6fe9f31ab0387e0334be Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 21 Jun 2016 13:46:48 -0400 Subject: [PATCH 17/24] Indentation fix. --- .../jenkinsci/plugins/github_branch_source/GitHubSCMSource.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 af958848c..8fb4b685a 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 @@ -597,7 +597,7 @@ protected SCMRevision doRetrieve(SCMHead head, TaskListener listener, GHReposito if (prhead.isMerge()) { PullRequestAction metadata = prhead.getAction(PullRequestAction.class); if (metadata == null) { - throw new IOException("Cannot find base branch metadata from " + prhead); + throw new IOException("Cannot find base branch metadata from " + prhead); } baseHash = repo.getRef("heads/" + metadata.getTarget().getName()).getObject().getSha(); } else { From e8898025a00e02ba8b76f3745f0b623377806735 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 21 Jun 2016 13:52:46 -0400 Subject: [PATCH 18/24] Typo. --- .../github_branch_source/GitHubSCMSource/config-detail.jelly | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/config-detail.jelly b/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/config-detail.jelly index d6e0cd1b9..397d77e7b 100644 --- a/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/config-detail.jelly +++ b/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/config-detail.jelly @@ -34,7 +34,7 @@ - + @@ -43,4 +43,4 @@ - \ No newline at end of file + From f7e51579b424bca54deb1d53a95ec7d817a6ccc0 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 21 Jun 2016 14:30:12 -0400 Subject: [PATCH 19/24] =?UTF-8?q?Improved=20form=20validation=20at=20@amun?= =?UTF-8?q?iz=E2=80=99s=20suggestion.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../github_branch_source/GitHubSCMSource.java | 36 ++++++++++++++----- 1 file changed, 28 insertions(+), 8 deletions(-) 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 8fb4b685a..cdffb09d7 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 @@ -775,7 +775,7 @@ public FormValidation doCheckScanCredentialsId(@AncestorInPath SCMSourceOwner co } @Restricted(NoExternalUse.class) - public FormValidation doCheckBuildOriginBranch/* arbitrary which one we pick for web method name */( + public FormValidation doCheckBuildOriginBranchWithPR( @QueryParameter boolean buildOriginBranch, @QueryParameter boolean buildOriginBranchWithPR, @QueryParameter boolean buildOriginPRMerge, @@ -783,17 +783,37 @@ public FormValidation doCheckScanCredentialsId(@AncestorInPath SCMSourceOwner co @QueryParameter boolean buildForkPRMerge, @QueryParameter boolean buildForkPRHead ) { - if (!buildOriginBranch && !buildOriginBranchWithPR && !buildOriginPRMerge && !buildOriginPRHead && !buildForkPRMerge && !buildForkPRHead) { - return FormValidation.warning("You need to build something!"); - } - if (buildOriginBranchWithPR && buildOriginPRHead) { - return FormValidation.warning("Redundant to build an origin PR both as a branch and as an unmerged PR"); - } if (buildOriginBranch && !buildOriginBranchWithPR && !buildOriginPRMerge && !buildOriginPRHead && !buildForkPRMerge && !buildForkPRHead) { // TODO in principle we could make doRetrieve populate originBranchesWithPR without actually including any PRs, but it would be more work and probably never wanted anyway. return FormValidation.warning("If you are not building any PRs, all origin branches will be built."); } - if (buildOriginPRMerge && buildOriginPRHead || buildForkPRMerge && buildForkPRHead) { + return FormValidation.ok(); + } + + @Restricted(NoExternalUse.class) + public FormValidation doCheckBuildOriginPRHead(@QueryParameter boolean buildOriginBranchWithPR, @QueryParameter boolean buildOriginPRMerge, @QueryParameter boolean buildOriginPRHead) { + if (buildOriginBranchWithPR && buildOriginPRHead) { + return FormValidation.warning("Redundant to build an origin PR both as a branch and as an unmerged PR."); + } + if (buildOriginPRMerge && buildOriginPRHead) { + return FormValidation.ok("Merged vs. unmerged PRs will be distinguished in the job name (*-merge vs. *-head)."); + } + return FormValidation.ok(); + } + + @Restricted(NoExternalUse.class) + public FormValidation doCheckBuildForkPRHead/* web method name controls UI position of message; we want this at the bottom */( + @QueryParameter boolean buildOriginBranch, + @QueryParameter boolean buildOriginBranchWithPR, + @QueryParameter boolean buildOriginPRMerge, + @QueryParameter boolean buildOriginPRHead, + @QueryParameter boolean buildForkPRMerge, + @QueryParameter boolean buildForkPRHead + ) { + if (!buildOriginBranch && !buildOriginBranchWithPR && !buildOriginPRMerge && !buildOriginPRHead && !buildForkPRMerge && !buildForkPRHead) { + return FormValidation.warning("You need to build something!"); + } + if (buildForkPRMerge && buildForkPRHead) { return FormValidation.ok("Merged vs. unmerged PRs will be distinguished in the job name (*-merge vs. *-head)."); } return FormValidation.ok(); From 57fa572d74b9c2f1258ab10ba4683830ef77e1e3 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 21 Jun 2016 15:02:08 -0400 Subject: [PATCH 20/24] I18N and help. --- .../GitHubSCMSource/config-detail.jelly | 14 ++++------ .../GitHubSCMSource/config-detail.properties | 28 +++++++++++++++++++ .../GitHubSCMSource/help-buildForkPRHead.html | 4 +++ .../help-buildForkPRMerge.html | 4 +++ .../help-buildOriginBranch.html | 4 +++ .../help-buildOriginBranchWithPR.html | 4 +++ .../help-buildOriginPRHead.html | 5 ++++ .../help-buildOriginPRMerge.html | 4 +++ 8 files changed, 59 insertions(+), 8 deletions(-) create mode 100644 src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/config-detail.properties create mode 100644 src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/help-buildForkPRHead.html create mode 100644 src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/help-buildForkPRMerge.html create mode 100644 src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/help-buildOriginBranch.html create mode 100644 src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/help-buildOriginBranchWithPR.html create mode 100644 src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/help-buildOriginPRHead.html create mode 100644 src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/help-buildOriginPRMerge.html diff --git a/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/config-detail.jelly b/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/config-detail.jelly index 397d77e7b..1cd9a08e5 100644 --- a/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/config-detail.jelly +++ b/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/config-detail.jelly @@ -22,24 +22,22 @@ - - + - + - - + - + - + - + diff --git a/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/config-detail.properties b/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/config-detail.properties new file mode 100644 index 000000000..1210f2546 --- /dev/null +++ b/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/config-detail.properties @@ -0,0 +1,28 @@ +# The MIT License +# +# Copyright 2016 CloudBees, Inc. +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in +# all copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +# THE SOFTWARE. + +buildOriginBranch.title=Build origin branches +buildOriginBranchWithPR.title=Build origin branches also filed as PRs +buildOriginPRMerge.title=Build origin PRs (merged with base branch) +buildOriginPRHead.title=Build origin PRs (unmerged head) +buildForkPRMerge.title=Build fork PRs (merged with base branch) +buildForkPRHead.title=Build fork PRs (unmerged head) diff --git a/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/help-buildForkPRHead.html b/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/help-buildForkPRHead.html new file mode 100644 index 000000000..e7d2acd52 --- /dev/null +++ b/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/help-buildForkPRHead.html @@ -0,0 +1,4 @@ +
+ Whether to build pull requests filed from forks of the main repository. + The job will be named according to the PR and builds will use the head of the pull request, ignoring subsequent changes to the base branch. +
diff --git a/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/help-buildForkPRMerge.html b/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/help-buildForkPRMerge.html new file mode 100644 index 000000000..f459fbab2 --- /dev/null +++ b/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/help-buildForkPRMerge.html @@ -0,0 +1,4 @@ +
+ Whether to build pull requests filed from forks of the main repository. + The job will be named according to the PR and builds will attempt to merge with the base branch. +
diff --git a/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/help-buildOriginBranch.html b/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/help-buildOriginBranch.html new file mode 100644 index 000000000..4a151a7ff --- /dev/null +++ b/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/help-buildOriginBranch.html @@ -0,0 +1,4 @@ +
+ Whether to build branches defined in the origin (primary) repository, not associated with any pull request. + The job name will match the branch name. +
diff --git a/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/help-buildOriginBranchWithPR.html b/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/help-buildOriginBranchWithPR.html new file mode 100644 index 000000000..7e8c21a89 --- /dev/null +++ b/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/help-buildOriginBranchWithPR.html @@ -0,0 +1,4 @@ +
+ Whether to build branches defined in the origin (primary) repository for which pull requests happen to have been filed. + The job name will match the branch name, not the pull request(s). +
diff --git a/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/help-buildOriginPRHead.html b/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/help-buildOriginPRHead.html new file mode 100644 index 000000000..8307c24b1 --- /dev/null +++ b/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/help-buildOriginPRHead.html @@ -0,0 +1,5 @@ +
+ Whether to build pull requests filed from branches in the origin repository. + The job will be named according to the PR and builds will use the head of the pull request, ignoring subsequent changes to the base branch. + Other than naming, the behavior is similar to Build origin branches also filed as PRs. +
diff --git a/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/help-buildOriginPRMerge.html b/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/help-buildOriginPRMerge.html new file mode 100644 index 000000000..03564dd39 --- /dev/null +++ b/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMSource/help-buildOriginPRMerge.html @@ -0,0 +1,4 @@ +
+ Whether to build pull requests filed from branches in the origin repository. + The job will be named according to the PR and builds will attempt to merge with the base branch. +
From 71d72d8ea04e614cbef579f6d16002eabe89d3ef Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 21 Jun 2016 16:11:04 -0400 Subject: [PATCH 21/24] Copy-paste support for GitHubSCMNavigator, pending a cleaner way of sharing configuration code with GitHubSCMSource. --- .../GitHubSCMNavigator.java | 144 +++++++++++++++++- .../GitHubSCMNavigator/config.jelly | 18 +++ .../GitHubSCMNavigator/config.properties | 28 ++++ .../help-buildForkPRHead.html | 4 + .../help-buildForkPRMerge.html | 4 + .../help-buildOriginBranch.html | 4 + .../help-buildOriginBranchWithPR.html | 4 + .../help-buildOriginPRHead.html | 5 + .../help-buildOriginPRMerge.html | 4 + 9 files changed, 214 insertions(+), 1 deletion(-) create mode 100644 src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMNavigator/config.properties create mode 100644 src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMNavigator/help-buildForkPRHead.html create mode 100644 src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMNavigator/help-buildForkPRMerge.html create mode 100644 src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMNavigator/help-buildOriginBranch.html create mode 100644 src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMNavigator/help-buildOriginBranchWithPR.html create mode 100644 src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMNavigator/help-buildOriginPRHead.html create mode 100644 src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMNavigator/help-buildOriginPRMerge.html diff --git a/src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMNavigator.java b/src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMNavigator.java index ec28e26e2..944a459a6 100644 --- a/src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMNavigator.java +++ b/src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubSCMNavigator.java @@ -27,6 +27,7 @@ import com.cloudbees.plugins.credentials.CredentialsNameProvider; import com.cloudbees.plugins.credentials.common.StandardCredentials; import com.cloudbees.plugins.credentials.common.StandardListBoxModel; +import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import hudson.AbortException; import hudson.Extension; import hudson.Util; @@ -41,6 +42,7 @@ import java.util.regex.Pattern; import javax.annotation.CheckForNull; import javax.annotation.Nonnull; +import javax.inject.Inject; import jenkins.scm.api.SCMNavigator; import jenkins.scm.api.SCMNavigatorDescriptor; import jenkins.scm.api.SCMSourceObserver; @@ -70,7 +72,18 @@ public class GitHubSCMNavigator extends SCMNavigator { @CheckForNull private String includes; @CheckForNull private String excludes; - // TODO buildXXX to match GitHubSCMSource + /** Whether to build regular origin branches. */ + private @Nonnull Boolean buildOriginBranch = DescriptorImpl.defaultBuildOriginBranch; + /** Whether to build origin branches which happen to also have a PR filed from them (but here we are naming and building as a branch). */ + private @Nonnull Boolean buildOriginBranchWithPR = DescriptorImpl.defaultBuildOriginBranchWithPR; + /** Whether to build PRs filed from the origin, where the build is of the merge with the base branch. */ + private @Nonnull Boolean buildOriginPRMerge = DescriptorImpl.defaultBuildOriginPRMerge; + /** Whether to build PRs filed from the origin, where the build is of the branch head. */ + private @Nonnull Boolean buildOriginPRHead = DescriptorImpl.defaultBuildOriginPRHead; + /** Whether to build PRs filed from a fork, where the build is of the merge with the base branch. */ + private @Nonnull Boolean buildForkPRMerge = DescriptorImpl.defaultBuildForkPRMerge; + /** Whether to build PRs filed from a fork, where the build is of the branch head. */ + private @Nonnull Boolean buildForkPRHead = DescriptorImpl.defaultBuildForkPRHead; @DataBoundConstructor public GitHubSCMNavigator(String apiUri, String repoOwner, String scanCredentialsId, String checkoutCredentialsId) { this.repoOwner = repoOwner; @@ -79,6 +92,30 @@ public class GitHubSCMNavigator extends SCMNavigator { this.apiUri = Util.fixEmpty(apiUri); } + /** Use defaults for old settings. */ + @SuppressFBWarnings(value="RCN_REDUNDANT_NULLCHECK_OF_NONNULL_VALUE", justification="Only non-null after we set them here!") + private Object readResolve() { + if (buildOriginBranch == null) { + buildOriginBranch = DescriptorImpl.defaultBuildOriginBranch; + } + if (buildOriginBranchWithPR == null) { + buildOriginBranchWithPR = DescriptorImpl.defaultBuildOriginBranchWithPR; + } + if (buildOriginPRMerge == null) { + buildOriginPRMerge = DescriptorImpl.defaultBuildOriginPRMerge; + } + if (buildOriginPRHead == null) { + buildOriginPRHead = DescriptorImpl.defaultBuildOriginPRHead; + } + if (buildForkPRMerge == null) { + buildForkPRMerge = DescriptorImpl.defaultBuildForkPRMerge; + } + if (buildForkPRHead == null) { + buildForkPRHead = DescriptorImpl.defaultBuildForkPRHead; + } + return this; + } + @Nonnull public String getIncludes() { return includes != null ? includes : DescriptorImpl.defaultIncludes; } @@ -95,6 +132,60 @@ public class GitHubSCMNavigator extends SCMNavigator { this.excludes = excludes.equals(DescriptorImpl.defaultExcludes) ? null : excludes; } + public boolean getBuildOriginBranch() { + return buildOriginBranch; + } + + @DataBoundSetter + public void setBuildOriginBranch(boolean buildOriginBranch) { + this.buildOriginBranch = buildOriginBranch; + } + + public boolean getBuildOriginBranchWithPR() { + return buildOriginBranchWithPR; + } + + @DataBoundSetter + public void setBuildOriginBranchWithPR(boolean buildOriginBranchWithPR) { + this.buildOriginBranchWithPR = buildOriginBranchWithPR; + } + + public boolean getBuildOriginPRMerge() { + return buildOriginPRMerge; + } + + @DataBoundSetter + public void setBuildOriginPRMerge(boolean buildOriginPRMerge) { + this.buildOriginPRMerge = buildOriginPRMerge; + } + + public boolean getBuildOriginPRHead() { + return buildOriginPRHead; + } + + @DataBoundSetter + public void setBuildOriginPRHead(boolean buildOriginPRHead) { + this.buildOriginPRHead = buildOriginPRHead; + } + + public boolean getBuildForkPRMerge() { + return buildForkPRMerge; + } + + @DataBoundSetter + public void setBuildForkPRMerge(boolean buildForkPRMerge) { + this.buildForkPRMerge = buildForkPRMerge; + } + + public boolean getBuildForkPRHead() { + return buildForkPRHead; + } + + @DataBoundSetter + public void setBuildForkPRHead(boolean buildForkPRHead) { + this.buildForkPRHead = buildForkPRHead; + } + public String getRepoOwner() { return repoOwner; } @@ -221,6 +312,12 @@ private void add(TaskListener listener, SCMSourceObserver observer, GHRepository GitHubSCMSource ghSCMSource = new GitHubSCMSource(null, apiUri, checkoutCredentialsId, scanCredentialsId, repoOwner, name); ghSCMSource.setExcludes(getExcludes()); ghSCMSource.setIncludes(getIncludes()); + ghSCMSource.setBuildOriginBranch(getBuildOriginBranch()); + ghSCMSource.setBuildOriginBranchWithPR(getBuildOriginBranchWithPR()); + ghSCMSource.setBuildOriginPRMerge(getBuildOriginPRMerge()); + ghSCMSource.setBuildOriginPRHead(getBuildOriginPRHead()); + ghSCMSource.setBuildForkPRMerge(getBuildForkPRMerge()); + ghSCMSource.setBuildForkPRHead(getBuildForkPRHead()); projectObserver.addSource(ghSCMSource); projectObserver.complete(); @@ -233,6 +330,14 @@ private void add(TaskListener listener, SCMSourceObserver observer, GHRepository public static final String defaultIncludes = GitHubSCMSource.DescriptorImpl.defaultIncludes; public static final String defaultExcludes = GitHubSCMSource.DescriptorImpl.defaultExcludes; public static final String SAME = GitHubSCMSource.DescriptorImpl.SAME; + public static final boolean defaultBuildOriginBranch = GitHubSCMSource.DescriptorImpl.defaultBuildOriginBranch; + public static final boolean defaultBuildOriginBranchWithPR = GitHubSCMSource.DescriptorImpl.defaultBuildOriginBranchWithPR; + public static final boolean defaultBuildOriginPRMerge = GitHubSCMSource.DescriptorImpl.defaultBuildOriginPRMerge; + public static final boolean defaultBuildOriginPRHead = GitHubSCMSource.DescriptorImpl.defaultBuildOriginPRHead; + public static final boolean defaultBuildForkPRMerge = GitHubSCMSource.DescriptorImpl.defaultBuildForkPRMerge; + public static final boolean defaultBuildForkPRHead = GitHubSCMSource.DescriptorImpl.defaultBuildForkPRHead; + + @Inject private GitHubSCMSource.DescriptorImpl delegate; @Override public String getDisplayName() { return Messages.GitHubSCMNavigator_DisplayName(); @@ -300,6 +405,43 @@ public ListBoxModel doFillApiUriItems() { } return result; } + + // TODO repeating configuration blocks like this is clumsy; better to factor shared config into a Describable and use f:property + + @Restricted(NoExternalUse.class) + public FormValidation doCheckIncludes(@QueryParameter String value) { + return delegate.doCheckIncludes(value); + } + + @Restricted(NoExternalUse.class) + public FormValidation doCheckBuildOriginBranchWithPR( + @QueryParameter boolean buildOriginBranch, + @QueryParameter boolean buildOriginBranchWithPR, + @QueryParameter boolean buildOriginPRMerge, + @QueryParameter boolean buildOriginPRHead, + @QueryParameter boolean buildForkPRMerge, + @QueryParameter boolean buildForkPRHead + ) { + return delegate.doCheckBuildOriginBranchWithPR(buildOriginBranch, buildOriginBranchWithPR, buildOriginPRMerge, buildOriginPRHead, buildForkPRMerge, buildForkPRHead); + } + + @Restricted(NoExternalUse.class) + public FormValidation doCheckBuildOriginPRHead(@QueryParameter boolean buildOriginBranchWithPR, @QueryParameter boolean buildOriginPRMerge, @QueryParameter boolean buildOriginPRHead) { + return delegate.doCheckBuildOriginPRHead(buildOriginBranchWithPR, buildOriginPRMerge, buildOriginPRHead); + } + + @Restricted(NoExternalUse.class) + public FormValidation doCheckBuildForkPRHead/* web method name controls UI position of message; we want this at the bottom */( + @QueryParameter boolean buildOriginBranch, + @QueryParameter boolean buildOriginBranchWithPR, + @QueryParameter boolean buildOriginPRMerge, + @QueryParameter boolean buildOriginPRHead, + @QueryParameter boolean buildForkPRMerge, + @QueryParameter boolean buildForkPRHead + ) { + return delegate.doCheckBuildForkPRHead(buildOriginBranch, buildOriginBranchWithPR, buildOriginPRMerge, buildOriginPRHead, buildForkPRMerge, buildForkPRHead); + } + } } diff --git a/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMNavigator/config.jelly b/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMNavigator/config.jelly index d1675d9ed..11a42b6be 100644 --- a/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMNavigator/config.jelly +++ b/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMNavigator/config.jelly @@ -23,5 +23,23 @@ + + + + + + + + + + + + + + + + + + diff --git a/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMNavigator/config.properties b/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMNavigator/config.properties new file mode 100644 index 000000000..1210f2546 --- /dev/null +++ b/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMNavigator/config.properties @@ -0,0 +1,28 @@ +# The MIT License +# +# Copyright 2016 CloudBees, Inc. +# +# Permission is hereby granted, free of charge, to any person obtaining a copy +# of this software and associated documentation files (the "Software"), to deal +# in the Software without restriction, including without limitation the rights +# to use, copy, modify, merge, publish, distribute, sublicense, and/or sell +# copies of the Software, and to permit persons to whom the Software is +# furnished to do so, subject to the following conditions: +# +# The above copyright notice and this permission notice shall be included in +# all copies or substantial portions of the Software. +# +# THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +# IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +# FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +# AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +# LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, +# OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN +# THE SOFTWARE. + +buildOriginBranch.title=Build origin branches +buildOriginBranchWithPR.title=Build origin branches also filed as PRs +buildOriginPRMerge.title=Build origin PRs (merged with base branch) +buildOriginPRHead.title=Build origin PRs (unmerged head) +buildForkPRMerge.title=Build fork PRs (merged with base branch) +buildForkPRHead.title=Build fork PRs (unmerged head) diff --git a/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMNavigator/help-buildForkPRHead.html b/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMNavigator/help-buildForkPRHead.html new file mode 100644 index 000000000..e7d2acd52 --- /dev/null +++ b/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMNavigator/help-buildForkPRHead.html @@ -0,0 +1,4 @@ +
+ Whether to build pull requests filed from forks of the main repository. + The job will be named according to the PR and builds will use the head of the pull request, ignoring subsequent changes to the base branch. +
diff --git a/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMNavigator/help-buildForkPRMerge.html b/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMNavigator/help-buildForkPRMerge.html new file mode 100644 index 000000000..f459fbab2 --- /dev/null +++ b/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMNavigator/help-buildForkPRMerge.html @@ -0,0 +1,4 @@ +
+ Whether to build pull requests filed from forks of the main repository. + The job will be named according to the PR and builds will attempt to merge with the base branch. +
diff --git a/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMNavigator/help-buildOriginBranch.html b/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMNavigator/help-buildOriginBranch.html new file mode 100644 index 000000000..4a151a7ff --- /dev/null +++ b/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMNavigator/help-buildOriginBranch.html @@ -0,0 +1,4 @@ +
+ Whether to build branches defined in the origin (primary) repository, not associated with any pull request. + The job name will match the branch name. +
diff --git a/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMNavigator/help-buildOriginBranchWithPR.html b/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMNavigator/help-buildOriginBranchWithPR.html new file mode 100644 index 000000000..7e8c21a89 --- /dev/null +++ b/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMNavigator/help-buildOriginBranchWithPR.html @@ -0,0 +1,4 @@ +
+ Whether to build branches defined in the origin (primary) repository for which pull requests happen to have been filed. + The job name will match the branch name, not the pull request(s). +
diff --git a/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMNavigator/help-buildOriginPRHead.html b/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMNavigator/help-buildOriginPRHead.html new file mode 100644 index 000000000..8307c24b1 --- /dev/null +++ b/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMNavigator/help-buildOriginPRHead.html @@ -0,0 +1,5 @@ +
+ Whether to build pull requests filed from branches in the origin repository. + The job will be named according to the PR and builds will use the head of the pull request, ignoring subsequent changes to the base branch. + Other than naming, the behavior is similar to Build origin branches also filed as PRs. +
diff --git a/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMNavigator/help-buildOriginPRMerge.html b/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMNavigator/help-buildOriginPRMerge.html new file mode 100644 index 000000000..03564dd39 --- /dev/null +++ b/src/main/resources/org/jenkinsci/plugins/github_branch_source/GitHubSCMNavigator/help-buildOriginPRMerge.html @@ -0,0 +1,4 @@ +
+ Whether to build pull requests filed from branches in the origin repository. + The job will be named according to the PR and builds will attempt to merge with the base branch. +
From 67ccb38c9d07724df1d0e06c021582decf460498 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 21 Jun 2016 16:15:41 -0400 Subject: [PATCH 22/24] Tweak to indexing log; as @amuniz pointed out, it is otherwise hard to tell which is a merge vs. a head build. --- .../jenkinsci/plugins/github_branch_source/GitHubSCMSource.java | 1 + 1 file changed, 1 insertion(+) 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 cdffb09d7..69e8bee97 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 @@ -432,6 +432,7 @@ private void doRetrieve(SCMHeadObserver observer, TaskListener listener, GHRepos branchName += "-head"; } } + listener.getLogger().format(" Job name: %s%n", branchName); PullRequestSCMHead head = new PullRequestSCMHead(ghPullRequest, branchName, merge, trusted); if (criteria != null) { // Would be more precise to check whether the merge of the base branch head with the PR branch head contains a given file, etc., From 2e6048aa30ad509fb2f3577df5a597b1db660831 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 21 Jun 2016 16:52:57 -0400 Subject: [PATCH 23/24] Need to set author & committer prior to merging, in case the master has no Git configuration. --- .../jenkinsci/plugins/github_branch_source/GitHubSCMSource.java | 2 ++ 1 file changed, 2 insertions(+) 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 69e8bee97..c12b33f8e 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 @@ -657,6 +657,8 @@ public Revision decorateRevisionToBuild(GitSCM scm, Run build, GitClient g listener.getLogger().println("Merging " + baseName + " commit " + baseHash + " into PR head commit " + rev.getSha1String()); checkout(scm, build, git, listener, rev); try { + git.setAuthor("Jenkins", /* could parse out of JenkinsLocationConfiguration.get().getAdminAddress() but seems overkill */"nobody@nowhere"); + git.setCommitter("Jenkins", "nobody@nowhere"); MergeCommand cmd = git.merge().setRevisionToMerge(ObjectId.fromString(baseHash)); for (GitSCMExtension ext : scm.getExtensions()) { // By default we do a regular merge, allowing it to fast-forward. From 16e36f7fc9aaf6e416eacc77d920188f8550ba14 Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Tue, 21 Jun 2016 17:00:06 -0400 Subject: [PATCH 24/24] Added run-snapshot target. --- demo/.gitignore | 1 + demo/Dockerfile-snapshot | 2 ++ demo/Makefile | 12 ++++++++++-- 3 files changed, 13 insertions(+), 2 deletions(-) create mode 100644 demo/Dockerfile-snapshot diff --git a/demo/.gitignore b/demo/.gitignore index 9e6bf7cd6..e544bbd48 100644 --- a/demo/.gitignore +++ b/demo/.gitignore @@ -1 +1,2 @@ workspace +snapshot-plugins diff --git a/demo/Dockerfile-snapshot b/demo/Dockerfile-snapshot new file mode 100644 index 000000000..9617760ba --- /dev/null +++ b/demo/Dockerfile-snapshot @@ -0,0 +1,2 @@ +FROM jenkinsci/pipeline-as-code-github-demo:RELEASE +ADD snapshot-plugins /usr/share/jenkins/ref/plugins diff --git a/demo/Makefile b/demo/Makefile index cf415a3e2..4b945a53c 100644 --- a/demo/Makefile +++ b/demo/Makefile @@ -2,6 +2,7 @@ IMAGE=jenkinsci/pipeline-as-code-github-demo TAG=$(shell sed -ne s/github-branch-source://p < plugins.txt) # an arbitrary directory we can share with the host WORKSPACE=$(shell pwd)/workspace +DOCKER_RUN=docker run --rm -p 8080:8080 -p 4040:4040 -v /var/run/docker.sock:/var/run/docker.sock -v $(WORKSPACE):$(WORKSPACE) -e WORKSPACE=$(WORKSPACE) -ti build: docker build -t $(IMAGE):$(TAG) . @@ -9,9 +10,16 @@ build: run: build rm -rf $(WORKSPACE) mkdir $(WORKSPACE) - docker run --rm -p 8080:8080 -p 4040:4040 -v /var/run/docker.sock:/var/run/docker.sock -v $(WORKSPACE):$(WORKSPACE) -e WORKSPACE=$(WORKSPACE) -ti $(IMAGE):$(TAG) + $(DOCKER_RUN) $(IMAGE):$(TAG) -# TODO support snapshots using technique from workflow-aggregator-plugin/demo/Makefile +build-snapshot: + docker build -t $(IMAGE):RELEASE . + mkdir -p snapshot-plugins + for p in $$(cat plugins.txt|perl -pe s/:.+//g; cat snapshot-only-plugins.txt); do echo looking for snapshot builds of $$p; for g in org/jenkins-ci/plugins org/jenkins-ci/plugins/workflow org/jenkins-ci/plugins/pipeline-stage-view com/coravy/hudson/plugins/github; do if [ -f ~/.m2/repository/$$g/$$p/maven-metadata-local.xml ]; then cp -v $$(ls -1 ~/.m2/repository/$$g/$$p/*-SNAPSHOT/*.hpi | tail -1) snapshot-plugins/$$p.jpi; fi; done; done + docker build -f Dockerfile-snapshot -t $(IMAGE):SNAPSHOT . + +run-snapshot: build-snapshot + $(DOCKER_RUN) $(IMAGE):SNAPSHOT push: docker push $(IMAGE):$(TAG)