[META] Upgrading Folly #20302

hramos · 2018-07-19T22:00:37Z

For Discussion

The Folly version used in open source is very old. We're looking for volunteers interested in making the necessary changes to upgrade Folly in React Native.

Known Incompatibilities

This is in no way an exhaustive list, but be aware you'll need to tackle the following when upgrading Folly:

folly::Optional's has_value() doesn't exist
No template named 'Optional' in namespace 'folly' stuff

The text was updated successfully, but these errors were encountered:

fkgozali · 2018-07-22T00:31:00Z

to upgrade Folly in React Native

Ideally, we'd want to pull the latest folly directly from https://github.com/facebook/folly - or at least use the latest version for each RN release - automatically.

This will involve updating/changing the integration with CocoaPods + gradle that react-native repo has today, e.g. making them run the build scripts already provided by folly repo, instead of making their own wrapper scripts / build steps.

dulmandakh · 2018-07-29T15:02:46Z

I would like to volunteer to upgrade Folly in Android, and I think this experience will help me better learn C++.

Also it might be better to prepare RN for Clang and libc++ support, as it'll be the only compiler and STL in next NDK release. I tried to build RN with Clang and got many warnings, errors in Folly. So I think that it might be better to upgrade C++ or native tools beforehand, maybe merge #20357 which would land latest Android NDK version.

dulmandakh · 2018-07-31T12:27:51Z

maybe it's better to switch to CMake to build jni code on Android, because boost, glob and folly supports it, but also many C++ libraries.

gengjiawen · 2018-08-01T14:49:26Z

@dulmandakh Good idea. And with it we can also have better IDE support.

yinhangfeng · 2018-08-02T03:27:23Z

I have tried building with cmake
https://github.com/yinhangfeng/react-native/blob/0.53-stable-cmake/ReactAndroid/CMakeLists.txt
maybe have some reference value
i don't understand c++ but this does work.

for the convenience of testing I put all the libraries in ReactAndroid/third-party-ndk
Folly version 2018.02.05.00

problem:

c++_shared conflict with jsc-android-buildscripts
https://github.com/yinhangfeng/react-native/blob/0.53-stable-cmake/ReactAndroid/build.gradle#L255
JSC_JSGlobalContextEnableDebugger does not exist https://github.com/yinhangfeng/react-native/blob/0.53-stable-cmake/ReactCommon/cxxreact/JSCExecutor.cpp#L224

dulmandakh · 2018-08-06T15:48:37Z

@yinhangfeng could you please try to build master with CMake as is, or without bumping libraries. Once finished it will be easy to bump libraries to latest versions. Thank you.

Kudo · 2018-10-19T14:31:51Z

I am going to upgrade folly and build the code with clang.
So far the build is passed with folly's patch facebook/folly#953.
But there is still a bottleneck for runtime crash, that current prebuilt android-jsc (libjsc.so and libicu_common.so) are still built by GCC.
Both original JSC on jcenter and current upstream one are linked to libgnustl_shared.so.

It seems that the community maintained android-jsc builder could build JSC with clang.
I would clarify if there are any plans to upgrade android-jsc that built by clang.
or maybe @gengjiawen could share the steps that how you build the android-jsc at #18754.

dulmandakh · 2018-10-19T15:08:21Z

I think that it might be better to start with clang then upgrade folly, because clang is default toolchain for XCode so we'll have identical toolchains for Folly upgrade. But it's your choice, good luck 👍

Kudo · 2018-10-19T16:16:33Z

I've tried before and the old folly does not support NDK with clang build.
This issue relates to this facebook/folly#75.
That's why I upgrade folly directly.

Kudo · 2018-10-19T17:05:09Z

Just create #21863 to show my changes in case if someone has interests.

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

Summary: Fixes #20302 (For iOS) Note: ------ 1. Checked the changes did not break CocoaPods integration. 2. The change for glog copying header into exported/ is to prevent build break for folly. `folly/detail/Demangle.h` will try to use libstdc++'s demangle.h. Unfortunately, glog also has a demangle.h in source code. So I copy exported headers and only search headers in exported/ folder during build. Pull Request resolved: #21976 Reviewed By: hramos Differential Revision: D12818131 Pulled By: fkgozali fbshipit-source-id: b3c637d09d1b3adde0ea15c82eb56e28f846885b

fkgozali · 2018-10-29T19:49:24Z

I've merged both #21976 and #21977 - this should get us moving for the time being. However, I'd like to see if we can come up with a more scalable/automated way of upgrading Folly. For instance, right now we list out relevant files manually like so:

https://github.com/facebook/react-native/pull/21977/files#diff-ae2383f344153a01ce1b6130b453a8e1R5

This list usually changes release by release (not always, tho). I think to make it more maintainable (e.g. if we decide to upgrade it again 1 year from now), it would be great if somehow we can use whatever configuration folly repo already has. Perhaps there's no specific cmake/config for mobile builds atm, but maybe that's one way to approach this:

introduce mobile-only folly build config in folly repo
adjust gradle/cocoapods/xcode to execute such build script in the build step
all RN configs need to do is to point to a specific folly release

This will make upgrades much easier in the future. Thoughts?

fkgozali · 2018-10-29T20:06:17Z

Btw @Kudo, thank you for the PRs! They should unblock us for making Fabric C++ code compilable with gradle/cocoapods/xcode.

Kudo · 2018-10-30T18:59:14Z

@fkgozali Thank you for the review and landing.

Just for your information, in the Android PR, I wrote a folly patch in gradle to support gcc 4.9.
The patch works due to RN only includes partial folly library.
Unfortunately, folly does not support gcc 4.9 now and NDK only support gcc to 4.9.
We should use clang to support folly build, in case if we are going to import other parts of folly.

I am working on #21863 and hopefully we could use clang build soon.

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

cpojer · 2019-03-19T12:49:31Z

Just wanted to ask what is the status of this issue? Are we still looking for somebody to figure out a way to make folly upgrades easier in the future or are we alright with the current state of things?

gengjiawen · 2019-03-19T12:55:41Z

@cpojer I think you can close it for now. Folly has been upgraded. Upgrade easier or migrate to CMake is another issue.

Summary: Fixes facebook#20302 (For Android) Note: ------ 1. New folly will have build break for a gcc-4.9 and gcc-4.9 seems to be deprecated for latest folly. As we only use partial folly implementations, I just fixed the build break part. To support building RN on Windows, the patches are written by gradle ReplaceTokens. 2. The change for glog copying header into exported/ is to prevent build break for folly. `folly/detail/Demangle.h` will try to use libstdc++'s demangle.h. Unfortunately, glog also has a demangle.h in source code. So I copy exported headers and only search headers in exported/ folder during build. Pull Request resolved: facebook#21977 Reviewed By: hramos Differential Revision: D12818133 Pulled By: fkgozali fbshipit-source-id: 2c1f6f012663204581a86141d0c9ed0eb9d8c698

Summary: Fixes facebook#20302 (For iOS) Note: ------ 1. Checked the changes did not break CocoaPods integration. 2. The change for glog copying header into exported/ is to prevent build break for folly. `folly/detail/Demangle.h` will try to use libstdc++'s demangle.h. Unfortunately, glog also has a demangle.h in source code. So I copy exported headers and only search headers in exported/ folder during build. Pull Request resolved: facebook#21976 Reviewed By: hramos Differential Revision: D12818131 Pulled By: fkgozali fbshipit-source-id: b3c637d09d1b3adde0ea15c82eb56e28f846885b

Summary: Fixes facebook#20302 (For Android) Note: ------ 1. New folly will have build break for a gcc-4.9 and gcc-4.9 seems to be deprecated for latest folly. As we only use partial folly implementations, I just fixed the build break part. To support building RN on Windows, the patches are written by gradle ReplaceTokens. 2. The change for glog copying header into exported/ is to prevent build break for folly. `folly/detail/Demangle.h` will try to use libstdc++'s demangle.h. Unfortunately, glog also has a demangle.h in source code. So I copy exported headers and only search headers in exported/ folder during build. Pull Request resolved: facebook#21977 Reviewed By: hramos Differential Revision: D12818133 Pulled By: fkgozali fbshipit-source-id: 2c1f6f012663204581a86141d0c9ed0eb9d8c698

hramos added Help Wanted Issues ideal for external contributors. Type: Discussion Long running discussion. labels Jul 19, 2018

dulmandakh mentioned this issue Aug 8, 2018

[META] Build with Clang (Android) #20342

Closed

Kudo mentioned this issue Oct 19, 2018

Fix posix_memalign symbol not found with Android API 16 facebook/folly#953

Closed

Kudo mentioned this issue Oct 19, 2018

[ANDROID] [ENHANCEMENT] [build] - Use clang to build Android libraries #21863

Closed

1 task

This was referenced Oct 27, 2018

Upgrade folly to v2018.10.22.00 for iOS #21976

Closed

Upgrade folly to v2018.10.22.00 for Android #21977

Closed

facebook-github-bot closed this as completed in a316dc6 Oct 29, 2018

fkgozali reopened this Oct 29, 2018

hramos changed the title ~~Upgrade Folly~~ [META] Upgrading Folly Dec 19, 2018

fkgozali mentioned this issue Jan 15, 2019

[React Fabric] Updating Build Configuration (CocoaPods, .xcodeproj, gradle) #22989

Closed

cpojer mentioned this issue Mar 19, 2019

React Native London Issue Triage #24032

Closed

cpojer added the Follow Up label Mar 19, 2019

cpojer closed this as completed Mar 19, 2019

facebook locked as resolved and limited conversation to collaborators Mar 19, 2020

react-native-bot added the Resolution: Locked This issue was locked by the bot. label Mar 19, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[META] Upgrading Folly #20302

[META] Upgrading Folly #20302

hramos commented Jul 19, 2018

fkgozali commented Jul 22, 2018

dulmandakh commented Jul 29, 2018

dulmandakh commented Jul 31, 2018

gengjiawen commented Aug 1, 2018

yinhangfeng commented Aug 2, 2018

dulmandakh commented Aug 6, 2018

Kudo commented Oct 19, 2018

dulmandakh commented Oct 19, 2018 via email

Kudo commented Oct 19, 2018

Kudo commented Oct 19, 2018

fkgozali commented Oct 29, 2018

fkgozali commented Oct 29, 2018

Kudo commented Oct 30, 2018

cpojer commented Mar 19, 2019

gengjiawen commented Mar 19, 2019

[META] Upgrading Folly #20302

[META] Upgrading Folly #20302

Comments

hramos commented Jul 19, 2018

For Discussion

Known Incompatibilities

fkgozali commented Jul 22, 2018

dulmandakh commented Jul 29, 2018

dulmandakh commented Jul 31, 2018

gengjiawen commented Aug 1, 2018

yinhangfeng commented Aug 2, 2018

dulmandakh commented Aug 6, 2018

Kudo commented Oct 19, 2018

dulmandakh commented Oct 19, 2018 via email

Kudo commented Oct 19, 2018

Kudo commented Oct 19, 2018

fkgozali commented Oct 29, 2018

fkgozali commented Oct 29, 2018

Kudo commented Oct 30, 2018

cpojer commented Mar 19, 2019

gengjiawen commented Mar 19, 2019