Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import jenkins.model.Jenkins;
import jenkins.security.MasterToSlaveCallable;
import org.kohsuke.stapler.DataBoundConstructor;
import org.apache.commons.io.IOUtils;

/**
* Runs a Bourne shell script on a Unix node using {@code nohup}.
Expand Down Expand Up @@ -85,7 +86,9 @@ public String getScript() {
listener.getLogger().println("Warning: was asked to run an empty script");
}

ShellController c = new ShellController(ws);
String encoding = ws.act(new ZosCheck()) ? "IBM1047" : null;
Copy link
Member

Choose a reason for hiding this comment

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

Seems OK though may clash with the higher-priority #33.


ShellController c = new ShellController(ws, encoding);

FilePath shf = c.getScriptFile(ws);

Expand All @@ -95,7 +98,7 @@ public String getScript() {
String defaultShell = jenkins.getInjector().getInstance(Shell.DescriptorImpl.class).getShellOrDefault(ws.getChannel());
s = "#!"+defaultShell+" -xe\n" + s;
}
shf.write(s, "UTF-8");
shf.write(s, encoding==null ? "UTF-8" : encoding);
Copy link
Member

Choose a reason for hiding this comment

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

Would we not just want to use the system encoding on the agent, regardless of OS?

Copy link
Contributor Author

@lne3 lne3 Feb 8, 2017

Choose a reason for hiding this comment

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

For various reasons (e.g. JENKINS-13091) we might be in a situation that the agent does not run with the systems default encoding, i.e. the –Dfile.encoding=… jvm parameter changing it to something else. Therefore system default encoding might be a good default. However, it should be possible to set the encoding explicitly since the required script file encoding (IBM1047 on z/OS) does not necessarily match the encoding set by –Dfile.encoding.

shf.chmod(0755);

envVars.put(cookieVariable, "please-do-not-kill-me");
Expand Down Expand Up @@ -153,8 +156,8 @@ public String getScript() {
private int pid;
private final long startTime = System.currentTimeMillis();

private ShellController(FilePath ws) throws IOException, InterruptedException {
super(ws);
private ShellController(FilePath ws, String encoding) throws IOException, InterruptedException {
super(ws, encoding);
}

public FilePath getScriptFile(FilePath ws) throws IOException, InterruptedException {
Expand All @@ -170,7 +173,10 @@ private synchronized int pid(FilePath ws) throws IOException, InterruptedExcepti
FilePath pidFile = pidFile(ws);
if (pidFile.exists()) {
try {
pid = Integer.parseInt(pidFile.readToString().trim());
final String pidStr = this.getEncoding() == null ?
pidFile.readToString().trim() : // original behavior
IOUtils.toString(pidFile.read(),this.getEncoding());
pid = Integer.parseInt(pidStr.trim());
} catch (NumberFormatException x) {
throw new IOException("corrupted content in " + pidFile + ": " + x, x);
}
Expand Down Expand Up @@ -221,4 +227,10 @@ private static final class DarwinCheck extends MasterToSlaveCallable<Boolean,Run
}
}

private static final class ZosCheck extends MasterToSlaveCallable<Boolean,RuntimeException> {
@Override public Boolean call() throws RuntimeException {
return Platform.current() == Platform.UNIX && System.getProperty("os.name").equals("z/OS");
}
}

}
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,17 @@
import hudson.slaves.WorkspaceList;
import java.io.File;
import java.io.IOException;
import java.io.InputStreamReader;
import java.io.OutputStream;
import java.io.OutputStreamWriter;
import java.io.RandomAccessFile;
import java.util.Collections;
import java.util.UUID;
import java.util.logging.Level;
import java.util.logging.Logger;
import jenkins.MasterToSlaveFileCallable;
import org.apache.commons.io.IOUtils;
import org.apache.commons.io.output.WriterOutputStream;

/**
* A task which forks some external command and then waits for log and status files to be updated/created.
Expand Down Expand Up @@ -94,18 +97,41 @@ protected static class FileMonitoringController extends Controller {
*/
private long lastLocation;

/**
* Encoding of input/output of the sh.
* If null no character conversion to be done.
*/
private String encoding;

/**
* @return Encoding of the files to be monitored.
* If null handled without conversion just as byte stream.
*/
public String getEncoding() {
return encoding;
}

protected FileMonitoringController(FilePath ws) throws IOException, InterruptedException {
this(ws,null);
}

protected FileMonitoringController(FilePath ws, String encoding) throws IOException, InterruptedException {
// can't keep ws reference because Controller is expected to be serializable
ws.mkdirs();
FilePath cd = tempDir(ws).child("durable-" + Util.getDigestOf(UUID.randomUUID().toString()).substring(0,8));
cd.mkdirs();
controlDir = cd.getRemote();
this.encoding = encoding;
}

@Override public final boolean writeLog(FilePath workspace, OutputStream sink) throws IOException, InterruptedException {
FilePath log = getLogFile(workspace);
if (this.getEncoding() != null) {
sink = new WriterOutputStream(new OutputStreamWriter(sink, "UTF-8"), this.getEncoding());
Copy link
Member

Choose a reason for hiding this comment

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

This is probably superseded by encoding fixes in #29.

}
Long newLocation = log.act(new WriteLog(lastLocation, new RemoteOutputStream(sink)));
if (newLocation != null) {
if (this.getEncoding() != null) sink.flush();
LOGGER.log(Level.FINE, "copied {0} bytes from {1}", new Object[] {newLocation - lastLocation, log});
lastLocation = newLocation;
return true;
Expand Down Expand Up @@ -149,7 +175,10 @@ private static class WriteLog extends MasterToSlaveFileCallable<Long> {
FilePath status = getResultFile(workspace);
if (status.exists()) {
try {
return Integer.parseInt(status.readToString().trim());
final String statusStr = this.getEncoding() == null ?
status.readToString().trim() : // original behavior
IOUtils.toString(status.read(),this.getEncoding());
return Integer.parseInt(statusStr.trim());
} catch (NumberFormatException x) {
throw new IOException("corrupted content in " + status + ": " + x, x);
}
Expand All @@ -160,7 +189,9 @@ private static class WriteLog extends MasterToSlaveFileCallable<Long> {

@Override public byte[] getOutput(FilePath workspace, Launcher launcher) throws IOException, InterruptedException {
// TODO could perhaps be more efficient for large files to send a MasterToSlaveFileCallable<byte[]>
return IOUtils.toByteArray(getOutputFile(workspace).read());
return this.getEncoding() == null ?
Copy link
Member

Choose a reason for hiding this comment

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

Ditto.

IOUtils.toByteArray(getOutputFile(workspace).read()) :
IOUtils.toByteArray(new InputStreamReader(getOutputFile(workspace).read(),this.getEncoding()), "UTF-8");
}

@Override public final void stop(FilePath workspace, Launcher launcher) throws IOException, InterruptedException {
Expand Down