Closed
Bug 1480558
Opened 6 years ago
Closed 6 years ago
figure out how to make ICU's data file work with aarch64 windows
Categories
(Firefox Build System :: General, enhancement)
Tracking
(firefox63 fixed)
RESOLVED
FIXED
mozilla63
Tracking | Status | |
---|---|---|
firefox63 | --- | fixed |
People
(Reporter: froydnj, Assigned: froydnj)
References
(Blocks 1 open bug)
Details
Attachments
(4 files, 1 obsolete file)
1.62 KB,
patch
|
mshal
:
review+
|
Details | Diff | Splinter Review |
988 bytes,
patch
|
mshal
:
review+
|
Details | Diff | Splinter Review |
3.52 KB,
patch
|
mshal
:
review+
|
Details | Diff | Splinter Review |
798 bytes,
patch
|
froydnj
:
review+
|
Details | Diff | Splinter Review |
We generate a big data file for ICU, which we assume we can assembly with yasm or the GNU assembler. Neither one is a viable option on aarch64 windows, and we *probably* can't disable ICU entirely. So we need to find some kind of solution here.
Assignee | ||
Comment 1•6 years ago
|
||
Note that we cannot --without-intl-api our way to victory, because several places in the codebase depend on ICU's routines and have no fallback.
Comment 2•6 years ago
|
||
armasm64 might use INCBIN syntax to include binary. DATA_SYMBOL INCBIN icudt62l.dat END
Comment 3•6 years ago
|
||
In the worst case if you can't find a solution that works with the available assembler, you could generate an equivalent C file that contains the contents of the .dat file as an array of bytes.
Assignee | ||
Comment 4•6 years ago
|
||
Attachment #8998615 -
Flags: review?(core-build-config-reviews)
Assignee | ||
Comment 5•6 years ago
|
||
armasm64 doesn't accept the same options as its x86-ish counterparts, and passing options it doesn't understand causes assembly to fail. So let's just not pass any flags to the assembler for the moment.
Attachment #8998616 -
Flags: review?(core-build-config-reviews)
Assignee | ||
Comment 6•6 years ago
|
||
The assembler for this platform doesn't need the special handling AS_DASH_C_FLAG provides.
Attachment #8998617 -
Flags: review?(core-build-config-reviews)
Assignee | ||
Comment 7•6 years ago
|
||
yasm doesn't support aarch64, and trying to use GNU as with an MSVC build seems like sadness waiting to happen. Instead, we'll generate our own assembly file that armasm64 will accept.
Attachment #8998618 -
Flags: review?(core-build-config-reviews)
Comment 8•6 years ago
|
||
triaging, assigning to :froydnj since he attached patches
Assignee: nobody → nfroyd
Updated•6 years ago
|
Attachment #8998615 -
Flags: review?(core-build-config-reviews) → review+
Updated•6 years ago
|
Attachment #8998616 -
Flags: review?(core-build-config-reviews) → review+
Updated•6 years ago
|
Attachment #8998617 -
Flags: review?(core-build-config-reviews) → review+
Comment 9•6 years ago
|
||
Comment on attachment 8998618 [details] [diff] [review] part 4 - add icudata support for aarch64 windows >+import buildconfig Doesn't look like this gets used.
Attachment #8998618 -
Flags: review?(core-build-config-reviews) → review+
Assignee | ||
Comment 10•6 years ago
|
||
I thought TARGET_CPU would be more likely to have aarch64 in it, since target_cpu is set by old-configure/autoconf and we have such an old version of autoconf. However, TARGET_CPU apparently wasn't being set (I didn't notice on my AArch64 build because I have AS set), and that caused problems for "real" Windows builds. Actually looking at the code says that old-configure is perfectly willing to deal properly with aarch64-* targets, so we're good just adding a case here. Carrying over r+.
Attachment #9002751 -
Flags: review+
Assignee | ||
Updated•6 years ago
|
Attachment #8998615 -
Attachment is obsolete: true
Comment 11•6 years ago
|
||
Pushed by nfroyd@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/edc9c6244815 part 1 - set AS appropriately on aarch64 windows; r=mshal https://hg.mozilla.org/integration/mozilla-inbound/rev/7378a8999aea part 2 - don't add MOZ_DEBUG_FLAGS to ASFLAGS on aarch64 windows; r=mshal https://hg.mozilla.org/integration/mozilla-inbound/rev/90a93173d247 part 3 - unset AS_DASH_C_FLAG for aarch64 windows; r=mshal https://hg.mozilla.org/integration/mozilla-inbound/rev/8eac225243d8 part 4 - add icudata support for aarch64 windows; r=mshal
Comment 12•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/edc9c6244815 https://hg.mozilla.org/mozilla-central/rev/7378a8999aea https://hg.mozilla.org/mozilla-central/rev/90a93173d247 https://hg.mozilla.org/mozilla-central/rev/8eac225243d8
Status: NEW → RESOLVED
Closed: 6 years ago
status-firefox63:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla63
You need to log in
before you can comment on or make changes to this bug.
Description
•