Skip to content

Update CMake config#36

Open
yaigorian wants to merge 22 commits into
DafeCpp:mainfrom
yaigorian:feature
Open

Update CMake config#36
yaigorian wants to merge 22 commits into
DafeCpp:mainfrom
yaigorian:feature

Conversation

@yaigorian
Copy link
Copy Markdown

Улучшения:

  1. раньше корневой CMakeLists.txt рекурсивно сканировал всё дерево и сам пытался найти шаблон.
  2. sandbox и additional_tasks теперь опциональны. В CMakeLists.txt есть опции BUILD_SANDBOX и BUILD_ADDITIONAL_TASKS, по умолчанию OFF.
  3. улучшена читабельность добавлением cmake module
  4. раньше логика угадывала файлы через GLOB, теперь они явно прописаны, так как CMake принято писать в декларативном стиле
  5. улучшена логика сборки проекта, теперь можно собирать каждое задание по отдельности, это добавляет IDE-friendly, так как раньше CMake был настроен на codespaces
  6. теперь библиотека gtest подключается одиножды в корне, раньше подключение было в каждой задаче
  7. Упросщен обход sandbox и additional_tasks: теперь только прямые подпапки с CMakeLists.txt, без рекурсивного обхода всего дерева.
    Глобально: раньше CMake пытался сам догадаться, как устроен репозиторий - теперь репозиторий сам явно описывает свою структуру.

Comment thread task_04/CMakeLists.txt
string(REPLACE " " "_" PROJECT_NAME ${PROJECT_NAME})
project(${PROJECT_NAME} C CXX)

set(CMAKE_CXX_STANDARD 23)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

заголовочные файлы (.hpp) не нужно перечислять в SOURCES и TEST_SOURCES — CMake их не компилирует. Достаточно оставить только .cpp файлы. То же самое в task_05 и lib/CMakeLists.txt

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

убрал .hpp из SOURCES и TEST_SOURCES, они, действительно, не нужны там

Comment thread task_06/CMakeLists.txt
cmake_minimum_required(VERSION 3.10)

get_filename_component(PROJECT_NAME ${CMAKE_CURRENT_LIST_DIR} NAME)
string(REPLACE " " "_" PROJECT_NAME ${PROJECT_NAME})
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

здесь перечислен только main.cpp, но студенты обычно добавляют отдельные .cpp файлы с реализацией. Если они не допишут SOURCES в CMakeLists.txt, получат ошибку линковки. Стоит добавить это в README или в шаблон CMakeLists.txt как комментарий-подсказку

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Добавил в каждый CMakeList предупреждение, а также добавил рекомендацию в README

Comment thread CMakeLists.txt
task_10
task_11
task_12
task_13
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

тесты собираются только если передан флаг -DBUILD_TESTING=ON. Нужно убедиться, что GitHub Actions workflow явно передаёт этот флаг при вызове cmake, иначе тесты не будут собираться и CI будет «зелёным» без реальной проверки

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Добавил флаг -DBUILD_TESTING=ON в шаг run cmake в .github/workflows/tests.yaml, теперь тесты собираются и CI их гарантировано проверяет.

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.

2 participants