Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[FIXED JENKINS-7478]
Create Line Ending Conversion utility to convert scripts to proper OS
line ending type.  Tests included.
  • Loading branch information
David Ruhmann committed Aug 25, 2014
1 parent 3efe07f commit 67ac47a
Show file tree
Hide file tree
Showing 11 changed files with 233 additions and 53 deletions.
10 changes: 8 additions & 2 deletions core/src/main/java/hudson/tasks/BatchFile.java
Expand Up @@ -26,9 +26,11 @@
import hudson.FilePath;
import hudson.Extension;
import hudson.model.AbstractProject;
import hudson.util.LineEndingConversion;
import net.sf.json.JSONObject;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.StaplerRequest;
import java.io.ObjectStreamException;

/**
* Executes commands by using Windows batch file.
Expand All @@ -38,21 +40,25 @@
public class BatchFile extends CommandInterpreter {
@DataBoundConstructor
public BatchFile(String command) {
super(command);
super(LineEndingConversion.convertEOL(command, LineEndingConversion.EOLType.Windows));
}

public String[] buildCommandLine(FilePath script) {
return new String[] {"cmd","/c","call",script.getRemote()};
}

protected String getContents() {
return command+"\r\nexit %ERRORLEVEL%";
return LineEndingConversion.convertEOL(command+"\r\nexit %ERRORLEVEL%",LineEndingConversion.EOLType.Windows);
}

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

private Object readResolve() throws ObjectStreamException {
return new BatchFile(command);
}

@Extension
public static final class DescriptorImpl extends BuildStepDescriptor<Builder> {
@Override
Expand Down
34 changes: 9 additions & 25 deletions core/src/main/java/hudson/tasks/Shell.java
Expand Up @@ -32,6 +32,8 @@
import hudson.remoting.VirtualChannel;
import hudson.util.FormValidation;
import java.io.IOException;
import java.io.ObjectStreamException;
import hudson.util.LineEndingConversion;
import net.sf.json.JSONObject;
import org.kohsuke.stapler.DataBoundConstructor;
import org.kohsuke.stapler.StaplerRequest;
Expand All @@ -50,37 +52,15 @@
public class Shell extends CommandInterpreter {
@DataBoundConstructor
public Shell(String command) {
super(fixCrLf(command));
}

/**
* Fix CR/LF and always make it Unix style.
*/
private static String fixCrLf(String s) {
// eliminate CR
int idx;
while((idx=s.indexOf("\r\n"))!=-1)
s = s.substring(0,idx)+s.substring(idx+1);

//// add CR back if this is for Windows
//if(isWindows()) {
// idx=0;
// while(true) {
// idx = s.indexOf('\n',idx);
// if(idx==-1) break;
// s = s.substring(0,idx)+'\r'+s.substring(idx);
// idx+=2;
// }
//}
return s;
super(LineEndingConversion.convertEOL(command, LineEndingConversion.EOLType.Unix));
}

/**
* 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 @@ -105,7 +85,7 @@ public String[] buildCommandLine(FilePath script) {
}

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

protected String getFileExtension() {
Expand All @@ -117,6 +97,10 @@ public DescriptorImpl getDescriptor() {
return (DescriptorImpl)super.getDescriptor();
}

private Object readResolve() throws ObjectStreamException {
return new Shell(command);
}

@Extension
public static class DescriptorImpl extends BuildStepDescriptor<Builder> {
/**
Expand Down
15 changes: 1 addition & 14 deletions core/src/main/java/hudson/tools/AbstractCommandInstaller.java
Expand Up @@ -48,7 +48,7 @@ public abstract class AbstractCommandInstaller extends ToolInstaller {

public AbstractCommandInstaller(String label, String command, String toolHome) {
super(label);
this.command = fixCrLf(command);
this.command = command;
this.toolHome = toolHome;
}

Expand Down Expand Up @@ -84,19 +84,6 @@ public FilePath performInstallation(ToolInstallation tool, Node node, TaskListen
return dir.child(getToolHome());
}

/**
* Fix CR/LF and always make it Unix style.
*/
//TODO: replace by a Windows style
private static String fixCrLf(String s) {
// eliminate CR
int idx;
while ((idx = s.indexOf("\r\n")) != -1) {
s = s.substring(0, idx) + s.substring(idx + 1);
}
return s;
}

public static abstract class Descriptor<TInstallerClass extends AbstractCommandInstaller>
extends ToolInstallerDescriptor<TInstallerClass> {

Expand Down
8 changes: 7 additions & 1 deletion core/src/main/java/hudson/tools/BatchCommandInstaller.java
Expand Up @@ -26,7 +26,9 @@

import hudson.Extension;
import hudson.FilePath;
import hudson.util.LineEndingConversion;
import org.kohsuke.stapler.DataBoundConstructor;
import java.io.ObjectStreamException;

/**
* Installs tool via script execution of Batch script.
Expand All @@ -37,7 +39,7 @@ public class BatchCommandInstaller extends AbstractCommandInstaller {

@DataBoundConstructor
public BatchCommandInstaller(String label, String command, String toolHome) {
super(label, command, toolHome);
super(label, LineEndingConversion.convertEOL(command, LineEndingConversion.EOLType.Windows), toolHome);
}

@Override
Expand All @@ -50,6 +52,10 @@ public String[] getCommandCall(FilePath script) {
String[] cmd = {"cmd", "/c", "call", script.getRemote()};
return cmd;
}

private Object readResolve() throws ObjectStreamException {
return new BatchCommandInstaller(getLabel(), getCommand(), getToolHome());
}

@Extension
public static class DescriptorImpl extends Descriptor<BatchCommandInstaller> {
Expand Down
8 changes: 7 additions & 1 deletion core/src/main/java/hudson/tools/CommandInstaller.java
Expand Up @@ -26,7 +26,9 @@

import hudson.Extension;
import hudson.FilePath;
import hudson.util.LineEndingConversion;
import org.kohsuke.stapler.DataBoundConstructor;
import java.io.ObjectStreamException;

/**
* Installs a tool by running an arbitrary shell command.
Expand All @@ -36,7 +38,7 @@ public class CommandInstaller extends AbstractCommandInstaller {

@DataBoundConstructor
public CommandInstaller(String label, String command, String toolHome) {
super(label, command, toolHome);
super(label, LineEndingConversion.convertEOL(command, LineEndingConversion.EOLType.Unix), toolHome);
}

@Override
Expand All @@ -50,6 +52,10 @@ public String[] getCommandCall(FilePath script) {
return cmd;
}

private Object readResolve() throws ObjectStreamException {
return new CommandInstaller(getLabel(), getCommand(), getToolHome());
}

@Extension
public static class DescriptorImpl extends Descriptor<CommandInstaller> {

Expand Down
66 changes: 66 additions & 0 deletions core/src/main/java/hudson/util/LineEndingConversion.java
@@ -0,0 +1,66 @@
package hudson.util;

/**
* Converts line endings of a string.
*
* @since TODO
* @author David Ruhmann
*/
public class LineEndingConversion {

/**
* Supported line ending types for conversion
*/
public enum EOLType {
CR,
CRLF,
LF,
LFCR,
Mac,
Unix,
Windows
}

/**
* Convert line endings of a string to the given type. Default to Unix type.
*
* @param input
* The string containing line endings to be converted.
* @param type
* Type of line endings to convert the string into.
* @return
* String updated with the new line endings or null if given null.
*/
public static String convertEOL(String input, EOLType type) {
if (null == input || 0 == input.length()) {
return input;
}
// Convert line endings to Unix LF,
// which also sets up the string for other conversions
input = input.replace("\r\n","\n");
input = input.replace("\r","\n");
switch (type) {
case CR:
case Mac:
// Convert line endings to CR
input = input.replace("\n", "\r");
break;
case CRLF:
case Windows:
// Convert line endings to Windows CR/LF
input = input.replace("\n", "\r\n");
break;
default:
case LF:
case Unix:
// Conversion already completed
return input;
case LFCR:
// Convert line endings to LF/CR
input = input.replace("\n", "\n\r");
break;
}
return input;
}
}

30 changes: 30 additions & 0 deletions test/src/test/java/hudson/tasks/BatchFileTest.java
@@ -0,0 +1,30 @@
package hudson.tasks;

import org.junit.Rule;
import org.junit.Test;
import org.jvnet.hudson.test.Issue;
import org.jvnet.hudson.test.JenkinsRule;

/**
* Tests for the BatchFile tasks class.
*
* @author David Ruhmann
*/
public class BatchFileTest {

@Rule
public JenkinsRule rule = new JenkinsRule();

@Issue("JENKINS-7478")
@Test
public void validateBatchFileCommandEOL() throws Exception {
BatchFile obj = new BatchFile("echo A\necho B\recho C");
rule.assertStringContains(obj.getCommand(), "echo A\r\necho B\r\necho C");
}

@Test
public void validateBatchFileContents() throws Exception {
BatchFile obj = new BatchFile("echo A\necho B\recho C");
rule.assertStringContains(obj.getContents(), "echo A\r\necho B\r\necho C\r\nexit %ERRORLEVEL%");
}
}
43 changes: 33 additions & 10 deletions test/src/test/java/hudson/tasks/ShellTest.java
Expand Up @@ -7,31 +7,54 @@
import hudson.model.FreeStyleProject;
import org.apache.commons.io.FileUtils;
import org.jvnet.hudson.test.FakeLauncher;
import org.jvnet.hudson.test.HudsonTestCase;
import org.jvnet.hudson.test.PretendSlave;

import java.io.File;
import java.io.IOException;
import java.io.PrintStream;
import java.util.List;

import org.junit.Assert;
import org.junit.Rule;
import org.junit.Test;
import org.jvnet.hudson.test.JenkinsRule;

/**
* Tests for the Shell tasks class
*
* @author Kohsuke Kawaguchi
*/
public class ShellTest extends HudsonTestCase {
public class ShellTest {

@Rule
public JenkinsRule rule = new JenkinsRule();

@Test
public void validateShellCommandEOL() throws Exception {
Shell obj = new Shell("echo A\r\necho B\recho C");
rule.assertStringContains(obj.getCommand(), "echo A\necho B\necho C");
}

@Test
public void validateShellContents() throws Exception {
Shell obj = new Shell("echo A\r\necho B\recho C");
rule.assertStringContains(obj.getContents(), "\necho A\necho B\necho C");
}

@Test
public void testBasic() throws Exception {
// If we're on Windows, don't bother doing this.
if (Functions.isWindows())
return;

// TODO: define a FakeLauncher implementation with easymock so that this kind of assertions can be simplified.
PretendSlave s = createPretendSlave(new FakeLauncher() {
PretendSlave s = rule.createPretendSlave(new FakeLauncher() {
public Proc onLaunch(ProcStarter p) throws IOException {
// test the command line argument.
List<String> cmds = p.cmds();
assertEquals("/bin/sh",cmds.get(0));
assertEquals("-xe",cmds.get(1));
assertTrue(new File(cmds.get(2)).exists());
rule.assertStringContains("/bin/sh",cmds.get(0));
rule.assertStringContains("-xe",cmds.get(1));
Assert.assertTrue(new File(cmds.get(2)).exists());

// fake the execution
PrintStream ps = new PrintStream(p.stdout());
Expand All @@ -41,13 +64,13 @@ public Proc onLaunch(ProcStarter p) throws IOException {
return new FinishedProc(0);
}
});
FreeStyleProject p = createFreeStyleProject();
FreeStyleProject p = rule.createFreeStyleProject();
p.getBuildersList().add(new Shell("echo abc"));
p.setAssignedNode(s);

FreeStyleBuild b = assertBuildStatusSuccess(p.scheduleBuild2(0).get());
FreeStyleBuild b = rule.assertBuildStatusSuccess(p.scheduleBuild2(0).get());

assertEquals(1,s.numLaunch);
assertTrue(FileUtils.readFileToString(b.getLogFile()).contains("Hudson was here"));
Assert.assertEquals(1,s.numLaunch);
Assert.assertTrue(FileUtils.readFileToString(b.getLogFile()).contains("Hudson was here"));
}
}
22 changes: 22 additions & 0 deletions test/src/test/java/hudson/tools/BatchCommandInstallerTest.java
@@ -0,0 +1,22 @@
package hudson.tools;

import org.junit.Rule;
import org.junit.Test;
import org.jvnet.hudson.test.JenkinsRule;

/**
* Tests for the BatchCommandInstaller tools class.
*
* @author David Ruhmann
*/
public class BatchCommandInstallerTest {

@Rule
public JenkinsRule rule = new JenkinsRule();

@Test
public void validateBatchCommandInstallerCommandEOL() throws Exception {
BatchCommandInstaller obj = new BatchCommandInstaller("", "echo A\necho B\recho C", "");
rule.assertStringContains(obj.getCommand(), "echo A\r\necho B\r\necho C");
}
}

0 comments on commit 67ac47a

Please sign in to comment.