Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[META] Build with Clang (Android) #20342

Closed
dulmandakh opened this issue Jul 23, 2018 · 6 comments
Closed

[META] Build with Clang (Android) #20342

dulmandakh opened this issue Jul 23, 2018 · 6 comments
Labels
Platform: Android Android applications. Resolution: Locked This issue was locked by the bot. Type: Discussion Long running discussion.

Comments

@dulmandakh
Copy link
Contributor

dulmandakh commented Jul 23, 2018

For Discussion

The purpose of this issue is to create a "single point of truth" regarding the efforts related to building React Native using Clang and against libc++, in order to coordinate and recap all the efforts connected to this subject.

Background

Android NDK is about to drop GCC and GNU STL support in favor of Clang and libc++ in version 18. Below are notable changes in NDK

Version 11

  • GCC in the NDK is now deprecated in favor of Clang. We strongly recommend switching to Clang.

Version 13

  • GCC is no longer supported. It will not be removed from the NDK just yet, but is no longer receiving backports. It cannot be removed until after libc++ has become stable enough to be the default, as some parts of gnustl are still incompatible with Clang. It will likely be removed after that point.
  • NDK_TOOLCHAIN_VERSION now defaults to Clang.

Version 14

  • This release ends active support for GCC

Version 16

  • libc++ is out of beta and is now the preferred STL in the NDK. Starting in r17, libc++ is the default STL for CMake and standalone toolchains. If you manually selected a different STL, we strongly encourage you to move to libc++. For more details, see this blog post.

Version 17

  • GCC is no longer supported. It will be removed in NDK r18.
  • libc++ is now the default STL for CMake and standalone toolchains
    gnustl and stlport are deprecated and will be removed in NDK r18.

Documentations

TODO

  • Use CMake to build C++ code
  • Upgrade Folly ([META] Upgrading Folly #20302)
  • Compile RN using Clang
  • Compile JSC and RN using Clang against libc++, because gnustl and libc++ ABI are incompatible

How to

Compile with Clang

Open ReactAndroid/src/main/jni/Application.mk and remove or comment out NDK_TOOLCHAIN_VERSION

Compile with against libc++

Open ReactAndroid/src/main/jni/Application.mk and set APP_STL := c++_shared

Related issues

Related PRs

@dulmandakh
Copy link
Contributor Author

I was able to compile RN with NDK r17, and will create a PR once #19945 is merged.

@dulmandakh dulmandakh added Core Team Platform: Android Android applications. labels Jul 23, 2018
@react-native-bot react-native-bot added the Type: Discussion Long running discussion. label Jul 23, 2018
@gengjiawen
Copy link
Contributor

Amazing job,thanks.

@dulmandakh
Copy link
Contributor Author

dulmandakh commented Jul 25, 2018

If I understood correctly, iOS builds with clang agains libc++ and with almost the same native dependencies. So it won't be a big problem to build with Clang, but to use libc++ we may need to build JSC against it. Please correct me if wrong

@dulmandakh dulmandakh changed the title [META] Build with Clang [META] Build with Clang (Android) Jul 25, 2018
@dulmandakh
Copy link
Contributor Author

I'm not that proficient in C++ and don't know much about RN internals, therefore cannot make changes to C++ code to make it compile with Clang. @hramos could you please find someone from Facebook to help with it.

@dulmandakh
Copy link
Contributor Author

NDK r17b landed 6117a6c

facebook-github-bot pushed a commit to facebook/folly that referenced this issue Oct 19, 2018
Summary:
According to this android/ndk#647,
  posix_memalign may not exist on Android API 16.
  From Android NDK r17c, the API exists for Android API 17+.
```
    #if __ANDROID_API__ >= 17
    int posix_memalign(void** __memptr, size_t __alignment, size_t __size) __INTRODUCED_IN(17);
    #endif /* __ANDROID_API__ >= 17 */
```
  Change the code to use posix_memalign only after Android API 17+.
  This would also fix issue for OSS React Native to pack latest folly and building with clang.
  See: facebook/react-native#20302 and facebook/react-native#20342
Pull Request resolved: #953

Reviewed By: yfeldblum

Differential Revision: D10469757

Pulled By: Orvid

fbshipit-source-id: c63838f3f6e723ef3de77187f39597a4063043db
@dulmandakh
Copy link
Contributor Author

RN master now use clang

sandraiser pushed a commit to sandraiser/folly that referenced this issue Mar 4, 2019
Summary:
According to this android/ndk#647,
  posix_memalign may not exist on Android API 16.
  From Android NDK r17c, the API exists for Android API 17+.
```
    #if __ANDROID_API__ >= 17
    int posix_memalign(void** __memptr, size_t __alignment, size_t __size) __INTRODUCED_IN(17);
    #endif /* __ANDROID_API__ >= 17 */
```
  Change the code to use posix_memalign only after Android API 17+.
  This would also fix issue for OSS React Native to pack latest folly and building with clang.
  See: facebook/react-native#20302 and facebook/react-native#20342
Pull Request resolved: facebook#953

Reviewed By: yfeldblum

Differential Revision: D10469757

Pulled By: Orvid

fbshipit-source-id: c63838f3f6e723ef3de77187f39597a4063043db
@facebook facebook locked as resolved and limited conversation to collaborators Jan 23, 2020
@react-native-bot react-native-bot added the Resolution: Locked This issue was locked by the bot. label Jan 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Platform: Android Android applications. Resolution: Locked This issue was locked by the bot. Type: Discussion Long running discussion.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants