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

Update the new app template to use CMake instead of Android.mk #34354

Closed
wants to merge 1 commit into from

Conversation

cortinico
Copy link
Contributor

Summary:
This change simplifies the setup for New Architecture for users on Android.
Instead of using the Android.mk file, users can now use a CMake file which
encapsulate a lot of the complexities and reduces the maintainance cost.

Android.mk support is kept for backward compatibility.

Changelog:
[Android] [Changed] - Update the new app template to use CMake instead of Android.mk

Differential Revision: D38460536

@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: Facebook Partner: Facebook Partner fb-exported labels Aug 5, 2022
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D38460536

@react-native-bot react-native-bot added the Platform: Android Android applications. label Aug 5, 2022
@analysis-bot
Copy link

analysis-bot commented Aug 5, 2022

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 7,610,291 +0
android hermes armeabi-v7a 7,026,008 +0
android hermes x86 7,910,171 +0
android hermes x86_64 7,883,438 +0
android jsc arm64-v8a 9,488,707 +0
android jsc armeabi-v7a 8,266,585 +0
android jsc x86 9,426,083 +0
android jsc x86_64 10,018,824 +0

Base commit: 84fc580
Branch: main

@analysis-bot
Copy link

analysis-bot commented Aug 5, 2022

Platform Engine Arch Size (bytes) Diff
ios - universal n/a --

Base commit: 84fc580
Branch: main

…ook#34354)

Summary:
Pull Request resolved: facebook#34354

This change simplifies the setup for New Architecture for users on Android.
Instead of using the Android.mk file, users can now use a CMake file which
encapsulate a lot of the complexities and reduces the maintainance cost.

Android.mk support is kept for backward compatibility.

Changelog:
[Android] [Changed] - Update the new app template to use CMake instead of Android.mk

Reviewed By: cipolleschi

Differential Revision: D38460536

fbshipit-source-id: 244dbd3936c3ff238905bcb2b2c9e91a008af8bc
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D38460536

@kelset
Copy link
Contributor

kelset commented Aug 8, 2022

(do we want this in 0.70?)

@cortinico
Copy link
Contributor Author

(do we want this in 0.70?)

I discussed a bit about it with @cipolleschi. That's up for debate. Users could use this functionality in 0.70 if they wish. I just haven't updated the template to don't add too many 'moving pieces' to 0.70.

If we include it, it should simplify life for users on Windows and generally reduce the API surface of the template (which is always a good idea). If this is a concern for @Kudo or @tomekzaw or others building on top of the template, we can leave this for 0.71.

@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by @cortinico in dfd7f70.

When will my fix make it into a release? | Upcoming Releases

@react-native-bot react-native-bot added the Merged This PR has been merged. label Aug 8, 2022
@Kudo
Copy link
Contributor

Kudo commented Aug 9, 2022

If we include it, it should simplify life for users on Windows and generally reduce the API surface of the template (which is always a good idea). If this is a concern for @Kudo or @tomekzaw or others building on top of the template, we can leave this for 0.71.

there're no concern for us and i like to have cmake for app templates, too 🎉 thanks for the heads up.

@kelset
Copy link
Contributor

kelset commented Aug 9, 2022

ok I'll add it to the list of commits to cherry pick 👍

kelset pushed a commit that referenced this pull request Aug 11, 2022
Summary:
Pull Request resolved: #34354

This change simplifies the setup for New Architecture for users on Android.
Instead of using the Android.mk file, users can now use a CMake file which
encapsulate a lot of the complexities and reduces the maintainance cost.

Android.mk support is kept for backward compatibility.

Changelog:
[Android] [Changed] - Update the new app template to use CMake instead of Android.mk

Reviewed By: cipolleschi

Differential Revision: D38460536

fbshipit-source-id: 9d4c3b15be751921d34023b24c174044537e6f02
leotm added a commit to leotm/react-native-1 that referenced this pull request Aug 16, 2022
leotm added a commit to leotm/react-native-1 that referenced this pull request Aug 16, 2022
CMake gens running debug
- `android/app/.cxx/Debug/*`
- `android/app/.cxx/RelWithDebInfo/*`
Neither/nothing during release.

So probably want the 87 debug files untracked.

Follow-up: facebook#34354
facebook-github-bot pushed a commit that referenced this pull request Aug 17, 2022
Summary:
CMake gens running debug
- `android/app/.cxx/Debug/*`
- `android/app/.cxx/RelWithDebInfo/*`

Neither/nothing during release.

So probably want the 87 debug files untracked.

Follow-up: #34354

_macOS 13b, RN 0.70.0-rc.3_

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

[Android] [Added] - Update template to gitignore `android/app/.cxx`

Pull Request resolved: #34430

Test Plan: Everything builds and runs as expected

Reviewed By: cipolleschi

Differential Revision: D38752097

Pulled By: cortinico

fbshipit-source-id: 61c31317d5e45f831445841f3e14da871b3903e5
roryabraham pushed a commit to Expensify/react-native that referenced this pull request Aug 17, 2022
…ook#34354)

Summary:
Pull Request resolved: facebook#34354

This change simplifies the setup for New Architecture for users on Android.
Instead of using the Android.mk file, users can now use a CMake file which
encapsulate a lot of the complexities and reduces the maintainance cost.

Android.mk support is kept for backward compatibility.

Changelog:
[Android] [Changed] - Update the new app template to use CMake instead of Android.mk

Reviewed By: cipolleschi

Differential Revision: D38460536

fbshipit-source-id: 9d4c3b15be751921d34023b24c174044537e6f02
roryabraham pushed a commit to Expensify/react-native that referenced this pull request Aug 17, 2022
Summary:
CMake gens running debug
- `android/app/.cxx/Debug/*`
- `android/app/.cxx/RelWithDebInfo/*`

Neither/nothing during release.

So probably want the 87 debug files untracked.

Follow-up: facebook#34354

_macOS 13b, RN 0.70.0-rc.3_

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

[Android] [Added] - Update template to gitignore `android/app/.cxx`

Pull Request resolved: facebook#34430

Test Plan: Everything builds and runs as expected

Reviewed By: cipolleschi

Differential Revision: D38752097

Pulled By: cortinico

fbshipit-source-id: 61c31317d5e45f831445841f3e14da871b3903e5
roryabraham pushed a commit to Expensify/react-native that referenced this pull request Aug 17, 2022
…ook#34354)

Summary:
Pull Request resolved: facebook#34354

This change simplifies the setup for New Architecture for users on Android.
Instead of using the Android.mk file, users can now use a CMake file which
encapsulate a lot of the complexities and reduces the maintainance cost.

Android.mk support is kept for backward compatibility.

Changelog:
[Android] [Changed] - Update the new app template to use CMake instead of Android.mk

Reviewed By: cipolleschi

Differential Revision: D38460536

fbshipit-source-id: 9d4c3b15be751921d34023b24c174044537e6f02
roryabraham pushed a commit to Expensify/react-native that referenced this pull request Aug 17, 2022
Summary:
CMake gens running debug
- `android/app/.cxx/Debug/*`
- `android/app/.cxx/RelWithDebInfo/*`

Neither/nothing during release.

So probably want the 87 debug files untracked.

Follow-up: facebook#34354

_macOS 13b, RN 0.70.0-rc.3_

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

[Android] [Added] - Update template to gitignore `android/app/.cxx`

Pull Request resolved: facebook#34430

Test Plan: Everything builds and runs as expected

Reviewed By: cipolleschi

Differential Revision: D38752097

Pulled By: cortinico

fbshipit-source-id: 61c31317d5e45f831445841f3e14da871b3903e5
kelset pushed a commit that referenced this pull request Aug 22, 2022
Summary:
CMake gens running debug
- `android/app/.cxx/Debug/*`
- `android/app/.cxx/RelWithDebInfo/*`

Neither/nothing during release.

So probably want the 87 debug files untracked.

Follow-up: #34354

_macOS 13b, RN 0.70.0-rc.3_

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://reactnative.dev/contributing/changelogs-in-pull-requests
-->

[Android] [Added] - Update template to gitignore `android/app/.cxx`

Pull Request resolved: #34430

Test Plan: Everything builds and runs as expected

Reviewed By: cipolleschi

Differential Revision: D38752097

Pulled By: cortinico

fbshipit-source-id: 61c31317d5e45f831445841f3e14da871b3903e5
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. fb-exported Merged This PR has been merged. p: Facebook Partner: Facebook Partner Platform: Android Android applications.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants