-
Notifications
You must be signed in to change notification settings - Fork 41
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
DRAFT: WSL-Ubuntu-Android Build Guide #366
base: main
Are you sure you want to change the base?
Conversation
Rebased main, deleted WSL_APK_INSTALL.md and modified build-android.md so the wsl guide can be found there. Let me know if there's anything that needs to be updated @jarolrod |
Please add PR description, update PR title, fix linter issues and clean commit history. |
Took care of the above. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Concept ACK
I'll try to validate the instructions following them and checking the built is correct. Is this exclusive to WSL or can be done on Ubuntu 22.04.
Re: clean commit history, please let me know if that's the way to do it or if you'd prefer a different approach?
I think there's no need to have so many commits, you could squash them all into only one.
Also, perhaps you need to rebase?
Thanks @pablomartin4btc for Concept Ack this PR
I will squash the commits and rebase early next week as I'm traveling for
the holiday.
…On Wed, Nov 22, 2023, 20:33 pablomartin4btc ***@***.***> wrote:
***@***.**** commented on this pull request.
Concept ACK
I'll try to validate the instructions following them and checking the
built is correct. Is this exclusive to WSL or can be done on Ubuntu 22.04.
Re: clean commit history, please let me know if that's the way to do it or
if you'd prefer a different approach?
I think there's no need to have so many commits, you could squash them all
into only one.
Also, perhaps you need to rebase?
—
Reply to this email directly, view it on GitHub
<#366 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/A2P6LNZVG3DCPSXE4EDXAPLYFZHQJAVCNFSM6AAAAAA4VFKKCSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTONBVGI3DAMBXGI>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
576bbc3
to
a4bf253
Compare
It should work with Ubuntu 22.04 proper. Squashed the commits and rebased with Thanks again for reviewing @pablomartin4btc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
9ca8bb2
to
5f569dd
Compare
thanks @pablomartin4btc, for taking a look... squashed it down to 1 commit.
Sounds great!
with pleasure |
The diff looks too big (956 files) for a build guide, doesn't it? |
5f569dd
to
67150ef
Compare
@hebasto, yes thanks for pointing it out. I didn't do proper house cleaning when synching my fork. It should be resolved now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tACK till Step 4 included
I'll continue and finish it before the end of the next week. Looking forward to have this working and play with it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tACK till Step 6 included, only 1 step left to finish...
I have some minor nits that I'll write them out when I complete the guide entirely.
72f3c44
to
4523a0f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tACK 4523a0f
I've finished the entire guide, sorry it took me a while cos I've checked different devices and had to learn to use the tools.
I tried first with an old tablet (HUAWEI m3 lite 10") with Android 7.0 (API level #24) but the Bitcoin core app crashes due to some missing qt library (checked with adb logcat
I needed to install "Ministro II" but it didn't fix it... I'll try later to build a qt bundle from Android Studio and send it to the device).
Secondly I've tried on these (friends and family - 😄 🙏) phones:
Finally, I've re-built it for my old smart watch...
I've noticed some issues regarding the user experience, layout, etc., on the different platforms/ devices related to QML itself so I'll leave that out of this PR which approach is only the WSL - Ubuntu Android Build Guide.
I think I'd leave this guide open to use both WSL and Ubuntu itself so I'm not sure you need to but in case you do, you can also install usbipd
there on Ubuntu.
|
||
More details on this step can be found [here](https://github.com/bitcoin/bitcoin/blob/master/doc/build-windows.md#compiling-with-windows-subsystem-for-linux). | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
e.g.:
ANDROID_SDK=/home/ubunpolk/Android/Sdk ANDROID_NDK=/home/ubunpolk/Android/Sdk/ndk/26.1.10909125 make -C depends HOST=armv7a-linux-android ANDROID_API_LEVEL=26 ANDROID_TOOLCHAIN_BIN=/home/ubunpolk/Android/Sdk/ndk/26.1.10909125/toolchains/llvm/prebuilt/linux-x86_64/bin CC=/home/ubunpolk/Android/Sdk/ndk/26.1.10909125/toolchains/llvm/prebuilt/linux-x86_64/bin/armv7a-linux-androideabi26-clang CXX=/home/ubunpolk/Android/Sdk/ndk/26.1.10909125/toolchains/llvm/prebuilt/linux-x86_64/bin/armv7a-linux-androideabi26-clang++
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At the top of this document the example given is:
ANDROID_SDK=/home/user/Android/Sdk ANDROID_NDK=/home/user/Android/Sdk/ndk-bundle make HOST=aarch64-linux-android ANDROID_API_LEVEL=28 ANDROID_TOOLCHAIN_BIN=/home/user/Android/Sdk/ndk-bundle/toolchains/llvm/prebuilt/linux-x86_64/bin
in your example, what do the `CC=...` and `CXX=...` add in terms building the proper depends?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it forces to build depends
with the proper clang version for the required Android API Level.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ah got it... ok so will leave the the extra CC=
and CXX=
from example to minimize confusion, however if you feel strongly about having them let me know?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I prefer to use them in all make
calls to ensure the Android API Level for the particular architecture (arm64-v8a, armeabi-v7a, etc), but perhaps it's not required or I'm over-using them, maybe @hebasto can shed some light here.
Hi @pablomartin4btc , Thanks so much for going through the whole guide and testing it on different devices,,, amazing!
No worries, happy you got through and thanks for suggesting improvements! I will look them over, test them then update the commit in the coming days... |
96bfd55
to
8e20109
Compare
@pablomartin4btc , thanks for your reviews and suggestions I've implemented them here 8e20109 |
8e20109
to
657eee8
Compare
doc/build-android.md
Outdated
More details on this step can be found [here](https://github.com/bitcoin/bitcoin/blob/master/doc/build-windows.md#compiling-with-windows-subsystem-for-linux). | ||
|
||
Now, you can build the APKs using the guide found [here](https://github.com/bitcoin-core/gui-qml/blob/main/doc/build-android.md). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit:
Now, you can build the APKs using the guide found [here](https://github.com/bitcoin-core/gui-qml/blob/main/doc/build-android.md). | |
Now, you need to build the dependencies for the specific Android API Level, for which you can follow the guide found [here](https://github.com/bitcoin-core/gui-qml/blob/main/doc/build-android.md). |
Maybe that guide above needs to be updated as it shows an example for "a default build with no disabled dependencies", and could display the make -C depends
suggested previously. And later below you also specify run make && make apk
from src/qt
as on the build-android guide.
In my opinion, I'd be more clear on the sub-steps here in Step 7 in order to build the APK and transfer/ install it to the device (including for each the other commands to make it work as the qlogging.cpp
update, paths, etc):
- build dependencies
- configure with proper host
- make
- make apk
- adb install
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great idea on the having sub-steps
657eee8
to
7687e2c
Compare
new commit 7687e2c Added your suggestions @pablomartin4btc |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tACK 7687e2c
Tested on Ubuntu 22.04 (non-WSL), so I think we can use this guide for both, perhaps you need to identify and separate the steps specifically for WSL (including usage of usbip
, which is not required for non-WSL) and that's all I can say, it worked fine for me.
Sorry, what I mentioned above perhaps can be done on a follow-up, apologies if you started doing that already 🙏. |
No worries... Apologies for the late response was focused on the AssumeUTXO integration
That's a great idea, once I implement your latest suggestions perhaps it can be merged and we can open a new PR detailing both OSes or we can keep collaborating on this one till we feel it's ready to be merged... @pablomartin4btc thoughts? |
7ec2203
to
5b7da4a
Compare
updated and rebased 5b7da4a Thanks @pablomartin4btc for testing and the suggestions |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
re ACK 5b7da4a
perhaps it can be merged and we can open a new PR detailing both OSes or we can keep collaborating on this one till we feel it's ready to be merged...
IMO this PR can be merge as it is, and we can open a new one to incorporate Ubuntu
with some tweaks.
Building for Android is currently broken in the Bitcoin Core master branch. This is the main reason why there have not been syncing to the master branch recently. I suggest to draft / postpone this PR until CMake + Qt6 are landed to the master branch. |
Ah great to know... yes setting this PR to draft then |
This is an updated version of PR #351
It has been rebased and the WSL Ubuntu Android install guide has been added to build-android.md
Let me know if there are any issues