Simplify Android build and bring up to date #273

ignatk · 2017-12-29T12:50:30Z

This PR improves Themis Android build:

updates used Android build tools to latest versions
adds x86_64 build architecture (now the default for Android native code builds)
checks-in BoringSSL as a submodule to Themis as recommended by BoringSSL project: https://boringssl.googlesource.com/boringssl/+/HEAD/INCORPORATING.md
integrates BoringSSL build to main Themis build, so no separate "build BoringSSL" step needed
bumps API level to 21 for better support of 64 bit platforms

The PR also includes days of messing with Circle CI to ensure it does not OOM with the new build system.

Relates to #235

…ndroid gradle plugin 3.0.1

…rce.com/boringssl/+/HEAD/INCORPORATING.md

Otherwise, builds are failing with oom

Circle CI fails with out of memory. To reduce memory usage we will compile Themis before starting the emulator. This should also fail the build early if there are any issues with Android build without having to wait for emulator to boot (which takes a while).

… for Android

Lagovas

awesome, good job.it's great if after these changes tests will not down on android tests)

Lagovas · 2017-12-29T13:12:07Z

circle.yml

@@ -39,15 +46,21 @@ dependencies:
    - sudo make rubythemis_install
    - sudo make phpthemis_install
    - if [ ! -d $BORINGSSL_PATH ]; then cd $HOME && git clone https://boringssl.googlesource.com/boringssl && cd boringssl && git checkout chromium-stable && mkdir build && cd build && cmake .. && make && cp decrepit/libdecrepit.a crypto/; fi
-    - if [ ! -d $BORINGSSL_PATH/build-armeabi-v7a ]; then cd $BORINGSSL_PATH && mkdir build-armeabi-v7a && cd build-armeabi-v7a && cmake -DANDROID_ABI=armeabi-v7a -DCMAKE_TOOLCHAIN_FILE=../third_party/android-cmake/android.toolchain.cmake -DANDROID_NATIVE_API_LEVEL=16 -GNinja .. && ninja -j 20; fi


as I understand here run tests with different android abi. we don't need it more?

The tests used to run (and still are running) with only one ABI. The ABI depends on emulator we start in the CI (which is arm 32 bit ATM). The compilation is done for all ABIs however.

Lagovas · 2017-12-29T13:20:53Z

as I see now tests running ~50 minutes and gradlew download dependencies before building (... Download https://dl.google.com/dl/android/maven2/com/android/tools/external/org-jetbrains/uast/26.0.1/uast-26.0.1.pom ...). Can we cache something of that (external archives/files/tools) to decrease time of tests?

ignatk · 2017-12-29T14:01:59Z

Most of the time is taken not by downloading dependencies, but building BoringSSL. Because of Circle CI memory limit I had to almost disable concurrent compilation, so it takes ages to compile BoringSSL for 4 architectures with 2 flavour build (Debug, Release) - totally 8 builds.

vixentael · 2018-01-02T01:59:39Z

Thank you @secumod! These changes are great and very welcomed!

However, could you please help me building Android tests on macOS? I've pulled your branch to init dependences, installed SDK Platform-Tools 27, SDK Build Tools 27 and CMake using Android Studio.

When executing command ./gradlew build, I receive following error:

FAILURE: Build failed with an exception.

* What went wrong:
Execution failed for task ':boringssl:generateJsonModelDebug'.
> Build command failed.
  Error while executing process /Users/vxtl/Library/Android/sdk/cmake/3.6.4111459/bin/cmake with arguments {-H/Users/vxtl/projects/themis-test/mainfork/third_party/boringssl/src -B/Users/vxtl/projects/themis-test/mainfork/third_party/boringssl/.externalNativeBuild/cmake/debug/mips64 -DANDROID_ABI=mips64 -DANDROID_PLATFORM=android-21 -DCMAKE_LIBRARY_OUTPUT_DIRECTORY=/Users/vxtl/projects/themis-test/mainfork/third_party/boringssl/build/intermediates/cmake/debug/obj/mips64 -DCMAKE_BUILD_TYPE=Debug -DANDROID_NDK=/Users/vxtl/Library/Android/sdk/ndk-bundle -DCMAKE_TOOLCHAIN_FILE=/Users/vxtl/Library/Android/sdk/ndk-bundle/build/cmake/android.toolchain.cmake -DCMAKE_MAKE_PROGRAM=/Users/vxtl/Library/Android/sdk/cmake/3.6.4111459/bin/ninja -GAndroid Gradle - Ninja -DCMAKE_TOOLCHAIN_FILE=/Users/vxtl/Library/Android/sdk/ndk-bundle/build/cmake/android.toolchain.cmake -DANDROID_NATIVE_API_LEVEL=21 -DANDROID_TOOLCHAIN=gcc -DCMAKE_BUILD_TYPE=Release -GNinja}
  -- The C compiler identification is GNU 4.9.0
  -- Check for working C compiler: /Users/vxtl/Library/Android/sdk/ndk-bundle/toolchains/mips64el-linux-android-4.9/prebuilt/darwin-x86_64/bin/mips64el-linux-android-gcc
  -- Check for working C compiler: /Users/vxtl/Library/Android/sdk/ndk-bundle/toolchains/mips64el-linux-android-4.9/prebuilt/darwin-x86_64/bin/mips64el-linux-android-gcc -- works
  -- Detecting C compiler ABI info
  -- Detecting C compiler ABI info - done
  -- Detecting C compile features
  -- Detecting C compile features - done
  -- The CXX compiler identification is GNU 4.9.0
  -- Check for working CXX compiler: /Users/vxtl/Library/Android/sdk/ndk-bundle/toolchains/mips64el-linux-android-4.9/prebuilt/darwin-x86_64/bin/mips64el-linux-android-g++
  -- Check for working CXX compiler: /Users/vxtl/Library/Android/sdk/ndk-bundle/toolchains/mips64el-linux-android-4.9/prebuilt/darwin-x86_64/bin/mips64el-linux-android-g++ -- works
  -- Detecting CXX compiler ABI info
  -- Detecting CXX compiler ABI info - done
  -- Detecting CXX compile features
  -- Detecting CXX compile features - done
  CMake Error at CMakeLists.txt:339 (message):
    Unknown processor:mips64

This corresponds to the following line in CMakeLists.txt inside BoringSSL repo.

I found one Stack Overflow issue with similar question, but I don't think we were supposed to build BoringSSL for mips64, were we?

As far as I can say, there is no such line in CircleCI build output, I don't think that CircleCI was compiling BoringSSL for mips64.

Moreover, I've noticed that submodule is linked to master branch, but circle.yml is configured to download chromium-stable branch. However, changing submodule to use chromium-stable branch doesn't solve the issue :/

vixentael · 2018-01-02T02:30:23Z

I've specified ABI version adding the following line to the thrid_party/boringssl/build.gradle file:

"-DANDROID_ABI=armeabi-v7a"

This fixed compiling mips64 problem, but I can't push to your branch.

Is it a good fix?

ignatk · 2018-01-02T09:36:13Z

Circle CI is not building mips64. AFAIK, currently if you don't specify architectures, Android build system should build x86, x86_64, armeabi-v7a and arm64.

The fact that your environment tries to build mips64 may be some override in your configs. Adding "-DANDROID_ABI=armeabi-v7a" is probably not a valid fix, because you are configuring boringssl build for one architecture only.

As for branches, I noticed we used to use chromium-stable, but I explicitly changed it to master for Android (and probably we should change for regular PCs) according to https://groups.google.com/a/chromium.org/forum/#!topic/security-dev/coiM8IZ8Fsc

vixentael · 2018-01-02T12:30:10Z

Right, in my case I get incompatible target error

  [x86] SharedLibrary  : libthemis_jni.so
  /Users/vxtl/Library/Android/sdk/ndk-bundle/toolchains/x86-4.9/prebuilt/darwin-x86_64/bin/../lib/gcc/i686-linux-android/4.9.x/../../../../i686-linux-android/bin/ld: error: /Users/vxtl/projects/themis-test/mainfork/jni/../third_party/boringssl/.externalNativeBuild/cmake/debug/x86/crypto/libcrypto.a(bcm.c.o): incompatible target

How to explicitly set boringssl architecture omitting mips64?

Should I have several cmake statements for each architecture?

    defaultConfig {
        minSdkVersion 16
        targetSdkVersion 16
        externalNativeBuild {
		    cmake {
		        arguments "-DCMAKE_TOOLCHAIN_FILE=" + android.ndkDirectory + "/build/cmake/android.toolchain.cmake",
		                  "-DANDROID_NATIVE_API_LEVEL=21",
		                  "-DANDROID_TOOLCHAIN=gcc",
		                  "-DCMAKE_BUILD_TYPE=Release",
                          "-DANDROID_ABI=armeabi-v7a",
		                  "-GNinja"
		    }
            cmake {
                arguments "-DCMAKE_TOOLCHAIN_FILE=" + android.ndkDirectory + "/build/cmake/android.toolchain.cmake",
                          "-DANDROID_NATIVE_API_LEVEL=21",
                          "-DANDROID_TOOLCHAIN=gcc",
                          "-DCMAKE_BUILD_TYPE=Release",
                          "-DANDROID_ABI=x86",
                          "-GNinja"
            }
            cmake {
                arguments "-DCMAKE_TOOLCHAIN_FILE=" + android.ndkDirectory + "/build/cmake/android.toolchain.cmake",
                          "-DANDROID_NATIVE_API_LEVEL=21",
                          "-DANDROID_TOOLCHAIN=gcc",
                          "-DCMAKE_BUILD_TYPE=Release",
                          "-DANDROID_ABI=arm64-v8a",
                          "-GNinja"
            }
            cmake {
                arguments "-DCMAKE_TOOLCHAIN_FILE=" + android.ndkDirectory + "/build/cmake/android.toolchain.cmake",
                          "-DANDROID_NATIVE_API_LEVEL=21",
                          "-DANDROID_TOOLCHAIN=gcc",
                          "-DCMAKE_BUILD_TYPE=Release",
                          "-DANDROID_ABI=x86_64",
                          "-GNinja"
            }
	    }
    }

ignatk · 2018-01-02T12:37:02Z

I would prefer to rely on defaults - as I mentioned above, check your build environment why is it building mips64. Maybe, try to upgrade Android NDK to the latest version?

See https://developer.android.com/ndk/guides/abis.html
mips and mips64 are deprecated in NDK r16, so probably you are using an older one.

vixentael · 2018-01-02T12:45:18Z

Thank you, currently I'm waiting untill local build finishes, then I'll try to update NDK.

According to the documentation:

ANDROID_ABI - specifies the target Application Binary Interface (ABI). 
This option nearly matches to the APP_ABI variable used by ndk-build tool from Android NDK. 
If not specified then set to armeabi-v7a.

ignatk · 2018-01-02T12:47:10Z

That variable is set by Android gradle build system (depending on the project config), so no need to specify it manually.

vixentael · 2018-01-05T16:39:08Z

I've updated NDK, it helped to solve the issue with building on mips64, however, I still see various error while building. I'll clean environment, themis folder and put here error log.

vixentael · 2018-01-05T17:01:34Z

Good news, I've built .aar archives successfully.

I'll run tests using connectedAndroidTest, run our example code, and merge PR after that.

vixentael · 2018-01-05T17:05:32Z

@secumod can you please review build instructions on wiki page?

I'm planning to add requirements (like installing SDK Platform-Tools 27, SDK Build Tools 27, downloading cmake via Android Studio) and short note about using submodules (like git submodule update --remote).

Please add notes about building themis_jni.dylib with new BoringSSL location

What else you would add?

vixentael · 2018-01-05T23:58:11Z

merged.

@secumod please update docs

vixentael · 2018-01-06T00:29:41Z

@secumod if I understand correctly, we run tests on Android 22. However latest version is Android 27.

Test case are written using AndroidTestCase which is deprecated since Android 24.
https://developer.android.com/reference/android/test/AndroidTestCase.html

Trying to run tests on emulator 24+, I constantly receive error:

com.android.builder.testing.ConnectedDevice > No tests found.[Nexus_27(AVD) - 8.1.0] FAILED 
No tests found. This usually means that your test classes are not in the form that your test runner expects (e.g. don't inherit from TestCase or lack @Test annotations).

I believe we should update Android Tests to target to the latest Android version. What do you think?

ignatk and others added 12 commits December 29, 2017 12:43

Use latest Android platform and build-tools versions

0b99d62

Require ANDROID_NDK environment variable instead of ANDROID_NDK_ROOT

67787bb

Upgrade gradle wrapper and build.gradle to support gradle 4.4.1 and A…

3b212b0

…ndroid gradle plugin 3.0.1

Add BoringSSL as a submodule according to https://boringssl.googlesou…

5c47811

…rce.com/boringssl/+/HEAD/INCORPORATING.md

Add build.gradle and dummy manifest for BoringSSL

be57406

Add BoringSSL as a dependency to the main Android themis build.gradle

5ead6a5

Update Circle CI configuration to support updated Android build

4722d82

Limit JVM memory for gradle to 1GB

297e176

Otherwise, builds are failing with oom

Disable parallel build for gradle and limit number of workers

b91cdb5

Disable gradle deamon in CI

260c6b8

Limit CMake/Ninja build concurrency for Circle CI builds of BoringSSL…

edfed71

… for Android

ignatk requested review from vixentael, Lagovas and storojs72 December 29, 2017 13:11

Lagovas approved these changes Dec 29, 2017

View reviewed changes

vixentael approved these changes Jan 5, 2018

View reviewed changes

vixentael merged commit c42c452 into cossacklabs:master Jan 5, 2018

vixentael added O-Android 🤖 Operating system: Android tests Themis test suite infrastructure Automated building and packaging C-BoringSSL Crypto provider: BoringSSL and removed tests Themis test suite labels Jan 17, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Simplify Android build and bring up to date #273

Simplify Android build and bring up to date #273

ignatk commented Dec 29, 2017

Lagovas left a comment

Lagovas Dec 29, 2017

ignatk Dec 29, 2017

Lagovas commented Dec 29, 2017 •

edited

ignatk commented Dec 29, 2017

vixentael commented Jan 2, 2018 •

edited

vixentael commented Jan 2, 2018 •

edited

ignatk commented Jan 2, 2018 •

edited

vixentael commented Jan 2, 2018 •

edited

ignatk commented Jan 2, 2018 •

edited

vixentael commented Jan 2, 2018 •

edited

ignatk commented Jan 2, 2018

vixentael commented Jan 5, 2018

vixentael commented Jan 5, 2018

vixentael commented Jan 5, 2018 •

edited

vixentael commented Jan 5, 2018

vixentael commented Jan 6, 2018

Simplify Android build and bring up to date #273

Simplify Android build and bring up to date #273

Conversation

ignatk commented Dec 29, 2017

Lagovas left a comment

Choose a reason for hiding this comment

Lagovas Dec 29, 2017

Choose a reason for hiding this comment

ignatk Dec 29, 2017

Choose a reason for hiding this comment

Lagovas commented Dec 29, 2017 • edited

ignatk commented Dec 29, 2017

vixentael commented Jan 2, 2018 • edited

vixentael commented Jan 2, 2018 • edited

ignatk commented Jan 2, 2018 • edited

vixentael commented Jan 2, 2018 • edited

ignatk commented Jan 2, 2018 • edited

vixentael commented Jan 2, 2018 • edited

ignatk commented Jan 2, 2018

vixentael commented Jan 5, 2018

vixentael commented Jan 5, 2018

vixentael commented Jan 5, 2018 • edited

vixentael commented Jan 5, 2018

vixentael commented Jan 6, 2018

Lagovas commented Dec 29, 2017 •

edited

vixentael commented Jan 2, 2018 •

edited

vixentael commented Jan 2, 2018 •

edited

ignatk commented Jan 2, 2018 •

edited

vixentael commented Jan 2, 2018 •

edited

ignatk commented Jan 2, 2018 •

edited

vixentael commented Jan 2, 2018 •

edited

vixentael commented Jan 5, 2018 •

edited