-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Propose two new options for building #3291
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
base: master
Are you sure you want to change the base?
Conversation
ec0273d
to
8443c90
Compare
Hi @levlam! These modifications are ready to be reviewed, could you help take a look? |
Oh, a little bit surprising that there's already similar one being proposed years ago. I should've done some search there.
Effectively, 1127's modifications are almost identical to mine, except one extra switch for tgcli example. Honestly say, I wouldn't choose to add such switch, because downstreams always rely on the successful compilation of all existing sources. But running the tests/benchmarks is just another different situation, adding an option to disable is likely relaxing the burdens for downstreams. And my rationale above is largely based on my practical experience of using tdlib as an in-tree dependency, that using the exactly the same build system could maximize the integration. But I did suffer from the building process because of such missing configurations, during development. It's undoubtedly true they are essential for the release build. But isn't it more true that the frequency of making debug build is way higher than making a release one? And the current settings provide no options to reduce such pain, except we manually edit tdlib's source to disable it or tdlib offering such an built-in option for us.
I think, if they too essential to be offered to disable, they should be included by default(which is already true by this patch). And additionally, we can emphasize their essence by not offering the documents for these switches, or adding some warning comments, or things like that.
…-------- Original Message --------
On 5/2/25 21:45, Aliaksei Levin wrote:
levlam left a comment [(tdlib/td#3291)](#3291 (comment))
***@***.***(https://github.com/micl2e2)
The proposed changes mostly duplicate the changes proposed in [#1127](#1127).
—
Reply to this email directly, [view it on GitHub](#3291 (comment)), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/A2QUWQ6OL5LGVHRUJTGSZVT24NZGZAVCNFSM6AAAAAB2A4IPKSVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDQNBXGI2DSNRZGU).
You are receiving this because you were mentioned.Message ID: ***@***.***>
|
The changes from 1127 aren't merged, because for each issue they solve there are much better solutions then adding another configuration parameter.
If you use TDLib in-tree and
This way only targets explicitly used by your project will be ever built.
Debug builds are incremental. Tests and benchmarks are never rebuilt unless changed. They add nearly zero overhead to incremental builds. |
|
This patch is proposing two new build flags:
TD_BUILD_BENCHMARK
andTD_BUILD_TESTING
, both for the ability of enabling/disabling corresponding submodule's building and running.The major rationale is that, for some downstream projects that use tdlib as an in-tree dependency(rather than installed one), building the project means building the tdlib, and testing the project means testing the tdlib. While benchmarks and tests are essential submodules, it is not to be distributed to the end users. so in some cases where a downstream has full confidence for the quality of current tdlib's codebase, during a frequent edit-build-debug cycle, it is sensible to disable these two submodules, which could potentially help a third-party app gain a even smoother development experience, which fullfills the very first goal introduces at https://core.telegram.org/tdlib.
Here are some benefits we could gain:
build
dir, skipping these two modules would be undoubtedly significant savings:And for some downstream projects that use tdlib as an in-tree dependency, the build would be located in those projects' root or somewhere else if their any special preferences.
at least some minutes required for a relatively performant machine, and they are the cases where cache tools like ccache won't provide any help. These additional time won't be recognized by installed-tdlib downstreams, but in-tree-tdlib downstreams, who have their own tests and also use cmake's testing workflow to run the tests. While CTest already provides a builtin
BUILD_TESTING
switch, it is for more general cases, switching off it means switching off project's all ctest-based tests, not just tdlib's ones, so a dedicated option would help to customize tests running according to downstreams' special needs. And those new added options are ON by default to make behaviors consistent.