-
Notifications
You must be signed in to change notification settings - Fork 0
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
Implement CMake project #1
Conversation
8d12993
to
7ad30c2
Compare
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as off-topic.
This comment was marked as off-topic.
7ad30c2
to
2ba7b75
Compare
So there is a lot in this PR, but I will add some comments in the significant parts of the code to highlight a few paradimes |
option(TEMPLATE_TESTS "Template: Build test-suite" ${PROJECT_IS_TOP_LEVEL}) | ||
option(TEMPLATE_SHARED_LIBS "Template: Build as a shared library" ${PROJECT_IS_TOP_LEVEL}) | ||
option(TEMPLATE_INSTALL "Template: Install project" ${PROJECT_IS_TOP_LEVEL}) |
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.
Another common default I use are these 3 options with the defaults set according to if the project is included as FetchContent
or built as standalone. I believe SHARED_LIBS
is a sensible option here because downstream projects should avoid linking as shared library and potentially being overwritten by the system installed libraries, unless they know what they are doing
FetchContent_Declare(fmt | ||
GIT_REPOSITORY https://github.com/fmtlib/fmt | ||
GIT_TAG master | ||
FIND_PACKAGE_ARGS MODULE | ||
) | ||
|
||
FetchContent_MakeAvailable(fmt) |
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.
With FIND_PACKAGE_ARGS
, FetchContent
is the most flexible way of defining the external dependencies since it can be controlled via CMAKE_REQUIRE_FIND_PACKAGE_<PackageName>
and CMAKE_DISABLE_FIND_PACKAGE_<PackageName>
. Also see Findfmt.cmake
for further implementation.
Template_FindPackage(${CMAKE_FIND_PACKAGE_NAME} | ||
NAMES fmt | ||
PKG_MODULE_NAMES fmt | ||
) | ||
|
||
# Create appropriate aliases | ||
if (${CMAKE_FIND_PACKAGE_NAME}_PKGCONFIG) | ||
add_library(fmt::fmt ALIAS PkgConfig::${CMAKE_FIND_PACKAGE_NAME}) | ||
endif () | ||
set_package_properties(${CMAKE_FIND_PACKAGE_NAME} PROPERTIES | ||
URL https://github.com/fmtlib/fmt | ||
DESCRIPTION "A modern formatting library " | ||
) |
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.
The goal of this Findfmt.cmake
module is to make a unified interface of fmt::fmt
regardless of if it is imported via FetchContent
(outside of this file), or find_package(CONFIG)
or pkg-config
(handled in Template_FindPackage
). This could also be expanded to handled un-packaged libraries, but that has to be defined manually
test/CMakeLists.txt
Outdated
project(Template_test) | ||
|
||
set(CMAKE_CXX_STANDARD 17) | ||
set(CMAKE_CXX_STANDARD_REQUIRED ON) | ||
set(CMAKE_CXX_EXTENSIONS ON) | ||
|
||
set(external_libs) | ||
if (NOT Template_IS_TOP_LEVEL) | ||
FetchContent_Declare(Template | ||
GIT_REPOSITORY https://github.com/LecrisUT/CMake-Template | ||
GIT_TAG main | ||
FIND_PACKAGE_ARGS CONFIG | ||
) | ||
list(APPEND external_libs Template) | ||
endif () |
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.
This structure I think it is good to advertise. This allows for importing and running the tests individually from an external testing suite like tmt
or from a cross-compiled environment like in conda
.
set(test_list | ||
FetchContent | ||
find_package | ||
) | ||
if (NOT Template_IS_TOP_LEVEL) | ||
list(APPEND test_list pkg-config) | ||
endif () |
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.
Packaging tests, also callable from tmt
. pkg-config
I don't think can be easily tested outside of a tmt
environment because of the configure_file
format.
.packit.yaml
Outdated
- job: copr_build | ||
trigger: pull_request | ||
owner: lecris | ||
project: cmake-template | ||
targets: | ||
- fedora-all-x86_64 | ||
- fedora-all-aarch64 | ||
- epel-9-x86_64 | ||
- epel-9-aarch64 | ||
- job: tests | ||
trigger: pull_request | ||
targets: | ||
- fedora-all-x86_64 | ||
- fedora-all-aarch64 | ||
- epel-9-x86_64 | ||
- epel-9-aarch64 | ||
fmf_path: .distro |
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.
Fedora packaging with tests :)
@@ -0,0 +1,29 @@ | |||
repos: |
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.
Common pre-commits, maybe can include more/better tools?
This comment was marked as off-topic.
This comment was marked as off-topic.
Signed-off-by: Cristian Le <[email protected]>
Signed-off-by: Cristian Le <[email protected]>
Signed-off-by: Cristian Le <[email protected]>
Signed-off-by: Cristian Le <[email protected]>
Signed-off-by: Cristian Le <[email protected]>
380198f
to
6a769e4
Compare
Signed-off-by: Cristian Le <[email protected]>
Signed-off-by: Cristian Le <[email protected]>
Signed-off-by: Cristian Le <[email protected]>
6a769e4
to
ddc23b1
Compare
Welcome to Codecov 🎉Once merged to your default branch, Codecov will compare your coverage reports and display the results in this comment. Thanks for integrating Codecov - We've got you covered ☂️ |
3da388d
to
52d7a26
Compare
Signed-off-by: Cristian Le <[email protected]>
Signed-off-by: Cristian Le <[email protected]>
Signed-off-by: Cristian Le <[email protected]>
Signed-off-by: Cristian Le <[email protected]>
Signed-off-by: Cristian Le <[email protected]>
Signed-off-by: Cristian Le <[email protected]>
Signed-off-by: Cristian Le <[email protected]>
Signed-off-by: Cristian Le <[email protected]>
52d7a26
to
efb23f2
Compare
4f31eca
to
6b0c40b
Compare
Signed-off-by: Cristian Le <[email protected]>
Signed-off-by: Cristian Le <[email protected]>
Signed-off-by: Cristian Le <[email protected]>
Signed-off-by: Cristian Le <[email protected]>
Catch2 version is not compatible and backporting FetchContent(FIND_PACKAGE_ARGS) from CMake 3.24 is a hustle with nested projects Signed-off-by: Cristian Le <[email protected]>
37d2d0b
to
cee25b2
Compare
Kudos, SonarCloud Quality Gate passed! |
No description provided.