TheLartians / ModernCppStarter

🚀 Kick-start your C++! A template for modern C++ projects using CMake, CI, code coverage, clang-format, reproducible dependency management and much more.
https://thelartians.github.io/ModernCppStarter
The Unlicense
4.33k stars 381 forks source link

Use ${PROJECT_NAME} instead of writing projectname multiple times #133

Closed DominicD closed 2 years ago

DominicD commented 3 years ago

Is there a reason ${PROJECT_NAME} is not used? I thought this is the recommended way to go about it? Also it makes it easier to do the initial setup with your template.

Example: Instead of:

project(
  Greeter
  VERSION 1.0
  LANGUAGES CXX
)

add_library(Greeter ${headers} ${sources})

set_target_properties(Greeter PROPERTIES CXX_STANDARD 17)

target_compile_options(Greeter PUBLIC "$<$<COMPILE_LANG_AND_ID:CXX,MSVC>:/permissive->")

target_link_libraries(Greeter PRIVATE fmt::fmt)

use :

project(
  Greeter
  VERSION 1.0
  LANGUAGES CXX
)

add_library(${PROJECT_NAME}${headers} ${sources})

set_target_properties(${PROJECT_NAME}PROPERTIES CXX_STANDARD 17)

target_compile_options(${PROJECT_NAME}PUBLIC "$<$<COMPILE_LANG_AND_ID:CXX,MSVC>:/permissive->")

target_link_libraries(${PROJECT_NAME}PRIVATE fmt::fmt)
ClausKlein commented 3 years ago

OK, but do not forget the spaces!

DominicD commented 3 years ago

Should I maybe create a pull request? I actually think this is a big oversight for a starter template.

TheLartians commented 3 years ago

I think the main reason I didn't use it was for consistency with the derived targets (tests, standalone, etc), that create their own projects and therefore aren't aware of the main project's name. One way to circumvent this could be to create a config.cmake file that can be included to declare the project name and version, however imo it would remove some of the simplicity of the template. Anyways, you should usually run a simple search-and-replace when customising the starter, so that all include paths etc reference the new name.

DominicD commented 3 years ago

Fair point. However I still think for a template that shall serve as a reference to modern CMake and good practices it should use it. I don´t know how handle it for the derived targets but still I think it is an improvement. I prepared it already: https://github.com/DominicD/ModernCppStarter/commit/2528b

If you want I will create a pull request if not its also fine

TheLartians commented 2 years ago

Hm I do see the advantages of avoiding redundancy. On the other hand, sometime it may be better to be explicit and see directly what the target in question is. Any idea how other popular C++ projects handle this?

DominicD commented 2 years ago

Some examples I found:

https://github.com/nlohmann/json/blob/develop/CMakeLists.txt https://github.com/gabime/spdlog/blob/v1.x/CMakeLists.txt https://github.com/CoatiSoftware/Sourcetrail/blob/master/CMakeLists.txt https://github.com/amrayn/easyloggingpp/blob/master/CMakeLists.txt https://github.com/aws/aws-sdk-cpp/blob/main/aws-cpp-sdk-wellarchitected/CMakeLists.txt

However I cant really say how popular they are. Lot of the really famous c and c++ projects dont use cmake e.g. OpenSSL, Boost, QT.

Still for me redundancy is always bad. So im curious when do you think it makes sense to not use the ${PROJECT_NAME}?

TheLartians commented 2 years ago

Cool thanks for the list! So it seems that quite a few libraries do use the project name or other variables to define the library name.

Still for me redundancy is always bad. So im curious when do you think it makes sense to not use the ${PROJECT_NAME}?

TBH not much, just that at the time I didn't have strong flinging and I felt that many projects were explicit in their target naming. Imo it may actually be good idea to use it, especially as we are already using ${PROJECT_NAME} in the PackageProject command.

Would be happy to review a PR for your branch!