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.

Sign up for GitHub

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

Jump to bottom

feat(android): Add Android builds to CI #682

Open
wants to merge 68 commits into
base: main
Choose a base branch
from
Open

feat(android): Add Android builds to CI #682

wants to merge 68 commits into from

Conversation

rwalworth
Copy link
Contributor

@rwalworth rwalworth commented Apr 16, 2024 •
edited

Description:
This PR adds Android builds of the C++ SDK to the Github workflows. It will only generate the builds after a PR is merged as to not slow down development. A couple notes:

  • vcpkg needed to upgraded as the baseline we were using was too old for Android builds.
  • Switched to using spdlog logging library in place of log4cxx simply because vcpkg currently supports spdlog but not log4cxx android builds.
  • The Android build is currently done in a docker container, but it could be possibly be done straight on our Github runners as they are linux-x86_64 if this wanted to be looked into later. I tried this but there was an issue finding git that I couldn't figure out quickly.
  • There is an issue testing the Android test executables on an emulator running in an x86_64 environment; even running an arm64 docker container in an x86_64 host environment there were issues. If we would like to test these Android builds in CI, this will need to be figured out. However, I left instructions in the Dockerfile as to how to run the test executables.
  • Some work was done that involved building the examples into Android APKs. This was eventually abandoned, but I left some of the work I had done for that in the Dockerfile as well in case this wants to be done in the future.
  • The examples executables were being installed twice unnecessarily so I removed this.
  • We are now installing gtest via vcpkg instead of manually through CMake.
  • Some filepaths needed updating.
  • Many files had unnecessary includes that I removed.

Related issue(s):

Fixes #660

Checklist

  • Documented (Code comments, README, etc.)
  • Tested (unit, integration, etc.)

@rwalworth
WIP: Add android presets to cmake presets. Doesn't currently build
18a500a 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth
Try different path for android ndk
a2649b3 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth
Try correct architecture ndk
9e1d55a 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth
Wrong directory name for ndk
dceadbf 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth
Trying stuff
f879c59 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth
Revert back to x86_64
3bd293f 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth
Update paths
a0bbb01 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth
Replace log4cxx with spdlog since android builds for apr don't work o…
4763b55 
…n vcpkg

Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth
Fix issue of traditional make ports being incompatible with NDK r26 (…
67526c4 
…github.com/microsoft/vcpkg/issues/33881)

Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth
Try newer version of openssl
32d3e9b 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth
Manually add VCPKG_DETECTED_CMAKE_ANDROID_ARCH to CMakePresets.json
2e7f1a4 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth
Add update steps to dockerfile
1bd1e10 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth
Trying going back to r25c
445bc23 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth
Revert vcpkg
ee716e7 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth
Revert openssl version
c21b09a 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth
Downgrade spdlog
2a9aeda 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth
Install vim
5524e1e 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth
Reupdate to new stuff
f9718ca 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth
Trying stuff
f194cd6 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth
Forgot to update json
35ac9de 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth
Try r25c NDK
4fb16fd 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth
Forgot update to env var
368c99a 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth
Try master
517dcc4 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth
Try adding package
837d8d0 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth
Add version to vcpkg-cmake-get-vars
236eaa8 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth
Try an updated baseline
0ebb4c9 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth
Merge branch 'main' into 00660-add-android-and-ios-builds
2ceb723
@rwalworth
Use latest vcpkg release
934ef36 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth
Try no specific versions and just most recent packages
e70e8bf 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth
Get configuration working
ca44bc9 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth
Remove apk
8b36990 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth
remove pull from dockerfile
3ad3648 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth
AndroidHelper not in impl
639161d 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth
Fix arg bug
5ebff29 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth
Fix include path
1a206ec 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth
Try setting environment variables with adb
62df8dd 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth
Add debug statements
b74cb78 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth
AAssetManager isn't internal
46e6c46 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth
Move Android definitions to own source file
5ca4534 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth
Can't name the function main()
8066989 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth
Try using android logs
d9e89ce 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth
Fix typo
8d499e4 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth
Target newer API version
6739c93 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth
Clean up repo, remove example APK work
a07c9b4 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth
Add workflow changes to build Android in CI
5d030b9 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth rwalworth linked an issue Apr 16, 2024 that may be closed by this pull request
Add Android builds #660
Open
@rwalworth
Go back to latest of get-cmake
199c175 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth
Fix repo being cloned
efa231f 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth
Enable android build on PR merge
001a527 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth
Merge branch 'main' into 00660-add-android-and-ios-builds
d363e16 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
@rwalworth rwalworth changed the title 00660 add android and ios builds feat(android): Add Android builds to CI Apr 17, 2024
@rwalworth rwalworth self-assigned this Apr 17, 2024
@rwalworth rwalworth added Release Issues tied to release management functions New Feature A new feature, service, or documentation. Major changes that are not backwards compatible. labels Apr 17, 2024
@rwalworth rwalworth marked this pull request as ready for review April 17, 2024 16:03
gsstoykov
gsstoykov reviewed Apr 19, 2024
View reviewed changes
Copy link
Contributor

@gsstoykov gsstoykov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall LGTM but shouldn't we wait and see how the communication with the community member goes before pushing this into main? As I saw we still don't have answers from the community member and we would only introduce more configurations and docker pipeline overhead.

Dockerfile Show resolved Hide resolved
Dockerfile Outdated Show resolved Hide resolved
src/sdk/main/include/Logger.h Outdated Show resolved Hide resolved
Dockerfile Show resolved Hide resolved
@rwalworth
Copy link
Contributor Author

Overall LGTM but shouldn't we wait and see how the communication with the community member goes before pushing this into main? As I saw we still don't have answers from the community member and we would only introduce more configurations and docker pipeline overhead.

I could go either way. I have the Android builds only being generated on merge so it won't impact development time. Maybe @SimiHunjan can weigh in here.

@rwalworth
Address PR comments
7bce8a1 
Signed-off-by: Rob Walworth <robert.walworth@swirldslabs.com>
gsstoykov
gsstoykov approved these changes Apr 19, 2024
View reviewed changes
Copy link
Contributor

@gsstoykov gsstoykov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gsstoykov gsstoykov gsstoykov approved these changes

Assignees

@rwalworth rwalworth

Labels
New Feature A new feature, service, or documentation. Major changes that are not backwards compatible. Release Issues tied to release management functions
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add Android builds
2 participants
@rwalworth @gsstoykov