Skip to content
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

Update iOS example to build OpenSSL 3.1.1 #2517

Merged
merged 1 commit into from
Jul 30, 2023

Conversation

vincentneo
Copy link
Contributor

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
Copy link
Contributor

levlam commented Jun 30, 2023

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
Copy link
Contributor Author

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 added a commit to Swiftgram/TDLibFramework that referenced this pull request Jul 6, 2023
@Kylmakalle
Copy link
Contributor

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

My simple patch I'm using daily.

@vincentneo
Copy link
Contributor Author

@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
Copy link
Contributor

@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
Copy link
Contributor Author

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

@Kylmakalle
Copy link
Contributor

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

@vincentneo
Copy link
Contributor Author

vincentneo commented Jul 6, 2023

@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 levlam merged commit 6a6cd8a into tdlib:master Jul 30, 2023
@levlam
Copy link
Contributor

levlam commented Jul 30, 2023

Thank you, @vincentneo!

And thank you, @Kylmakalle for the valuable feedback.

Kylmakalle added a commit to Swiftgram/TDLibFramework that referenced this pull request Jul 31, 2023
Kylmakalle added a commit to Swiftgram/TDLibFramework that referenced this pull request Jul 31, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants