Skip to content
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

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

Closed
DominicD opened this issue Sep 7, 2021 · 7 comments · Fixed by #134
Closed

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

DominicD opened this issue Sep 7, 2021 · 7 comments · Fixed by #134

Comments

@DominicD
Copy link
Contributor

DominicD commented Sep 7, 2021

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
Copy link
Contributor

OK, but do not forget the spaces!

@DominicD
Copy link
Contributor Author

DominicD commented Sep 8, 2021

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

@TheLartians
Copy link
Owner

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
Copy link
Contributor Author

DominicD commented Sep 9, 2021

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
Copy link
Owner

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
Copy link
Contributor Author

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
Copy link
Owner

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!

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 a pull request may close this issue.

3 participants