-
-
Notifications
You must be signed in to change notification settings - Fork 589
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
Added options to enable/disable targets to be added to a cmake build. #1650
base: master
Are you sure you want to change the base?
Conversation
To support this change and prevent erroneous cmake variable values, logic around choosing test dependency was also modified.
This concept arose in conversation between @Torets and I while discussing the changes in master. Being able to disable/enable the targets gives a best of both worlds scenario when dealing with CMake and IDE's that have cluttered target lists, and build everything like Visual Studio does. But does not prevent building everything all at once if desired. |
Hey @Torets I was doing some testing, and setup a new project which fetched godot-cpp. Visual studio didnt build anything at all when triggering the all build. Can you re-check with a fresh source tree that the build-all in visual studio does/does not infact try to build all targets for you? I had to consume a target, or remove EXLCUDE_FROM_ALL to get it to want to build the library when clicking the build all, i htink f6, or from the build menu. |
@enetheru |
The only EXCLUDE_FROM_ALL I have in my version is in CMakeLists.txt from my folder. And regardless of having it for the rest of projects or no and having folders for them (which is always good), I think having an option to remove projects from *.sln is needed QoL feature |
what? there should be an ... crap add_library cmake docs
I agree with you, I think it is a good idea, I just had conflicting information I needed to clean up in my mind. |
yeah, that EXCLUDE is always empty right now from what I see. I can assume it could get in a way of passing test or some other default behavior. If not, may then I return it in your PR? |
Since this PR will also have conflicts with PR CMake: Alleviate target name clashes, visibility, and grouping. I will solve conflicts after that one will be successfully merged. |
In CMake build system all of the targets were added to build (solution for Visual Studio), which might case confusion
and unnecessary additional build time.
I have added following options to CMake:
GODOT_ADD_TARGET_TEMPLATE_RELEASE
- for target template_releaseGODOT_ADD_TARGET_TEMPLATE_DEBUG
- for target template_debugGODOT_ADD_TARGET_EDITOR
- for target editorThese options are all
ON
by default to preserve the same default behavior as prior their addition. By turning optionsOFF
correlating targets will be excluded from build. If all options areOFF
CMake will rise an error during configuration phase.Additionally to make sure that 'Test' target dependency stays valid, the following logic was added. If dependency gets turned off by corresponding
GODOT_ADD_TARGET_*
, then the dependency is switched to next target in following ordertemplate_debug
->template_release
->editor
.Also separate flag
GODOT_CPP_USE_TEST
with default value ON was added to have an ability to turn off testing as well.These changes can be further supported in Fix break prior modernization PR by adding
${GODOT_DEFAULT_TARGET}
ALIAS instead of hardcodedtemplate_debug
after both PRs are merged. Even without the change default behavior of CMake build should stay the same as it was before