Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 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 @@ -34,6 +34,7 @@
import hudson.model.TaskListener;
import hudson.tasks.Shell;
import java.io.IOException;
import java.nio.charset.Charset;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.List;
Expand All @@ -46,15 +47,14 @@
import jenkins.model.Jenkins;
import jenkins.security.MasterToSlaveCallable;
import org.kohsuke.stapler.DataBoundConstructor;

/**
* Runs a Bourne shell script on a Unix node using {@code nohup}.
*/
public final class BourneShellScript extends FileMonitoringTask {

private static final Logger LOGGER = Logger.getLogger(BourneShellScript.class.getName());

private static enum OsType {DARWIN, UNIX, WINDOWS}
private static enum OsType {DARWIN, UNIX, WINDOWS, ZOS}

/** Number of times we will show launch diagnostics in a newly encountered workspace before going mute to save resources. */
@SuppressWarnings("FieldMayBeFinal")
Expand Down Expand Up @@ -107,7 +107,10 @@ public String getScript() {
if (script.isEmpty()) {
listener.getLogger().println("Warning: was asked to run an empty script");
}

OsType os = ws.act(new getOsType());

String encoding = os == OsType.ZOS ? "IBM1047" : null;
if(encoding != null) charset(Charset.forName(encoding));
ShellController c = new ShellController(ws);

FilePath shf = c.getScriptFile(ws);
Expand All @@ -118,14 +121,12 @@ 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);
shf.chmod(0755);

scriptPath = shf.getRemote();
List<String> args = new ArrayList<>();

OsType os = ws.act(new getOsType());

if (os != OsType.DARWIN) { // JENKINS-25848
args.add("nohup");
}
Expand Down Expand Up @@ -195,9 +196,8 @@ public String getScript() {
private transient long checkedTimestamp;

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

super(ws);
}
public FilePath getScriptFile(FilePath ws) throws IOException, InterruptedException {
return controlDir(ws).child("script.sh");
}
Expand Down Expand Up @@ -264,11 +264,13 @@ private int recordExitStatus(FilePath workspace, int code) throws IOException, I
private static final class getOsType extends MasterToSlaveCallable<OsType,RuntimeException> {
@Override public OsType call() throws RuntimeException {
if (Platform.isDarwin()) {
return OsType.DARWIN;
return OsType.DARWIN;
} else if (Platform.current() == Platform.WINDOWS) {
return OsType.WINDOWS;
return OsType.WINDOWS;
} else if(Platform.current() == Platform.UNIX && System.getProperty("os.name").equals("z/OS")) {
return OsType.ZOS;
} else {
return OsType.UNIX; // Default Value
return OsType.UNIX; // Default Value
}
}
private static final long serialVersionUID = 1L;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@
import hudson.remoting.VirtualChannel;
import hudson.slaves.WorkspaceList;
import hudson.util.StreamTaskListener;
import hudson.Platform;
import java.io.File;
import java.io.IOException;
import java.io.OutputStream;
Expand Down Expand Up @@ -180,6 +181,7 @@ protected FileMonitoringController(FilePath ws) throws IOException, InterruptedE
CountingOutputStream cos = new CountingOutputStream(transcodedSink);
try {
log.act(new WriteLog(lastLocation, new RemoteOutputStream(cos)));
sink.flush();
Copy link
Member

Choose a reason for hiding this comment

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

What does this have to do with z/OS support?

Copy link
Author

Choose a reason for hiding this comment

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

Well, this is due to weird IBM Java behavior on z/OS. Without flushing stream some of the job output isn't timely appears in a log. No side effects on OpenJDK or Sun Java.

Copy link
Member

Choose a reason for hiding this comment

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

Without this, does the output not appear at all, or does it just take a long time?

return cos.getByteCount() > 0;
} finally { // even if RemoteOutputStream write was interrupted, record what we actually received
transcodedSink.flush(); // writeImmediately flag does not seem to work
Expand Down Expand Up @@ -223,13 +225,15 @@ private static class TranscodingCharsetForSystemDefault extends MasterToSlaveCal
}

/** Avoids excess round-tripping when reading status file. */
static class StatusCheck extends MasterToSlaveFileCallable<Integer> {
/* replaced static class STATUS_CHECK with anonymous to capture charset */
final MasterToSlaveFileCallable<Integer> STATUS_CHECK_INSTANCE = new MasterToSlaveFileCallable<Integer>() {
@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());
Charset cs = transcodingCharset(charset);
Copy link
Member

Choose a reason for hiding this comment

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

Similarly, this makes me nervous (and its impact does not seem to be limited to z/OS): we expect the status file to be in EBCDIC if on z/OS, and in ASCII on any other platform. So if the JVM on z/OS fails to report its default charset as EBCDIC (why??), then we would just need to hard-code that value here.

Copy link
Author

Choose a reason for hiding this comment

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

Well, we do expect that IBM Java on z/OS will report a value of ibm.system.encoding with getProperty() call. If such property doesn't exist we will get NULL returned (that means we aren't on z/OS IBM as this property is unique for z/OS Java). And in this case all code flow will be absolutely the same as on ASCII platform, with specifying Charset.defaultCharset() in readFirstLine call.

Copy link
Member

Choose a reason for hiding this comment

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

I am not following your logic. If we are not on z/OS then getIBMzOsEncoding is not even called. Therefore this part of the patch is changing a status.txt read call on Linux from using Charset.defaultCharset() to using transcodingCharset(charset), which could be anything—probably an ASCII superset, but not necessarily.

Copy link
Author

Choose a reason for hiding this comment

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

I misunderstood your question then. On platforms rather than z/OS - behavior doesn't change much at all... it just uses a same logic as for getOutput(). it will read status file with encoding specified either by defaultCharset() for the platform encoding or from sh step encoding: parameter. This logic seems to be more consistent if i'm not mistaken. Indeed i can leave as it was, creating an additional check for z/OS platform and transcode status file only we are running on z/OS, np here. Just let me know if such change needed.

String fileString = Files.readFirstLine(f, cs == null ? Charset.defaultCharset() : cs );
if (fileString == null || fileString.isEmpty()) {
return null;
} else {
Expand All @@ -246,9 +250,7 @@ public Integer invoke(File f, VirtualChannel channel) throws IOException, Interr
}
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);
Expand Down