Feature: Enhanced CMake Build Script #12
Conversation
CMakeLists.txt
Outdated
| # What do we want to build? | ||
| option(GGPO_BUILD_SDK "Enable the build of the GGPO SDK" ON) | ||
| option(GGPO_BUILD_VECTORWAR "Enable the build of the Vector War example app" ON) | ||
| option(GGPO_BUILD_SHARED_LIBS "Enable the build of shared libraries (.dll/.so) instead of static ones (.lib/.a)" ON) |
There was a problem hiding this comment.
Is there a reason why we need to use a re-definition of BUILD_SHARED_LIBS? It means we have to explicitly state the library type when calling add_library().
There was a problem hiding this comment.
Yeah, I didn't know that BUILD_SHARED_LIBS was a built-it option and I wanted to be more explicit (and it was late when I was finishing this). Will change that.
There was a problem hiding this comment.
Done. Now BUILD_SHARED_LIBS just inherits the GGPO_BUILD_SHARED_LIBS value. I've also reverted the changes to the add_library and BUILD_SHARED_LIBS stuff to what was before.
There was a problem hiding this comment.
Why not just remove GGPO_BUILD_SHARED_LIBS altogether? Seeing as BUILD_SHARED_LIBS is builtin, having the two options linked together could be confusing, especially if one is set explicitly on the cli making them out of sync.
There was a problem hiding this comment.
Wanted to make the naming uniform with the other options, but I can understand the concern. Change reverted.
src/cmake/helper_methods.cmake
Outdated
| if(MSVC) | ||
| set_property(TARGET ${target} APPEND_STRING PROPERTY LINK_FLAGS_DEBUG "/DEBUG /INCREMENTAL") | ||
| set_property(TARGET ${target} APPEND_STRING PROPERTY LINK_FLAGS_RELEASE "/DEBUG /LTCG /INCREMENTAL:NO /OPT:REF") | ||
| set_property(TARGET ${target} APPEND PROPERTY COMPILE_OPTIONS -DWIN32 -D_WINDOWS -D_WINSOCK_DEPRECATED_NO_WARNINGS -D_CRT_SECURE_NO_WARNINGS) |
There was a problem hiding this comment.
I was planning on leaving the warnings in and fixing them later down the road, but I guess that's up to @pond3r
There was a problem hiding this comment.
Fixing them would be nice. We had to disable the "use safe C library functions" warnings explicitly within the code at my company, since they can be such an annoyance with third party code (and last time I've checked they were only implemented by MSVC, C11 seems to provide them, but IDK about the compiler/C library support). But I'm also will leave the decision up to Tony.
There was a problem hiding this comment.
I think the right path here is to eventually fix all the warnings and remove the defines which explicitly ignore them so the code just builds clean.
There was a problem hiding this comment.
OK. Removed -D_WINSOCK_DEPRECATED_NO_WARNINGS and -D_CRT_SECURE_NO_WARNINGS from the compile options.
src/cmake/properties.cmake
Outdated
|
|
||
| ## GGPO uses C-with-classes C++ flavour, so technically it should compile with C++03 | ||
| ## but to be more safe, we set the standard to C++11 | ||
| set(CMAKE_CXX_STANDARD 11) |
There was a problem hiding this comment.
I see no problem with modernising the codebase somewhat (and bringing up the cpp standard). However, I know Tony has expressed concerns with bringing in the STL so this is a discussion that we should probably have at some point.
There was a problem hiding this comment.
Can understand. STL while convenient, does add some bloat. I just set it to C++11 to be more on the safe side, but I will try to lower it down to C++03.
There was a problem hiding this comment.
Changed the set C++ standard to C++98 (CMake doesn't define C++03, but GGPO compiles fine with C++98).
37a4393
to
04f76dc
Compare
This extends and refactors the original CMake script to make it nicer. The structure of it is loosely based on that used in bs::framework project. - Separate the Main SDK build definitions to its own CMakeLists.txt. Source file lists for each project were also moved to file CMakeSources.cmake, cataloged based on a folder they're in and the filter definitions for VS were added. - During build the executable/library artifacts are now stored in <BUILD_DIR>/<BIN_OR_LIB>/<ARCHITECTURE>/<BUILD_TYPE>. The scripts were modified to reflect that change. - Added most of the build flags from the original VS project with some new one. - Added some properties to explicitly configure the build environment. - Add ability to install the libraries for distribution. - Other small renaming, cleanups and things that I've forgotten.
|
Sorry for the delay, friends. I've been super tanked at work! LGTM. |
Feature: Enhanced CMake Build Script
This extends and refactors the original CMake script to make it nicer.
The structure of it is loosely based on that used in bs::framework project.
CMakeLists.txt.Source file lists for each project were also moved to file
CMakeSources.cmake,cataloged based on a folder they're in and the filter definitions for VS were added.
<BUILD_DIR>/<BIN_OR_LIB>/<ARCHITECTURE>/<BUILD_TYPE>. The scripts weremodified to reflect that change.
with some new ones.