Repository navigation
backport: bitcoin#30049 (remove Android-related build support) - #7788
Conversation
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (10)
💤 Files with no reviewable changes (9)
Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review. WalkthroughThe changes remove Android-specific host detection, toolchain settings, package mappings, and Qt build configuration. Android hosts now follow the remaining host-detection paths, including the Linux target branch in Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to Android build support is temporarily removed as part of the stated migration plan. No unintended actionable issue is established for this change. Architecture SummaryArchitecture risk: 🔵 Low · up to The change affects 5 systems. Changed systems: Architecture concerns Review detailsSystems and components
Before / after behavior
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
✅ Final review complete — no blockers (commit 94960cf) · triage: low |
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Final validation — Phase 1 + Phase 2
Static verification at head 34e09ff confirms that the one-line rename supplies the Android system-name variable consumed by depends/funcs.mk without changing native build-package configuration; no in-scope code defects were found. The failed PR-title check is independently confirmed, but is an automated naming-policy issue rather than a code-review blocker. The supplied CI snapshot still has pending arm64 container jobs and does not establish full Android build validation; no builds or tests were run in this lane.
Review provenance
Source: reviewer 1: glm-5.3-flash (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: dash-core-commit-history); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
lowbygpt-6.1-sol(effort low) — The one-line Android depends variable rename changes cross-build behavior but is narrowly scoped and straightforward to verify against the variable consumed by funcs.mk. - Phase 1 reviewers:
glm-5.3-flash— general (completed, effort high); agentphase1-reviewer - Phase 1 model:
glm-5.3-flash— zai quota: 5h 99% left, weekly 69% left; passed overgemini-3.8-flash-high(antigravity below 15% reserve: weekly 15% left, 5h 100% left) - Fresh verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort medium); agentphase2-reviewer,gpt-6.1-sol— dash-core-commit-history (completed, effort medium); agentphase2-reviewer
thepastaclaw
added
the
pastaclaw:approved
PastaPastaPasta
changed the title
knst
left a comment
knst
left a comment
There was a problem hiding this comment.
NACK there's no android support, moreover these files are removed by bitcoin#30049
| android_RANLIB=$(ANDROID_TOOLCHAIN_BIN)/llvm-ranlib | ||
|
|
||
| android_cmake_system=Android | ||
| android_cmake_system_name=Android |
There was a problem hiding this comment.
depends/hosts/android.mk this file and other related are removed by bitcoin#30049
Instead fixing it or applying any fixes better just remove depends/hosts/android.m
There was a problem hiding this comment.
Agreed. I've reworked the PR into a backport of bitcoin#30049 (94960cf), which deletes depends/hosts/android.mk, the Android Qt patch and config, and the Android branches in configure.ac and bitcoin_qt.m4. I've updated the title and description to match.
🤖 Posted autonomously by Claude on behalf of pasta.
…ated stuff 5deb0b0 build, test, doc: Temporarily remove Android-related stuff (Hennadii Stepanov) Pull request description: Previously, our Android builds were geared towards generating APKs, which relied on Qt. However, after migrating to C++20, compiling for Android became unfeasible due to Qt 5.15's compatibility limitations with NDK only up to r25, which includes an outdated embedded libc++ (see bitcoin#29360). All removed stuff will be reinstated after migrating the build system to CMake and upgrading Qt to version 6.x. This PR makes possible a clean migration to the CMake-based build system as it removes code, which is not used at this moment. ACKs for top commit: vasild: ACK 5deb0b0 fanquake: ACK 5deb0b0 - given none of this is currently tested/wont compile. Can be revisted in future. Tree-SHA512: 3bc2ccfe881e11cc1d78c27acd6f1d86cfba86821ef3bb5eca2e80d978fdfa13659ec82284dcaadc507e2394524dea91d4b8f81d0030c1cef9708df8be76bf07 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
34e09ff to
94960cf
Compare
PastaPastaPasta
changed the title
thepastaclaw
removed
the
pastaclaw:approved
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Re-review — Final validation — Phase 1 + Phase 2
Independently reviewed the complete diff at 94960cf against Bitcoin Core PR bitcoin#30049; no actionable correctness, backport-completeness, or Dash-specific integration issues were found. The Android removal matches the stated scope, the omitted upstream areas are explicitly documented, and Dash's FreeBSD configuration and QT_STATIC plugin guards remain intact. Verification was static only: the supplied CI snapshot at 2026-10-05T18:08:28Z shows lint and all six depends builds passing, with ten source-build jobs still in progress.
🔴 0 blocking | 🟡 0 suggestion(s) | 💬 0 nitpick(s)
Review provenance
Source: reviewer 1: glm-5.3-flash (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: backport-reviewer); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: dash-core-commit-history); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: backport-reviewer); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: dash-core-commit-history); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)
- Triage:
lowbygpt-6.1-sol(effort low) — The diff is a small, contained removal of unused Android build support, with only a straightforward change to the shared Qt plugin check and no critical-surface changes. - Phase 1 reviewers:
glm-5.3-flash— general (completed, effort high); agentphase1-reviewer - Phase 1 model:
glm-5.3-flash— zai quota: 5h 99% left, weekly 99% left; passed overgemini-3.8-flash-high(antigravity below 15% reserve: weekly 15% left, 5h 100% left) - Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
- Fresh verifier:
gpt-6.1-sol— final-verifier; agentsol-verifier - Phase 2 reviewers:
gpt-6.1-sol— general (completed, effort medium); agentphase2-reviewer,gpt-6.1-sol— backport-reviewer (completed, effort medium); agentphase2-reviewer,gpt-6.1-sol— dash-core-commit-history (completed, effort medium); agentphase2-reviewer,gpt-6.1-sol— general (completed, effort medium); agentphase2-reviewer,gpt-6.1-sol— backport-reviewer (completed, effort medium); agentphase2-reviewer,gpt-6.1-sol— dash-core-commit-history (completed, effort medium); agentphase2-reviewer
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify the current code and confirm that no unresolved issues remain.
No unresolved findings remain from the prior review on this head.
fbe6526 Merge #7807: feat(consensus): hold EvoNode shares and multiple payouts behind a new evo_shares deployment (pasta) 57fd2d9 Merge #7776: fix: keep a pending ProUpServTx across a registrar update that keeps the operator key (pasta) ca69801 Merge #7803: fix: return non-zero amount of accounts after sethdseed (pasta) 57a8f07 Merge #7796: backport: bitcoin#26720 (remaining), partial bitcoin#29523: weight-check the knapsack exact-match selection (pasta) 4eb82f6 Merge #7786: fix(net): keep a dedicated onion listener when -bind is given and correct the bind release note (pasta) 70626a0 Merge #7793: backport: partial bitcoin#28868 (wallet: reload wallet if migration exited early, keep mixed watchonly txs) (pasta) 250a3cd Merge #7790: fix: guard cached governance object flags with the object lock (pasta) 700c195 Merge #7787: fix: only sign EHF signals inside the deployment's start/timeout window (pasta) a3b0f3c Merge #7785: fix: restore the txindex requirement when governance validation is enabled (pasta) 2d1735d Merge #7794: fix(wallet): floor the denomination gap threshold so rebalancing cannot oscillate (pasta) d9f6592 Merge #7783: fix: stop fish completion from re-running the typed dash-cli command (pasta) 89d44c8 Merge #7795: backport: bitcoin#34272, partial bitcoin#31650 (pass PSBT by const reference in PSBTInputSignedAndVerified) (pasta) a1d7411 Merge #7779: fix!: report legacy evonode Platform addresses when listdiff changes its address (pasta) 73f6ea2 Merge #7789: fix(qt): report malformed lang/font settings instead of aborting at startup (pasta) 40de493 Merge #7781: fix(rpc): use a consistent tip height in quorum dkgstatus (pasta) 7d1a90d Merge #7791: fix(net): request object votes from nPeersPerHashMax peers (pasta) 51b145c Merge #7778: test: use single-member in functional tests: feature_mnehf, feature_notifications, p2p_instantsend (pasta) Pull request description: ## Issue being fixed or feature implemented Backports for v24.0.0-rc.3. `v24.0.x` was cut at the v24.0.0-rc.2 tag (1e239d4), and develop has since moved to 24.1.0 (#7775), so rc.3 is built from this branch instead of being tagged on develop like rc.1 and rc.2. ## What was done? Cherry-picked (`git cherry-pick -m1 -x`) every develop merge since rc.2 that carries `backport-candidate-24.0.x`, plus #7778, which #7807's test needs. They are in develop merge order: - #7778 test: use single-member in functional tests: feature_mnehf, feature_notifications, p2p_instantsend (test-only. Needed because #7807's `feature_evo_shares_activation.py` depends on its `mine_quorum_single_member()` fix and fails without it.) - #7791 fix(net): request object votes from nPeersPerHashMax peers - #7781 fix(rpc): use a consistent tip height in quorum dkgstatus - #7789 fix(qt): report malformed lang/font settings instead of aborting at startup - #7779 fix!: report legacy evonode Platform addresses when listdiff changes its address - #7795 backport: bitcoin#34272, partial bitcoin#31650 (pass PSBT by const reference in PSBTInputSignedAndVerified) - #7783 fix: stop fish completion from re-running the typed dash-cli command - #7794 fix(wallet): floor the denomination gap threshold so rebalancing cannot oscillate - #7785 fix: restore the txindex requirement when governance validation is enabled - #7787 fix: only sign EHF signals inside the deployment's start/timeout window - #7790 fix: guard cached governance object flags with the object lock - #7793 backport: partial bitcoin#28868 (wallet: reload wallet if migration exited early, keep mixed watchonly txs) - #7786 fix(net): keep a dedicated onion listener when -bind is given and correct the bind release note. Includes partial bitcoin#36170 (5f40d56). Upstream's dedicated-onion-bind changes to `feature_torcontrol.py` and `p2p_private_broadcast.py` are not included, because those tests depend on upstream commits (5693833, e74d54e) that Dash does not have. - #7796 backport: bitcoin#26720 (remaining), partial bitcoin#29523: weight-check the knapsack exact-match selection - #7803 fix: return non-zero amount of accounts after sethdseed - #7776 fix: keep a pending ProUpServTx across a registrar update that keeps the operator key - #7807 feat(consensus): hold EvoNode shares and multiple payouts behind a new evo_shares deployment One pick needed conflict resolution, in a test only, described in its commit message: #7793, `test/functional/wallet_migration.py`. `test_conflict_txs()` comes from bitcoin#28542 (#7762), which is not on this branch, so it is left out. The C++ part of #7793 is identical to develop. Merged since rc.2 but intentionally not included: #7775 (24.1 version bump), #7260 and #7804 (new Qt feature and its CI fix), #7584 (refactor), routine upstream backports #7257, #7759, #7762, #7768, #7788, #7801, and test/CI-only changes #7777 and #7797. No version change is needed. `configure.ac` is already 24.0.0 with `CLIENT_VERSION_IS_RELEASE` false, so the build takes its version string from the `v24.0.0-rc.3` tag, as it did for rc.2. ## How Has This Been Tested? Built on macOS arm64 against depends with `--enable-debug --enable-werror` (dashd, dash-qt, tests). - `make check`: all unit test suites pass, including `test_dash-qt`. - Functional tests, all passing: `feature_evo_shares_activation.py`, `feature_masternode_shares.py`, `feature_mnehf.py`, `feature_governance.py --descriptors`, `feature_governance_objects.py`, `feature_governance_txindex_devnet.py`, `feature_proxy.py`, `feature_notifications.py`, `feature_llmq_connections.py`, `feature_llmq_chainlocks_automatic.py`, `feature_llmq_singlenode.py`, `p2p_instantsend.py`, `rpc_blockchain.py` (v1 and v2 transport), `rpc_netinfo.py`, `rpc_quorum.py`, `rpc_verifychainlock.py`, `wallet_hd.py` (legacy and descriptors), `wallet_migration.py`. - `feature_bind_extra.py` (from #7786) only runs on Linux and was skipped locally, so CI covers it. Every touched file was diffed against develop. The remaining differences come only from develop PRs that are not part of this backport. ## Breaking Changes Carried over from the picked PRs: - #7807: consensus change behind the new `evo_shares` deployment (bit 14). Mainnet and testnet are unaffected until it activates. Devnets with `v24` already active and an EvoNode carrying several owner payouts need to check or reset. New RPC `protx shared_register_prepare_evo`. - #7785: outside regtest, a node with `-txindex=0` and without `-disablegovernance` refuses to start again, as before v23.1.0. - #7786: carries a partial bitcoin#36170. A node started with `-bind` but no specific `-bind=<addr:port>=onion` now refuses to start while `-listenonion` is enabled. That is the default when listening, even without Tor configured. A wildcard onion bind is also refused. Such nodes need `-bind=127.0.0.1:<port>=onion` or `-listenonion=0`. Nodes without `-bind`, including those using only `-whitebind`, keep the default `127.0.0.1:9996` (`19996` testnet) onion target. See `doc/release-notes-7300.md`. - #7779: `protx listdiff` now includes `platform_p2p` and `platform_https` for a legacy-address evonode whose core P2P address changes. ## Checklist: - [x] I have performed a self-review of my own code - [ ] I have made corresponding changes to the documentation - [ ] I have assigned this pull request to a milestone 🤖 Generated with [Claude Code](https://claude.com/claude-code) Top commit has no ACKs. Tree-SHA512: 8cb01bdb0234f31b5dd2c105b69673de148922991a7523627395c0e0ccfa22b94be5ffbecc842398d443bc90fd9baf2eee6a9cf6fcdbbd1626eb00b93726a55a
4 participants
Issue being fixed or feature implemented
Dash does not support Android builds. The Android depends host, the Android Qt configuration, and the Android branches in
configure.acandbitcoin_qt.m4are untested and have already rotted: since the partial bitcoin#30454 backport (9df31ce),depends/hosts/android.mkstill defines the oldandroid_cmake_systemname, so every CMake package configured for an*-linux-androidhost gets an empty-DCMAKE_SYSTEM_NAME=and is built for the build machine instead of Android.This PR originally fixed that one variable name. Review pointed out that upstream removed all of this code in bitcoin#30049, and that removing it is better than repairing a path nobody builds or tests. This PR now backports bitcoin#30049 instead.
What was done?
Backports bitcoin#30049 ("build, test, doc: Temporarily remove Android-related stuff"), commit 5deb0b0:
depends/hosts/android.mkanddepends/patches/qt/fix_android_jni_static.patchare deleted.androidhost-OS mapping independs/Makefile, the Android Qt and qrencode package lists, and the Android Qt configure options are removed.*android*host case,ANDROID_ARCH, and the Android Qt plugin and library handling are removed fromconfigure.acandbuild-aux/m4/bitcoin_qt.m4. Theqminimalstatic plugin is now checked unconditionally.depends/README.mdand the Android build notes link fromdoc/README.md.src/qt/android/.gitignore, the only file Dash still had undersrc/qt/android/, is deleted.Upstream hunks with no Dash counterpart:
ci/test/00_setup_env_android.shand the Android steps inci/test/01_base_install.sh,02_run_container.shand03_test_script.sh: Dash has no Android CI job and does not have these upstream files.doc/build-android.md, thebitcoin_qt_apktarget insrc/Makefile.qt.include, and the APK project files undersrc/qt/android/(manifest, Gradle files, icons,BitcoinQtActivity.java): Dash does not have them.As in upstream, the
Q_OS_ANDROIDandQT_QPA_PLATFORM_ANDROIDguards insrc/qt/bitcoin.cpp,src/qt/guiutil.handsrc/qt/test/test_main.cppare kept.Conflicts were resolved against Dash's tree. In
bitcoin_qt.m4, upstream's context line definingQT_STATICPLUGINcomes from an earlier upstream change that Dash does not have (Dash still guards static plugins withQT_STATIC), so only the unconditionalqminimalcheck was taken. Inpackages.mk, Dash'sqt_freebsd_packagesline is kept.How Has This Been Tested?
./autogen.shsucceeds, and the generatedconfigurepassessh -nand no longer containsANDROID.make -s -C depends HOST=<host> print-packagesgives the same package list before and after forx86_64-linux-gnu,x86_64-w64-mingw32andarm64-apple-darwin. The only change inprint-qt_patchesis thatfix_android_jni_static.patchis gone.git grep -i androidfinds nothing left in the build system, CI, or docs, apart from the Qt source guards that upstream also keeps.test/lint/lint-whitespace.pyandgit diff --checkpass.I did not run a full depends or Qt build. The Qt package's build ID changes because its patch list changed, so CI rebuilds the Qt depends cache.
Breaking Changes
HOST=*-linux-androidis no longer a supported depends target, and./configureno longer has an Android target. These paths were not built in CI or Guix, and their CMake packages were already misconfigured.Checklist:
🤖 Generated with Claude Code