Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] workflow support? #26

Closed
wants to merge 16 commits into from
Original file line number Diff line number Diff line change
@@ -1,15 +1,19 @@
package org.jenkinsci.plugins.github.pullrequest.builders;

import hudson.Extension;
import hudson.FilePath;
import hudson.Launcher;
import hudson.model.AbstractBuild;
import hudson.model.AbstractProject;
import hudson.model.BuildListener;
import hudson.model.Run;
import hudson.model.TaskListener;
import hudson.tasks.BuildStepDescriptor;
import hudson.tasks.Builder;
import jenkins.tasks.SimpleBuildStep;
import org.jenkinsci.plugins.github.pullrequest.GitHubPRCause;
import org.jenkinsci.plugins.github.pullrequest.GitHubPRMessage;
import org.jenkinsci.plugins.github.pullrequest.GitHubPRTrigger;
import org.jenkinsci.plugins.github.util.JobInfoHelpers;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.NoExternalUse;
import org.kohsuke.github.GHCommitState;
Expand All @@ -18,14 +22,15 @@
import org.slf4j.LoggerFactory;

import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;
import java.io.IOException;

import static org.apache.commons.lang3.StringUtils.isNotBlank;

/**
* Sets pr status for build caused by GitHubPRCause
*/
public class GitHubPRStatusBuilder extends Builder {
public class GitHubPRStatusBuilder extends Builder implements SimpleBuildStep {
private static final Logger LOGGER = LoggerFactory.getLogger(GitHubPRStatusBuilder.class);

public static final GitHubPRMessage DEFAULT_MESSAGE = new GitHubPRMessage("${GITHUB_PR_COND_REF} run started");
Expand All @@ -52,41 +57,52 @@ public GitHubPRMessage getStatusMessage() {
}

@Override
public boolean perform(AbstractBuild<?, ?> build, Launcher launcher, BuildListener listener)
throws InterruptedException, IOException {
GitHubPRTrigger trigger = build.getProject().getTrigger(GitHubPRTrigger.class);
public void perform(@Nonnull Run<?, ?> run, @Nonnull FilePath workspace, @Nonnull Launcher launcher,
@Nonnull TaskListener listener) throws InterruptedException, IOException {
// No triggers in Run class, but we need it
Copy link

Choose a reason for hiding this comment

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

ParameterizedJobMixIn.getTrigger since 1.621+; till then, inline.

Copy link

Choose a reason for hiding this comment

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

final GitHubPRTrigger trigger = JobInfoHelpers.triggerFrom(run.getParent(), GitHubPRTrigger.class);
Copy link
Contributor

Choose a reason for hiding this comment

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

for what it's worth, I'd like to see something like that method (and probably the whole class) in core somewhere to deal with the Run/AbstractProject pain.

if (trigger == null) {
return true;
// silently skip. TODO implement error handler, like in publishers
Copy link

Choose a reason for hiding this comment

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

Can you please show me an example of what you mean "like in the publishers". I am trying to convert the http-request plugin and I have the same question.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Plugin Publishers has errorHandler, this class is Builder and i forgot to use it also. ErrorHandler is useful only for linear jobs, workflow allows handle errors in DSL directly.

return;
}

GitHubPRCause cause = build.getCause(GitHubPRCause.class);
GitHubPRCause cause = run.getCause(GitHubPRCause.class);
if (cause == null) {
return true;
return;
Copy link
Contributor

Choose a reason for hiding this comment

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

Just curious - why the change from return true to just return?

Copy link
Owner Author

Choose a reason for hiding this comment

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

perform for Runs doesn't expect boolean. And boolean as return value is old style, it was replaced with AbortException. There are 2 ways of handling steps misuses (configured step but has no expected data):

  • fail
  • silently skip.

So this code do only silently skips (for publishers i wrote errorHandler with UI).

Copy link
Contributor

Choose a reason for hiding this comment

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

Aaaaaah, gotcha.

}

// GitHub status for commit
try {
if (statusMessage != null) {
String url = trigger.getDescriptor().getJenkinsURL() + build.getUrl();

trigger.getRemoteRepo().createCommitStatus(cause.getHeadSha(),
String url = trigger.getDescriptor().getJenkinsURL() + run.getUrl();
String expandedMessage;
if (run instanceof AbstractBuild<?, ?>) {
final AbstractBuild<?, ?> build = (AbstractBuild<?, ?>) run;
expandedMessage = statusMessage.expandAll(build, listener);
} else {
// Workflow doesn't expand variables with token macro!
expandedMessage = statusMessage.getContent();
Copy link

Choose a reason for hiding this comment

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

May need to specifically expand ${GITHUB_PR_COND_REF}.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Sounds like i should wrote Util that will try expand all possible variables from JobMixIns. That will at least expand ParameterActions.

}

trigger.getRemoteRepo().createCommitStatus(
cause.getHeadSha(),
GHCommitState.PENDING,
url,
statusMessage.expandAll(build, listener),
build.getProject().getFullName());
expandedMessage,
run.getParent().getFullName()
);
}
} catch (IOException e) {
listener.getLogger().println("Can't update build description");
LOGGER.error("Can't set commit status", e);
}

return true;
}

@Extension
public static class DescriptorImpl extends BuildStepDescriptor<Builder> {
@Override
public boolean isApplicable(Class<? extends AbstractProject> jobType) {
// workflow wants Run, but API doesn't support it
return true;
}

Expand Down