-
Notifications
You must be signed in to change notification settings - Fork 568
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
Support arm64v8a and x86_64 (64-bit) in Sqlcipher #151
Conversation
…s64 libs are missing - compiled from master branch - will update the external library pointers accordingly) Modifeid Makefiles to get 64-bit architectures - all64 not used yet Modified Java code to use longs for the variables which store pointers, rather than int Modifed C++ code to use longs rather than ints for pointers shared between java
…s64 libs are missing - compiled from master branch - will update the external library pointers accordingly) Modifeid Makefiles to get 64-bit architectures - all64 not used yet Modified Java code to use longs for the variables which store pointers, rather than int Modifed C++ code to use longs rather than ints for pointers shared between java
Split String include files per architecture Modified external/Android.mk to reflect different architecture needs
Removed unnecessary file from external/Android.mk Aded armeabi-v7a arch in Application.mk
Conflicts: Makefile external/android-libs/arm64-v8a/libandroid_runtime.so external/android-libs/arm64-v8a/libbinder.so external/android-libs/arm64-v8a/libcutils.so external/android-libs/arm64-v8a/libnativehelper.so external/android-libs/arm64-v8a/libutils.so external/android-libs/x86_64/libandroid_runtime.so external/android-libs/x86_64/libbinder.so external/android-libs/x86_64/libcutils.so external/android-libs/x86_64/libnativehelper.so external/android-libs/x86_64/libutils.so jni/Application.mk
Added static openssl binaries for compilation
Hi @gmetal, Thanks so much for this pull request, it is quite large! We look forward to reviewing and getting back with any feedback we may have. |
Any news on this? It would be a nice feature to have. |
Hi @gmetal With regard to this pull request, due to the number of changes we would need you to complete a contributor agreement before we can proceed, more information can be found here. Next, we would like to ask you to par down the changes to only those required to adding support for arm64v8a and x86_64. You have included additional changes such as including alternative source locations for OpenSSL which we would prefer not to include. Also, there are changes to the minimum SDK version, we plan to continue to support Android 2.1 and up. As a point of reference, please make sure your changes are compatible with the current SQLCipher for Android test suite. |
Hello, Now in terms of the patch, I removed the partial mips64 support and the alternative OpenSSL source location. My patch does not change the minimum SDK version. However, since 64-bit support was introduced in android-21, I did include some alternative submodules needed to compile the 64-bit version libraries. However, if you have a closer look, you'll see that the 32-bit versions of the library have not been touched, and are being built like they used too. I also had my patched tested with the test suite, on an x86_64 emulator and on Nexus 9 (arm64-v8a) and in both cases all tests were successful. |
Hello @gmetal, Again, thank you for the time you have put into this pull request, we do certainly appreciate it. After reviewing the contents I do have one concern moving forward. We prefer to build all OpenSSL binary artifacts from source based on our submodule checkout, assembled using this script for the various platforms needed. Your pull request includes libssl.a binaries for both Can you provide any feedback on this? |
Hello @developernotes The included libcrypto.a and libssl.a have been built from the AOSP openssl repository, which provides all the necessary support for building openssl for all AOSP platforms. I can understand not wanting to include such binaries in the codebase. However, after doing a quick check on the OpenSSL releases, it appears that they started supporting the arm64-v8a architecture only very recently (e.g. on the 1.0.2a). I can have a go at updating the build script with arm64-v8a support, but I will also have to bump the openssl repository to the 1.0.2a release. |
Hi @gmetal, That would be great, please note the OpenSSL submodule in external/openssl has already been updated to the 1.0.2a tag with our latest release. |
@developernotes Ok, so it has been some time, what is the status of support for arm64-v8a and x86_64? |
@kayaatakan Why do you think its not running on Samsung Galaxy S6? Its working perfectly fine on x64-platforms - try it out on yourself... Its afaik just not optimized for x64... |
Hello @kayaatakan Per this portion of the thread, we are still awaiting direct support for building the arm64v8a and x86_64 versions of OpenSSL. This will require changes within the OpenSSL build system which appears to still be pending. Once OpenSSL directly supports targeting those platforms we will be able to proceed in adding support to SQLCipher for Android. |
@developernotes So, I see that there is no direct support for arm64-v8a and x86_64 yet. |
Basically is there any way of making SqlCipher work on those devices at the moment? |
@atakankaya I got it to work with devices like the Samsung S6 by downloading the @gmetal fork and building from source. |
@kayaatakan @atakankaya: What @TheNephilim88 was saying is simply that the library is not built specifically for that architecture, however, SQLCipher should run just fine on AArch64 platforms, since they will fall back to armv7a. If you are actually seeing a crash when running on a device, then it is almost definitely related to including a different native library (i.e. other than SQLCipher) with arm64-v8a support due to Android’s preferential loading process. To fix this, you can simply remove the packaging of any platform libraries for arm64-v8a, so that you are bundling the native libraries only for the versions supported by all components (e.g. armv7a). Once the architecture for all native components are consistent then the application should run fine on arm64-v8a devices. Please let us know if that clarifies the situation, and if the suggestion resolves any issues you are seeing. |
@atakankaya Can you help to build the @gmetal source or send me directly binairies please ? |
Hello, I've setup a travis job to build my patch and upload the produced binaries in github. You can find my builds here: https://github.com/gmetal/android-database-sqlcipher/releases |
Alright, a big thanks to you for your work and help ! 2016-01-22 13:49 GMT+01:00 gmetal notifications@github.com:
|
Hi all, sorry for the late response. I used the suggestion of @sjlombardo |
Hello @kayaatakan - excellent, I'm very glad that worked for you! |
@developernotes my patch to openssl has been rejected, since the functionality has been implemented in their master branch. When 1.1.0 is released, you will be able to produce 64-bit android openssl builds without any additional patches, etc. |
Hi ! Thanks you for notice me, i will look at their version ! :) 2016-03-02 15:56 GMT+01:00 gmetal notifications@github.com:
|
Hi @gmetal Thank you for the follow up, we are looking forward to their 1.1.0 release. Take care! |
With the 64-bit capable android devices already out there, it is necessary to be able to have native 64-bit compilation for increased performance. MIPS64 has not been included, because the corresponding AOSP version does not seem to compile.