tdlib / td

Cross-platform library for building Telegram clients
https://core.telegram.org/tdlib
Boost Software License 1.0
7.11k stars 1.44k forks source link

Update iOS example to build OpenSSL 3.1.1 #2517

Closed vincentneo closed 1 year ago

vincentneo commented 1 year ago

This pull request includes modified script and patch which I personally use to compile the latest OpenSSL for tdlib.

openssl-1.0.2n-darwin-arm64.patch added in #1620, has also been removed as the script has been modified to use the currently newest commit of Python-Apple-support, which does supports macOS arm64 compilation (Reference)

levlam commented 1 year ago

OpenSSL 3.* is much slower and has significantly bigger binary size than previous OpenSSL versions. Any older version of OpenSSL is much better as a general purpose cryptographic library, and hence it is more preferable for TDLib.

Unfortunately, OpenSSL 1.1.1 will reach EOL in less than 3 month, which forces switching to OpenSSL 3.*. Therefore, I will accept the pull request after testing anyway, but overall it will bring only downsides to the iOS build example.

vincentneo commented 1 year ago

OpenSSL 3.* is much slower and has significantly bigger binary size

Yes, it looks to be about twice as large as before, though I hadn't realised (at least TDLib as a whole) being slower because of OpenSSL 3

OpenSSL 1.1.1 will reach EOL in less than 3 month

Yeah that was why I personally used the 3.x release, more of a precaution for me to get things ready when the time comes.

Kylmakalle commented 1 year ago

@vincentneo Please consider removing armv7 support from other parts of TDLib. cc @levlam

My simple patch I'm using daily.

vincentneo commented 1 year ago

@Kylmakalle If you mean all armv7 support, I don't think that is a good idea.

With my pull request I remember that only armv7k of watchOS is retained, out of all armv7 architectures support. While it is perfectly reasonable to drop armv7 support for iOS, since the last device that uses it is already 10 years old (iPhone 5), I don't think it is a good idea to drop armv7 support for watchOS.

That is because for projects targeting at minimum watchOS 8.x, armv7k is still a supported architecture (used for Series 3 watches), which in my opinion is still worth preserving for now, as watchOS 8 is still a fairly recent release, one that is still supported, and is last updated in June 2023, with watchOS 8.8.1. (Reference)

Kylmakalle commented 1 year ago

@vincentneo Let me clarify: Previously with v1.1.1 TARGETS-iOS=iphoneos.armv7 iphoneos.armv7s iphoneos.arm64

now with v3 TARGETS-iOS=iphoneos.arm64

vincentneo commented 1 year ago

@Kylmakalle thanks for clarifying. Agreed on your sentiment on armv7 and armv7s for iOS.

Kylmakalle commented 1 year ago

Gave this PR a try on my CI. Builds just fine. OpenSSL is slightly smaller for iOS btw.

vincentneo commented 1 year ago

@Kylmakalle Actually to think about it, probably tdlib won't build for iOS without patching away armv7 since this openssl build doesn't have it.

openssl on iOS is smaller due to lack of armv7 and armv7s binaries by the way

levlam commented 1 year ago

Thank you, @vincentneo!

And thank you, @Kylmakalle for the valuable feedback.