Privatise and Tidy

There are tiny issues that don't effect the build that can be cleaned. And flags that can be made private to not effect consumers. Remove RPATH_USE_ORIGIN from static libs add INTERFACE_POSITION_INDEPENDENT_CODE ON reset GODOT_ARCH property to SYSTEM_ARCH for macos because universal add MACOSX_RPATH to test target remove redundant properties from test target remove redundant and PUBLIC cxx_std_17 compile feature Change compiler options to PRIVATE, tested on godot-cpp-test, and orchestrator Transform BITS if statement into simple math expression Transform binding parameters if statements into a genex
godotengine · Dec 11, 2024 · 04da0ef · 04da0ef
1 parent 28ed5df
commit 04da0ef
Show file tree

Hide file tree

Showing 4 changed files with 64 additions and 80 deletions.
diff --git a/cmake/common_compiler_flags.cmake b/cmake/common_compiler_flags.cmake
@@ -19,11 +19,11 @@ set( IS_GNU "$<CXX_COMPILER_ID:GNU>" )
 set( IS_MSVC "$<CXX_COMPILER_ID:MSVC>" )
 set( NOT_MSVC "$<NOT:$<CXX_COMPILER_ID:MSVC>>" )
 
-set( GNU_LT_V8 "$<VERSION_LESS:$<CXX_COMPILER_VERSION>,8>" )
-set( GNU_GE_V9 "$<VERSION_GREATER_EQUAL:$<CXX_COMPILER_VERSION>,9>" )
-set( GNU_GT_V11 "$<VERSION_GREATER_EQUAL:$<CXX_COMPILER_VERSION>,11>" )
-set( GNU_LT_V11 "$<VERSION_LESS:$<CXX_COMPILER_VERSION>,11>" )
-set( GNU_GE_V12 "$<VERSION_GREATER_EQUAL:$<CXX_COMPILER_VERSION>,12>" )
+set( LT_V8  "$<VERSION_LESS:$<CXX_COMPILER_VERSION>,8>" )
+set( GE_V9  "$<VERSION_GREATER_EQUAL:$<CXX_COMPILER_VERSION>,9>" )
+set( GT_V11 "$<VERSION_GREATER_EQUAL:$<CXX_COMPILER_VERSION>,11>" )
+set( LT_V11 "$<VERSION_LESS:$<CXX_COMPILER_VERSION>,11>" )
+set( GE_V12 "$<VERSION_GREATER_EQUAL:$<CXX_COMPILER_VERSION>,12>" )
 
 #[[ Check for clang-cl with MSVC frontend
 The compiler is tested and set when the project command is called.
@@ -46,19 +46,14 @@ endfunction(  )
 
 function( common_compiler_flags )
 
-    target_compile_features(${TARGET_NAME}
-            PUBLIC
-            cxx_std_17
-    )
-
     # These compiler options reflect what is in godot/SConstruct.
     target_compile_options( ${TARGET_NAME}
+        # The public flag tells CMake that the following options are transient,
+        #and will propagate to consumers.
         PUBLIC
             # Disable exception handling. Godot doesn't use exceptions anywhere, and this
             # saves around 20% of binary size and very significant build time.
-            $<${DISABLE_EXCEPTIONS}:
-                $<${NOT_MSVC}:-fno-exceptions>
-            >
+            $<${DISABLE_EXCEPTIONS}:$<${NOT_MSVC}:-fno-exceptions>>
 
             # Enabling Debug Symbols
             $<${DEBUG_SYMBOLS}:
@@ -70,20 +65,19 @@ function( common_compiler_flags )
                 >
             >
 
-            $<${IS_DEV_BUILD}:
-                $<${NOT_MSVC}:-fno-omit-frame-pointer -O0>
-            >
+            $<${IS_DEV_BUILD}:$<${NOT_MSVC}:-fno-omit-frame-pointer -O0>>
 
-            $<${HOT_RELOAD}:
-                $<${IS_GNU}:-fno-gnu-unique>
-            >
+            $<${HOT_RELOAD}:$<${IS_GNU}:-fno-gnu-unique>>
+
+        # Warnings below, these do not need to propagate to consumers.
+        PRIVATE
 
         # MSVC only
         $<${IS_MSVC}:
             # /MP isn't valid for clang-cl with msvc frontend
             $<$<CXX_COMPILER_ID:MSVC>:/MP${PROC_N}>
-            /W4
 
+            /W4      # Warning level 4 (informational) warnings that aren't off by default.
             # Disable warnings which we don't plan to fix.
             /wd4100  # C4100 (unreferenced formal parameter): Doesn't play nice with polymorphism.
             /wd4127  # C4127 (conditional expression is constant)
@@ -95,8 +89,6 @@ function( common_compiler_flags )
             /wd4514  # C4514 (unreferenced inline function has been removed)
             /wd4714  # C4714 (function marked as __forceinline not inlined)
             /wd4820  # C4820 (padding added after construct)
-
-            /utf-8
         >
 
         # Clang and GNU common options
@@ -124,21 +116,22 @@ function( common_compiler_flags )
             -Wplacement-new=1
             -Wshadow-local
             -Wstringop-overflow=4
+        >
 
-            # Bogus warning fixed in 8+.
-            $<${GNU_LT_V8}:-Wno-strict-overflow>
+        # Bogus warning fixed in 8+.
+        $<${IS_GNU}:$<${LT_V8}:-Wno-strict-overflow>>
 
-            $<${GNU_GE_V9}:-Wattribute-alias=2>
+        $<${IS_GNU}:$<${GE_V9}:-Wattribute-alias=2>>
 
-            # Broke on MethodBind templates before GCC 11.
-            $<${GNU_GT_V11}:-Wlogical-op>
+        # Broke on MethodBind templates before GCC 11.
+        $<${IS_GNU}:$<${GT_V11}:-Wlogical-op>>
 
-            # Regression in GCC 9/10, spams so much in our variadic templates that we need to outright disable it.
-            $<${GNU_LT_V11}:-Wno-type-limits>
+        # Regression in GCC 9/10, spams so much in our variadic templates that we need to outright disable it.
+        $<${IS_GNU}:$<${LT_V11}:-Wno-type-limits>>
+
+        # False positives in our error macros, see GH-58747.
+        $<${IS_GNU}:$<${GE_V12}:-Wno-return-type>>
 
-            # False positives in our error macros, see GH-58747.
-            $<${GNU_GE_V12}:-Wno-return-type>
-        >
     )
 
     target_compile_definitions(${TARGET_NAME}
@@ -158,13 +151,11 @@ function( common_compiler_flags )
     )
 
     target_link_options( ${TARGET_NAME}
-        PUBLIC
-            $<${IS_MSVC}:
-                /WX             # treat link warnings as errors.
-                /MANIFEST:NO    # We dont need a manifest
-            >
+        PRIVATE
+            $<${IS_MSVC}:/WX /MANIFEST:NO>
+            # /WX             # treat link warnings as errors.
+            # /MANIFEST:NO    # We dont need a manifest
 
-            $<${DEBUG_SYMBOLS}:$<${IS_MSVC}:/DEBUG:FULL>>
             $<$<NOT:${DEBUG_SYMBOLS}>:
                 $<${IS_GNU}:-s>
                 $<${IS_CLANG}:-s>

diff --git a/cmake/godotcpp.cmake b/cmake/godotcpp.cmake
@@ -193,32 +193,21 @@ function( godotcpp_generate )
         set(GODOT_SYSTEM_HEADERS_ATTRIBUTE SYSTEM)
     endif ()
 
-    #[[ Generate Bindings ]]
-    if(NOT DEFINED BITS)
-        set(BITS 32)
-        if(CMAKE_SIZEOF_VOID_P EQUAL 8)
-            set(BITS 64)
-        endif(CMAKE_SIZEOF_VOID_P EQUAL 8)
-    endif()
-
+    #[[ Configure Binding Variables ]]
     set(GODOT_GDEXTENSION_API_FILE "${GODOT_GDEXTENSION_DIR}/extension_api.json")
-    if (NOT "${GODOT_CUSTOM_API_FILE}" STREQUAL "")  # User-defined override.
+    if( GODOT_CUSTOM_API_FILE )  # User-defined override.
         set(GODOT_GDEXTENSION_API_FILE "${GODOT_CUSTOM_API_FILE}")
     endif()
 
-    # Code Generation option
-    if(GODOT_GENERATE_TEMPLATE_GET_NODE)
-        set(GENERATE_BINDING_PARAMETERS "True")
-    else()
-        set(GENERATE_BINDING_PARAMETERS "False")
-    endif()
+    set( GENERATE_BINDING_PARAMETERS "$<IF:$<BOOL:${GODOT_GENERATE_TEMPLATE_GET_NODE}>,True,False>" )
+    math( EXPR BITS "${CMAKE_SIZEOF_VOID_P} * 8" )
 
     execute_process(COMMAND "${Python3_EXECUTABLE}" "-c" "import binding_generator; binding_generator.print_file_list('${GODOT_GDEXTENSION_API_FILE}', '${CMAKE_CURRENT_BINARY_DIR}', headers=True, sources=True)"
             WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
             OUTPUT_VARIABLE GENERATED_FILES_LIST
             OUTPUT_STRIP_TRAILING_WHITESPACE
     )
-
+    
     add_custom_command(OUTPUT ${GENERATED_FILES_LIST}
             COMMAND "${Python3_EXECUTABLE}" "-c" "import binding_generator; binding_generator.generate_bindings('${GODOT_GDEXTENSION_API_FILE}', '${GENERATE_BINDING_PARAMETERS}', '${BITS}', '${GODOT_PRECISION}', '${CMAKE_CURRENT_BINARY_DIR}')"
             VERBATIM
@@ -298,8 +287,9 @@ function( godotcpp_generate )
                 CXX_VISIBILITY_PRESET ${GODOT_SYMBOL_VISIBILITY}
 
                 COMPILE_WARNING_AS_ERROR ${GODOT_WARNING_AS_ERROR}
+
                 POSITION_INDEPENDENT_CODE ON
-                BUILD_RPATH_USE_ORIGIN ON
+                INTERFACE_POSITION_INDEPENDENT_CODE ON
 
                 PREFIX lib
                 OUTPUT_NAME "${PROJECT_NAME}.${SYSTEM_NAME}.${TARGET_ALIAS}${DEV_TAG}.${SYSTEM_ARCH}"

diff --git a/cmake/macos.cmake b/cmake/macos.cmake
@@ -36,7 +36,9 @@ function( macos_generate )
     set_target_properties( ${TARGET_NAME}
             PROPERTIES
 
+            # Specify multiple architectures for universal builds
             OSX_ARCHITECTURES "${OSX_ARCH}"
+            GODOT_ARCH ${SYSTEM_ARCH}
     )
 
     target_compile_definitions(${TARGET_NAME}

diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
@@ -12,6 +12,22 @@ foreach( TARGET_ALIAS template_debug template_release editor )
     set( TARGET_NAME "godot-cpp.test.${TARGET_ALIAS}" )
     set( LINK_TARGET "godot-cpp::${TARGET_ALIAS}" )
 
+    ### Get useful properties of the library
+    get_target_property( GODOT_PLATFORM ${LINK_TARGET} GODOT_PLATFORM   )
+    get_target_property( GODOT_TARGET   ${LINK_TARGET} GODOT_TARGET     )
+    get_target_property( GODOT_ARCH     ${LINK_TARGET} GODOT_ARCH       )
+    get_target_property( OSX_ARCH       ${LINK_TARGET} OSX_ARCHITECTURES )
+
+    set( DEV_TAG "$<$<BOOL:${GODOT_DEV_BUILD}>:.dev>" )
+
+    if( CMAKE_SYSTEM_NAME STREQUAL Darwin )
+        set( OUTPUT_DIR "${OUTPUT_DIR}/libgdexample.macos.${TEST_TARGET}.framework")
+        set( OUTPUT_NAME "gdexample.macos.${TEST_TARGET}${DEV_TAG}" )
+    else()
+        set( OUTPUT_DIR "${CMAKE_CURRENT_SOURCE_DIR}/project/bin/" )
+        set( OUTPUT_NAME "gdexample.${GODOT_PLATFORM}.${GODOT_TARGET}${DEV_TAG}.${GODOT_ARCH}" )
+    endif()
+
     add_library( ${TARGET_NAME} SHARED EXCLUDE_FROM_ALL )
 
     target_sources( ${TARGET_NAME}
@@ -25,21 +41,13 @@ foreach( TARGET_ALIAS template_debug template_release editor )
 
     target_link_libraries( ${TARGET_NAME} PRIVATE ${LINK_TARGET} )
 
-    ### Get useful properties of the library
-    get_target_property( GODOT_PLATFORM ${LINK_TARGET} GODOT_PLATFORM   )
-    get_target_property( GODOT_TARGET   ${LINK_TARGET} GODOT_TARGET     )
-    get_target_property( GODOT_ARCH     ${LINK_TARGET} GODOT_ARCH       )
-
-    set( OUTPUT_DIR "${CMAKE_CURRENT_SOURCE_DIR}/project/bin/" )
-    set( DEV_TAG "$<$<BOOL:${GODOT_DEV_BUILD}>:.dev>" )
-
     set_target_properties( ${TARGET_NAME}
             PROPERTIES
             CXX_STANDARD 17
             CXX_EXTENSIONS OFF
             CXX_VISIBILITY_PRESET ${GODOT_SYMBOL_VISIBILITY}
 
-            POSITION_INDEPENDENT_CODE ON
+            # This flag adds the runtime path at build time
             BUILD_RPATH_USE_ORIGIN ON
 
             # Try to ensure only static libraries are selected to be linked to.
@@ -55,27 +63,20 @@ foreach( TARGET_ALIAS template_debug template_release editor )
             PREFIX "lib"
             OUTPUT_NAME "gdexample.${GODOT_PLATFORM}.${GODOT_TARGET}${DEV_TAG}.${GODOT_ARCH}"
 
+            #macos options, ignored on other platforms
+            OSX_ARCHITECTURES "${OSX_ARCH}"
+
+            # enable RPATH on MACOS, with the BUILD_RPATH_USE_ORIGIN
+            # this should allow loading libraries from relative paths on macos.
+            MACOSX_RPATH ON
+
             # Some IDE's respect this property to logically group targets
             FOLDER "godot-cpp"
     )
 
-    # CMAKE_SYSTEM_NAME refers to the target system
+    # Only blank the suffix on osx to match SCons
     if( CMAKE_SYSTEM_NAME STREQUAL Darwin )
-        get_target_property( OSX_ARCH ${LINK_TARGET} OSX_ARCHITECTURES )
-
-        set( OUTPUT_DIR "${OUTPUT_DIR}/libgdexample.macos.${TEST_TARGET}.framework")
-
-        set_target_properties( ${TARGET_NAME}
-                PROPERTIES
-                LIBRARY_OUTPUT_DIRECTORY "$<1:${OUTPUT_DIR}>"
-                RUNTIME_OUTPUT_DIRECTORY "$<1:${OUTPUT_DIR}>"
-
-                OUTPUT_NAME "gdexample.macos.${TEST_TARGET}${DEV_TAG}"
-                SUFFIX ""
-
-                #macos options
-                OSX_ARCHITECTURES "${OSX_ARCH}"
-        )
+        set_target_properties( ${TARGET_NAME} PROPERTIES SUFFIX "" )
     endif ()
 
 endforeach()