Skip to content

Commit

Permalink
Fix for 23786 Permit 'Execute shell' jobs to return 2 for 'unstable'
Browse files Browse the repository at this point in the history 
refactor and fix

remove duplication and fix FB errors

fix integration tests

consistency fixes

consistency fixes

changed error message to warning

fix comment and warning message

fix comment

fix junit tests

fix junit tests

fix junit tests

fix junit tests

clean import

a commit to trigger jenkins checks
  • Loading branch information
stochmim committed Oct 10, 2016
1 parent 4b067fd commit 359548e
Show file tree
Hide file tree
Showing 12 changed files with 321 additions and 186 deletions.
62 changes: 44 additions & 18 deletions core/src/main/java/hudson/tasks/BatchFile.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,31 +25,33 @@

import hudson.FilePath;
import hudson.Extension;
import hudson.Util;
import hudson.model.AbstractProject;
import hudson.util.FormValidation;
import hudson.util.LineEndingConversion;
import net.sf.json.JSONObject;
import org.jenkinsci.Symbol;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.DoNotUse;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.StaplerRequest;
import org.kohsuke.stapler.DataBoundSetter;
import org.kohsuke.stapler.QueryParameter;

import java.io.ObjectStreamException;

import javax.annotation.CheckForNull;

/**
* Executes commands by using Windows batch file.
*
* @author Kohsuke Kawaguchi
*/
public class BatchFile extends CommandInterpreter {
@DataBoundConstructor
public BatchFile(String command, Integer unstableReturn) {
super(LineEndingConversion.convertEOL(command, LineEndingConversion.EOLType.Windows));
this.unstableReturn = unstableReturn;
}

public BatchFile(String command) {
this(command, null);
super(LineEndingConversion.convertEOL(command, LineEndingConversion.EOLType.Windows));
}

private final Integer unstableReturn;
private Integer unstableReturn;

public String[] buildCommandLine(FilePath script) {
return new String[] {"cmd","/c","call",script.getRemote()};
Expand All @@ -63,8 +65,19 @@ protected String getFileExtension() {
return ".bat";
}

@CheckForNull
public final Integer getUnstableReturn() {
return unstableReturn;
return new Integer(0).equals(unstableReturn) ? null : unstableReturn;
}

@DataBoundSetter
public void setUnstableReturn(Integer unstableReturn) {
this.unstableReturn = unstableReturn;
}

@Override
protected boolean isErrorlevelForUnstableBuild(int exitCode) {
return this.unstableReturn != null && exitCode != 0 && this.unstableReturn.equals(exitCode);
}

private Object readResolve() throws ObjectStreamException {
Expand All @@ -82,15 +95,28 @@ public String getDisplayName() {
return Messages.BatchFile_DisplayName();
}

@Override
public Builder newInstance(StaplerRequest req, JSONObject data) {
final String unstableReturnStr = data.getString("unstableReturn");
Integer unstableReturn = null;
if (unstableReturnStr != null && ! unstableReturnStr.isEmpty()) {
/* Already validated by f.number in the form */
unstableReturn = (Integer)Integer.parseInt(unstableReturnStr, 10);
/**
* Performs on-the-fly validation of the errorlevel.
*/
@Restricted(DoNotUse.class)
public FormValidation doCheckUnstableReturn(@QueryParameter String value) {
value = Util.fixEmptyAndTrim(value);
if (value == null) {
return FormValidation.ok();
}
long unstableReturn;
try {
unstableReturn = Long.parseLong(value);
} catch (NumberFormatException e) {
return FormValidation.error(hudson.model.Messages.Hudson_NotANumber());
}
if (unstableReturn == 0) {
return FormValidation.warning(hudson.tasks.Messages.BatchFile_invalid_exit_code_zero());
}
if (unstableReturn < Integer.MIN_VALUE || unstableReturn > Integer.MAX_VALUE) {
return FormValidation.error(hudson.tasks.Messages.BatchFile_invalid_exit_code_range(unstableReturn));
}
return new BatchFile(data.getString("command"), unstableReturn);
return FormValidation.ok();
}

public boolean isApplicable(Class<? extends AbstractProject> jobType) {
Expand Down
48 changes: 22 additions & 26 deletions core/src/main/java/hudson/tasks/CommandInterpreter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import hudson.model.AbstractBuild;
import hudson.model.BuildListener;
import hudson.model.Node;
import hudson.model.Result;
import hudson.model.TaskListener;
import hudson.remoting.ChannelClosedException;

Expand All @@ -51,11 +52,6 @@ public abstract class CommandInterpreter extends Builder {
*/
protected final String command;

/**
* The build being run. Valid only during perform(...).
*/
protected AbstractBuild<?,?> build;

public CommandInterpreter(String command) {
this.command = command;
}
Expand All @@ -64,26 +60,24 @@ public final String getCommand() {
return command;
}

/**
* Access the current build object.
*
* Useful for {@link #join(Proc p)} for setting build results.
*
* @return The build being run, or null if outside perform(...)
* @since 1.573
*/
protected final AbstractBuild<?,?> getBuild()
{
return build;
}

@Override
public boolean perform(AbstractBuild<?,?> build, Launcher launcher, BuildListener listener) throws InterruptedException {
return perform(build,launcher,(TaskListener)listener);
}

/**
* Determines whether a non-zero exit code from the process should change the build
* status to {@link Result#UNSTABLE} instead of default {@link Result#FAILURE}.
*
* Changing to {@link Result#UNSTABLE} does not abort the build, next steps are continued.
*
* @since TODO
*/
protected boolean isErrorlevelForUnstableBuild(int exitCode) {
return false;
}

public boolean perform(AbstractBuild<?,?> build, Launcher launcher, TaskListener listener) throws InterruptedException {
this.build = build;
FilePath ws = build.getWorkspace();
if (ws == null) {
Node node = build.getBuiltOn();
Expand Down Expand Up @@ -112,11 +106,15 @@ public boolean perform(AbstractBuild<?,?> build, Launcher launcher, TaskListener
envVars.put(e.getKey(),e.getValue());

r = join(launcher.launch().cmds(buildCommandLine(script)).envs(envVars).stdout(listener).pwd(ws).start());

if(isErrorlevelForUnstableBuild(r)) {
build.setResult(Result.UNSTABLE);
r = 0;
}
} catch (IOException e) {
Util.displayIOException(e, listener);
e.printStackTrace(listener.fatalError(Messages.CommandInterpreter_CommandFailed()));
}
this.build = null;
return r==0;
} finally {
try {
Expand Down Expand Up @@ -145,12 +143,10 @@ public boolean perform(AbstractBuild<?,?> build, Launcher launcher, TaskListener
/**
* Reports the exit code from the process.
*
* This allows subtypes to treat the exit code differently (for example by
* treating non-zero exit code as if it's zero). Any non-zero exit code
* will cause the build step to fail.
*
* To set the status to {@link Result#UNSTABLE}, use {@link #getBuild()} and
* call {@code getBuild().setResult(BuildResult.UNSTABLE); }.
* This allows subtypes to treat the exit code differently (for example by treating non-zero exit code
* as if it's zero, or to set the status to {@link Result#UNSTABLE}). Any non-zero exit code will cause
* the build step to fail. Use {@link #isErrorlevelForUnstableBuild(int exitCode)} to redefine the default
* behaviour.
*
* @since 1.549
*/
Expand Down
88 changes: 48 additions & 40 deletions core/src/main/java/hudson/tasks/Shell.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,7 @@
import hudson.Functions;
import hudson.Util;
import hudson.Extension;
import hudson.Proc;
import hudson.model.AbstractProject;
import hudson.model.Result;
import hudson.remoting.Callable;
import hudson.remoting.VirtualChannel;
import hudson.util.FormValidation;
import java.io.IOException;
Expand All @@ -39,7 +36,10 @@
import jenkins.security.MasterToSlaveCallable;
import net.sf.json.JSONObject;
import org.jenkinsci.Symbol;
import org.kohsuke.accmod.Restricted;
import org.kohsuke.accmod.restrictions.DoNotUse;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.DataBoundSetter;
import org.kohsuke.stapler.StaplerRequest;
import org.kohsuke.stapler.QueryParameter;

Expand All @@ -49,32 +49,30 @@
import java.util.logging.Level;
import java.util.logging.Logger;

import javax.annotation.CheckForNull;

/**
* Executes a series of commands by using a shell.
*
* @author Kohsuke Kawaguchi
*/
public class Shell extends CommandInterpreter {

@DataBoundConstructor
public Shell(String command, Integer unstableReturn) {
public Shell(String command) {
super(LineEndingConversion.convertEOL(command, LineEndingConversion.EOLType.Unix));
if (unstableReturn != null && unstableReturn.equals(0))
unstableReturn = null;
this.unstableReturn = unstableReturn;
}

public Shell(String command) {
this(command, null);
}
private Integer unstableReturn;


private final Integer unstableReturn;

/**
* Older versions of bash have a bug where non-ASCII on the first line
* makes the shell think the file is a binary file and not a script. Adding
* a leading line feed works around this problem.
*/
private static String addCrForNonASCII(String s) {
private static String addLineFeedForNonASCII(String s) {
if(!s.startsWith("#!")) {
if (s.indexOf('\n')!=0) {
return "\n" + s;
Expand All @@ -94,34 +92,31 @@ public String[] buildCommandLine(FilePath script) {
args.add(script.getRemote());
args.set(0,args.get(0).substring(2)); // trim off "#!"
return args.toArray(new String[args.size()]);
} else
} else
return new String[] { getDescriptor().getShellOrDefault(script.getChannel()), "-xe", script.getRemote()};
}

protected String getContents() {
return addCrForNonASCII(fixCrLf(command));
return addLineFeedForNonASCII(LineEndingConversion.convertEOL(command,LineEndingConversion.EOLType.Unix));
}

protected String getFileExtension() {
return ".sh";
}

@CheckForNull
public final Integer getUnstableReturn() {
return unstableReturn;
return new Integer(0).equals(unstableReturn) ? null : unstableReturn;
}

@DataBoundSetter
public void setUnstableReturn(Integer unstableReturn) {
this.unstableReturn = unstableReturn;
}

/**
* Allow the user to define a result for "unstable":
*/
@Override
protected int join(Proc p) throws IOException, InterruptedException {
final int result = p.join();
if (this.unstableReturn != null && result != 0 && this.unstableReturn.equals(result)) {
getBuild().setResult(Result.UNSTABLE);
return 0;
}
else
return result;
protected boolean isErrorlevelForUnstableBuild(int exitCode) {
return this.unstableReturn != null && exitCode != 0 && this.unstableReturn.equals(exitCode);
}

@Override
Expand Down Expand Up @@ -164,7 +159,7 @@ public String getShellOrDefault() {
}

public String getShellOrDefault(VirtualChannel channel) {
if (shell != null)
if (shell != null)
return shell;

String interpreter = null;
Expand All @@ -181,7 +176,7 @@ public String getShellOrDefault(VirtualChannel channel) {

return interpreter;
}

public void setShell(String shell) {
this.shell = Util.fixEmptyAndTrim(shell);
save();
Expand All @@ -191,15 +186,28 @@ public String getDisplayName() {
return Messages.Shell_DisplayName();
}

@Override
public Builder newInstance(StaplerRequest req, JSONObject data) {
final String unstableReturnStr = data.getString("unstableReturn");
Integer unstableReturn = null;
if (unstableReturnStr != null && ! unstableReturnStr.isEmpty()) {
/* Already validated by f.number in the form */
unstableReturn = (Integer)Integer.parseInt(unstableReturnStr, 10);
/**
* Performs on-the-fly validation of the exit code.
*/
@Restricted(DoNotUse.class)
public FormValidation doCheckUnstableReturn(@QueryParameter String value) {
value = Util.fixEmptyAndTrim(value);
if (value == null) {
return FormValidation.ok();
}
long unstableReturn;
try {
unstableReturn = Long.parseLong(value);
} catch (NumberFormatException e) {
return FormValidation.error(hudson.model.Messages.Hudson_NotANumber());
}
return new Shell(data.getString("command"), unstableReturn);
if (unstableReturn == 0) {
return FormValidation.warning(hudson.tasks.Messages.Shell_invalid_exit_code_zero());
}
if (unstableReturn < 1 || unstableReturn > 255) {
return FormValidation.error(hudson.tasks.Messages.Shell_invalid_exit_code_range(unstableReturn));
}
return FormValidation.ok();
}

@Override
Expand All @@ -213,9 +221,9 @@ public boolean configure(StaplerRequest req, JSONObject data) throws FormExcepti
*/
public FormValidation doCheckShell(@QueryParameter String value) {
// Executable requires admin permission
return FormValidation.validateExecutable(value);
return FormValidation.validateExecutable(value);
}

private static final class Shellinterpreter extends MasterToSlaveCallable<String, IOException> {

private static final long serialVersionUID = 1L;
Expand All @@ -224,8 +232,8 @@ public String call() throws IOException {
return Functions.isWindows() ? "sh" : "/bin/sh";
}
}

}

private static final Logger LOGGER = Logger.getLogger(Shell.class.getName());
}
7 changes: 3 additions & 4 deletions core/src/main/resources/hudson/tasks/BatchFile/config.jelly
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,13 +25,12 @@ THE SOFTWARE.
<?jelly escape-by-default='true'?>
<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define" xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:f="/lib/form">
<f:entry title="${%Command}"
description="${%command_description(rootURL)}">
description="${%description(rootURL)}">
<f:textarea name="command" value="${instance.command}" class="fixed-width" />
</f:entry>
<f:advanced>
<f:entry title="errorlevel to set build unstable"
description="${%unstableReturn_description(rootURL)}">
<f:number clazz="positive-number" name="unstableReturn" value="${instance.unstableReturn}" min="1" max="255" step="1" />
<f:entry title="${%ERRORLEVEL to set build unstable}" field="unstableReturn" >
<f:number value="${instance.unstableReturn}" min="-2147483648" max="2147483647" step="1" />
</f:entry>
</f:advanced>
</j:jelly>
Loading

0 comments on commit 359548e

Please sign in to comment.