It would be useful to have support for building against the Android NDK in the repository as well as the depends system.
Greenaddress is currently doing this and has scripts and patches for doing so in a separate repository.
Integrating this into the upstream build would likely be more maintainable, and it would make it possible to make the ndk build part of the gitian build set at some point, providing higher reassurance for the Android executables.
(sister issue for iOS is #11720)
Thanks @laanwj
Some things of note:
ndk r16 works on:
aarch_64, x86_64
fails on: arm, x86
/opt/arm-linux-androideabi-clang/bin/../lib/gcc/arm-linux-androideabi/4.9.x/../../../../include/c++/4.9.x/fstream:826:20: error: use of undeclared identifier ‘ftello’
from a quick investigation this seems an issue with the ndk and the work around is to not set D_FILE_OFFSET_BITS=64 (but I couldn’t see an easy way to do it yet)
I’ve updated the bitcoin_ndk repo with a new branch for the 0.16 release candidate.
Had to modify the patch to take into account the 0.16 changes but so far looking good https://github.com/greenaddress/bitcoin_ndk/tree/core_0.16 and smaller patches (which is great)
@laanwj I updated to 0.16.1 and managed to upgrade to latest ndk compiler (r17b) for both 32/64 bit by passing ac_cv_sys_file_offset_bits accordingly.
I also had to hack zmq depend because of tautological-constant-compare and new compiler (patch is from zmq upstream)
See https://github.com/greenaddress/bitcoin_ndk/commit/a925df4caa94abdac8727dfac1df191bc2c89277
@fanquake eventually yes, with some guidance from you and others :)
For the ZMQ patch since it is upstream I don’t think we need to do anything other than wait until it trickles down in a stable release we move to? Also, I checked the latest update in the one we use in bitcoin core master and still suffers from the tautological-constant-compare Error (in zmq depends).
Actually I think the actual issue there is that we are not passing through/overriding depends compile flags when we build depends libraries but we do set tautological-constant-compare as a warning when we build bitcoin core. If we were to pass through/override depends CFLAGS/CPPFLAGS then we wouldn’t even need the ZMQ patch.
@greenaddress we are already applying patches for zeromq in depends, so if we need to pull that patch in to fix a build issue, that isn’t a problem.
Given that zeromq/libzmq#3140 was only merged recently, I’d assume it’ll be part of zmq 4.2.6 if/when that’s released. However, a zeromq bump in the 0.16.x branch is pretty unlikely now, so just pulling that patch into depends is probably the best approach.
@theuni In https://github.com/greenaddress/bitcoin_ndk I have patches required for 0.16.1 (and 0.16.2rc2 for that matter) to build bitcoind using NDK latest (r17b) on arm/x86 32/64 bit both.
It has patches for depends, adds ifaddrs (missing on android) and patches leveldb fread/write/flush_unlocked to fread/write/flush. It also has the ZMQ fix for tautological constant compare directly from upstream (which, as you suggested, in bitcoin master already could be avoided by using –disable-Werror though that’s more impactful)
What do you think, if anything, that we should try to backport either into core or even upstream as applicable?
By the way, if you have other ideas (like –disable-Werror) or comments about the patches that would improve them or simplify them I am keen on feedback!
@fanquake I raised #13689 for the zmq patches (changed from patching in the disabling of tautological constant compare to patch in disabling of Werror as per @theuni feedback)
Note: the –disable-Werror can only be used in master/0.17.x because we use 4.2.3 zmq, in 0.16,{1,2} we use 4.2.2 which doesn’t support –disable-Werror.
@greenaddress I took a look at the ndk patches, a few quick notes:
great, thanks a ton!
Looks like #13659 leveldb fixes works fine https://travis-ci.org/greenaddress/bitcoin_ndk/builds/406349358 thanks for fixing that (at least for 0.17.x/master, for 0.16.2 that wasn’t back ported)
https://github.com/theuni/bitcoin/commit/6c32dd097755595025f6a94551d77d76cdff2569 also looks much better to me, and it works too - would that be a candidate for merge in master? want me to raise a PR for it?
Can’t wait to make the bitcoin_ndk repo unnecessary and looks like we are on the right path :) see branch https://github.com/greenaddress/bitcoin_ndk/tree/v0.17.0_master
@greenaddress I took a look at the ndk patches, a few quick notes:
- #13689 should be good to go
- #13659 should have fixed the leveldb build for android, I assume you can drop the fread patches now.
- The libevent patch is annoying to have to carry, but looks good
- I’d prefer to just drop the use of ifaddrs if unsupported by android (api level < 24) by adding an autoconf check for getifaddrs
Hi, while building for both android arm64-v8a and arm-v7a i noted that in the ifaddrs.h provided by the ndk , it states that
- getifaddrs(3) creates a linked list
Are only supported by ANDROID_API__ >= 24
