search

Kurama622 / StartUp

Terminal Dashboard
GNU General Public License v3.0
12 stars 2 forks source link

Could you please replace FetchContent_Declare(ftxui) with find_package(ftxui REQUIRED)? #3

Closed yurivict closed 1 year ago

Kurama622 commented 1 year ago

Why do you want to replace it？ I just kept it consistent with FTXUI.

yurivict commented 1 year ago

Why: Because downloads aren't allowed during package build.

I just kept it consistent with FTXUI.

But it is inconsistent with FTXUI: FTXUI provides cmake files and expects users to use find_package to find it, and you don't do this.

Kurama622 commented 1 year ago

Maybe the information we consulted is different.

I read the description of FTXUI, and some tools written by the author.

https://github.com/ArthurSonzogni/FTXUI/tree/master#external-package

It is highly recommended to use CMake FetchContent to depend on FTXUI. This way you can specify which commit you would like to depend on.


include(FetchContent)

FetchContent_Declare(ftxui GIT_REPOSITORY https://github.com/ArthurSonzogni/ftxui GIT_TAG v3.0.0 )

FetchContent_GetProperties(ftxui) if(NOT ftxui_POPULATED) FetchContent_Populate(ftxui) add_subdirectory(${ftxui_SOURCE_DIR} ${ftxui_BINARY_DIR} EXCLUDE_FROM_ALL) endif()



[json-tui cmakelist](https://github.com/ArthurSonzogni/json-tui/blob/main/CMakeLists.txt)
yurivict commented 1 year ago

It is highly recommended to use CMake FetchContent to depend on FTXUI. This way you can specify which commit you would like to depend on.

This is simply a wrong recommendation.

Imagine that you replace FTXUI with software package "X", and you would follow this recommendation everywhere and for everything. All software would download dependencies, and these dependencies would download their dependencies, and so on and so forth. This would cause exponential explosion of CPU time needed to build everything, exponential explosion of executable sizes, required disk space and network bandwidth. This is obviously an unsustainable situation.