search

nunchuk-io / tap-protocol

Coinkite Tap Protocol
MIT License
15 stars 4 forks source link

build_ios.sh inconsistent behaviour #12

Closed PeteClubSeven closed 7 months ago

PeteClubSeven commented 8 months ago

Hi, I've been adding iOS support to my cktap-protocol-flutter library and I noticed that the behaviour build script for iOS/macOS is different from build_android.sh and build_linux.sh.

The purpose of build_android.sh and build_linux.sh seems to be to ONLY build libsecp256k1 so that it's available when running cmake separately. However the build_ios.sh script runs cmake after compiling libsecp256k1. The reason this I see this as an issue is because in my use case where I wish to consume your library. I end up having to compile tap-protocol twice, once because build_ios.sh does it and then a second time because I need to perform my own cmake build with different parameters than build_ios.sh uses.

I figured it makes sense to raise an issue since the intention of the build_*.sh scripts is a bit ambiguous right now. Are they just for libsecp256k1 or should they also build the tap-protocol project? For now I've worked around this by taking advantage of a bug in line #75 of build_ios.sh where cmake is never ran if the "build" folder already exists.

giahuy98 commented 8 months ago

This PR #13 should eliminate the need for tools/* entirely. I've already tested it on Linux. Please assist me in testing it on Android/iOS, @PeteClubSeven. Thank you!

PeteClubSeven commented 8 months ago

That’s amazing, I will absolutely help in testing it. That will also mean it’s possible to build for Android on Windows since it all uses cmake, correct? That would also solve another problem I was going to have to tackle later. Thanks!

giahuy98 commented 8 months ago

That will also mean it’s possible to build for Android on Windows since it all uses cmake, correct?

Correct, it should work, but I haven't had a chance to try it yet.

PeteClubSeven commented 7 months ago

Fixed with the merge of #13