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

jglick · 2018-02-12T22:21:46Z

Extracted from #21. Downstream of jenkinsci/durable-task-plugin#61. Also downstream of jenkinsci/workflow-job-plugin#89 for testing, but really as a user you want to update workflow-job as well, unless your master happens to be running in UTF-8.

jglick · 2018-02-13T19:11:56Z

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


    private boolean returnStdout;
-    private String encoding = DurableTaskStepDescriptor.defaultEncoding;
+    private String encoding;


Note that this is a behavioral change—we are changing the default value of encoding from UTF-8 to null (i.e., use the node’s system encoding). Also the encoding now applies to regular streamed output, whereas previously it only mattered for returnStdout: true. (Which is the main point of this whole exercise: JENKINS-31096.)

I tried to think of a way to keep the previous default but it was just going to be ugly. With the change of the overall build log to UTF-8, it would mean that you would get mojibake in case you were running, say, entirely on CP-1252 Windows (with or without agents) and just did something simple like

node { bat 'something producing Western European characters…' }

which seems wrong.

This interpretation of encoding is also consistent with the readFile and writeFile steps, as well as the behavior of freestyle builds (which define the entire build’s log encoding by the node’s system encoding), and I think would be considered most intuitive going forward. Otherwise we would need a special value ('', 'system', …) indicating system default encoding, and then try to explain that in the snippet generator.

Obviously 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 make that explicit with encoding: 'UTF-8'.

Obviously 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 make that explicit with encoding: 'UTF-8'.

🐛 This should be called out really explicitly in the documentation / UI here.

Yeah I can emphasize that.

Seems reasonable. Whatever is done with encodings in Durable Task and this plugin, it is a risky change anyway. Since the idea is to NOT enforce the UTF encoding, the current approach looks preferable

svanoort · 2018-02-13T22:18:17Z

pom.xml

    </pluginRepositories>
    <properties>
-        <jenkins.version>2.60.3</jenkins.version>
+        <jenkins.version>2.73.3</jenkins.version>


Is this gratuitous or do we need a new API?

Test dep on workflow-job requires this.

svanoort

Okay, so help me out here Jesse: is there a way we can execute these PRs so users don't have to update the plugins all in lockstep in order not to result in some mangled output?

Or am I misreading your comments here / in the upstream?

jglick · 2018-02-19T17:36:08Z

is there a way we can execute these PRs so users don't have to update the plugins all in lockstep in order not to result in some mangled output?

No, as noted elsewhere you need to update both Pipeline: Job and Pipeline: Nodes and Processes. But if you care about Unicode, you probably have mangled output already, so 🤷‍♂️

jglick · 2018-02-19T19:00:06Z

and bye-bye @rsandell

oleg-nenashev · 2018-04-02T13:41:57Z

Jenkinsfile

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


2.107.1 please

oleg-nenashev · 2018-04-02T13:42:13Z

pom.xml

        <groupId>org.jenkins-ci.plugins</groupId>
        <artifactId>plugin</artifactId>
-        <version>3.2</version>
+        <version>3.4</version>


3.6 if 2.107.1 is added to CI

oleg-nenashev · 2018-04-02T14:25:47Z

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


    private boolean returnStdout;
-    private String encoding = DurableTaskStepDescriptor.defaultEncoding;
+    private String encoding;


Seems reasonable. Whatever is done with encodings in Durable Task and this plugin, it is a risky change anyway. Since the idea is to NOT enforce the UTF encoding, the current approach looks preferable

oleg-nenashev · 2018-04-02T14:26:58Z

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

-        public static final String defaultEncoding = "UTF-8";
-
        public FormValidation doCheckEncoding(@QueryParameter boolean returnStdout, @QueryParameter String encoding) {
+            if (encoding.isEmpty()) {


StringUtils.isBlank()? otherwise the behavior will differ from Util.fixEmpty
🐜

This is a web method; the text field may be blank but not missing.

oleg-nenashev · 2018-04-02T14:27:36Z

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

                durableTask.captureOutput();
            }
+            if (step.encoding != null) {
+                durableTask.charset(Charset.forName(step.encoding));


Catch/rethrow exception with more debug details?

Which debug details are you looking for here?

jglick · 2018-06-11T13:10:00Z

Test failure looks like a flake.

carlossg · 2018-06-20T15:47:55Z

@svanoort can you check the comments addressing your review ?

svanoort · 2018-06-21T12:49:28Z

@carlossg I'm mostly booked up for the week due to the Java 10 hackathon, but will take a look as soon as we're out of that, thanks.

svanoort

AFAICT this is fine -- although we'll probably have some issues from the incompatible aspects of this, at least it moves in the right direction.

svanoort · 2018-07-12T02:13:24Z

@jglick Although I can't tell if that test failure is a flake or actual issue here?

jglick · 2018-07-17T01:32:05Z

I can't tell if that test failure is a flake or actual issue

IIRC there were subsuming PRs which did not fail, strongly suggesting a flake. I can try to retrigger the build.

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

[JENKINS-31096] Gather command output in a local encoding.

dec68db

jglick requested review from abayer, oleg-nenashev, rsandell and svanoort February 12, 2018 22:21

jglick mentioned this pull request Feb 12, 2018

[JEP-206] Always use UTF-8 for the main Pipeline log file jenkinsci/workflow-job-plugin#89

Merged

abayer approved these changes Feb 12, 2018

View reviewed changes

Cannot downgrade Jenkins in tests!

6f2bf2e

jglick mentioned this pull request Feb 13, 2018

Watching plus Unicode #65

Closed

jglick commented Feb 13, 2018

View reviewed changes

svanoort reviewed Feb 13, 2018

View reviewed changes

jglick mentioned this pull request Feb 16, 2018

[JEP-206] Always use UTF-8 for per-step log files jenkinsci/workflow-support-plugin#56

Merged

svanoort requested changes Feb 16, 2018

View reviewed changes

jglick added 3 commits February 19, 2018 13:39

Merge branch 'master' into UTF-8-JENKINS-31096

23a1dde

Bump.

1f4493f

Emphasizing that specifying an explicit encoding is wise.

0dcf951

jglick requested review from svanoort and removed request for rsandell February 19, 2018 18:59

svanoort requested a review from rsandell February 23, 2018 22:17

rsandell approved these changes Mar 2, 2018

View reviewed changes

oleg-nenashev approved these changes Apr 2, 2018

View reviewed changes

jglick added 4 commits June 7, 2018 18:46

Merge branch 'incrementals' into UTF-8-JENKINS-31096

35b0234

Review comments.

36722de

Merge branch 'master' into UTF-8-JENKINS-31096

75181a5

Restricting doCheckEncoding.

54b2187

jglick requested a review from dwnusbaum June 22, 2018 21:38

svanoort approved these changes Jul 12, 2018

View reviewed changes

jglick closed this Jul 17, 2018

jglick reopened this Jul 17, 2018

jglick changed the title ~~[JENKINS-31096] Gather command output in a local encoding~~ [JEP-206] Gather command output in a local encoding Aug 6, 2018

jglick and others added 2 commits August 7, 2018 11:44

Updated parent and incremental deps.

8c763a4

Consume released workflow-support and durable-task plugins

d3508c3

svanoort merged commit 0220678 into jenkinsci:master Aug 7, 2018

jglick deleted the UTF-8-JENKINS-31096 branch August 7, 2018 21:54

orgi pushed a commit to orgi/workflow-durable-task-step-plugin that referenced this pull request Nov 13, 2018

Merge pull request jenkinsci#64 from jglick/UTF-8-JENKINS-31096

863f9b0

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

smerle33 mentioned this pull request Mar 10, 2025

Windows agents have different character set than in the past jenkins-infra/helpdesk#4568

Closed

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

Uh oh!

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

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

Uh oh!

Conversation

jglick commented Feb 12, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

svanoort left a comment

Choose a reason for hiding this comment

Uh oh!

jglick commented Feb 19, 2018

Uh oh!

jglick commented Feb 19, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jglick commented Jun 11, 2018

Uh oh!

carlossg commented Jun 20, 2018

Uh oh!

svanoort commented Jun 21, 2018

Uh oh!

svanoort left a comment

Choose a reason for hiding this comment

Uh oh!

svanoort commented Jul 12, 2018

Uh oh!

jglick commented Jul 17, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

jglick commented Feb 12, 2018 •

edited

Loading