Closed
Bug 1283611
Opened 8 years ago
Closed 8 years ago
add NDK r13 support
Categories
(Firefox Build System :: General, defect)
Tracking
(firefox50 affected, firefox55 fixed)
RESOLVED
FIXED
mozilla55
People
(Reporter: froydnj, Assigned: m_kato)
Details
Attachments
(2 files)
I've been filing bugs on the Android NDK over at Github, and a person from the NDK team, Dan Albert, has dove in and wrote some patches to make Fennec compile with NDK r13 (beta coming later this summer, release in Q3)!
Comment 1•8 years ago
|
||
The order of these includes is important with the updated libc++ in
NDK r13. libc++ now includes a math.h that needs to be able to
include_next the same and find definitions for all the math
functions. libandroid_support also needs to include_next. The previous
include order resulted in the following resolution when math.h was
included:
1. libandroid_support's math.h
2. libc++'s math.h
3. libc's math.h.
We enter 2 at the top of 1, so when we continue processing 2 after
including 1, we haven't picked up all the definitions we need from 1
yet, and the build fails because we're missing a huge portion of the
math definitions.
Moving the -I later gets us the resolution order we need:
1. libc++'s math.h
2. libandroid_support's math.h
3. libc's math.h
Review commit: https://reviewboard.mozilla.org/r/61642/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/61642/
Attachment #8766923 -
Flags: review?(nfroyd)
Reporter | ||
Updated•8 years ago
|
Attachment #8766923 -
Flags: review?(nfroyd) → review+
Reporter | ||
Comment 2•8 years ago
|
||
Comment on attachment 8766923 [details]
Bug 1283611 - Add support for NDK r13's libc++ paths.
https://reviewboard.mozilla.org/r/61642/#review58578
Thanks for the explanation.
Comment 3•8 years ago
|
||
https://reviewboard.mozilla.org/r/61642/#review58578
Hmm, it's still not letting me trigger a try build. Any idea why?
Reporter | ||
Comment 4•8 years ago
|
||
(In reply to Dan Albert from comment #3)
> https://reviewboard.mozilla.org/r/61642/#review58578
>
> Hmm, it's still not letting me trigger a try build. Any idea why?
Sorry about the PTO-induced latency here. I asked around, and folks said that following the steps here:
https://mozilla-version-control-tools.readthedocs.io/en/latest/mozreview/install.html#manually-associating-your-ldap-account-with-mozreview
should enable you to push try builds from inside mozreview. For generating a Bugzilla API key, you'll want to select "Preferences" from the dropdown menu on the upper right of any Bugzilla page, go to the "API Keys" sub-menu, and then generate a new API key according to the bits there.
You'll also want to set up ~/.ssh/config for reviewboard-hg.mozilla.org.
Let me know if you have any more questions.
Comment hidden (mozreview-request) |
Assignee | ||
Comment 6•8 years ago
|
||
When using NDK r13+ with part 1, the following build error still occurs when using cmath.
0:21.01 /mozilla/android-ndk-r14b/sources/cxx-stl/llvm-libc++/include/math.h:661:105: error: 'acosl' was not declared in this scope
0:21.01 inline _LIBCPP_INLINE_VISIBILITY long double acos(long double __lcpp_x) _NOEXCEPT {return acosl(__lcpp_x);}
To fix this, we need change the order of include path.
Assignee: nobody → m_kato
Attachment #8851302 -
Flags: review?(nfroyd)
Assignee | ||
Comment 7•8 years ago
|
||
Reporter | ||
Comment 8•8 years ago
|
||
Comment on attachment 8851302 [details] [diff] [review]
Change order include path to support NDK r13+
Review of attachment 8851302 [details] [diff] [review]:
-----------------------------------------------------------------
r=me assuming this still works with the NDK we use.
Attachment #8851302 -
Flags: review?(nfroyd) → review+
Pushed by m_kato@ga2.so-net.ne.jp:
https://hg.mozilla.org/integration/mozilla-inbound/rev/7de73b4bbf32
Add support for NDK r13's libc++ paths. r=froydnj
https://hg.mozilla.org/integration/mozilla-inbound/rev/17052b6a31bf
Change order of include path to support NDK r13+ r=froydnj
Comment 10•8 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/7de73b4bbf32
https://hg.mozilla.org/mozilla-central/rev/17052b6a31bf
Status: NEW → RESOLVED
Closed: 8 years ago
status-firefox55:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Updated•7 years ago
|
Product: Core → Firefox Build System
You need to log in
before you can comment on or make changes to this bug.
Description
•