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

Reviews

See the guideline for information on the review process.

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Concept ACK

It should be noted the primary role of the task is to check that a change doesn’t prevent us from building for android, a platform with 3 billion users and a platform that we care about building for. A manual check will be needed for build system changes to ensure that this compatibility is kept.

@dergoegge

can you show an example to back up this claim: "I have only ever seen this task pass (even if all others fail)..." - The android CI won't pass if we can't build core, if all other tests fail we can't build core (macOS 10.15 ci task for example). So this sounds impossible.

why are there no issues posted about these intermittent failures

Because no one reported them. But they do exist, if you grep for "Warning: An error occurred while preparing SDK package Android Emulator: Connection reset." For example, https://cirrus-ci.com/task/5766445213155328?logs=build#L2700

It's great to speed up a task, and I wish all tasks could run as quickly as possible;

It only checks that it can build, and doesn't run any tests, so I hope the other tasks remain to run the tests.

This feels, in a small way, motivated by the want to remove the GUI by some.

I think this is a far stretch. I've never encouraged to remove the GUI and would strongly object the removal. What this does is remove the Android task from the CI config, nothing more. The CI env is explicitly kept, to be able to continue running the task outside. And in fact it is already running on a nightly basis on my nightly CI repo: https://cirrus-ci.com/task/5872120266227712 , along with other important to check tasks that have no place in this repo.

Needless to say this is also no attempt to remove Android compatibility. I agree with you that this is an important platform and demographic to support.

This is merely removing a task that is failing more often due to unrelated issues (network) than to indicate a useful result (true red CI). If you disagree it would be helpful to demonstrate or link to one instance where this task has caught or would have caught an issue.

I don't feel too strongly, and I am happy to close this, but I think we should be having a discussion based on real data points and not feelings.

@MarcoFalke withdrew NACK, I've misunderstood intentions and outcomes of this PR. It is true that right now outside of checking for build support, this doesn't do anything for this repo.

I think it should be fine to re-add this the next time it finds an issue, or when the (unit) tests are run, or some other reason appears to re-add it.

Another failure on a doc change: https://cirrus-ci.com/task/5213373952950272?logs=ci#L1404

  CXX      qt/libbitcoinqt_a-qrc_bitcoin.o
  CXX      qt/libbitcoinqt_a-qrc_bitcoin_locale.o
  AR       libbitcoin_node.a
  AR       libbitcoin_util.a
  AR       libbitcoin_wallet.a
  AR       libbitcoin_zmq.a
  AR       libbitcoin_cli.a
  AR       libbitcoin_common.a
  AR       libbitcoin_consensus.a
  CXXLD    crypto/libbitcoin_crypto_base.la
  CXXLD    crypto/libbitcoin_crypto_arm_shani.la
  CXXLD    libunivalue.la
  CXXLD    leveldb/libleveldb.la
  CXXLD    crc32c/libcrc32c.la
  CXXLD    leveldb/libmemenv.la
  AR       libtest_util.a
  AR       qt/libbitcoinqt.a
  CXXLD    qt/bitcoin-qt
  CXXLD    qt/test/test_bitcoin-qt
make[2]: Leaving directory '/tmp/cirrus-ci-build/src'
make[1]: Leaving directory '/tmp/cirrus-ci-build/src'
Making all in doc/man
make[1]: Entering directory '/tmp/cirrus-ci-build/doc/man'
make[1]: Nothing to be done for 'all'.
make[1]: Leaving directory '/tmp/cirrus-ci-build/doc/man'
make[1]: Entering directory '/tmp/cirrus-ci-build'
make[1]: Nothing to be done for 'all-am'.
make[1]: Leaving directory '/tmp/cirrus-ci-build'
+ cd src/qt
+ ANDROID_HOME=/tmp/cirrus-ci-build/depends/SDKs/android
+ ANDROID_NDK_HOME=/tmp/cirrus-ci-build/depends/SDKs/android/ndk/23.2.8568313
+ make apk
make -C .. bitcoin_qt_apk
make[1]: Entering directory '/tmp/cirrus-ci-build/src'
tar: --exclude=*/*: Cannot open: No such file or directory
tar: Error is not recoverable: exiting now
tar: --exclude=*/*: Cannot open: No such file or directory
tar: Error is not recoverable: exiting now
mkdir -p qt/android/libs/arm64-v8a
cp /tmp/cirrus-ci-build/depends/SDKs/android/ndk/23.2.8568313/toolchains/llvm/prebuilt/linux-x86_64/bin//../sysroot/usr/lib/aarch64-linux-android/libc++_shared.so qt/android/libs/arm64-v8a
tar xf  -C qt/android/src/ src/android/jar/src --strip-components=5
tar: -C: Cannot open: No such file or directory
tar: Error is not recoverable: exiting now
make[1]: *** [Makefile:22313: bitcoin_qt_apk] Error 2
make[1]: Leaving directory '/tmp/cirrus-ci-build/src'
make: *** [Makefile:11: apk] Error 2
Exit status: 2

can you show an example to back up this claim: "I have only ever seen this task pass (even if all others fail)..."

@jarolrod I can't find the PR this happened on and I've only seen that one time but I swear I'm not making this up. The macOS task has other build flags, so it must have been some weird combination of that and tests/lint failing but some builds succeeding.