Skip to content

Commit dec68db

Browse files
jglickjglick
committed
[JENKINS-31096] Gather command output in a local encoding.
1 parent 9f09a0c commit dec68db

File tree

5 files changed

+65
-25
lines changed

5 files changed

+65
-25
lines changed

‎pom.xml

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@
2828
<parent>
2929
<groupId>org.jenkins-ci.plugins</groupId>
3030
<artifactId>plugin</artifactId>
31-
<version>3.2</version>
31+
<version>3.4</version>
3232
<relativePath />
3333
</parent>
3434
<groupId>org.jenkins-ci.plugins.workflow</groupId>
@@ -62,9 +62,10 @@
6262
</pluginRepository>
6363
</pluginRepositories>
6464
<properties>
65-
<jenkins.version>2.60.3</jenkins.version>
65+
<jenkins.version>2.73.3</jenkins.version>
6666
<java.level>8</java.level>
6767
<workflow-step-api-plugin.version>2.13</workflow-step-api-plugin.version>
68+
<workflow-support-plugin.version>2.19-20180209.204154-1</workflow-support-plugin.version> <!-- TODO https://github.com/jenkinsci/workflow-support-plugin/pull/56 -->
6869
</properties>
6970
<dependencies>
7071
<dependency>
@@ -75,17 +76,17 @@
7576
<dependency>
7677
<groupId>org.jenkins-ci.plugins</groupId>
7778
<artifactId>durable-task</artifactId>
78-
<version>1.18-20180125.194359-1</version> <!-- TODO https://github.com/jenkinsci/durable-task-plugin/pull/57 -->
79+
<version>1.18-20180212.221815-3</version> <!-- TODO https://github.com/jenkinsci/durable-task-plugin/pull/61 -->
7980
</dependency>
8081
<dependency>
8182
<groupId>org.jenkins-ci.plugins.workflow</groupId>
8283
<artifactId>workflow-api</artifactId>
83-
<version>2.22</version>
84+
<version>2.25</version>
8485
</dependency>
8586
<dependency>
8687
<groupId>org.jenkins-ci.plugins.workflow</groupId>
8788
<artifactId>workflow-support</artifactId>
88-
<version>2.16</version>
89+
<version>${workflow-support-plugin.version}</version>
8990
</dependency>
9091
<dependency>
9192
<groupId>org.jenkins-ci.plugins.workflow</groupId>
@@ -96,7 +97,7 @@
9697
<dependency>
9798
<groupId>org.jenkins-ci.plugins.workflow</groupId>
9899
<artifactId>workflow-job</artifactId>
99-
<version>2.9</version>
100+
<version>2.18-20180209.215341-1</version> <!-- TODO https://github.com/jenkinsci/workflow-job-plugin/pull/89 -->
100101
<scope>test</scope>
101102
</dependency>
102103
<dependency>
@@ -121,7 +122,7 @@
121122
<dependency>
122123
<groupId>org.jenkins-ci.plugins.workflow</groupId>
123124
<artifactId>workflow-support</artifactId>
124-
<version>2.13</version>
125+
<version>${workflow-support-plugin.version}</version>
125126
<classifier>tests</classifier>
126127
<scope>test</scope>
127128
</dependency>
@@ -134,12 +135,17 @@
134135
<dependency>
135136
<groupId>org.jenkins-ci.plugins</groupId>
136137
<artifactId>script-security</artifactId>
137-
<version>1.27</version>
138+
<version>1.39</version>
138139
</dependency>
139140
<dependency>
140141
<groupId>org.jenkins-ci.plugins</groupId>
141142
<artifactId>structs</artifactId>
142-
<version>1.7</version>
143+
<version>1.10</version>
144+
</dependency>
145+
<dependency>
146+
<groupId>org.jenkins-ci.plugins</groupId>
147+
<artifactId>scm-api</artifactId>
148+
<version>2.2.6</version>
143149
</dependency>
144150
</dependencies>
145151
</project>

‎src/main/java/org/jenkinsci/plugins/workflow/steps/durable_task/DurableTaskStep.java

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -30,13 +30,15 @@
3030
import hudson.EnvVars;
3131
import hudson.FilePath;
3232
import hudson.Launcher;
33+
import hudson.Util;
3334
import hudson.model.TaskListener;
3435
import hudson.util.DaemonThreadFactory;
3536
import hudson.util.FormValidation;
3637
import hudson.util.LogTaskListener;
3738
import hudson.util.NamingThreadFactory;
3839
import java.io.IOException;
3940
import java.nio.charset.Charset;
41+
import java.nio.charset.StandardCharsets;
4042
import java.util.Set;
4143
import java.util.concurrent.ScheduledFuture;
4244
import java.util.concurrent.ScheduledThreadPoolExecutor;
@@ -67,7 +69,7 @@ public abstract class DurableTaskStep extends Step {
6769
private static final Logger LOGGER = Logger.getLogger(DurableTaskStep.class.getName());
6870

6971
private boolean returnStdout;
70-
private String encoding = DurableTaskStepDescriptor.defaultEncoding;
72+
private String encoding;
7173
private boolean returnStatus;
7274

7375
protected abstract DurableTask task();
@@ -85,7 +87,7 @@ public String getEncoding() {
8587
}
8688

8789
@DataBoundSetter public void setEncoding(String encoding) {
88-
this.encoding = encoding;
90+
this.encoding = Util.fixEmpty(encoding);
8991
}
9092

9193
public boolean isReturnStatus() {
@@ -102,17 +104,15 @@ public boolean isReturnStatus() {
102104

103105
public abstract static class DurableTaskStepDescriptor extends StepDescriptor {
104106

105-
public static final String defaultEncoding = "UTF-8";
106-
107107
public FormValidation doCheckEncoding(@QueryParameter boolean returnStdout, @QueryParameter String encoding) {
108+
if (encoding.isEmpty()) {
109+
return FormValidation.ok();
110+
}
108111
try {
109112
Charset.forName(encoding);
110113
} catch (Exception x) {
111114
return FormValidation.error(x, "Unrecognized encoding");
112115
}
113-
if (!returnStdout && !encoding.equals(DurableTaskStepDescriptor.defaultEncoding)) {
114-
return FormValidation.warning("encoding is ignored unless returnStdout is checked.");
115-
}
116116
return FormValidation.ok();
117117
}
118118

@@ -154,7 +154,6 @@ static final class Execution extends AbstractStepExecutionImpl implements Runnab
154154
private String node;
155155
private String remote;
156156
private boolean returnStdout; // serialized default is false
157-
private String encoding; // serialized default is irrelevant
158157
private boolean returnStatus; // serialized default is false
159158

160159
Execution(StepContext context, DurableTaskStep step) {
@@ -164,7 +163,6 @@ static final class Execution extends AbstractStepExecutionImpl implements Runnab
164163

165164
@Override public boolean start() throws Exception {
166165
returnStdout = step.returnStdout;
167-
encoding = step.encoding;
168166
returnStatus = step.returnStatus;
169167
StepContext context = getContext();
170168
ws = context.get(FilePath.class);
@@ -173,6 +171,11 @@ static final class Execution extends AbstractStepExecutionImpl implements Runnab
173171
if (returnStdout) {
174172
durableTask.captureOutput();
175173
}
174+
if (step.encoding != null) {
175+
durableTask.charset(Charset.forName(step.encoding));
176+
} else {
177+
durableTask.defaultCharset();
178+
}
176179
controller = durableTask.launch(context.get(EnvVars.class), ws, context.get(Launcher.class), context.get(TaskListener.class));
177180
this.remote = ws.getRemote();
178181
setupTimer();
@@ -327,7 +330,7 @@ private void check() {
327330
LOGGER.log(Level.FINE, "last-minute output in {0} on {1}", new Object[] {remote, node});
328331
}
329332
if (returnStatus || exitCode == 0) {
330-
getContext().onSuccess(returnStatus ? exitCode : returnStdout ? new String(controller.getOutput(workspace, launcher()), encoding) : null);
333+
getContext().onSuccess(returnStatus ? exitCode : returnStdout ? new String(controller.getOutput(workspace, launcher()), StandardCharsets.UTF_8) : null);
331334
} else {
332335
if (returnStdout) {
333336
listener.getLogger().write(controller.getOutput(workspace, launcher())); // diagnostic

‎src/main/resources/org/jenkinsci/plugins/workflow/steps/durable_task/DurableTaskStep/config.jelly

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ THE SOFTWARE.
3131
<f:checkbox/>
3232
</f:entry>
3333
<f:entry field="encoding" title="${%Encoding of standard output}">
34-
<f:textbox default="${descriptor.defaultEncoding}"/>
34+
<f:textbox/>
3535
</f:entry>
3636
<f:entry field="returnStatus" title="${%Return exit status}">
3737
<f:checkbox/>

‎src/main/resources/org/jenkinsci/plugins/workflow/steps/durable_task/DurableTaskStep/help-encoding.html

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
11
<div>
2-
Encoding of standard output, if it is being captured.
2+
Encoding of process output.
3+
In the case of <code>returnStdout</code>, applies to the return value of this step;
4+
otherwise, or always for standard error, controls how text is copied to the build log.
5+
If unspecified, uses the system default encoding of the node on which the step is run.
36
</div>

‎src/test/java/org/jenkinsci/plugins/workflow/steps/durable_task/ShellStepTest.java

Lines changed: 32 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,14 +2,15 @@
22

33
import com.google.common.base.Predicate;
44
import hudson.EnvVars;
5+
import hudson.FilePath;
56
import hudson.Functions;
67
import hudson.Launcher;
78
import hudson.LauncherDecorator;
8-
import hudson.Platform;
99
import hudson.model.BallColor;
1010
import hudson.model.FreeStyleProject;
1111
import hudson.model.Node;
1212
import hudson.model.Result;
13+
import hudson.model.Slave;
1314
import hudson.slaves.DumbSlave;
1415
import hudson.slaves.EnvironmentVariablesNodeProperty;
1516
import hudson.tasks.BatchFile;
@@ -39,10 +40,12 @@
3940
import org.junit.ClassRule;
4041
import org.junit.Rule;
4142
import org.junit.Test;
43+
import org.junit.rules.TemporaryFolder;
4244
import org.jvnet.hudson.test.BuildWatcher;
4345
import org.jvnet.hudson.test.Issue;
4446
import org.jvnet.hudson.test.JenkinsRule;
4547
import org.jvnet.hudson.test.LoggerRule;
48+
import org.jvnet.hudson.test.SimpleCommandLauncher;
4649
import org.jvnet.hudson.test.TestExtension;
4750
import org.kohsuke.stapler.DataBoundConstructor;
4851

@@ -52,6 +55,7 @@ public class ShellStepTest {
5255
public static BuildWatcher buildWatcher = new BuildWatcher();
5356

5457
@Rule public JenkinsRule j = new JenkinsRule();
58+
@Rule public TemporaryFolder tmp = new TemporaryFolder();
5559

5660
@Rule public LoggerRule logging = new LoggerRule();
5761

@@ -197,7 +201,7 @@ public DescriptorImpl() {
197201
s = new StepConfigTester(j).configRoundTrip(s);
198202
assertEquals("echo hello", s.getScript());
199203
assertFalse(s.isReturnStdout());
200-
assertEquals(DurableTaskStep.DurableTaskStepDescriptor.defaultEncoding, s.getEncoding());
204+
assertNull(s.getEncoding());
201205
assertFalse(s.isReturnStatus());
202206
s.setReturnStdout(true);
203207
s.setEncoding("ISO-8859-1");
@@ -207,12 +211,12 @@ public DescriptorImpl() {
207211
assertEquals("ISO-8859-1", s.getEncoding());
208212
assertFalse(s.isReturnStatus());
209213
s.setReturnStdout(false);
210-
s.setEncoding(DurableTaskStep.DurableTaskStepDescriptor.defaultEncoding);
214+
s.setEncoding("UTF-8");
211215
s.setReturnStatus(true);
212216
s = new StepConfigTester(j).configRoundTrip(s);
213217
assertEquals("echo hello", s.getScript());
214218
assertFalse(s.isReturnStdout());
215-
assertEquals(DurableTaskStep.DurableTaskStepDescriptor.defaultEncoding, s.getEncoding());
219+
assertEquals("UTF-8", s.getEncoding());
216220
assertTrue(s.isReturnStatus());
217221
}
218222

@@ -233,6 +237,30 @@ public DescriptorImpl() {
233237
j.assertLogContains("truth is 0 but falsity is 1", j.assertBuildStatusSuccess(p.scheduleBuild2(0)));
234238
}
235239

240+
@Issue("JENKINS-31096")
241+
@Test public void encoding() throws Exception {
242+
// Like JenkinsRule.createSlave but passing a system encoding:
243+
Slave remote = new DumbSlave("remote", tmp.getRoot().getAbsolutePath(),
244+
new SimpleCommandLauncher("'" + System.getProperty("java.home") + "/bin/java' -Dfile.encoding=ISO-8859-2 -jar '" + new File(j.jenkins.getJnlpJars("slave.jar").getURL().toURI()) + "'"));
245+
j.jenkins.addNode(remote);
246+
j.waitOnline(remote);
247+
WorkflowJob p = j.createProject(WorkflowJob.class, "p");
248+
FilePath ws;
249+
while ((ws = remote.getWorkspaceFor(p)) == null) {
250+
Thread.sleep(100);
251+
}
252+
ws.child("message").write("Čau ty vole!\n", "ISO-8859-2");
253+
p.setDefinition(new CpsFlowDefinition("node('remote') {if (isUnix()) {sh 'cat message'} else {bat 'type message'}}", true));
254+
j.assertLogContains("Čau ty vole!", j.buildAndAssertSuccess(p));
255+
p.setDefinition(new CpsFlowDefinition("node('remote') {echo(/received: ${isUnix() ? sh(script: 'cat message', returnStdout: true) : bat(script: '@type message', returnStdout: true)}/)}", true)); // http://stackoverflow.com/a/8486061/12916
256+
j.assertLogContains("received: Čau ty vole!", j.buildAndAssertSuccess(p));
257+
p.setDefinition(new CpsFlowDefinition("node('remote') {if (isUnix()) {sh script: 'cat message', encoding: 'US-ASCII'} else {bat script: 'type message', encoding: 'US-ASCII'}}", true));
258+
j.assertLogContains("�au ty vole!", j.buildAndAssertSuccess(p));
259+
ws.child("message").write("¡Čau → there!\n", "UTF-8");
260+
p.setDefinition(new CpsFlowDefinition("node('remote') {if (isUnix()) {sh script: 'cat message', encoding: 'UTF-8'} else {bat script: 'type message', encoding: 'UTF-8'}}", true));
261+
j.assertLogContains("¡Čau → there!", j.buildAndAssertSuccess(p));
262+
}
263+
236264
@Issue("JENKINS-34021")
237265
@Test public void deadStep() throws Exception {
238266
logging.record(DurableTaskStep.class, Level.INFO).record(CpsStepContext.class, Level.INFO).capture(100);

0 commit comments

Comments
 (0)