-
Notifications
You must be signed in to change notification settings - Fork 24.3k
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
add x86_64 arm64-v8a support. Fixes #2814 #18754
Conversation
b617f49
to
668204c
Compare
any progress about this support? |
This comment has been minimized.
This comment has been minimized.
@hramos Can you help on this ? |
I'm not the right person to review this, sorry. |
@hramos maybe you know the right person? |
668204c
to
fc3ea39
Compare
This comment has been minimized.
This comment has been minimized.
f5e579d
to
cba56df
Compare
@mdvacca Any thought to merge this or wait for user-specific jsc support ? |
We have been stuck with #2814 too long, maybe merge this first. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hramos is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
Looks like take too long to import. @hramos |
One quick question about this. How can we guaranty that there is no keylogger or bitcoin mining program inside the binary and it is doing only what it is suppose to do? |
get the so to any virus scan software or push fb to merge my pr to android-jsc. |
The import failed due to a binary size limit. I've whitelisted the PR and I am trying to land this again. Please do not add any commits, to ensure the land is not cancelled. |
Summary: add arm64 support, related issue : #2814. If we are okay with binary aar android-jsc, then the pr can be directly merged. Otherwise merge facebookarchive/android-jsc#30 first and do a new release. RNTester all variant works. You can also test the apk from here: https://github.com/gengjiawen/react-native/releases/tag/v0.56beta. facebookarchive/android-jsc#30. [ANDROID] [ENHANCEMENT] [ABI] - add x86_64 arm64-v8a support. Differential Revision: D9491481 Pulled By: hramos fbshipit-source-id: d6ec6992768eb0c0866a0317273e09fae5b8935e
@gengjiawen. So I generate a new project using
Pretty sure this commit is the problem since its parent works fine, but this commit doesn't. Are you familiar with what could have caused this problem, @gengjiawen? |
what device are you running ? Is it a x86_64 emulator ? |
@RSNara did you pass some custom parameters to |
@kelset I used @gengjiawen It's a x86_64 emulator. Here are the details (from IntelliJ):
|
I doesn't put x86_64 abi in template, only in RNTester. Normally from my experience a x86 emulator is sufficient. |
@gengjiawen, I'm not sure I follow your comment. What concrete steps can we take to fix this? |
If you want to run this on x86_64 emulator, add x86_64 to this line https://github.com/facebook/react-native/blob/master/local-cli/templates/HelloWorld/android/app/build.gradle#L112. |
@gengjiawen, I just added |
@RSNara Can you try RNTester on that emulator ? |
@gengjiawen RNTester works fine. In case you're curious, here's the script I'm using to create a react native project: #/bin/bash
set -e
APP_NAME=rnapp$(date +%s)
RN_GITUB=~/fbsource/xplat/js/react-native-github
echo 'Installing dependencies...'
cd $RN_GITHUB
npm install prompt
echo ''
echo 'Temporarily modify local files'
sed -i '' "s/execSync(installCommand/execSync('yarn link react-native'/g" $RN_GITHUB/react-native-cli/index.js
sed -i '' "s|ReactNativeVersionCheck.checkVersions();|// ReactNativeVersionCheck.checkVersions();|g" $RN_GITHUB/Libraries/Core/InitializeCore.js
echo ''
echo 'Link react-native'
yarn link
echo ''
echo "Create react-native app: $APP_NAME"
node ./react-native-cli/index.js init $APP_NAME
cd $APP_NAME && npm install react-native
echo ''
echo "Copy source files into \"$APP_NAME\""
cp -r ../{React,ReactAndroid,ReactCommon,Libraries} ./node_modules/react-native
echo ''
echo "Move \"$APP_NAME\" to desktop"
cd .. && mv $APP_NAME ~/Desktop/$APP_NAME
echo ''
echo "Undo file modifications"
sed -i '' "s/execSync('yarn link react-native'/execSync(installCommand/g" $RN_GITHUB/react-native-cli/index.js
sed -i '' "s|// ReactNativeVersionCheck.checkVersions();|ReactNativeVersionCheck.checkVersions();|g" $RN_GITHUB/Libraries/Core/InitializeCore.js
echo ''
echo "Generated app: $APP_NAME"
echo "Run \"cd ~/Desktop/$APP_NAME\" to get to app"
echo 'Run "yarn start" to start metro'
echo 'Run "yarn android" to install to a running android emulator'
echo 'Run "yarn ios" to open up and install to an ios emulator'
|
Can you check the so in the generated apk using android studio ? |
Summary: add arm64 support, related issue : facebook#2814. If we are okay with binary aar android-jsc, then the pr can be directly merged. Otherwise merge facebookarchive/android-jsc#30 first and do a new release. RNTester all variant works. You can also test the apk from here: https://github.com/gengjiawen/react-native/releases/tag/v0.56beta. facebookarchive/android-jsc#30. [ANDROID] [ENHANCEMENT] [ABI] - add x86_64 arm64-v8a support. Differential Revision: D9491481 Pulled By: hramos fbshipit-source-id: d6ec6992768eb0c0866a0317273e09fae5b8935e
Motivaion
add arm64 support, related issue : #2814.
If we are okay with binary aar android-jsc, then the pr can be directly merged. Otherwise merge facebookarchive/android-jsc#30 first and do a new release.
Test Plan
RNTester all variant works. You can also test the apk from here: https://github.com/gengjiawen/react-native/releases/tag/v0.56beta.
Related PRs
facebookarchive/android-jsc#30.
Release Notes
[ANDROID] [ENHANCEMENT] [ABI] - add x86_64 arm64-v8a support.