search

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

Add option to install only BUILD_SHARED_LIBS-matching files #3081

Closed OPNA2608 closed 1 week ago

OPNA2608 commented 3 weeks ago

For Debian, I've been asked to get rid of the non-shared libraries that get installed by tdlib. This was previously mentioned in #1810, with the TL;DR there being "libtdjson_static must be static, the rest are internal and maybe shouldn't be installed".

Here is a shot at resolving this by:

levlam commented 3 weeks ago

Adding the option BUILD_SHARED_LIBS explicitly is redundant and harmful and must never be done. This just breaks all projects that add the current project as subproject and doesn't add the option explicitly, which is described in the mentioned documentation.

tdmtproto, tdcore and other mentioned libraries are not suitable to be used as separate shared libraries and must not be used as such. libtdjson.so must be installed as a single shared library.

Also, see the last paragraph at https://github.com/tdlib/td/issues/2696#issuecomment-2037015283, which seems to be relevant.

If you prefer, another option can be added that installs only shared/static libraries if the option is enabled.

OPNA2608 commented 3 weeks ago

Adding the option BUILD_SHARED_LIBS explicitly is redundant and harmful and must never be done. This just breaks all projects that add the current project as subproject and doesn't add the option explicitly, which is described in the mentioned documentation.

The documentation explicitly mentions that projects often define this as an option(), so I'm not sure about calling this "must never be done" behaviour. If you wish to care about the use-cases you describe, then it should merely require some more careful handling no? I.e. only defining the option when the project is the top-level one would prolly work.

But sure, can not introduce this if absolutely undesired.

tdmtproto, tdcore and other mentioned libraries are not suitable to be used as separate shared libraries and must not be used as such.

Then they likely shouldn't be installed in the first place. If you're keeping them around to facilitate linking of the static tdjson, where the dependencies must be available & provided when linking against it, then maybe it would make sense to make them object libraries and only link them into one final user-available static library at the end?

libtdjson.so must be installed as a single shared library.

Even when BUILD_SHARED_LIBS:BOOL=OFF is set, which is the user explicitly telling you "I don't want shared libraries"?

If you prefer, another option can be added that installs only shared/static libraries if the option is enabled.

If always linking both variants is a requirement, then that would be a compromise I'm okay with.

levlam commented 3 weeks ago

The documentation explicitly mentions that projects often define this as an option(), so I'm not sure about calling this "must never be done" behaviour. If you wish to care about the use-cases you describe, then it should merely require some more careful handling no? I.e. only defining the option when the project is the top-level one would prolly work.

The documentation mentions this because other projects must be aware, that doing so can break building of the outer project, unless the outer project does the same. Hence, if you are forced to include such harmful subproject, then you must also become a harmful project. Otherwise, everything works by default correctly without any explicit options.

Then they likely shouldn't be installed in the first place.

They are needed for native C++ interface and libtdjson_static, which aren't usually used. That's why an option to install only the shared library without C++ headers can be added.

then maybe it would make sense to make them object libraries and only link them into one final user-available static library at the end

Object libraries can't be used in target_link_libraries() before CMake 3.12. Therefore, this can't be done without dropping support for older CMake versions and corresponding operating systems. Someday, this will be done, but not yet.

Even when BUILD_SHARED_LIBS:BOOL=OFF is set, which is the user explicitly telling you "I don't want shared libraries"?

No, it doesn't mean this. As any other options BUILD_SHARED_LIBS is false by default, hence, it means "build static libraries by default". It tells nothing about libraries like tdjson that must be shared, because otherwise they can't be used from most programming languages.

If always linking both variants is a requirement, then that would be a compromise I'm okay with.

The option can remove static/shared target libraries also, but this will take neglible effect on the build time.

OPNA2608 commented 2 weeks ago

I've introduced an option TD_ENABLE_FILTERED_INSTALL (feel free to suggest a better / change the name for this) to enable the discussed behaviour, defaulting to OFF to preserve current behaviour.

Listing of installed files for each relevant option combination ##### `-DTD_ENABLE_FILTERED_INSTALL:BOOL=ON -DBUILD_SHARED_LIBS:BOOL=ON` ``` -- Installing: /nix/store/s22jhr5ijix95nn9rf8ak4xxx643n0c7-tdlib-local/lib/libtdjson.so.1.8.37 -- Set non-toolchain portion of runtime path of "/nix/store/s22jhr5ijix95nn9rf8ak4xxx643n0c7-tdlib-local/lib/libtdjson.so.1.8.37" to "" -- Installing: /nix/store/s22jhr5ijix95nn9rf8ak4xxx643n0c7-tdlib-local/lib/libtdjson.so -- Installing: /nix/store/s22jhr5ijix95nn9rf8ak4xxx643n0c7-tdlib-local/lib/libtdclient.so.1.8.37 -- Set non-toolchain portion of runtime path of "/nix/store/s22jhr5ijix95nn9rf8ak4xxx643n0c7-tdlib-local/lib/libtdclient.so.1.8.37" to "" -- Installing: /nix/store/s22jhr5ijix95nn9rf8ak4xxx643n0c7-tdlib-local/lib/libtdclient.so -- Installing: /nix/store/s22jhr5ijix95nn9rf8ak4xxx643n0c7-tdlib-local/lib/libtdapi.so.1.8.37 -- Installing: /nix/store/s22jhr5ijix95nn9rf8ak4xxx643n0c7-tdlib-local/lib/libtdapi.so -- Installing: /nix/store/s22jhr5ijix95nn9rf8ak4xxx643n0c7-tdlib-local/lib/pkgconfig/tdclient.pc -- Installing: /nix/store/s22jhr5ijix95nn9rf8ak4xxx643n0c7-tdlib-local/lib/pkgconfig/tdapi.pc -- Installing: /nix/store/s22jhr5ijix95nn9rf8ak4xxx643n0c7-tdlib-local/lib/pkgconfig/tdjson.pc -- Installing: /nix/store/s22jhr5ijix95nn9rf8ak4xxx643n0c7-tdlib-local/lib/cmake/Td/TdTargets.cmake -- Installing: /nix/store/s22jhr5ijix95nn9rf8ak4xxx643n0c7-tdlib-local/lib/cmake/Td/TdTargets-release.cmake -- Installing: /nix/store/s22jhr5ijix95nn9rf8ak4xxx643n0c7-tdlib-local/include/td/telegram/td_json_client.h -- Installing: /nix/store/s22jhr5ijix95nn9rf8ak4xxx643n0c7-tdlib-local/include/td/telegram/td_log.h -- Installing: /nix/store/s22jhr5ijix95nn9rf8ak4xxx643n0c7-tdlib-local/include/td/telegram/tdjson_export.h -- Installing: /nix/store/s22jhr5ijix95nn9rf8ak4xxx643n0c7-tdlib-local/include/td/telegram/Client.h -- Installing: /nix/store/s22jhr5ijix95nn9rf8ak4xxx643n0c7-tdlib-local/include/td/telegram/Log.h -- Installing: /nix/store/s22jhr5ijix95nn9rf8ak4xxx643n0c7-tdlib-local/include/td/tl/TlObject.h -- Installing: /nix/store/s22jhr5ijix95nn9rf8ak4xxx643n0c7-tdlib-local/include/td/telegram/td_api.h -- Installing: /nix/store/s22jhr5ijix95nn9rf8ak4xxx643n0c7-tdlib-local/include/td/telegram/td_api.hpp -- Installing: /nix/store/s22jhr5ijix95nn9rf8ak4xxx643n0c7-tdlib-local/lib/cmake/Td/TdConfig.cmake -- Installing: /nix/store/s22jhr5ijix95nn9rf8ak4xxx643n0c7-tdlib-local/lib/cmake/Td/TdConfigVersion.cmake ``` ##### `-DTD_ENABLE_FILTERED_INSTALL:BOOL=ON -DBUILD_SHARED_LIBS:BOOL=OFF` ``` -- Installing: /nix/store/6s62fqhgb93p1hbwvmjhmh6wm2l5314j-tdlib-local/lib/libtdjson_static.a -- Installing: /nix/store/6s62fqhgb93p1hbwvmjhmh6wm2l5314j-tdlib-local/lib/libtdjson_private.a -- Installing: /nix/store/6s62fqhgb93p1hbwvmjhmh6wm2l5314j-tdlib-local/lib/libtdcore.a -- Installing: /nix/store/6s62fqhgb93p1hbwvmjhmh6wm2l5314j-tdlib-local/lib/libtdmtproto.a -- Installing: /nix/store/6s62fqhgb93p1hbwvmjhmh6wm2l5314j-tdlib-local/lib/libtdclient.a -- Installing: /nix/store/6s62fqhgb93p1hbwvmjhmh6wm2l5314j-tdlib-local/lib/libtdapi.a -- Installing: /nix/store/6s62fqhgb93p1hbwvmjhmh6wm2l5314j-tdlib-local/lib/pkgconfig/tdutils.pc -- Installing: /nix/store/6s62fqhgb93p1hbwvmjhmh6wm2l5314j-tdlib-local/lib/pkgconfig/tdactor.pc -- Installing: /nix/store/6s62fqhgb93p1hbwvmjhmh6wm2l5314j-tdlib-local/lib/pkgconfig/tdnet.pc -- Installing: /nix/store/6s62fqhgb93p1hbwvmjhmh6wm2l5314j-tdlib-local/lib/pkgconfig/tdsqlite.pc -- Installing: /nix/store/6s62fqhgb93p1hbwvmjhmh6wm2l5314j-tdlib-local/lib/pkgconfig/tddb.pc -- Installing: /nix/store/6s62fqhgb93p1hbwvmjhmh6wm2l5314j-tdlib-local/lib/pkgconfig/tdmtproto.pc -- Installing: /nix/store/6s62fqhgb93p1hbwvmjhmh6wm2l5314j-tdlib-local/lib/pkgconfig/tdcore.pc -- Installing: /nix/store/6s62fqhgb93p1hbwvmjhmh6wm2l5314j-tdlib-local/lib/pkgconfig/tdclient.pc -- Installing: /nix/store/6s62fqhgb93p1hbwvmjhmh6wm2l5314j-tdlib-local/lib/pkgconfig/tdapi.pc -- Installing: /nix/store/6s62fqhgb93p1hbwvmjhmh6wm2l5314j-tdlib-local/lib/pkgconfig/tdjson_private.pc -- Installing: /nix/store/6s62fqhgb93p1hbwvmjhmh6wm2l5314j-tdlib-local/lib/pkgconfig/tdjson_static.pc -- Installing: /nix/store/6s62fqhgb93p1hbwvmjhmh6wm2l5314j-tdlib-local/lib/cmake/Td/TdStaticTargets.cmake -- Installing: /nix/store/6s62fqhgb93p1hbwvmjhmh6wm2l5314j-tdlib-local/lib/cmake/Td/TdStaticTargets-release.cmake -- Installing: /nix/store/6s62fqhgb93p1hbwvmjhmh6wm2l5314j-tdlib-local/include/td/telegram/td_json_client.h -- Installing: /nix/store/6s62fqhgb93p1hbwvmjhmh6wm2l5314j-tdlib-local/include/td/telegram/td_log.h -- Installing: /nix/store/6s62fqhgb93p1hbwvmjhmh6wm2l5314j-tdlib-local/include/td/telegram/tdjson_export.h -- Installing: /nix/store/6s62fqhgb93p1hbwvmjhmh6wm2l5314j-tdlib-local/include/td/telegram/Client.h -- Installing: /nix/store/6s62fqhgb93p1hbwvmjhmh6wm2l5314j-tdlib-local/include/td/telegram/Log.h -- Installing: /nix/store/6s62fqhgb93p1hbwvmjhmh6wm2l5314j-tdlib-local/include/td/tl/TlObject.h -- Installing: /nix/store/6s62fqhgb93p1hbwvmjhmh6wm2l5314j-tdlib-local/include/td/telegram/td_api.h -- Installing: /nix/store/6s62fqhgb93p1hbwvmjhmh6wm2l5314j-tdlib-local/include/td/telegram/td_api.hpp -- Installing: /nix/store/6s62fqhgb93p1hbwvmjhmh6wm2l5314j-tdlib-local/lib/cmake/Td/TdConfig.cmake -- Installing: /nix/store/6s62fqhgb93p1hbwvmjhmh6wm2l5314j-tdlib-local/lib/cmake/Td/TdConfigVersion.cmake -- Installing: /nix/store/6s62fqhgb93p1hbwvmjhmh6wm2l5314j-tdlib-local/lib/libtdutils.a -- Installing: /nix/store/6s62fqhgb93p1hbwvmjhmh6wm2l5314j-tdlib-local/lib/libtdactor.a -- Installing: /nix/store/6s62fqhgb93p1hbwvmjhmh6wm2l5314j-tdlib-local/lib/libtdnet.a -- Installing: /nix/store/6s62fqhgb93p1hbwvmjhmh6wm2l5314j-tdlib-local/lib/libtdsqlite.a -- Installing: /nix/store/6s62fqhgb93p1hbwvmjhmh6wm2l5314j-tdlib-local/lib/libtddb.a ``` ##### `-DTD_ENABLE_FILTERED_INSTALL:BOOL=OFF` ``` -- Installing: /nix/store/gjjvqv5vdkm2d8jkl3sx2173iknw8271-tdlib-local/lib/libtdjson.so.1.8.37 -- Set non-toolchain portion of runtime path of "/nix/store/gjjvqv5vdkm2d8jkl3sx2173iknw8271-tdlib-local/lib/libtdjson.so.1.8.37" to "" -- Installing: /nix/store/gjjvqv5vdkm2d8jkl3sx2173iknw8271-tdlib-local/lib/libtdjson.so -- Installing: /nix/store/gjjvqv5vdkm2d8jkl3sx2173iknw8271-tdlib-local/lib/libtdclient.so.1.8.37 -- Set non-toolchain portion of runtime path of "/nix/store/gjjvqv5vdkm2d8jkl3sx2173iknw8271-tdlib-local/lib/libtdclient.so.1.8.37" to "" -- Installing: /nix/store/gjjvqv5vdkm2d8jkl3sx2173iknw8271-tdlib-local/lib/libtdclient.so -- Installing: /nix/store/gjjvqv5vdkm2d8jkl3sx2173iknw8271-tdlib-local/lib/libtdapi.so.1.8.37 -- Installing: /nix/store/gjjvqv5vdkm2d8jkl3sx2173iknw8271-tdlib-local/lib/libtdapi.so -- Installing: /nix/store/gjjvqv5vdkm2d8jkl3sx2173iknw8271-tdlib-local/lib/libtdjson_static.a -- Installing: /nix/store/gjjvqv5vdkm2d8jkl3sx2173iknw8271-tdlib-local/lib/libtdjson_private.a -- Installing: /nix/store/gjjvqv5vdkm2d8jkl3sx2173iknw8271-tdlib-local/lib/libtdcore.a -- Installing: /nix/store/gjjvqv5vdkm2d8jkl3sx2173iknw8271-tdlib-local/lib/libtdmtproto.a -- Installing: /nix/store/gjjvqv5vdkm2d8jkl3sx2173iknw8271-tdlib-local/lib/pkgconfig/tdutils.pc -- Installing: /nix/store/gjjvqv5vdkm2d8jkl3sx2173iknw8271-tdlib-local/lib/pkgconfig/tdactor.pc -- Installing: /nix/store/gjjvqv5vdkm2d8jkl3sx2173iknw8271-tdlib-local/lib/pkgconfig/tdnet.pc -- Installing: /nix/store/gjjvqv5vdkm2d8jkl3sx2173iknw8271-tdlib-local/lib/pkgconfig/tdsqlite.pc -- Installing: /nix/store/gjjvqv5vdkm2d8jkl3sx2173iknw8271-tdlib-local/lib/pkgconfig/tddb.pc -- Installing: /nix/store/gjjvqv5vdkm2d8jkl3sx2173iknw8271-tdlib-local/lib/pkgconfig/tdmtproto.pc -- Installing: /nix/store/gjjvqv5vdkm2d8jkl3sx2173iknw8271-tdlib-local/lib/pkgconfig/tdcore.pc -- Installing: /nix/store/gjjvqv5vdkm2d8jkl3sx2173iknw8271-tdlib-local/lib/pkgconfig/tdclient.pc -- Installing: /nix/store/gjjvqv5vdkm2d8jkl3sx2173iknw8271-tdlib-local/lib/pkgconfig/tdapi.pc -- Installing: /nix/store/gjjvqv5vdkm2d8jkl3sx2173iknw8271-tdlib-local/lib/pkgconfig/tdjson_private.pc -- Installing: /nix/store/gjjvqv5vdkm2d8jkl3sx2173iknw8271-tdlib-local/lib/pkgconfig/tdjson.pc -- Installing: /nix/store/gjjvqv5vdkm2d8jkl3sx2173iknw8271-tdlib-local/lib/pkgconfig/tdjson_static.pc -- Installing: /nix/store/gjjvqv5vdkm2d8jkl3sx2173iknw8271-tdlib-local/lib/cmake/Td/TdTargets.cmake -- Installing: /nix/store/gjjvqv5vdkm2d8jkl3sx2173iknw8271-tdlib-local/lib/cmake/Td/TdTargets-release.cmake -- Installing: /nix/store/gjjvqv5vdkm2d8jkl3sx2173iknw8271-tdlib-local/lib/cmake/Td/TdStaticTargets.cmake -- Installing: /nix/store/gjjvqv5vdkm2d8jkl3sx2173iknw8271-tdlib-local/lib/cmake/Td/TdStaticTargets-release.cmake -- Installing: /nix/store/gjjvqv5vdkm2d8jkl3sx2173iknw8271-tdlib-local/include/td/telegram/td_json_client.h -- Installing: /nix/store/gjjvqv5vdkm2d8jkl3sx2173iknw8271-tdlib-local/include/td/telegram/td_log.h -- Installing: /nix/store/gjjvqv5vdkm2d8jkl3sx2173iknw8271-tdlib-local/include/td/telegram/tdjson_export.h -- Installing: /nix/store/gjjvqv5vdkm2d8jkl3sx2173iknw8271-tdlib-local/include/td/telegram/Client.h -- Installing: /nix/store/gjjvqv5vdkm2d8jkl3sx2173iknw8271-tdlib-local/include/td/telegram/Log.h -- Installing: /nix/store/gjjvqv5vdkm2d8jkl3sx2173iknw8271-tdlib-local/include/td/tl/TlObject.h -- Installing: /nix/store/gjjvqv5vdkm2d8jkl3sx2173iknw8271-tdlib-local/include/td/telegram/td_api.h -- Installing: /nix/store/gjjvqv5vdkm2d8jkl3sx2173iknw8271-tdlib-local/include/td/telegram/td_api.hpp -- Installing: /nix/store/gjjvqv5vdkm2d8jkl3sx2173iknw8271-tdlib-local/lib/cmake/Td/TdConfig.cmake -- Installing: /nix/store/gjjvqv5vdkm2d8jkl3sx2173iknw8271-tdlib-local/lib/cmake/Td/TdConfigVersion.cmake -- Installing: /nix/store/gjjvqv5vdkm2d8jkl3sx2173iknw8271-tdlib-local/lib/libtdutils.a -- Installing: /nix/store/gjjvqv5vdkm2d8jkl3sx2173iknw8271-tdlib-local/lib/libtdactor.a -- Installing: /nix/store/gjjvqv5vdkm2d8jkl3sx2173iknw8271-tdlib-local/lib/libtdnet.a -- Installing: /nix/store/gjjvqv5vdkm2d8jkl3sx2173iknw8271-tdlib-local/lib/libtdsqlite.a -- Installing: /nix/store/gjjvqv5vdkm2d8jkl3sx2173iknw8271-tdlib-local/lib/libtddb.a ```
levlam commented 1 week ago

Thank you, @OPNA2608!

I will merge the changes, despite I don't like that combination of options is required to achieve behavior changes and that the standard option BUILD_SHARED_LIBS is used for what it isn't supposed to be used.

Also, it is odd that behavior of generate_pkgconfig and libraries from subdirectories is affected by the top-level option TD_ENABLE_FILTERED_INSTALL, which makes hard to reuse them.

I will fix both issues myself after merging the commit.

levlam commented 1 week ago

I replaced the option TD_ENABLE_FILTERED_INSTALL with separate options TD_INSTALL_STATIC_LIBRARIES and TD_INSTALL_SHARED_LIBRARIES, which can be used to control whether static/shared libraries are installed.