Closed Bug 1283611 Opened 8 years ago Closed 8 years ago

add NDK r13 support

Categories

(Firefox Build System :: General, defect)

All
Android
defect
Not set
normal

Tracking

(firefox50 affected, firefox55 fixed)

RESOLVED FIXED
mozilla55
Tracking Status
firefox50 --- affected
firefox55 --- fixed

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)!
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)
Attachment #8766923 - Flags: review?(nfroyd) → review+
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.
https://reviewboard.mozilla.org/r/61642/#review58578 Hmm, it's still not letting me trigger a try build. Any idea why?
(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.
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)
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
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla55
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: