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

fix(android): fix ndkVersion is unset when building from source #43131

Closed
wants to merge 1 commit into from

Conversation

tido64
Copy link
Collaborator

@tido64 tido64 commented Feb 21, 2024

Summary:

ndkVersion is unset when building from source using this guide: https://reactnative.dev/contributing/how-to-build-from-source

Changelog:

[ANDROID] [FIXED] - Fix ndkVersion is unset when building from source

Test Plan:

git clone https://github.com/microsoft/react-native-test-app.git
cd react-native-test-app
npm run set-react-version nightly
yarn

# Manually apply the patch in node_modules/react-native/ReactAndroid/build.gradle.kts
# Enable building from source
sed -i '' 's/#react.buildFromSource/react.buildFromSource/' example/android/gradle.properties

# Build
cd example/android
./gradlew assembleDebug

@tido64 tido64 requested a review from cortinico February 21, 2024 15:23
@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. p: Microsoft Partner: Microsoft Partner Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Feb 21, 2024
@facebook-github-bot
Copy link
Contributor

@cortinico has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 17,935,067 +1
android hermes armeabi-v7a n/a --
android hermes x86 n/a --
android hermes x86_64 n/a --
android jsc arm64-v8a 21,293,279 -2
android jsc armeabi-v7a n/a --
android jsc x86 n/a --
android jsc x86_64 n/a --

Base commit: 7fffe69
Branch: main

@facebook-github-bot
Copy link
Contributor

@cortinico merged this pull request in a1171f7.

@tido64 tido64 deleted the tido/set-ndk-version branch February 21, 2024 18:23
@kelset
Copy link
Contributor

kelset commented Feb 22, 2024

@cortinico do we need to get this in the 73/74 branches? we only build nightlies from sources so for us in main should be enough, but not sure if it might be needed elsewhere

@cortinico
Copy link
Contributor

@cortinico do we need to get this in the 73/74 branches? we only build nightlies from sources so for us in main should be enough, but not sure if it might be needed elsewhere

Nope I don't think we do unless we got reports of specific failures on a given version

cortinico pushed a commit that referenced this pull request Mar 22, 2024
…3131)

Summary:
`ndkVersion` is unset when building from source using this guide: https://reactnative.dev/contributing/how-to-build-from-source

## Changelog:

[ANDROID] [FIXED] - Fix `ndkVersion` is unset when building from source

Pull Request resolved: #43131

Test Plan:
```
git clone https://github.com/microsoft/react-native-test-app.git
cd react-native-test-app
npm run set-react-version nightly
yarn

# Manually apply the patch in node_modules/react-native/ReactAndroid/build.gradle.kts
# Enable building from source
sed -i '' 's/#react.buildFromSource/react.buildFromSource/' example/android/gradle.properties

# Build
cd example/android
./gradlew assembleDebug
```

Reviewed By: christophpurrer

Differential Revision: D54006425

Pulled By: cortinico

fbshipit-source-id: 9ede64bc14af4cf609b7a4c12c5a1082bbc31f09
huntie pushed a commit that referenced this pull request Mar 25, 2024
* fix(android): fix `ndkVersion` is unset when building from source (#43131)

Summary:
`ndkVersion` is unset when building from source using this guide: https://reactnative.dev/contributing/how-to-build-from-source

## Changelog:

[ANDROID] [FIXED] - Fix `ndkVersion` is unset when building from source

Pull Request resolved: #43131

Test Plan:
```
git clone https://github.com/microsoft/react-native-test-app.git
cd react-native-test-app
npm run set-react-version nightly
yarn

# Manually apply the patch in node_modules/react-native/ReactAndroid/build.gradle.kts
# Enable building from source
sed -i '' 's/#react.buildFromSource/react.buildFromSource/' example/android/gradle.properties

# Build
cd example/android
./gradlew assembleDebug
```

Reviewed By: christophpurrer

Differential Revision: D54006425

Pulled By: cortinico

fbshipit-source-id: 9ede64bc14af4cf609b7a4c12c5a1082bbc31f09

* Fix build from source for hermes-engine (#43609)

Summary:
Pull Request resolved: #43609

When users are building from source for React Native they don't have an ndkVersion variable specified. So we want to fallback to the global NDK version we set for the whole build here.

Changelog:
[Android] [Fixed] - Fix build from source for hermes-engine

Reviewed By: dmytrorykun

Differential Revision: D55240603

fbshipit-source-id: 3c725a164b40e176548af8ada9fcb13d391ef017

---------

Co-authored-by: Tommy Nguyen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. p: Microsoft Partner: Microsoft Partner Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants