[cmake] Make gtest include directories a part of the library interface

[labath]

Summary

This applies the same fix that D84748 did for macro definitions.
Appropriate include path is now automatically set for all libraries
which link against gtest targets, which avoids the need to set
include_directories in various parts of the project.

[ldionne]

LGTM, but I'm not an owner for any of the projects touched by this change.

[teemperor]

LGTM too

[sivachandra]

Libc change LGTM.

[labath]

In D86616#2238946, @ldionne wrote:

LGTM, but I'm not an owner for any of the projects touched by this change.

I picked you because you seemed interested in the overall direction that our cmake support is going :), and I believe that one of our biggest problem with cmake is the lack of a unified direction of where our cmake support is going. (Also, my previous go-to person for that (@beanz), seems to be busy with other stuff these days.)

[ldionne]

In D86616#2241936, @labath wrote:

In D86616#2238946, @ldionne wrote:

LGTM, but I'm not an owner for any of the projects touched by this change.

I picked you because you seemed interested in the overall direction that our cmake support is going :), and I believe that one of our biggest problem with cmake is the lack of a unified direction of where our cmake support is going. (Also, my previous go-to person for that (@beanz), seems to be busy with other stuff these days.)

Yup, I am interested by that indeed :). I just wanted to make it clear my LGTM alone shouldn't be considered enough to commit to these other projects that might have active owners. But it does look like you've got enough approvals for an obviously correct improvement, IMO.

flang/CMakeLists.txt

option(FLANG_INCLUDE_TESTS
          ON)
 
 #Handle unittests when out-of-tree
 #LLVM_BUILD_MAIN_SRC_DIR - Path to llvm source when out-of-tree.
   set(FLANG_GTEST_AVAIL 0)
   if (FLANG_INCLUDE_TESTS)
     set(UNITTEST_DIR ${LLVM_BUILD_MAIN_SRC_DIR}/utils/unittest)
     if(EXISTS ${UNITTEST_DIR}/googletest/include/gtest/gtest.h)
-      if (TARGET gtest)
-        # LLVM Doesn't export gtest's include directorys, so do that here
-        set_target_properties(gtest
-          PROPERTIES INTERFACE_INCLUDE_DIRECTORIES
-          "${UNITTEST_DIR}/googletest/include;${UNITTEST_DIR}/googlemock/include"
-          )
-      else()
+      if (NOT TARGET gtest)
         add_library(gtest
           ${UNITTEST_DIR}/googletest/src/gtest-all.cc
           ${UNITTEST_DIR}/googlemock/src/gmock-all.cc
           )
         target_include_directories(gtest
           PUBLIC
           "${UNITTEST_DIR}/googletest/include"
           "${UNITTEST_DIR}/googlemock/include"

libc/benchmarks/CMakeLists.txt

cmake_parse_arguments(
     "SRCS;DEPENDS" # Multi-value arguments
     ${ARGN}
   )
 
   add_executable(${target_name}
     EXCLUDE_FROM_ALL
     ${LIBC_BENCHMARKS_UNITTEST_SRCS}
   )
-  target_include_directories(${target_name}
-    PRIVATE
-    ${LLVM_MAIN_SRC_DIR}/utils/unittest/googletest/include
-    ${LLVM_MAIN_SRC_DIR}/utils/unittest/googlemock/include
-  )
   target_link_libraries(${target_name}
     PRIVATE
     gtest_main
     gtest
     ${LIBC_BENCHMARKS_UNITTEST_DEPENDS}
   )
 
   add_custom_command(

lldb/unittests/TestingSupport/Symbol/CMakeLists.txt

 set_property(DIRECTORY PROPERTY EXCLUDE_FROM_ALL ON)
 add_lldb_library(lldbSymbolHelpers
   YAMLModuleTester.cpp
   )
-
-target_include_directories(lldbSymbolHelpers PUBLIC
-  ${LLVM_MAIN_SRC_DIR}/utils/unittest/googletest/include
-  ${LLVM_MAIN_SRC_DIR}/utils/unittest/googlemock/include)

llvm/cmake/modules/AddLLVM.cmake

 endfunction(add_llvm_implicit_projects)
 
 # Generic support for adding a unittest.
 function(add_unittest test_suite test_name)
   if( NOT LLVM_BUILD_TESTS )
     set(EXCLUDE_FROM_ALL ON)
   endif()
 
-  include_directories(${LLVM_MAIN_SRC_DIR}/utils/unittest/googletest/include)
-  include_directories(${LLVM_MAIN_SRC_DIR}/utils/unittest/googlemock/include)
-
   if (SUPPORTS_VARIADIC_MACROS_FLAG)
     list(APPEND LLVM_COMPILE_FLAGS "-Wno-variadic-macros")
   endif ()
   # Some parts of gtest rely on this GNU extension, don't warn on it.
   if(SUPPORTS_GNU_ZERO_VARIADIC_MACRO_ARGUMENTS_FLAG)
     list(APPEND LLVM_COMPILE_FLAGS "-Wno-gnu-zero-variadic-macro-arguments")
   endif()
 

llvm/lib/Testing/Support/CMakeLists.txt

 add_llvm_library(LLVMTestingSupport
   Annotations.cpp
   Error.cpp
   SupportHelpers.cpp
 
   BUILDTREE_ONLY
   
   ADDITIONAL_HEADER_DIRS
   ${LLVM_MAIN_INCLUDE_DIR}/llvm/Testing/Support
 
   LINK_COMPONENTS
   Support
   )
 
-include_directories(${LLVM_MAIN_SRC_DIR}/utils/unittest/googletest/include)
-include_directories(${LLVM_MAIN_SRC_DIR}/utils/unittest/googlemock/include)
 target_link_libraries(LLVMTestingSupport PRIVATE gtest)

llvm/utils/unittest/CMakeLists.txt

 ########################################################################
 # Experimental CMake build script for Google Test.
 #
 # Consider this a prototype.  It will change drastically.  For now,
 # this is only for people on the cutting edge.
 #
 # To run the tests for Google Test itself on Linux, use 'make test' or
 # ctest.  You can select which tests to run using 'ctest -R regex'.
 # For more options, run 'ctest --help'.
 ########################################################################
 #
 # Project-wide settings
 
-# Where gtest's .h files can be found.
-include_directories(
-  googletest/include
-  googletest
-  googlemock/include
-  googlemock
-  )
-
 if(WIN32)
   add_definitions(-DGTEST_OS_WINDOWS=1)
 endif()
 
 # Google Test requires headers which need _ALL_SOURCE to build on AIX
 if (UNIX AND ${CMAKE_SYSTEM_NAME} MATCHES "AIX")
   remove_definitions("-D_XOPEN_SOURCE=700")
   add_definitions("-D_ALL_SOURCE")

 
 # Gtest 1.8.0 uses tr1/tuple which is deprecated on MSVC, so we force it off.
 target_compile_definitions(gtest PUBLIC GTEST_HAS_TR1_TUPLE=0)
 
 if (NOT LLVM_ENABLE_THREADS)
   target_compile_definitions(gtest PUBLIC GTEST_HAS_PTHREAD=0)
 endif ()
 
+target_include_directories(gtest
+  PUBLIC googletest/include googlemock/include
+  PRIVATE googletest googlemock
+  )
+
 add_subdirectory(UnitTestMain)
 
 # When LLVM_LINK_LLVM_DYLIB is enabled, libLLVM.so is added to the interface
 # link libraries for gtest and gtest_main.  This means that any target, like
 # unittests for example, that links against gtest will be forced to link
 # against libLLVM.so.  In some cases we may want to statically unittests if they
 # need access to symbols that are marked private in libLLVM.so.  The only
 # way we can make this work is to remove libLLVM.so from the list of interface

polly/CMakeLists.txt

if (NOT DEFINED LLVM_MAIN_SRC_DIR)
 
   # Enable unit tests if available.
   set(POLLY_GTEST_AVAIL 0)
   set(UNITTEST_DIR ${LLVM_SOURCE_ROOT}/utils/unittest)
   if(EXISTS ${UNITTEST_DIR}/googletest/include/gtest/gtest.h)
     if (NOT TARGET gtest)
       add_subdirectory(${UNITTEST_DIR} utils/unittest)
     endif()
-
-    # LLVM Doesn't export gtest's include directorys, so do that here
-    set_target_properties(gtest
-      PROPERTIES INTERFACE_INCLUDE_DIRECTORIES
-      "${UNITTEST_DIR}/googletest/include;${UNITTEST_DIR}/googlemock/include"
-      )
     set(POLLY_GTEST_AVAIL 1)
   endif()
 
   if (LLVM_ENABLE_PIC)
     # Make sure the isl c files are built as fPIC if possible
     set(CMAKE_C_FLAGS "${CMAKE_C_FLAGS} -fPIC")
   endif ()