-
Notifications
You must be signed in to change notification settings - Fork 103
use cmake #91
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
base: main
Are you sure you want to change the base?
use cmake #91
Conversation
What file is this referencing? it's not in the PR |
CMakeLists.txt
Outdated
| endif () | ||
|
|
||
|
|
||
| add_library(rlImGui STATIC) |
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.
Why is this STATIC?
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.
I'll work on the SHARED option, I'll get back to you in a couple of days.
cmake/rlImGuiConfig.cmake.in
Outdated
| @@ -0,0 +1,3 @@ | |||
| @PACKAGE_INIT@ | |||
|
|
|||
| include("${CMAKE_CURRENT_LIST_DIR}/rlImGuiTargets.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.
What file does this reference? it's not in the PR.
Could you explain what the purpose of this file is?
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.
Target files are created automatically by cmake on executing cmake --target install
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.
Files .cmake.in are templates. When we call cmake install cmake replaces @var@ with the value VAR from CMakeLists.txt and the result is what the consumer of the library will get after the installation.
examples/CMakeLists.txt
Outdated
| ) | ||
| target_link_libraries(imgui_style_example PRIVATE rlImGui) | ||
| add_custom_command(TARGET imgui_style_example POST_BUILD | ||
| COMMAND ${CMAKE_COMMAND} -E copy |
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.
I'd prefer if this didn't copy files
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.
I'll remove them
|
I am not against having cmake as an option, but I have some concerns. I don't use cmake, so I have concerns about maintenance, I would not want to have a build system that is not maintained. |
|
Alright, valid concerns, it is probably better to cancel this PR then. What I did was enough for my project, I'll keep it on my fork only. |
can your system be modified to work from a single folder? |
|
Yes. There are three files now, CMakeLists.txt in the root level for the library setup, a CMakeLists.txt for the examples and config.cmake.in. The setup CMakeLists must be in the root level, the others I can move anywhere. There is another possible solution to put everything in one single CMakeLists.txt. I prefer to have multiple files, I feel it keeps things organized, but it's your call.
|
|
I have made a new commit, one single CMakeLists.txt to handle all. static/shared, config, examples. |
Is there anyway to have noting in the root and everything in a cmake_build folder? |
|
Unusual, I had to ask online but yes, it is possible. I'll review it some other time. |
|
done, last commit has all in one file in /cmake folder |
added cmake , and it can compile the examples and install as a lib