Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
996cd93
[JENKINS-33161] Draft of support for customized PR build behavior.
jglick Jun 16, 2016
ce5f941
Oops, forgot to record entries in originBranchesWithPR.
jglick Jun 16, 2016
63a6006
Warning about another anomalous combination.
jglick Jun 16, 2016
ed48654
Fixing a few warnings and a test failure.
jglick Jun 16, 2016
6ce16b1
We are converting this anyway, can strip out more implementation.
jglick Jun 16, 2016
870125b
Comments.
jglick Jun 16, 2016
32a5de3
We need to be sure to feed SpecificRevisionBuildChooser the PR head h…
jglick Jun 16, 2016
7dc37d0
Revert "We are converting this anyway, can strip out more implementat…
jglick Jun 16, 2016
56f47c5
typo
jglick Jun 16, 2016
413f0b0
Noting when job names will distinguish kinds of PRs.
jglick Jun 20, 2016
5a98d2a
Specify a notification per job, so that we can distinctly see merged …
jglick Jun 20, 2016
cdfe0b3
Override toString for debugging.
jglick Jun 20, 2016
7353a02
Building merged PRs seems to work now.
jglick Jun 20, 2016
bff827f
@KostyaSha’s suggestion of *-head vs. *-merge job naming seems as goo…
jglick Jun 21, 2016
d067c0c
The getters and setters were intended to be of type boolean, not Bool…
jglick Jun 21, 2016
e003af9
Did my IDE just go nuts?
jglick Jun 21, 2016
c1feec5
Indentation fix.
jglick Jun 21, 2016
e889802
Typo.
jglick Jun 21, 2016
f7e5157
Improved form validation at @amuniz’s suggestion.
jglick Jun 21, 2016
57fa572
I18N and help.
jglick Jun 21, 2016
71d72d8
Copy-paste support for GitHubSCMNavigator, pending a cleaner way of s…
jglick Jun 21, 2016
67ccb38
Tweak to indexing log; as @amuniz pointed out, it is otherwise hard t…
jglick Jun 21, 2016
2e6048a
Need to set author & committer prior to merging, in case the master h…
jglick Jun 21, 2016
16e36f7
Added run-snapshot target.
jglick Jun 21, 2016
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions demo/.gitignore
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
workspace
snapshot-plugins
2 changes: 2 additions & 0 deletions demo/Dockerfile-snapshot
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
FROM jenkinsci/pipeline-as-code-github-demo:RELEASE
ADD snapshot-plugins /usr/share/jenkins/ref/plugins
12 changes: 10 additions & 2 deletions demo/Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,24 @@ 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) .

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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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;

Expand All @@ -73,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());

Choose a reason for hiding this comment

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

This causes issues right now as it's not possible to require Jenkins status checks to pass anymore without GitHub. It relies on the name being consistent across checks, which isn't possible when the check name is always changing.

Choose a reason for hiding this comment

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

I see this issue has already been filed as JENKINS-36574.

}

@SuppressWarnings("deprecation") // Run.getAbsoluteUrl appropriate here
Expand All @@ -96,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");
Expand Down Expand Up @@ -198,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?");
Expand Down Expand Up @@ -239,35 +237,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<GHCommit> 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);
Copy link
Member Author

Choose a reason for hiding this comment

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

By keeping a reasonable representation of the SCMRevisions to begin with, we avoid the need to try to reconstruct what we just built.

}
throw new IllegalArgumentException();
}

private GitHubBuildStatusNotification() {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down Expand Up @@ -70,6 +72,18 @@ public class GitHubSCMNavigator extends SCMNavigator {

@CheckForNull private String includes;
@CheckForNull private String excludes;
/** 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;
Expand All @@ -78,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;
}
Expand All @@ -94,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;
}
Expand Down Expand Up @@ -220,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();
Expand All @@ -232,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();
Expand Down Expand Up @@ -299,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);
}

}

}
Loading