-
Notifications
You must be signed in to change notification settings - Fork 22
(Draft) Use CPM for package fetching #560
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
Conversation
| option(YDB_SDK_EXAMPLES "Build YDB C++ SDK examples" On) | ||
| set(YDB_SDK_GOOGLE_COMMON_PROTOS_TARGET "" CACHE STRING "Name of cmake target preparing google common proto library") | ||
| option(YDB_SDK_USE_RAPID_JSON "Search for rapid json library in system" ON) | ||
| option(YDB_SDK_DOWNLOAD_PACKAGES "Download with CPM if there is no system-provided libraries" ON) # todo изменить сообщение |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Сложное поведение как по мне. Проще дать пользователю выбор download, system или none для каждого пакета. А по умолчанию сделать download
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Подглядел в доке усервера, там есть небольшое объяснение
Как мне кажется, получается более гибко, так можно будет одной переменной отключить скачивание всех пакетов разом. Полезно будет потом на PPA, где не будет интернета, или в случае, когда хотим устанавливать все
А так ниже для каждого пакета появляются свои переменные для установки, которые по-умолчанию равны этой и их можно переопределить вручную
| ) | ||
| endfunction() | ||
|
|
||
| function(mark_targets_as_system directory) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
А эта функция для чего нужна?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Там была проблема, что установка и сборка зависимостей была уже во время нашей сборки, а из них вылетали предупреждения. Увидел что в усервере отключали так, помогло
| # If A uses find_package(B), and we install A and B using CPM, then: 1. make sure to call write_package_stub in SetupB | ||
| # 2. make sure to call SetupB at the beginning of SetupA | ||
| function(write_package_stub PACKAGE_NAME) | ||
| file(WRITE "${CMAKE_BINARY_DIR}/package_stubs/${PACKAGE_NAME}Config.cmake" ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| file(WRITE "${CMAKE_BINARY_DIR}/package_stubs/${PACKAGE_NAME}Config.cmake" ) | |
| file(WRITE "${CMAKE_BINARY_DIR}/package_stubs/${PACKAGE_NAME}Config.cmake") |
| @@ -0,0 +1,75 @@ | |||
| include_guard() | |||
|
|
|||
| if(NOT DEFINED CPM_USE_NAMED_CACHE_DIRECTORIES) | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
А что этот флаг делает?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Вот тут увидел, вроде он не вредит и есть польза ссылка
| option(YDB_SDK_FORCE_DOWNLOAD_ABSEIL "Download Abseil even if it exists in a system" ${YDB_SDK_DOWNLOAD_PACKAGES}) | ||
|
|
||
| if(NOT YDB_SDK_FORCE_DOWNLOAD_ABSEIL) | ||
| set(ABSL_PROPAGATE_CXX_STD ON) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
А зачем этот флаг тут нужен? Он же нужен для сборки и установки abseil, тут-то зачем?
| abseil/abseil-cpp | ||
| OPTIONS | ||
| "ABSL_PROPAGATE_CXX_STD ON" | ||
| "ABSL_ENABLE_INSTALL ON" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
А тут точно нужен этот флаг?
| endif() | ||
| endmacro() | ||
|
|
||
| if(NOT YDB_SDK_FORCE_DOWNLOAD_GRPC) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
А почему тут поиск gRPC так усложнился?
| # include(${CMAKE_CURRENT_LIST_DIR}/SetupProtobuf.cmake) | ||
| include(DownloadUsingCPM) | ||
|
|
||
| set(YDB_SDK_GPRC_BUILD_FROM_SOURCE ON) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
А зачем этот флаг?
| "BUILD_SHARED_LIBS OFF" | ||
| "CARES_BUILD_TOOLS OFF" | ||
| "RE2_BUILD_TESTING OFF" | ||
| "OPENSSL_NO_ASM ON" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
А зачем OPENSSL_NO_ASM ставить?
|
Duplicates #566 |
No description provided.