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

Add compile time checks to detect Android #90

Merged
merged 2 commits into from
Mar 21, 2021

Conversation

jt9897253
Copy link
Contributor

Builds on Android, tested using the swift-everywhere-toolchain

All instances of #if os(Linux) were changed to #if os(Linux) || os(Android)

Additionaly, ParseConstants.deviceType was extended for Android.

@codecov
Copy link

codecov bot commented Mar 12, 2021

Codecov Report

Merging #90 (e9b7e9d) into main (e1137fc) will decrease coverage by 0.27%.
The diff coverage is 94.44%.

❗ Current head e9b7e9d differs from pull request most recent head 32d4983. Consider uploading reports for the commit 32d4983 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##             main      #90      +/-   ##
==========================================
- Coverage   79.76%   79.49%   -0.28%     
==========================================
  Files          63       63              
  Lines        5170     5170              
==========================================
- Hits         4124     4110      -14     
- Misses       1046     1060      +14     
Impacted Files Coverage Δ
Sources/ParseSwift/Coding/AnyEncodable.swift 30.49% <ø> (ø)
Sources/ParseSwift/Internal/ParseHash.swift 94.11% <ø> (ø)
Sources/ParseSwift/LiveQuery/LiveQuerySocket.swift 61.40% <ø> (ø)
Sources/ParseSwift/LiveQuery/ParseLiveQuery.swift 74.61% <ø> (ø)
...t/LiveQuery/Protocols/ParseLiveQueryDelegate.swift 20.00% <ø> (ø)
Sources/ParseSwift/Storage/KeychainStore.swift 97.75% <ø> (ø)
...ources/ParseSwift/Storage/ParseKeyValueStore.swift 88.00% <ø> (ø)
Sources/ParseSwift/Storage/ParseFileManager.swift 81.81% <80.00%> (-8.34%) ⬇️
Sources/ParseSwift/Objects/ParseInstallation.swift 82.60% <100.00%> (ø)
Sources/ParseSwift/Objects/ParseUser.swift 79.16% <100.00%> (ø)
... and 6 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e1137fc...32d4983. Read the comment docs.

@cbaker6
Copy link
Contributor

cbaker6 commented Mar 12, 2021

@jt9897253 thanks for the PR!

Do you think you can add the Android build to the CI? I see some build info here: https://github.com/apple/swift/blob/main/docs/Android.md

@cbaker6
Copy link
Contributor

cbaker6 commented Mar 12, 2021

Also, can you add an android badge to the readme? The link I posted earlier states there’s a minimum NDK required

@jt9897253
Copy link
Contributor Author

Yes, sure, I can try to set up an Android CI. However, this looks rather complicated to me and it may take some time until I can look into it.

I see two possible approaches:

  • building the toolchain from source
  • using somebody else's toolchain binaries

I'd prefer building the toolchain from source to be able to update the swift version independently from other projects/people. Building the toolchain using the official documentation is definitely an option but also a lot of manual work. Also, I tested various swift android toolchain projects on GitHub which try to automate the build. A lot of what I found was outdated, I ended up using swift-everywhere-toolchain as it worked best for me.
Concerning CI / GitHub actions, it may make sense to have a look at this GitHub workflow. If I understand it correctly, the workflow builds the swift android toolchain from source, uses a recent version of swift and is published under the apache license. That could be a starting point.
The swift forum thread "android-call-for-the-community" can be recommended as it explains the current swift/Android situation.

@cbaker6
Copy link
Contributor

cbaker6 commented Mar 13, 2021

Yes, sure, I can try to set up an Android CI. However, this looks rather complicated to me and it may take some time until I can look into it.

Definitely looks a little complicated. In addition, the links you pointed to take 5-10 hours to build from source, so we should stay away from that. I think it's okay to use a pre-built toolchain as long as it looks like they update regularly. The swift-everywhere-toolchain looks good enough to me!

If I'm understanding the swift-everywhere-toolchain repo correctly, that toolchain is built in macOS, so it may be possible to build Parse-Swift for Android on macOS in the CI (I'm guessing)? If the toolchain can be used in Linux for the CI, it's actually better as we get more Linux builds (I think ~20 at a time) than macOS builds (5 at a time) in Github Actions. If I'm right, we can use the repo releases for Android and save 5-10 hours when building.

@cbaker6
Copy link
Contributor

cbaker6 commented Mar 13, 2021

It looks like the current linux CI in the ci.yml "might" contain almost everything we need to build for Android. We might can copy/paste it and try to install the swift-everywhere-toolchain to it and see if it works. Below is the environment where I see Android NDK from the Linux CI:

ANDROID_SDK_ROOT=/usr/local/lib/android/sdk HOMEBREW_NO_AUTO_UPDATE=1 PIPX_HOME=/opt/pipx GITHUB_REPOSITORY=parse-community/Parse-Swift GITHUB_GRAPHQL_URL=https://api.github.com/graphql JAVA_HOME=/usr/lib/jvm/adoptopenjdk-8-hotspot-amd64 SWIFT_PATH=/usr/share/swift/usr/bin DEPLOYMENT_BASEPATH=/opt/runner GITHUB_JOB=linux SHLVL=1 BOOTSTRAP_HASKELL_NONINTERACTIVE=1 ANT_HOME=/usr/share/ant CHROME_BIN=/usr/bin/google-chrome GITHUB_RETENTION_DAYS=90 DOTNET_NOLOGO=1 LEIN_HOME=/usr/local/lib/lein GITHUB_HEAD_REF=transactions PERFLOG_LOCATION_SETTING=RUNNER_PERFLOG HOMEBREW_PREFIX="/home/linuxbrew/.linuxbrew" VCPKG_INSTALLATION_ROOT=/usr/local/share/vcpkg LEIN_JAR=/usr/local/lib/lein/self-installs/leiningen-2.9.5-standalone.jar ImageOS=ubuntu18 NVM_DIR=/home/runner/.nvm GITHUB_ACTIONS=true DEBIAN_FRONTEND=noninteractive GITHUB_REPOSITORY_OWNER=parse-community GITHUB_REF=refs/pull/89/merge GITHUB_BASE_REF=main RUNNER_USER=runner _=/opt/swift/swift-5.3.3-release-swift-5.3.3-RELEASE-ubuntu18.04/usr/bin/swift ANDROID_NDK_HOME=/usr/local/lib/android/sdk/ndk-bundle GITHUB_ACTOR=cbaker6 GITHUB_RUN_NUMBER=401 RUNNER_WORKSPACE=/home/runner/work/Parse-Swift HOMEBREW_CLEANUP_PERIODIC_FULL_DAYS=3650 CI_XCODE_VER=/Applications/Xcode_11.7.app/Contents/Developer XDG_CONFIG_HOME=/home/runner/.config HOME=/home/runner JAVA_HOME_8_X64=/usr/lib/jvm/adoptopenjdk-8-hotspot-amd64 BASH_ENV=/etc/profile.d/env_vars.sh ANDROID_HOME=/usr/local/lib/android/sdk ImageVersion=20210309.1 RUNNER_TEMP=/home/runner/work/_temp HOMEBREW_REPOSITORY="/home/linuxbrew/.linuxbrew/Homebrew" INVOCATION_ID=eedcec81b6df4c8aadf0fbe331ce3b67 GITHUB_WORKSPACE=/home/runner/work/Parse-Swift/Parse-Swift DOTNET_SKIP_FIRST_TIME_EXPERIENCE=1 GITHUB_PATH=/home/runner/work/_temp/_runner_file_commands/add_path_1e033f33-de45-4e8b-81fa-38cc9920211c GOROOT_1_14_X64=/opt/hostedtoolcache/go/1.14.15/x64 GOROOT_1_15_X64=/opt/hostedtoolcache/go/1.15.8/x64 GRADLE_HOME=/usr/share/gradle GITHUB_RUN_ID=648118186 GECKOWEBDRIVER=/usr/local/share/gecko_driver PIPX_BIN_DIR=/opt/pipx_bin JAVA_HOME_11_X64=/usr/lib/jvm/adoptopenjdk-11-hotspot-amd64 ANDROID_NDK_ROOT=/usr/local/lib/android/sdk/ndk-bundle GITHUB_ACTION=run2 AZURE_EXTENSION_DIR=/opt/az/azcliextensions CONDA=/usr/share/miniconda POWERSHELL_DISTRIBUTION_CHANNEL=GitHub-Actions-ubuntu18 LANG=C.UTF-8 CHROMEWEBDRIVER=/usr/local/share/chrome_driver RUNNER_OS=Linux HOMEBREW_CELLAR="/home/linuxbrew/.linuxbrew/Cellar" GITHUB_EVENT_PATH=/home/runner/work/_temp/_github_workflow/event.json GITHUB_SERVER_URL=https://github.com GOROOT_1_13_X64=/opt/hostedtoolcache/go/1.13.15/x64 CI=true PATH=/opt/swift/swift-5.3.3-release-swift-5.3.3-RELEASE-ubuntu18.04/usr/bin:/home/linuxbrew/.linuxbrew/bin:/home/linuxbrew/.linuxbrew/sbin:/opt/pipx_bin:/usr/share/rust/.cargo/bin:/usr/local/.ghcup/bin:/snap/bin:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin:/usr/games:/usr/local/games:/snap/bin:/home/runner/.dotnet/tools:/home/runner/.config/composer/vendor/bin:/home/runner/.local/bin GITHUB_WORKFLOW=ci AGENT_TOOLSDIRECTORY=/opt/hostedtoolcache RUNNER_PERFLOG=/home/runner/perflog *** JAVA_HOME_12_X64=/usr/lib/jvm/adoptopenjdk-12-hotspot-amd64 USER=runner RUNNER_TOOL_CACHE=/opt/hostedtoolcache GITHUB_ACTION_REF= GITHUB_ENV=/home/runner/work/_temp/_runner_file_commands/set_env_1e033f33-de45-4e8b-81fa-38cc9920211c SELENIUM_JAR_PATH=/usr/share/java/selenium-server-standalone.jar GITHUB_API_URL=https://api.github.com RUNNER_TRACKING_ID=github_fb898df9-5cdb-4863-b8b8-a5e97b27873a GITHUB_ACTION_REPOSITORY= GITHUB_SHA=b03da01e68fd76f0c1499d587e1ba0d9a60d31cc GITHUB_EVENT_NAME=pull_request JOURNAL_STREAM=9:21729 DOTNET_MULTILEVEL_LOOKUP=0 /opt/swift/swift-5.3.3-release-swift-5.3.3-RELEASE-ubuntu18.04/usr/bin/llvm-profdata merge -sparse -o /home/runner/work/Parse-Swift/Parse-Swift/.build/x86_64-unknown-linux-gnu/debug/codecov/default.profdata output:

@cbaker6 cbaker6 marked this pull request as draft March 13, 2021 15:53
@cbaker6
Copy link
Contributor

cbaker6 commented Mar 21, 2021

@jt9897253 I'm going to merge this PR so your changes don't get lost and to avoid merge conflicts in the future.

If you find out a way to add the Android build to the CI, feel free to open another PR.

Thanks for your contribution!

@cbaker6 cbaker6 marked this pull request as ready for review March 21, 2021 14:41
@cbaker6 cbaker6 merged commit 403a02b into parse-community:main Mar 21, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants