Closed Bug 1121132 Opened 5 years ago Closed 5 years ago
A broken commit message can break builds
MozReview Request: Bug 1121132 - script.py's write_to_file should use codec.open for unicode support, r=jlund
39 bytes, text/x-review-board-request
The commit: https://hg.mozilla.org/integration/fx-team/rev/291e3a83a122 The bustage: https://treeherder.mozilla.org/logviewer.html#?job_id=1680280&repo=fx-team Apparently mozharness doesn't like × in a commit message!
/r/2459 - Bug 1121132 - script.py's write_to_file should use codec.open for unicode support, r=jlund Pull down this commit: hg pull review -r c8963cc8dbb654118618d041345e90b8bf410540
I think this should fix it (according to ). Here's a try run: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7be2d228d4ad  https://docs.python.org/2/howto/unicode.html#reading-and-writing-unicode-data
(In reply to Andrew Halberstadt [:ahal] from comment #3) > I think this should fix it (according to ). Here's a try run: > https://treeherder.mozilla.org/#/jobs?repo=try&revision=7be2d228d4ad > >  > https://docs.python.org/2/howto/unicode.html#reading-and-writing-unicode-data FYI, the original commit message that caused bustage on fx-team worked when pushed to try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f9671b6be4cd (test failures unrelated).
https://reviewboard.mozilla.org/r/2457/#review1731 ah, this reminds me, I hadn't followed up with: https://bugzilla.mozilla.org/show_bug.cgi?id=1106342 I guess unicode characters are more popular than I thought :) in that bug I used a similar approach which was r- by catlee. I agree it is probably safer to add a parameter to write_to_file and that encodes only when we want: e.g. for a commit message. feel free to leave with me or else add the option to write_to_file and I'll r+
Heh, originally I had added an encoding parameter to that function, but then noticed that elsewhere in script.py we just use 'utf-8' directly, so decided to follow that convention: https://dxr.mozilla.org/build:mozharness/source/mozharness/base/script.py#1343 Is what you're proposing simply to add an encoding='utf-8' to the method signature?
(In reply to Andrew Halberstadt [:ahal] from comment #6) > Heh, originally I had added an encoding parameter to that function, but then > noticed that elsewhere in script.py we just use 'utf-8' directly, so decided > to follow that convention: > https://dxr.mozilla.org/build:mozharness/source/mozharness/base/script. > py#1343 which might be safe to follow that convention. poke catlee if you feel strongly about this. I suspect his concerns are the unknown outcome of switching all the current working instances where we call write_to_file without encoding open() > > Is what you're proposing simply to add an encoding='utf-8' to the method > signature? we want encoding passed but we want the default to be None (i.e. encoding=None in write_to_file meth sig). Then for the commit message, explicitly pass encoding='utf-8'. at least that's my interpretation of https://bugzilla.mozilla.org/show_bug.cgi?id=1106342#c1
Comment on attachment 8548430 [details] MozReview Request: bz://1121132/ahal putting r- to clear my bugzilla queue while the patch is iterated on. if you feel strongly about defaulting to utf-8 over None, I don't want to block but you should pass it by catlee since he r- me on it.
Attachment #8548430 - Flags: review?(jlund) → review-
Whoops... just uploaded this to the wrong bug. Can this be deleted?
Attachment #8554824 - Attachment is obsolete: true
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1146035
You need to log in before you can comment on or make changes to this bug.