CMake: plumbing for modern CMake usage

This is the minimal change required to start using modern CMake in the blender build system. This change is designed to allow small incremental changes to the build system rather than doing it in one big bang which would be unmaintainable (for me) The biggest functional change is, previously all libraries in the `LIB` section of a `blender_add_lib` call had the `INTERFACE` scope, which is rarely, if ever the correct scope. This diff changes this to `PRIVATE` Concrete implications of this diff : The `LIB`, `INC` and `INC_SYS` sections of an `blender_add_lib` call now allow scoping keywords (`PUBLIC`, `PRIVATE,` `INTERFACE`) to declare the scope of the dependency. Right now the only library using any modern cmake is `bf_intern_atomic` which is an header only interface library that will just advertise its include directories. This allows us to clean up any `CMakeLists.txt` that adds `../../../intern/atomic` to its `INC` section to remove it in `INC` by adding a `PRIVATE bf_intern_atomic` to the `LIB` section. Pull Request: https://projects.blender.org/blender/blender/pulls/107858
2023-06-27 20:57:50 +02:00
parent d53862351d
commit eff9e2f4ce
7 changed files with 159 additions and 75 deletions
--- a/build_files/cmake/Modules/GTestTesting.cmake
+++ b/build_files/cmake/Modules/GTestTesting.cmake
@@ -47,17 +47,17 @@ macro(BLENDER_SRC_GTEST_EX)
    target_compile_definitions(${TARGET_NAME} PRIVATE ${GLOG_DEFINES})
    target_include_directories(${TARGET_NAME} PUBLIC "${TEST_INC}")
    target_include_directories(${TARGET_NAME} SYSTEM PUBLIC "${TEST_INC_SYS}")
-    target_link_libraries(${TARGET_NAME} ${ARG_EXTRA_LIBS} ${PLATFORM_LINKLIBS})
+    blender_link_libraries(${TARGET_NAME} "${ARG_EXTRA_LIBS};${PLATFORM_LINKLIBS}")
    if(WITH_TBB)
      # Force TBB libraries to be in front of MKL (part of OpenImageDenoise), so
      # that it is initialized before MKL and static library initialization order
      # issues are avoided.
-      target_link_libraries(${TARGET_NAME} ${TBB_LIBRARIES})
+      target_link_libraries(${TARGET_NAME} PRIVATE ${TBB_LIBRARIES})
      if(WITH_OPENIMAGEDENOISE)
-        target_link_libraries(${TARGET_NAME} ${OPENIMAGEDENOISE_LIBRARIES})
+        target_link_libraries(${TARGET_NAME} PRIVATE ${OPENIMAGEDENOISE_LIBRARIES})
      endif()
    endif()
-    target_link_libraries(${TARGET_NAME}
+    target_link_libraries(${TARGET_NAME} PRIVATE
                          bf_testing_main
                          bf_intern_eigen
                          bf_intern_guardedalloc
@@ -68,16 +68,16 @@ macro(BLENDER_SRC_GTEST_EX)
                          ${GLOG_LIBRARIES}
                          ${GFLAGS_LIBRARIES})
    if(WITH_OPENMP_STATIC)
-      target_link_libraries(${TARGET_NAME} ${OpenMP_LIBRARIES})
+      target_link_libraries(${TARGET_NAME} PRIVATE ${OpenMP_LIBRARIES})
    endif()
    if(UNIX AND NOT APPLE)
-      target_link_libraries(${TARGET_NAME} bf_intern_libc_compat)
+      target_link_libraries(${TARGET_NAME} PRIVATE bf_intern_libc_compat)
    endif()
    if(WITH_TBB)
-      target_link_libraries(${TARGET_NAME} ${TBB_LIBRARIES})
+      target_link_libraries(${TARGET_NAME} PRIVATE ${TBB_LIBRARIES})
    endif()
    if(WITH_GMP)
-      target_link_libraries(${TARGET_NAME} ${GMP_LIBRARIES})
+      target_link_libraries(${TARGET_NAME} PRIVATE ${GMP_LIBRARIES})
    endif()

    GET_BLENDER_TEST_INSTALL_DIR(TEST_INSTALL_DIR)
@@ -109,25 +109,3 @@ macro(BLENDER_SRC_GTEST_EX)
    unset(TARGET_NAME)
  endif()
 endmacro()
-
-macro(BLENDER_SRC_GTEST NAME SRC EXTRA_LIBS)
-  BLENDER_SRC_GTEST_EX(
-    NAME "${NAME}"
-    SRC "${SRC}"
-    EXTRA_LIBS "${EXTRA_LIBS}")
-endmacro()
-
-macro(BLENDER_TEST NAME EXTRA_LIBS)
-  BLENDER_SRC_GTEST_EX(
-    NAME "${NAME}"
-    SRC "${NAME}_test.cc"
-    EXTRA_LIBS "${EXTRA_LIBS}")
-endmacro()
-
-macro(BLENDER_TEST_PERFORMANCE NAME EXTRA_LIBS)
-  BLENDER_SRC_GTEST_EX(
-    NAME "${NAME}"
-    SRC "${NAME}_test.cc"
-    EXTRA_LIBS "${EXTRA_LIBS}"
-    SKIP_ADD_TEST)
-endmacro()
--- a/build_files/cmake/macros.cmake
+++ b/build_files/cmake/macros.cmake
@@ -159,31 +159,60 @@ function(absolute_include_dirs

  set(_ALL_INCS "")
  foreach(_INC ${ARGN})
-    get_filename_component(_ABS_INC ${_INC} ABSOLUTE)
-    list(APPEND _ALL_INCS ${_ABS_INC})
-    # for checking for invalid includes, disable for regular use
-    # if(NOT EXISTS "${_ABS_INC}/")
-    #   message(FATAL_ERROR "Include not found: ${_ABS_INC}/")
-    # endif()
+    # Pass any scoping keywords as is
+    if(("${_INC}" STREQUAL "PUBLIC") OR
+       ("${_INC}" STREQUAL "PRIVATE") OR
+       ("${_INC}" STREQUAL "INTERFACE"))
+      list(APPEND _ALL_INCS ${_INC})
+    else()
+      get_filename_component(_ABS_INC ${_INC} ABSOLUTE)
+      list(APPEND _ALL_INCS ${_ABS_INC})
+      # for checking for invalid includes, disable for regular use
+      # if(NOT EXISTS "${_ABS_INC}/")
+      #   message(FATAL_ERROR "Include not found: ${_ABS_INC}/")
+      # endif()
+    endif()
  endforeach()

  set(${includes_absolute} ${_ALL_INCS} PARENT_SCOPE)
 endfunction()

-function(blender_target_include_dirs
-  name
+function(blender_target_include_dirs_impl
+  target
+  system
+  includes
  )
+  set(next_interface_mode "PRIVATE")
+  foreach(_INC ${includes})
+    if(("${_INC}" STREQUAL "PUBLIC") OR
+       ("${_INC}" STREQUAL "PRIVATE") OR
+       ("${_INC}" STREQUAL "INTERFACE"))
+      set(next_interface_mode "${_INC}")
+    else()
+      if(system)
+        target_include_directories(${target} SYSTEM ${next_interface_mode} ${_INC})
+      else()
+        target_include_directories(${target} ${next_interface_mode} ${_INC})
+      endif()
+      set(next_interface_mode "PRIVATE")
+    endif()
+  endforeach()
+endfunction()

+# Nicer makefiles with -I/1/foo/ instead of -I/1/2/3/../../foo/
+# use it instead of target_include_directories()
+function(blender_target_include_dirs
+  target
+  )
  absolute_include_dirs(_ALL_INCS ${ARGN})
-  target_include_directories(${name} PRIVATE ${_ALL_INCS})
+  blender_target_include_dirs_impl(${target} FALSE "${_ALL_INCS}")
 endfunction()

 function(blender_target_include_dirs_sys
-  name
+  target
  )
-
  absolute_include_dirs(_ALL_INCS ${ARGN})
-  target_include_directories(${name} SYSTEM PRIVATE ${_ALL_INCS})
+  blender_target_include_dirs_impl(${target} TRUE "${_ALL_INCS}")
 endfunction()

 # Set include paths for header files included with "*.h" syntax.
@@ -277,23 +306,11 @@ macro(add_cc_flags_custom_test

 endmacro()

-
-# only MSVC uses SOURCE_GROUP
-function(blender_add_lib__impl
-  name
-  sources
-  includes
-  includes_sys
+function(blender_link_libraries
+  target
  library_deps
  )

-  # message(STATUS "Configuring library ${name}")
-
-  add_library(${name} ${sources})
-
-  blender_target_include_dirs(${name} ${includes})
-  blender_target_include_dirs_sys(${name} ${includes_sys})
-
  # On Windows certain libraries have two sets of binaries: one for debug builds and one for
  # release builds. The root of this requirement goes into ABI, I believe, but that's outside
  # of a scope of this comment.
@@ -331,23 +348,49 @@ function(blender_add_lib__impl
  # NOT: "optimized libfoo libbar debug libfoo_d libbar_d"
  if(NOT "${library_deps}" STREQUAL "")
    set(next_library_mode "")
+    set(next_interface_mode "PRIVATE")
    foreach(library ${library_deps})
      string(TOLOWER "${library}" library_lower)
      if(("${library_lower}" STREQUAL "optimized") OR
         ("${library_lower}" STREQUAL "debug"))
        set(next_library_mode "${library_lower}")
+      elseif(("${library}" STREQUAL "PUBLIC") OR
+             ("${library}" STREQUAL "PRIVATE") OR
+             ("${library}" STREQUAL "INTERFACE"))
+        set(next_interface_mode "${library}")
      else()
        if("${next_library_mode}" STREQUAL "optimized")
-          target_link_libraries(${name} INTERFACE optimized ${library})
+          target_link_libraries(${target} ${next_interface_mode} optimized ${library})
        elseif("${next_library_mode}" STREQUAL "debug")
-          target_link_libraries(${name} INTERFACE debug ${library})
+          target_link_libraries(${target} ${next_interface_mode} debug ${library})
        else()
-          target_link_libraries(${name} INTERFACE ${library})
+          target_link_libraries(${target} ${next_interface_mode} ${library})
        endif()
        set(next_library_mode "")
      endif()
    endforeach()
  endif()
+endfunction()
+
+# only MSVC uses SOURCE_GROUP
+function(blender_add_lib__impl
+  name
+  sources
+  includes
+  includes_sys
+  library_deps
+  )
+
+  # message(STATUS "Configuring library ${name}")
+
+  add_library(${name} ${sources})
+
+  blender_target_include_dirs(${name} ${includes})
+  blender_target_include_dirs_sys(${name} ${includes_sys})
+
+  if(library_deps)
+    blender_link_libraries(${name} "${library_deps}")
+  endif()

  # works fine without having the includes
  # listed is helpful for IDE's (QtCreator/MSVC)
@@ -477,8 +520,9 @@ endfunction()
 # To be used for smaller isolated libraries, that do not have many dependencies.
 # For libraries that do drag in many other Blender libraries and would create a
 # very large executable, blender_add_test_lib() should be used instead.
-function(blender_add_test_executable
+function(blender_add_test_executable_impl
  name
+  add_test_suite
  sources
  includes
  includes_sys
@@ -496,14 +540,48 @@ function(blender_add_test_executable
    EXTRA_LIBS "${library_deps}"
    SKIP_ADD_TEST
  )
-
+  if(add_test_suite)
+    blender_add_test_suite(
+      TARGET ${name}_test
+      SUITE_NAME ${name}
+      SOURCES "${sources}"
+    )
+  endif()
  blender_target_include_dirs(${name}_test ${includes})
  blender_target_include_dirs_sys(${name}_test ${includes_sys})
+endfunction()

-  blender_add_test_suite(
-    TARGET ${name}_test
-    SUITE_NAME ${name}
-    SOURCES "${sources}"
+function(blender_add_test_executable
+  name
+  sources
+  includes
+  includes_sys
+  library_deps
+  )
+  blender_add_test_executable_impl(
+    "${name}"
+    TRUE
+    "${sources}"
+    "${includes}"
+    "${includes_sys}"
+    "${library_deps}"
+   )
+endfunction()
+
+function(blender_add_performancetest_executable
+  name
+  sources
+  includes
+  includes_sys
+  library_deps
+  )
+  blender_add_test_executable_impl(
+    "${name}"
+    FALSE
+    "${sources}"
+    "${includes}"
+    "${includes_sys}"
+    "${library_deps}"
  )
 endfunction()

@@ -547,17 +625,17 @@ function(setup_platform_linker_libs
  )
  # jemalloc must be early in the list, to be before pthread (see #57998).
  if(WITH_MEM_JEMALLOC)
-    target_link_libraries(${target} ${JEMALLOC_LIBRARIES})
+    target_link_libraries(${target} PRIVATE ${JEMALLOC_LIBRARIES})
  endif()

  if(WIN32 AND NOT UNIX)
    if(DEFINED PTHREADS_LIBRARIES)
-      target_link_libraries(${target} ${PTHREADS_LIBRARIES})
+      target_link_libraries(${target} PRIVATE ${PTHREADS_LIBRARIES})
    endif()
  endif()

  # target_link_libraries(${target} ${PLATFORM_LINKLIBS} ${CMAKE_DL_LIBS})
-  target_link_libraries(${target} ${PLATFORM_LINKLIBS})
+  target_link_libraries(${target} PRIVATE ${PLATFORM_LINKLIBS})
 endfunction()

 macro(TEST_SSE_SUPPORT
--- a/intern/atomic/CMakeLists.txt
+++ b/intern/atomic/CMakeLists.txt
@@ -9,6 +9,25 @@ set(INC
 set(INC_SYS
 )

+add_library(bf_intern_atomic INTERFACE)
+target_include_directories(bf_intern_atomic INTERFACE .)
+
+# CMake 3.19+ allows one to populate the interface library with
+# source files to show in the IDE, for people on older CMake versions
+# these headers will be visible in the bf_intern_guardedalloc project
+# where they historically have been.
+if(${CMAKE_VERSION} VERSION_GREATER_EQUAL "3.19")
+  set(SRC
+    atomic_ops.h
+    intern/atomic_ops_ext.h
+    intern/atomic_ops_msvc.h
+    intern/atomic_ops_unix.h
+    intern/atomic_ops_utils.h
+  )
+  target_sources(bf_intern_atomic PRIVATE ${SRC})
+  blender_source_group(bf_intern_atomic ${SRC})
+endif()
+
 if(WITH_GTESTS)
  set(TEST_SRC
    tests/atomic_test.cc
@@ -16,6 +35,7 @@ if(WITH_GTESTS)
  set(TEST_INC
  )
  set(TEST_LIB
+    PRIVATE bf_intern_atomic
  )
  include(GTestTesting)
  blender_add_test_executable(atomic "${TEST_SRC}" "${INC};${TEST_INC}" "${INC_SYS}" "${LIB};${TEST_LIB}")
--- a/intern/cycles/test/CMakeLists.txt
+++ b/intern/cycles/test/CMakeLists.txt
@@ -55,5 +55,6 @@ if(NOT APPLE)
 endif()

 if(WITH_GTESTS AND WITH_CYCLES_LOGGING)
-  blender_src_gtest(cycles "${SRC}" "${LIB}")
+  set(INC_SYS )
+  blender_add_test_executable(cycles "${SRC}" "${INC}" "${INC_SYS}" "${LIB}")
 endif()
--- a/source/blender/blenlib/tests/performance/CMakeLists.txt
+++ b/source/blender/blenlib/tests/performance/CMakeLists.txt
@@ -11,7 +11,14 @@ set(INC
  ../../../../../intern/guardedalloc
 )

-include_directories(${INC})
+set(INC_SYS
+)

-blender_test_performance(BLI_ghash_performance "bf_blenlib")
-blender_test_performance(BLI_task_performance "bf_blenlib")
+set(LIB
+  PRIVATE bf_blenlib
+  PRIVATE bf_intern_guardedalloc
+  PRIVATE bf_intern_atomic
+)
+
+blender_add_performancetest_executable(BLI_ghash_performance "BLI_ghash_performance_test.cc" "${INC}" "${INC_SYS}" "${LIB}")
+blender_add_performancetest_executable(BLI_task_performance "BLI_task_performance_test.cc" "${INC}" "${INC_SYS}" "${LIB}")
--- a/source/creator/CMakeLists.txt
+++ b/source/creator/CMakeLists.txt
@@ -1575,7 +1575,7 @@ endif()
 # Setup link libraries

 add_dependencies(blender makesdna)
-target_link_libraries(blender ${LIB})
+target_link_libraries(blender PRIVATE ${LIB})
 unset(LIB)

 setup_platform_linker_flags(blender)
--- a/tests/gtests/runner/CMakeLists.txt
+++ b/tests/gtests/runner/CMakeLists.txt
@@ -41,7 +41,7 @@ if(WIN32)
    # Both target_link_libraries and target_link_options are required here
    # target_link_libraries will add any dependent libraries, while just setting
    # the wholearchive flag in target link options will not.
-    target_link_libraries(blender_test ${_lib})
+    target_link_libraries(blender_test PRIVATE ${_lib})
    target_link_options(blender_test PRIVATE /wholearchive:$<TARGET_FILE:${_lib}>)
  endforeach()
  set_target_properties(blender_test PROPERTIES VS_DEBUGGER_ENVIRONMENT "${PLATFORM_ENV_INSTALL};$<TARGET_FILE_DIR:blender>")
@@ -49,7 +49,7 @@ elseif(APPLE)
  foreach(_lib ${_test_libs})
    # We need -force_load for every test library and target_link_libraries will
    # deduplicate it. So explicitly set as linker option for every test lib.
-    target_link_libraries(blender_test ${_lib} "-Wl,-force_load,$<TARGET_PROPERTY:${_lib},IMPORTED_LOCATION>")
+    target_link_libraries(blender_test PRIVATE ${_lib} "-Wl,-force_load,$<TARGET_PROPERTY:${_lib},IMPORTED_LOCATION>")
  endforeach()
 endif()