Closed
Bug 1121132
Opened 10 years ago
Closed 10 years ago
A broken commit message can break builds
Categories
(Testing :: Mozbase, defect)
Testing
Mozbase
Tracking
(Not tracked)
RESOLVED
DUPLICATE
of bug 1146035
People
(Reporter: RyanVM, Unassigned)
Details
Attachments
(1 file, 2 obsolete files)
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!
Comment 1•10 years ago
|
||
Attachment #8548430 -
Flags: review?(jlund)
Comment 2•10 years ago
|
||
/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
Comment 3•10 years ago
|
||
I think this should fix it (according to [1]). Here's a try run:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=7be2d228d4ad
[1] https://docs.python.org/2/howto/unicode.html#reading-and-writing-unicode-data
Comment 4•10 years ago
|
||
(In reply to Andrew Halberstadt [:ahal] from comment #3)
> I think this should fix it (according to [1]). Here's a try run:
> https://treeherder.mozilla.org/#/jobs?repo=try&revision=7be2d228d4ad
>
> [1]
> 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).
Comment 5•10 years ago
|
||
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+
Comment 6•10 years ago
|
||
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?
Comment 7•10 years ago
|
||
(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 8•10 years ago
|
||
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-
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment hidden (Legacy TBPL/Treeherder Robot) |
Comment 21•10 years ago
|
||
Comment 22•10 years ago
|
||
Whoops... just uploaded this to the wrong bug. Can this be deleted?
Updated•10 years ago
|
Attachment #8554824 -
Attachment is obsolete: true
Updated•10 years ago
|
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → DUPLICATE
Comment 24•10 years ago
|
||
Attachment #8548430 -
Attachment is obsolete: true
Attachment #8619123 -
Flags: review-
Comment 25•10 years ago
|
||
You need to log in
before you can comment on or make changes to this bug.
Description
•