diff --git a/src/main/java/org/jenkinsci/plugins/durabletask/BourneShellScript.java b/src/main/java/org/jenkinsci/plugins/durabletask/BourneShellScript.java index e93b7be3..3c15d3c3 100644 --- a/src/main/java/org/jenkinsci/plugins/durabletask/BourneShellScript.java +++ b/src/main/java/org/jenkinsci/plugins/durabletask/BourneShellScript.java @@ -140,7 +140,7 @@ public String getScript() { FilePath resultFile = c.getResultFile(ws); FilePath controlDir = c.controlDir(ws); if (capturingOutput) { - cmd = String.format("{ while [ -d '%s' -a \\! -f '%s' ]; do touch '%s'; sleep 3; done } & jsc=%s; %s=$jsc '%s' > '%s' 2> '%s'; echo $? > '%s'; wait", + cmd = String.format("{ while [ -d '%s' -a \\! -f '%s' ]; do touch '%s'; sleep 3; done } & jsc=%s; %s=$jsc '%s' > '%s' 2> '%s'; echo $? > '%s.tmp'; mv '%s.tmp' '%s'; wait", controlDir, resultFile, logFile, @@ -149,9 +149,9 @@ public String getScript() { scriptPath, c.getOutputFile(ws), logFile, - resultFile); + resultFile, resultFile, resultFile); } else { - cmd = String.format("{ while [ -d '%s' -a \\! -f '%s' ]; do touch '%s'; sleep 3; done } & jsc=%s; %s=$jsc '%s' > '%s' 2>&1; echo $? > '%s'; wait", + cmd = String.format("{ while [ -d '%s' -a \\! -f '%s' ]; do touch '%s'; sleep 3; done } & jsc=%s; %s=$jsc '%s' > '%s' 2>&1; echo $? > '%s.tmp'; mv '%s.tmp' '%s'; wait", controlDir, resultFile, logFile, @@ -159,7 +159,7 @@ public String getScript() { cookieVariable, scriptPath, logFile, - resultFile); + resultFile, resultFile, resultFile); } cmd = cmd.replace("$", "$$"); // escape against EnvVars jobEnv in LocalLauncher.launch @@ -207,6 +207,7 @@ private FilePath pidFile(FilePath ws) throws IOException, InterruptedException { return controlDir(ws).child("pid"); } + // TODO run as one big MasterToSlaveCallable to avoid extra network roundtrips @Override public Integer exitStatus(FilePath workspace, Launcher launcher, TaskListener listener) throws IOException, InterruptedException { Integer status = super.exitStatus(workspace, launcher, listener); if (status != null) { diff --git a/src/main/java/org/jenkinsci/plugins/durabletask/FileMonitoringTask.java b/src/main/java/org/jenkinsci/plugins/durabletask/FileMonitoringTask.java index 7ef7d3d7..cbae4446 100644 --- a/src/main/java/org/jenkinsci/plugins/durabletask/FileMonitoringTask.java +++ b/src/main/java/org/jenkinsci/plugins/durabletask/FileMonitoringTask.java @@ -24,6 +24,7 @@ package org.jenkinsci.plugins.durabletask; +import com.google.common.io.Files; import hudson.EnvVars; import hudson.FilePath; import hudson.Launcher; @@ -40,6 +41,7 @@ import java.io.OutputStream; import java.io.RandomAccessFile; import java.io.StringWriter; +import java.nio.charset.Charset; import java.util.Collections; import java.util.Map; import java.util.TreeMap; @@ -48,6 +50,9 @@ import java.util.logging.Logger; import jenkins.MasterToSlaveFileCallable; import org.apache.commons.io.IOUtils; +import org.jenkinsci.remoting.RoleChecker; + +import javax.annotation.CheckForNull; /** * A task which forks some external command and then waits for log and status files to be updated/created. @@ -160,20 +165,39 @@ private static class WriteLog extends MasterToSlaveFileCallable { } } - // TODO would be more efficient to allow API to consolidate writeLog with exitStatus (save an RPC call) - @Override public Integer exitStatus(FilePath workspace, Launcher launcher, TaskListener listener) throws IOException, InterruptedException { - FilePath status = getResultFile(workspace); - if (status.exists()) { - try { - return Integer.parseInt(status.readToString().trim()); - } catch (NumberFormatException x) { - throw new IOException("corrupted content in " + status + ": " + x, x); + /** Avoids excess round-tripping when reading status file. */ + static class StatusCheck extends MasterToSlaveFileCallable { + @Override + @CheckForNull + public Integer invoke(File f, VirtualChannel channel) throws IOException, InterruptedException { + if (f.exists() && f.length() > 0) { + try { + String fileString = Files.readFirstLine(f, Charset.defaultCharset()); + if (fileString == null || fileString.isEmpty()) { + return null; + } else { + fileString = fileString.trim(); + if (fileString.isEmpty()) { + return null; + } else { + return Integer.parseInt(fileString); + } + } + } catch (NumberFormatException x) { + throw new IOException("corrupted content in " + f + ": " + x, x); + } } - } else { return null; } } + static final StatusCheck STATUS_CHECK_INSTANCE = new StatusCheck(); + + @Override public Integer exitStatus(FilePath workspace, Launcher launcher, TaskListener listener) throws IOException, InterruptedException { + FilePath status = getResultFile(workspace); + return status.act(STATUS_CHECK_INSTANCE); + } + @Override public byte[] getOutput(FilePath workspace, Launcher launcher) throws IOException, InterruptedException { // TODO could perhaps be more efficient for large files to send a MasterToSlaveFileCallable try (InputStream is = getOutputFile(workspace).read()) { diff --git a/src/main/java/org/jenkinsci/plugins/durabletask/WindowsBatchScript.java b/src/main/java/org/jenkinsci/plugins/durabletask/WindowsBatchScript.java index 7fac3075..4d4ec94d 100644 --- a/src/main/java/org/jenkinsci/plugins/durabletask/WindowsBatchScript.java +++ b/src/main/java/org/jenkinsci/plugins/durabletask/WindowsBatchScript.java @@ -60,17 +60,18 @@ public String getScript() { BatchController c = new BatchController(ws); String cmd; + String quotedResultFile = quote(c.getResultFile(ws)); if (capturingOutput) { - cmd = String.format("@echo off \r\ncmd /c \"\"%s\"\" > \"%s\" 2> \"%s\"\r\necho %%ERRORLEVEL%% > \"%s\"\r\n", + cmd = String.format("@echo off \r\ncmd /c \"\"%s\"\" > \"%s\" 2> \"%s\"\r\necho %%ERRORLEVEL%% > \"%s.tmp\"\r\nmove \"%s.tmp\" \"%s\"\r\n", quote(c.getBatchFile2(ws)), quote(c.getOutputFile(ws)), quote(c.getLogFile(ws)), - quote(c.getResultFile(ws))); + quotedResultFile, quotedResultFile, quotedResultFile); } else { - cmd = String.format("@echo off \r\ncmd /c \"\"%s\"\" > \"%s\" 2>&1\r\necho %%ERRORLEVEL%% > \"%s\"\r\n", + cmd = String.format("@echo off \r\ncmd /c \"\"%s\"\" > \"%s\" 2>&1\r\necho %%ERRORLEVEL%% > \"%s.tmp\"\r\nmove \"%s.tmp\" \"%s\"\n", quote(c.getBatchFile2(ws)), quote(c.getLogFile(ws)), - quote(c.getResultFile(ws))); + quotedResultFile, quotedResultFile, quotedResultFile); } c.getBatchFile1(ws).write(cmd, "UTF-8"); c.getBatchFile2(ws).write(script, "UTF-8");