-
-
Notifications
You must be signed in to change notification settings - Fork 140
[JEP-206] Always use UTF-8 for the main Pipeline log file #89
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| </pluginRepositories> | ||
| <properties> | ||
| <jenkins.version>2.62</jenkins.version> | ||
| <jenkins.version>2.73.3</jenkins.version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unrelated, but easier to remember LTS baselines.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's not bump the baseline unless there's a need to - this limits who can take advantage of new plugin releases.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bumped only to the nearest LTS line. There is no reason for anyone to still be using intermediate weeklies. If they are, they need to update.
| WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "p"); | ||
| String message = "¡Čau → there!"; | ||
| p.setDefinition(new CpsFlowDefinition("echo '" + message + "'", true)); | ||
| r.assertLogContains(message, r.buildAndAssertSuccess(p)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On Windows (ISO-8859-1), this fails with mojibake with the charset = "UTF-8" line commented out, or with the upstream PR not integrated.
Test can be strengthened only after jenkinsci/workflow-job-plugin#89 is released, so we do not get into a cyclic dependency (or by rewriting this aspect of the test to save content into a file and read it from test code).
| <workflow-support-plugin.version>2.17</workflow-support-plugin.version> | ||
| <scm-api-plugin.version>2.1.1</scm-api-plugin.version> | ||
| <workflow-support-plugin.version>2.19-20180209.204154-1</workflow-support-plugin.version> <!-- TODO https://github.com/jenkinsci/workflow-support-plugin/pull/56 --> | ||
| <scm-api-plugin.version>2.2.6</scm-api-plugin.version> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something in the transitive deps needs it.
svanoort
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks OK to me, but as noted I want to test with the full stack.
|
"Without jenkinsci/workflow-durable-task-step-plugin#64, you will get mojibake, but possibly no worse than you already do." -- This worries me however. |
|
IOW: if you are silly and only update certain plugins, things will not work very well. OTOH things are already broken, as can be seen from the bug report. If you just update all, things will be fine. |
oleg-nenashev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It may cause regressions on not-UTF systems where encodings were somehow aligned between components. I do not actually believe that Jenkins core may work reliably on non-UTF-8 setups , hence 🐝
Again, only if you selectively update plugins, pending JENKINS-49651. |
svanoort
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems clean AFAICT. It'll go into release over the next few days once the rest of the batch of logging-encoding work is ready to release.
JEP-206
Subsumes #102. Downstream of jenkinsci/workflow-support-plugin#56. A tiny piece pulled out of #27 as one component of a solution to JENKINS-31096. Without jenkinsci/workflow-durable-task-step-plugin#64, you will get mojibake, but possibly no worse than you already do. Compare the attempt to do something similar for freestyle in jenkinsci/jenkins#3231.