1283611 - add NDK r13 support

Status: RESOLVED FIXED

(Firefox Build System :: General, defect)

Description

[Nathan Froyd [:froydnj]]

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

[Dan Albert]

Attachment: Bug 1283611 - Add support for NDK r13's libc++ paths.

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/

Comment 2

[Nathan Froyd [:froydnj]]

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

[Dan Albert]

https://reviewboard.mozilla.org/r/61642/#review58578

Hmm, it's still not letting me trigger a try build. Any idea why?

Comment 4

[Nathan Froyd [:froydnj]]

(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 5

[Dan Albert]

Comment on attachment 8766923 [details]
Bug 1283611 - Add support for NDK r13's libc++ paths.

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/61642/diff/1-2/

Comment 6

[Makoto Kato [:m_kato]]

Attachment: Change order include path to support NDK r13+

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.

Comment 7

[Makoto Kato [:m_kato]]

https://treeherder.mozilla.org/#/jobs?repo=try&revision=cbef4a41328833aeafeeeb90638da85d64cc6992

Comment 8

[Nathan Froyd [:froydnj]]

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.

Comment 9

[Pulsebot]

Pushed by [e-mail]:
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

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/7de73b4bbf32
https://hg.mozilla.org/mozilla-central/rev/17052b6a31bf