Error in mozbuild/config_status.py when MOZ_APP_DISPLAYNAME uses non-ASCII strings

RESOLVED FIXED in Firefox 42

Status

Firefox Build System
General
RESOLVED FIXED
3 years ago
2 months ago

People

(Reporter: rnewman, Assigned: glandium)

Tracking

Trunk
mozilla42
Unspecified
Android

Firefox Tracking Flags

(p11+, firefox42 fixed)

Details

MozReview Requests

Submitter Diff Changes Open Issues Last Updated
Loading...
Error loading review requests:

Attachments

(2 attachments)

(Reporter)

Description

3 years ago
Add this to a branding configure.sh:

MOZ_APP_DISPLAYNAME=ブラウザ

You'll get this build failure:

 1:39.35 UnicodeEncodeError: 'ascii' codec can't encode characters in position 518-521: ordinal not in range(128)

Updated

3 years ago
tracking-p11: --- → +
(Reporter)

Comment 1

3 years ago
Full stack: 

1:26.68 Reticulating splines...
 1:39.34 Traceback (most recent call last):
 1:39.34   File "./config.status", line 992, in <module>
 1:39.34     config_status(**args)
 1:39.34   File "/Users/rnewman/moz/hg/fx-team/python/mozbuild/mozbuild/config_status.py", line 149, in config_status
 1:39.34     summary = the_backend.consume(definitions)
 1:39.34   File "/Users/rnewman/moz/hg/fx-team/python/mozbuild/mozbuild/backend/base.py", line 200, in consume
 1:39.34     self.consume_finished()
 1:39.34   File "/Users/rnewman/moz/hg/fx-team/python/mozbuild/mozbuild/backend/recursivemake.py", line 789, in consume_finished
 1:39.34     self._check_blacklisted_variables(makefile_in, content)
 1:39.34   File "/usr/local/Cellar/python/2.7.9/Frameworks/Python.framework/Versions/2.7/lib/python2.7/contextlib.py", line 24, in __exit__
 1:39.34     self.gen.next()
 1:39.35   File "/Users/rnewman/moz/hg/fx-team/python/mozbuild/mozbuild/backend/base.py", line 277, in _write_file
 1:39.35     existed, updated = fh.close()
 1:39.35   File "/Users/rnewman/moz/hg/fx-team/python/mozbuild/mozbuild/backend/recursivemake.py", line 212, in close
 1:39.35     return self.fh.close()
 1:39.35   File "/Users/rnewman/moz/hg/fx-team/python/mozbuild/mozbuild/util.py", line 161, in close
 1:39.35     file.write(buf)
 1:39.35 UnicodeEncodeError: 'ascii' codec can't encode characters in position 518-521: ordinal not in range(128)

Comment 2

3 years ago
Created attachment 8636210 [details]
MozReview Request: Bug 1185671 - Support writing Unicode from FileAvoidWrite; r?glandium

Bug 1185671 - Support writing Unicode from FileAvoidWrite; r?glandium

This is a quick hack that solves the problem at hand. A better solution
is to make FileAvoidWrite instances strict with regards to str/unicode
types. I've attempted to make this change before and it opens a can of
worms, as various parts of the build system don't consistently send
str/unicode types to FileAvoidWrite instances. Assuming the only part
that fails is the final .write() of unicode to a file handle, this hack
is sufficient for making the write succeed.
Attachment #8636210 - Flags: review?(mh+mozilla)
(Reporter)

Updated

3 years ago
Assignee: nobody → gps
Status: NEW → ASSIGNED
(Assignee)

Updated

3 years ago
Attachment #8636210 - Flags: review?(mh+mozilla) → review+
(Assignee)

Comment 3

3 years ago
Comment on attachment 8636210 [details]
MozReview Request: Bug 1185671 - Support writing Unicode from FileAvoidWrite; r?glandium

https://reviewboard.mozilla.org/r/13669/#review12357

Ship it, BUT see upcoming patch first.
(Assignee)

Comment 4

3 years ago
Created attachment 8636926 [details] [diff] [review]
Use BytesIO instead of StringIO as backing store for FileAvoidWrite, and handle writing unicode to it explicitly

So, I wanted to check the claim in your commit message, and it didn't take much to get something that works and should be the long-term thing to do. This should also allow to remove http://hg.mozilla.org/mozilla-central/file/8e5c888d0d89/python/mozbuild/mozbuild/util.py#l170 but I didn't test that.
Attachment #8636926 - Flags: review?(gps)

Comment 5

3 years ago
Comment on attachment 8636926 [details] [diff] [review]
Use BytesIO instead of StringIO as backing store for FileAvoidWrite, and handle writing unicode to it explicitly

Review of attachment 8636926 [details] [diff] [review]:
-----------------------------------------------------------------

This is definitely a better solution.

I think my attempts at this uncovered bustage when pushed to Try. We should definitely test this before landing.

Also, I may have been too architecturally pure and didn't implement the automatic .encode('utf-8') when passed a unicode type. This would have led to tons of bustage.
Attachment #8636926 - Flags: review?(gps) → review+
(Assignee)

Comment 7

3 years ago
I'll land this when inbound reopens.
Assignee: gps → mh+mozilla
https://hg.mozilla.org/mozilla-central/rev/f592ff085124
Status: ASSIGNED → RESOLVED
Last Resolved: 3 years ago
status-firefox42: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla42
(Reporter)

Updated

3 years ago
See Also: → bug 1188550

Updated

2 months ago
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.