build: modest Android improvements

icota commented at 5:06 pm on November 6, 2019: contributor

I’ve been trying to build for Android on different OSes/Gitian with varying success. Build system is quite the beast and sometimes it doesn’t get it right. To make sure it does these three little tweaks make the Android build more robust:

disable D-Bus (Android doesn’t support it and has its own way to trigger notifications)
don’t flag -lpthread when linking Boost, Bionic has built-in support
~~add -static-libstdc++ to linker flags. This avoids having to bundle libc++_shared with CLI apps, still necessary with bitcoin-qt though (thanks Sjors)~~

I think these are small and fairly straightforward so I put them all into this one PR.

fanquake added the label Build system on Nov 6, 2019

in build-aux/m4/bitcoin_qt.m4:79 in 99cfcf98aa outdated

75@@ -76,6 +76,13 @@ AC_DEFUN([BITCOIN_QT_INIT],[
76     [use_dbus=$withval],
77     [use_dbus=auto])
78 
79+  dnl Android doesn't support D-Bus and certainly doesn't use it for notifications

laanwj commented at 6:00 pm on November 6, 2019:

Please update the help message above for --with-qtdbus’s default

Edit: Though if you don’t want dbus, why build QtDbus at all?

icota commented at 8:46 am on November 7, 2019:

I was considering that but I feel that the configuration should be as independent as possible from the depends build or whatever libs we decide to use. For example a user might point to prebuilt Qt for Android libs (with QtDbus) and if that breaks the build he is going to blame the configure script.

There’s definitely no use for D-Bus on Android so I think it okay to disable even if the lib is found.

luke-jr commented at 6:40 pm on November 9, 2019:

Maybe some Android fork might add D-Bus, though? At least allow –with-dbus to override…

DrahtBot commented at 8:08 pm on November 6, 2019: member

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Conflicts

No conflicts as of last run.

icota force-pushed on Nov 7, 2019

in build-aux/m4/ax_boost_thread.m4:75 in 1a9c7abe65 outdated

70@@ -71,6 +71,8 @@ AC_DEFUN([AX_BOOST_THREAD],
71                  CXXFLAGS="-pthreads $CXXFLAGS"
72              elif test "x$host_os" = "xmingw32" ; then
73                  CXXFLAGS="-mthreads $CXXFLAGS"
74+             elif test "x$host_os" = "xlinux-android" ; then
75+                 CXXFLAGS="$CXXFLAGS"

luke-jr commented at 6:39 pm on November 9, 2019:

Does -pthread actually fail targeting Android?

icota commented at 10:20 am on November 11, 2019:

Yes it does, see here. I’m not sure why it worked before but right now building the master branch on Linux is broken.

in configure.ac:602 in 1a9c7abe65 outdated

598@@ -599,6 +599,7 @@ case $host in
599      dnl make sure android stays above linux for hosts like *linux-android*
600      TARGET_OS=android
601      LEVELDB_TARGET_FLAGS="-DOS_ANDROID"
602+     LDFLAGS="$LDFLAGS -static-libstdc++"

luke-jr commented at 6:40 pm on November 9, 2019:

Better to bundle a shared library than to link statically.

icota commented at 10:25 am on November 11, 2019:

Google recommends static.

luke-jr commented at 1:13 pm on November 11, 2019:

“If all of your application’s native code is contained in a single shared library, …”

Which it isn’t.

icota commented at 2:36 pm on November 11, 2019:

I assumed that most people will want bitcoind without the utils. I don’t have strong opinions about this so if you think bundling more than one bin + shared lib is a more common usecase I’ll drop this commit.

luke-jr commented at 4:43 pm on November 11, 2019:

More likely bitcoin-qt!

luke-jr changes_requested

icota force-pushed on Nov 11, 2019

icota force-pushed on Nov 12, 2019

MarcoFalke added the label Needs gitian build on Nov 25, 2019

DrahtBot removed the label Needs gitian build on Nov 28, 2019

greenaddress commented at 0:09 am on December 14, 2019: contributor

Seems to improve some issues found on android but not all - on top of doing checks for “xlinux-android” it should also do checks for “xlinux-androideabi” otherwise you cut off armv7

I’m doing some testing on this, will report my findings

greenaddress commented at 1:21 pm on December 14, 2019: contributor

@icota I can confirm that with xlinux-androideabi as well as xlinux-android all android ndk builds work fine for me/abcore - see https://github.com/greenaddress/bitcoin_ndk/blob/master/0001-android-patches-1bc9988993ee84bc814e5a7f33cc90f670a19f6a.patch

icota commented at 9:26 pm on December 14, 2019: contributor

Thanks @greenaddress! In that case for the case statement we should probably use wildcard(s) to look for xlinux-android* or even *android*.

I’m not sure if the test "x$host_os" = "x... bits allow for wildcards, if not I’d convert them to use case since it’s neater. I’ll investigate.

icota force-pushed on Dec 19, 2019

icota commented at 2:01 pm on December 19, 2019: contributor

Rebased and in ax_boost_thread.m4 converted if test to case statements for easier matching of Android flavours.

in build-aux/m4/ax_boost_thread.m4:70 in dfdc8b3df5 outdated

72-             elif test "x$host_os" = "xmingw32" ; then
73-                 CXXFLAGS="-mthreads $CXXFLAGS"
74-             else
75-                CXXFLAGS="-pthread $CXXFLAGS"
76-             fi
77+             case "x$host_os" in

fanquake commented at 2:24 pm on December 19, 2019:

Have you sent these changes upstream? If so, can you link to the upstream commits/PR. I’d rather not modify the macros in our tree unless absolutely necessary.

icota commented at 2:43 pm on December 19, 2019:

To be honest I didn’t even know there was an upstream. I’ll send a patch file (or whatever GNU people do nowadays) and report back here.

fanquake commented at 2:53 pm on December 19, 2019:

Great, thanks. You should be able to open a PR against the autoconf-archive repo on GitHub.

icota commented at 9:29 am on December 31, 2019:

Done.

icota commented at 5:43 pm on January 26, 2020:

Merged upstream.

MarcoFalke deleted a comment on Jan 27, 2020

MarcoFalke added the label Needs gitian build on Feb 25, 2020

DrahtBot removed the label Needs gitian build on Feb 26, 2020

MarcoFalke deleted a comment on Jun 9, 2020

MarcoFalke commented at 5:34 pm on June 21, 2020: member

Concept ACK

fanquake commented at 0:50 am on August 15, 2020: member

@icota If you’d like to move forward here, can you do the following. 77a347c7115125263561ab76389b5a47e1fb5a4a and dfdc8b3df58024884049db057a3d9d1312aa2722 should be combined into a single commit, that is actually just pulling in the latest version of the macro from upstream (looks like all the changes you made have been upstreamed), with an appropriate commit message. See 9c5bef450f7bf4dd4307cae8bce76dc131643574 in #19558 for an example of what I mean. I also think you can squash the other two commits.

build: disable D-Bus on Android by default

Android uses a different notification system and doesn't support D-Bus

cf0681133a

icota force-pushed on Aug 16, 2020

icota commented at 4:34 pm on August 16, 2020: contributor

Thanks @fanquake. I’ve rebased and done exactly that.

Did a test Android ARM64 full build which completed successfully. I ran a normal native build as well and it also completed without error.

MarcoFalke added the label Needs gitian build on Aug 16, 2020

MarcoFalke added the label Needs Guix build on Aug 16, 2020

icota force-pushed on Aug 16, 2020

build: AX_BOOST_THREAD serial 33 366913e307

icota force-pushed on Aug 16, 2020

DrahtBot commented at 2:53 pm on August 19, 2020: member

Guix builds

File	commit 1bc8e8eae2dc4b47ef67afc6880ea48b8e84a086(master)	commit 87e946cc0e0c936ab3e9e4051f021b7c5dda9e05(master and this pull)
*-aarch64-linux-gnu-debug.tar.gz	`ba41f394f562c9ec...`	`6a84d624fb5325c2...`
*-aarch64-linux-gnu.tar.gz	`70d920e06497e77b...`	`e719a87a710ea7d8...`
*-arm-linux-gnueabihf-debug.tar.gz	`1af0f9da257d8cdb...`	`3ade00e81b0dcfb9...`
*-arm-linux-gnueabihf.tar.gz	`4017f134a463404c...`	`44276d4fabc3c461...`
*-riscv64-linux-gnu-debug.tar.gz	`ac6e7216502ab750...`	`5df0a7b338dad0ea...`
*-riscv64-linux-gnu.tar.gz	`fded0f57f7a0fe6c...`	`d4a6568e2209f187...`
*-win-unsigned.tar.gz	`ee2b6e81325c9277...`	`be7a04b47ab090df...`
*-win64-debug.zip	`c27b3c9b14d2aa2f...`	`f014c2e43caacafe...`
*-win64-setup-unsigned.exe	`8a09c5a88d0f5735...`	`1014bf550db2dd02...`
*-win64.zip	`e7f5f963a6e490be...`	`7e34a9e2285a9838...`
*-x86_64-linux-gnu-debug.tar.gz	`bb2303a5a9e2d901...`	`10a5e410f0b4920c...`
*-x86_64-linux-gnu.tar.gz	`6c0be417f285c32c...`	`8d0bc3db25fdb02a...`
*.tar.gz	`9e674ed0081c5f0b...`	`539d80986cc500f8...`
guix_build.log	`9d3c81b395c6f903...`	`cdfbfd8fdb92fc1c...`
guix_build.log.diff		`7f4ee5f0b904ec89...`

DrahtBot removed the label Needs Guix build on Aug 19, 2020

fanquake commented at 1:51 am on August 20, 2020: member

Thanks @fanquake. I’ve rebased and done exactly that. @icota Thanks for following up, this looks good now. Just waiting for the gitian build.

DrahtBot commented at 9:05 pm on August 22, 2020: member

Gitian builds

File	commit e9b30126545d6ddd8772363e4079d1e4908ad117(master)	commit 3ecb83dcfed9e763d28adcea79cff53f65cd3e41(master and this pull)
bitcoin-core-linux-0.21-res.yml	`cea1ba265603d02f...`	`b063b10aa1146450...`
bitcoin-core-osx-0.21-res.yml	`77d7884f6a574134...`	`0727d16ea478b325...`
bitcoin-core-win-0.21-res.yml	`0b0f6bbbd9a5c0ef...`	`b9edd272fccc40d8...`
*-aarch64-linux-gnu-debug.tar.gz	`4ed42dd1dc70bea9...`	`82ad07fb63117104...`
*-aarch64-linux-gnu.tar.gz	`8a7a6ef9cccbae35...`	`7a6ab24abc2d47e3...`
*-arm-linux-gnueabihf-debug.tar.gz	`e40e4858ee8bd4d1...`	`2b7f2ff41e079dc6...`
*-arm-linux-gnueabihf.tar.gz	`7ddca7cad75ef53b...`	`345a923bfdcdfea1...`
*-osx-unsigned.dmg	`79a2858ea685e8b0...`	`474dad4346a269b1...`
*-osx64.tar.gz	`e77905a2f3abb599...`	`06e6e9636f1e4469...`
*-riscv64-linux-gnu-debug.tar.gz	`4ccc4f940f998b62...`	`19e77cc72ae822a7...`
*-riscv64-linux-gnu.tar.gz	`561c5ed7ebaf0ea5...`	`07de76fa08620940...`
*-win64-debug.zip	`e0b30b67a2217a50...`	`9f91995acfe56f88...`
*-win64-setup-unsigned.exe	`3e5025cde6a4d3d5...`	`0e1405eabab12c0e...`
*-win64.zip	`caee7761af12305b...`	`27209cf126ce030e...`
*-x86_64-linux-gnu-debug.tar.gz	`4036b1cb7c0177b0...`	`44559b8710ba0b5c...`
*-x86_64-linux-gnu.tar.gz	`e15879353155c44c...`	`a973eab824c42213...`
*.tar.gz	`e8cbb99f0f9f02a0...`	`16e3beea8b965f08...`
linux-build.log	`43933168846d907b...`	`d6d8a8da8a69b6f6...`
osx-build.log	`f0d612ea7fbc1c07...`	`ea58f0a1e6a1d8af...`
win-build.log	`54a3cd2d3d8f9dd4...`	`87458f9c85f65af9...`
bitcoin-core-linux-0.21-res.yml.diff		`e383ffc9ce0af076...`
bitcoin-core-osx-0.21-res.yml.diff		`a91b370f0b6f88d5...`
bitcoin-core-win-0.21-res.yml.diff		`c327e8e5eacf6f25...`
linux-build.log.diff		`88a98fa52872a42a...`
osx-build.log.diff		`cfe3b915ebc6f89f...`
win-build.log.diff		`7d7efd4912b1d6ba...`

DrahtBot removed the label Needs gitian build on Aug 22, 2020

practicalswift commented at 5:36 am on August 23, 2020: contributor

Concept ACK

fanquake approved

fanquake commented at 1:26 pm on August 24, 2020: member

ACK 366913e307d2dc13bc00d6bf7b6b2426c359ac30

fanquake merged this on Aug 24, 2020

fanquake closed this on Aug 24, 2020

sidhujag referenced this in commit 60ab73fe5e on Aug 24, 2020

PastaPastaPasta referenced this in commit 524536a2b6 on Jun 27, 2021

PastaPastaPasta referenced this in commit 590c8dbba1 on Jun 28, 2021

PastaPastaPasta referenced this in commit 5741b4bd1d on Jun 29, 2021

PastaPastaPasta referenced this in commit d515d483cc on Jul 1, 2021

PastaPastaPasta referenced this in commit 69d4755a3b on Jul 1, 2021

PastaPastaPasta referenced this in commit 51ee280873 on Jul 15, 2021

PastaPastaPasta referenced this in commit 68f8cbf043 on Jul 16, 2021

DrahtBot locked this on Feb 15, 2022

build: modest Android improvements #17396

Conflicts

Guix builds

Gitian builds