[JEP-206] Gather command output in a local encoding

.mvn/extensions.xml

@@ -2,6 +2,6 @@
   <extension>
     <groupId>io.jenkins.tools.incrementals</groupId>
     <artifactId>git-changelist-maven-extension</artifactId>
-    <version>1.0-beta-3</version>
+    <version>1.0-beta-4</version>
   </extension>
 </extensions>

Jenkinsfile

@@ -1 +1 @@
-buildPlugin(jenkinsVersions: [null, '2.73.1'])
+buildPlugin(jenkinsVersions: [null, '2.121.1'])

pom.xml

@@ -28,7 +28,7 @@
     <parent>
         <groupId>org.jenkins-ci.plugins</groupId>
         <artifactId>plugin</artifactId>
-        <version>3.14</version>
+        <version>3.19</version>
         <relativePath />
     </parent>
     <groupId>org.jenkins-ci.plugins.workflow</groupId>
@@ -64,9 +64,10 @@
     <properties>
         <revision>2.20</revision>
         <changelist>-SNAPSHOT</changelist>
-        <jenkins.version>2.60.3</jenkins.version>
+        <jenkins.version>2.73.3</jenkins.version>
         <java.level>8</java.level>
         <workflow-step-api-plugin.version>2.13</workflow-step-api-plugin.version>
+        <workflow-support-plugin.version>2.20</workflow-support-plugin.version>
     </properties>
     <dependencies>
         <dependency>
@@ -77,17 +78,17 @@
         <dependency>
             <groupId>org.jenkins-ci.plugins</groupId>
             <artifactId>durable-task</artifactId>
-            <version>1.18</version>
+            <version>1.24</version>
         </dependency>
         <dependency>
             <groupId>org.jenkins-ci.plugins.workflow</groupId>
             <artifactId>workflow-api</artifactId>
-            <version>2.22</version>
+            <version>2.25</version>
         </dependency>
         <dependency>
             <groupId>org.jenkins-ci.plugins.workflow</groupId>
             <artifactId>workflow-support</artifactId>
-            <version>2.16</version>
+            <version>${workflow-support-plugin.version}</version>
         </dependency>
         <dependency>
             <groupId>org.jenkins-ci.plugins.workflow</groupId>
@@ -98,7 +99,7 @@
         <dependency>
             <groupId>org.jenkins-ci.plugins.workflow</groupId>
             <artifactId>workflow-job</artifactId>
-            <version>2.9</version>
+            <version>2.24-rc765.2c5d9ffed127</version> <!-- TODO https://github.com/jenkinsci/workflow-job-plugin/pull/89 -->
             <scope>test</scope>
         </dependency>
         <dependency>
@@ -123,7 +124,7 @@
         <dependency>
             <groupId>org.jenkins-ci.plugins.workflow</groupId>
             <artifactId>workflow-support</artifactId>
-            <version>2.13</version>
+            <version>${workflow-support-plugin.version}</version>
             <classifier>tests</classifier>
             <scope>test</scope>
         </dependency>
@@ -136,12 +137,17 @@
         <dependency>
             <groupId>org.jenkins-ci.plugins</groupId>
             <artifactId>script-security</artifactId>
-            <version>1.27</version>
+            <version>1.39</version>
         </dependency>
         <dependency>
             <groupId>org.jenkins-ci.plugins</groupId>
             <artifactId>structs</artifactId>
-            <version>1.7</version>
+            <version>1.10</version>
+        </dependency>
+        <dependency>
+            <groupId>org.jenkins-ci.plugins</groupId>
+            <artifactId>scm-api</artifactId>
+            <version>2.2.6</version>
         </dependency>
     </dependencies>
 </project>

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

@@ -30,13 +30,15 @@
 import hudson.EnvVars;
 import hudson.FilePath;
 import hudson.Launcher;
+import hudson.Util;
 import hudson.model.TaskListener;
 import hudson.util.DaemonThreadFactory;
 import hudson.util.FormValidation;
 import hudson.util.LogTaskListener;
 import hudson.util.NamingThreadFactory;
 import java.io.IOException;
 import java.nio.charset.Charset;
+import java.nio.charset.StandardCharsets;
 import java.util.Set;
 import java.util.concurrent.ScheduledFuture;
 import java.util.concurrent.ScheduledThreadPoolExecutor;
@@ -56,6 +58,8 @@
 import org.jenkinsci.plugins.workflow.steps.StepExecution;
 import org.jenkinsci.plugins.workflow.support.concurrent.Timeout;
 import org.jenkinsci.plugins.workflow.support.concurrent.WithThreadName;
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.DoNotUse;
 import org.kohsuke.stapler.DataBoundSetter;
 import org.kohsuke.stapler.QueryParameter;
 
@@ -67,7 +71,7 @@ public abstract class DurableTaskStep extends Step {
     private static final Logger LOGGER = Logger.getLogger(DurableTaskStep.class.getName());
 
     private boolean returnStdout;
-    private String encoding = DurableTaskStepDescriptor.defaultEncoding;
+    private String encoding;
     private boolean returnStatus;
 
     protected abstract DurableTask task();
@@ -85,7 +89,7 @@ public String getEncoding() {
     }
 
     @DataBoundSetter public void setEncoding(String encoding) {
-        this.encoding = encoding;
+        this.encoding = Util.fixEmpty(encoding);
     }
 
     public boolean isReturnStatus() {
@@ -102,17 +106,16 @@ public boolean isReturnStatus() {
 
     public abstract static class DurableTaskStepDescriptor extends StepDescriptor {
 
-        public static final String defaultEncoding = "UTF-8";
-
+        @Restricted(DoNotUse.class)
         public FormValidation doCheckEncoding(@QueryParameter boolean returnStdout, @QueryParameter String encoding) {
+            if (encoding.isEmpty()) {
+                return FormValidation.ok();
+            }
             try {
                 Charset.forName(encoding);
             } catch (Exception x) {
                 return FormValidation.error(x, "Unrecognized encoding");
             }
-            if (!returnStdout && !encoding.equals(DurableTaskStepDescriptor.defaultEncoding)) {
-                return FormValidation.warning("encoding is ignored unless returnStdout is checked.");
-            }
             return FormValidation.ok();
         }
 
@@ -154,7 +157,6 @@ static final class Execution extends AbstractStepExecutionImpl implements Runnab
         private String node;
         private String remote;
         private boolean returnStdout; // serialized default is false
-        private String encoding; // serialized default is irrelevant
         private boolean returnStatus; // serialized default is false
 
         Execution(StepContext context, DurableTaskStep step) {
@@ -164,7 +166,6 @@ static final class Execution extends AbstractStepExecutionImpl implements Runnab
 
         @Override public boolean start() throws Exception {
             returnStdout = step.returnStdout;
-            encoding = step.encoding;
             returnStatus = step.returnStatus;
             StepContext context = getContext();
             ws = context.get(FilePath.class);
@@ -173,6 +174,11 @@ static final class Execution extends AbstractStepExecutionImpl implements Runnab
             if (returnStdout) {
                 durableTask.captureOutput();
             }
+            if (step.encoding != null) {
+                durableTask.charset(Charset.forName(step.encoding));
+            } else {
+                durableTask.defaultCharset();
+            }
             controller = durableTask.launch(context.get(EnvVars.class), ws, context.get(Launcher.class), context.get(TaskListener.class));
             this.remote = ws.getRemote();
             setupTimer();
@@ -327,7 +333,7 @@ private void check() {
                                 LOGGER.log(Level.FINE, "last-minute output in {0} on {1}", new Object[] {remote, node});
                             }
                             if (returnStatus || exitCode == 0) {
-                                getContext().onSuccess(returnStatus ? exitCode : returnStdout ? new String(controller.getOutput(workspace, launcher()), encoding) : null);
+                                getContext().onSuccess(returnStatus ? exitCode : returnStdout ? new String(controller.getOutput(workspace, launcher()), StandardCharsets.UTF_8) : null);
                             } else {
                                 if (returnStdout) {
                                     listener.getLogger().write(controller.getOutput(workspace, launcher())); // diagnostic

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

@@ -31,7 +31,7 @@ THE SOFTWARE.
             <f:checkbox/>
         </f:entry>
         <f:entry field="encoding" title="${%Encoding of standard output}">
-            <f:textbox default="${descriptor.defaultEncoding}"/>
+            <f:textbox/>
         </f:entry>
         <f:entry field="returnStatus" title="${%Return exit status}">
             <f:checkbox/>

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

@@ -1,3 +1,12 @@
 <div>
-    Encoding of standard output, if it is being captured.
+    Encoding of process output.
+    In the case of <code>returnStdout</code>, applies to the return value of this step;
+    otherwise, or always for standard error, controls how text is copied to the build log.
+    If unspecified, uses the system default encoding of the node on which the step is run.
+    If there is any expectation that process output might include non-ASCII characters,
+    it is best to specify the encoding explicitly.
+    For example, if you have specific knowledge that a given process is going to be producing UTF-8
+    yet will be running on a node with a different system encoding
+    (typically Windows, since every Linux distribution has defaulted to UTF-8 for a long time),
+    you can ensure correct output by specifying: <code>encoding: 'UTF-8'</code>
 </div>

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

@@ -2,14 +2,15 @@
 
 import com.google.common.base.Predicate;
 import hudson.EnvVars;
+import hudson.FilePath;
 import hudson.Functions;
 import hudson.Launcher;
 import hudson.LauncherDecorator;
-import hudson.Platform;
 import hudson.model.BallColor;
 import hudson.model.FreeStyleProject;
 import hudson.model.Node;
 import hudson.model.Result;
+import hudson.model.Slave;
 import hudson.slaves.DumbSlave;
 import hudson.slaves.EnvironmentVariablesNodeProperty;
 import hudson.tasks.BatchFile;
@@ -39,10 +40,12 @@
 import org.junit.ClassRule;
 import org.junit.Rule;
 import org.junit.Test;
+import org.junit.rules.TemporaryFolder;
 import org.jvnet.hudson.test.BuildWatcher;
 import org.jvnet.hudson.test.Issue;
 import org.jvnet.hudson.test.JenkinsRule;
 import org.jvnet.hudson.test.LoggerRule;
+import org.jvnet.hudson.test.SimpleCommandLauncher;
 import org.jvnet.hudson.test.TestExtension;
 import org.kohsuke.stapler.DataBoundConstructor;
 
@@ -52,6 +55,7 @@ public class ShellStepTest {
     public static BuildWatcher buildWatcher = new BuildWatcher();
 
     @Rule public JenkinsRule j = new JenkinsRule();
+    @Rule public TemporaryFolder tmp = new TemporaryFolder();
 
     @Rule public LoggerRule logging = new LoggerRule();
 
@@ -197,7 +201,7 @@ public DescriptorImpl() {
         s = new StepConfigTester(j).configRoundTrip(s);
         assertEquals("echo hello", s.getScript());
         assertFalse(s.isReturnStdout());
-        assertEquals(DurableTaskStep.DurableTaskStepDescriptor.defaultEncoding, s.getEncoding());
+        assertNull(s.getEncoding());
         assertFalse(s.isReturnStatus());
         s.setReturnStdout(true);
         s.setEncoding("ISO-8859-1");
@@ -207,12 +211,12 @@ public DescriptorImpl() {
         assertEquals("ISO-8859-1", s.getEncoding());
         assertFalse(s.isReturnStatus());
         s.setReturnStdout(false);
-        s.setEncoding(DurableTaskStep.DurableTaskStepDescriptor.defaultEncoding);
+        s.setEncoding("UTF-8");
         s.setReturnStatus(true);
         s = new StepConfigTester(j).configRoundTrip(s);
         assertEquals("echo hello", s.getScript());
         assertFalse(s.isReturnStdout());
-        assertEquals(DurableTaskStep.DurableTaskStepDescriptor.defaultEncoding, s.getEncoding());
+        assertEquals("UTF-8", s.getEncoding());
         assertTrue(s.isReturnStatus());
     }
 
@@ -233,6 +237,30 @@ public DescriptorImpl() {
         j.assertLogContains("truth is 0 but falsity is 1", j.assertBuildStatusSuccess(p.scheduleBuild2(0)));
     }
 
+    @Issue("JENKINS-31096")
+    @Test public void encoding() throws Exception {
+        // Like JenkinsRule.createSlave but passing a system encoding:
+        Slave remote = new DumbSlave("remote", tmp.getRoot().getAbsolutePath(),
+            new SimpleCommandLauncher("'" + System.getProperty("java.home") + "/bin/java' -Dfile.encoding=ISO-8859-2 -jar '" + new File(j.jenkins.getJnlpJars("slave.jar").getURL().toURI()) + "'"));
+        j.jenkins.addNode(remote);
+        j.waitOnline(remote);
+        WorkflowJob p = j.createProject(WorkflowJob.class, "p");
+        FilePath ws;
+        while ((ws = remote.getWorkspaceFor(p)) == null) {
+            Thread.sleep(100);
+        }
+        ws.child("message").write("Čau ty vole!\n", "ISO-8859-2");
+        p.setDefinition(new CpsFlowDefinition("node('remote') {if (isUnix()) {sh 'cat message'} else {bat 'type message'}}", true));
+        j.assertLogContains("Čau ty vole!", j.buildAndAssertSuccess(p));
+        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
+        j.assertLogContains("received: Čau ty vole!", j.buildAndAssertSuccess(p));
+        p.setDefinition(new CpsFlowDefinition("node('remote') {if (isUnix()) {sh script: 'cat message', encoding: 'US-ASCII'} else {bat script: 'type message', encoding: 'US-ASCII'}}", true));
+        j.assertLogContains("�au ty vole!", j.buildAndAssertSuccess(p));
+        ws.child("message").write("¡Čau → there!\n", "UTF-8");
+        p.setDefinition(new CpsFlowDefinition("node('remote') {if (isUnix()) {sh script: 'cat message', encoding: 'UTF-8'} else {bat script: 'type message', encoding: 'UTF-8'}}", true));
+        j.assertLogContains("¡Čau → there!", j.buildAndAssertSuccess(p));
+    }
+
     @Issue("JENKINS-34021")
     @Test public void deadStep() throws Exception {
         logging.record(DurableTaskStep.class, Level.INFO).record(CpsStepContext.class, Level.INFO).capture(100);