Replace include_directories() with target_include_directories().

- Now that we require CMake >= 3.x, we can roll the definition of the
  include directories for clients linking against Ceres into the
  exported Ceres CMake target via target_include_directories().
- This removes the requirement for CERES_INCLUDE_DIRS.

Change-Id: Ibe3bbf796339871d66138fc9520053d1766f09a8

CMakeLists.txt

@@ -472,39 +472,6 @@ if (BUILD_BENCHMARKS)
   mark_as_advanced(benchmark_DIR)
 endif()
 
-include_directories(
-  include
-  internal
-  internal/ceres
-  ${GLOG_INCLUDE_DIRS})
-
-# Eigen SparseQR generates various compiler warnings related to unused and
-# uninitialised local variables.  To avoid having to individually suppress these
-# warnings around the #include statments for Eigen headers across all GCC/Clang
-# versions, we tell CMake to treat Eigen headers as system headers.  This
-# results in all compiler warnings from them being suppressed.
-#
-# Note that this is *not* propagated to clients, ie CERES_INCLUDE_DIRS
-# used by clients after find_package(Ceres) does not identify Eigen as
-# as system headers.
-include_directories(SYSTEM ${EIGEN_INCLUDE_DIRS})
-
-if (SUITESPARSE)
-  include_directories(${SUITESPARSE_INCLUDE_DIRS})
-endif (SUITESPARSE)
-
-if (CXSPARSE)
-  include_directories(${CXSPARSE_INCLUDE_DIRS})
-endif (CXSPARSE)
-
-if (ACCELERATESPARSE)
-  include_directories(${AccelerateSparse_INCLUDE_DIRS})
-endif()
-
-if (GFLAGS)
-  include_directories(${GFLAGS_INCLUDE_DIRS})
-endif (GFLAGS)
-
 if (BUILD_SHARED_LIBS)
   message("-- Building Ceres as a shared library.")
   # The CERES_BUILDING_SHARED_LIBRARY compile definition is NOT stored in
@@ -651,12 +618,6 @@ list(REMOVE_DUPLICATES CERES_COMPILE_OPTIONS)
 include(CreateCeresConfig)
 create_ceres_config("${CERES_COMPILE_OPTIONS}"
   ${Ceres_BINARY_DIR}/config/ceres/internal)
-# Force the location containing the configured config.h to the front of the
-# include_directories list (by default it is appended to the back) to ensure
-# that if the user has an installed version of Ceres in the same location as one
-# of the dependencies (e.g. /usr/local) that we find the config.h we just
-# configured, not the (older) installed config.h.
-include_directories(BEFORE ${Ceres_BINARY_DIR}/config)
 
 add_subdirectory(internal/ceres)
 

cmake/CeresConfig.cmake.in

@@ -50,21 +50,15 @@
 #
 # CERES_VERSION: Version of Ceres found.
 #
-# CERES_INCLUDE_DIRS: Include directories for Ceres and the
-#                     dependencies which appear in the Ceres public
-#                     API and are thus required to use Ceres.
-#
 # CERES_LIBRARIES: Libraries for Ceres and all
 #                  dependencies against which Ceres was
 #                  compiled. This will not include any optional
 #                  dependencies that were disabled when Ceres was
 #                  compiled.
 #
-# The following variables are also defined for legacy compatibility
-# only.  Any new code should not use them as they do not conform to
-# the standard CMake FindPackage naming conventions.
-#
-# CERES_INCLUDES = ${CERES_INCLUDE_DIRS}.
+# NOTE: There is no equivalent of CERES_INCLUDE_DIRS as the exported
+#       CMake target already includes the definition of its public
+#       include directories.
 
 # Called if we failed to find Ceres or any of its required dependencies,
 # unsets all public (designed to be used externally) variables and reports
@@ -74,6 +68,7 @@ macro(CERES_REPORT_NOT_FOUND REASON_MSG)
   # explicitly set FALSE to denote not found (not merely undefined).
   set(Ceres_FOUND FALSE)
   set(CERES_FOUND FALSE)
+  unset(CERES_INCLUDE_DIR)
   unset(CERES_INCLUDE_DIRS)
   unset(CERES_LIBRARIES)
 
@@ -143,18 +138,6 @@ if (CERES_WAS_INSTALLED)
       "outside of CMake after Ceres was built.")
   endif (NOT EXISTS ${CURRENT_ROOT_INSTALL_DIR})
 
-  # Set the include directories for Ceres (itself).
-  set(CERES_INCLUDE_DIR "${CURRENT_ROOT_INSTALL_DIR}/include")
-  if (NOT EXISTS ${CERES_INCLUDE_DIR}/ceres/ceres.h)
-    ceres_report_not_found(
-      "Ceres install root: ${CURRENT_ROOT_INSTALL_DIR}, "
-      "determined from relative path from CeresConfig.cmake install location: "
-      "${CERES_CURRENT_CONFIG_DIR}, does not contain Ceres headers. "
-      "Either the install directory was deleted, or the install tree was only "
-      "partially relocated outside of CMake after Ceres was built.")
-  endif (NOT EXISTS ${CERES_INCLUDE_DIR}/ceres/ceres.h)
-  list(APPEND CERES_INCLUDE_DIRS ${CERES_INCLUDE_DIR})
-
 else(CERES_WAS_INSTALLED)
   # Ceres was exported from the build tree.
   set(CERES_EXPORTED_BUILD_DIR ${CERES_CURRENT_CONFIG_DIR})
@@ -173,28 +156,6 @@ else(CERES_WAS_INSTALLED)
   # with Ceres to find Ceres' dependencies, even if the user has equivalently
   # named FindPackage() scripts in their project.
   set(CMAKE_MODULE_PATH ${CERES_EXPORTED_SOURCE_DIR}/cmake)
-
-  # Set the include directories for Ceres (itself).
-  set(CERES_INCLUDE_DIR "${CERES_EXPORTED_SOURCE_DIR}/include")
-  if (NOT EXISTS ${CERES_INCLUDE_DIR}/ceres/ceres.h)
-    ceres_report_not_found(
-      "Ceres exported source directory: ${CERES_EXPORTED_SOURCE_DIR}, "
-      "determined from relative path from CeresConfig.cmake exported build "
-      "directory: ${CERES_EXPORTED_BUILD_DIR}, does not contain Ceres headers.")
-  endif (NOT EXISTS ${CERES_INCLUDE_DIR}/ceres/ceres.h)
-  list(APPEND CERES_INCLUDE_DIRS ${CERES_INCLUDE_DIR})
-
-  # Append the path to the configured config.h in the exported build directory
-  # to the Ceres include directories.
-  set(CERES_CONFIG_FILE
-    ${CERES_EXPORTED_BUILD_DIR}/config/ceres/internal/config.h)
-  if (NOT EXISTS ${CERES_CONFIG_FILE})
-    ceres_report_not_found(
-      "Ceres exported build directory: ${CERES_EXPORTED_BUILD_DIR}, "
-      "does not contain required configured Ceres config.h, it is not here: "
-      "${CERES_CONFIG_FILE}.")
-  endif (NOT EXISTS ${CERES_CONFIG_FILE})
-  list(APPEND CERES_INCLUDE_DIRS ${CERES_EXPORTED_BUILD_DIR}/config)
 endif(CERES_WAS_INSTALLED)
 
 # Set the version.
@@ -234,33 +195,24 @@ else (EIGEN_FOUND)
     "dependency: Eigen version ${CERES_EIGEN_VERSION}, please set "
     "EIGEN_INCLUDE_DIR.")
 endif (EIGEN_FOUND)
-list(APPEND CERES_INCLUDE_DIRS ${EIGEN_INCLUDE_DIRS})
 
 # Glog.
 # Flag set during configuration and build of Ceres.
 set(CERES_USES_MINIGLOG @MINIGLOG@)
 set(CERES_USES_GFLAGS @GFLAGS@)
 if (CERES_USES_MINIGLOG)
-  set(MINIGLOG_INCLUDE_DIR ${CERES_INCLUDE_DIR}/ceres/internal/miniglog)
-  if (NOT CERES_WAS_INSTALLED)
-    # When Ceres was exported from the build tree, the miniglog headers
-    # will be in Ceres internal source directory, not in the public headers
-    # directory (they are copied with the public headers when installed).
-    set(MINIGLOG_INCLUDE_DIR
-      ${CERES_EXPORTED_SOURCE_DIR}/internal/ceres/miniglog)
-  endif()
-  if (NOT EXISTS ${MINIGLOG_INCLUDE_DIR})
-    ceres_report_not_found(
-      "Failed to find miniglog headers in expected include directory: "
-      "${MINIGLOG_INCLUDE_DIR}, but Ceres was compiled with MINIGLOG enabled "
-      "(in place of glog).")
-  endif (NOT EXISTS ${MINIGLOG_INCLUDE_DIR})
-  list(APPEND CERES_INCLUDE_DIRS ${MINIGLOG_INCLUDE_DIR})
   # Output message at standard log level (not the lower STATUS) so that
   # the message is output in GUI during configuration to warn user.
   message("-- Found Ceres compiled with miniglog substitute "
     "for glog, beware this will likely cause problems if glog is later linked.")
-else (CERES_USES_MINIGLOG)
+else(CERES_USES_MINIGLOG)
+  # As imported CMake targets are not re-exported when a dependent target is
+  # exported, we must invoke find_package(XXX) here to reload the definition
+  # of their targets.  Without this, the dependency target names (e.g.
+  # 'gflags-shared') which will be present in the ceres target would not be
+  # defined, and so CMake will assume that they refer to a library name and
+  # fail to link correctly.
+
   # Append the locations of glog when Ceres was built to the search path hints.
   set(GLOG_WAS_BUILT_WITH_CMAKE @FOUND_INSTALLED_GLOG_CMAKE_CONFIGURATION@)
   if (GLOG_WAS_BUILT_WITH_CMAKE)
@@ -271,29 +223,19 @@ else (CERES_USES_MINIGLOG)
     get_filename_component(CERES_BUILD_GLOG_LIBRARY_DIR @GLOG_LIBRARY@ PATH)
     list(APPEND GLOG_LIBRARY_DIR_HINTS ${CERES_BUILD_GLOG_LIBRARY_DIR})
   endif()
-
   # Search quietly s/t we control the timing of the error message if not found.
   find_package(Glog QUIET)
   if (GLOG_FOUND)
     message(STATUS "Found required Ceres dependency: glog")
-  else (GLOG_FOUND)
+  else()
     ceres_report_not_found("Missing required Ceres "
       "dependency: glog. Searched using GLOG_INCLUDE_DIR_HINTS: "
       "${GLOG_INCLUDE_DIR_HINTS} and glog_DIR: ${glog_DIR}.")
-  endif (GLOG_FOUND)
-  list(APPEND CERES_INCLUDE_DIRS ${GLOG_INCLUDE_DIRS})
+  endif()
 
   # gflags is only a public dependency of Ceres via glog, thus is not required
   # if Ceres was built with MINIGLOG.
   if (CERES_USES_GFLAGS)
-    # If gflags was found as an imported CMake target, we need to call
-    # find_packge(Gflags) again here, as imported CMake targets are not
-    # re-exported.  Without this, the 'gflags-shared' target name which is
-    # present in CERES_LIBRARIES in this case would not be defined, and so
-    # CMake will assume it is a library name (which it is not) and fail to link.
-    #
-    # Append the locations of gflags when Ceres was built to the search path
-    # hints.
     set(GFLAGS_WAS_BUILT_WITH_CMAKE @FOUND_INSTALLED_GFLAGS_CMAKE_CONFIGURATION@)
     if (GFLAGS_WAS_BUILT_WITH_CMAKE)
       set(gflags_DIR @gflags_DIR@)
@@ -303,7 +245,6 @@ else (CERES_USES_MINIGLOG)
       get_filename_component(CERES_BUILD_GFLAGS_LIBRARY_DIR @GFLAGS_LIBRARY@ PATH)
       list(APPEND GFLAGS_LIBRARY_DIR_HINTS ${CERES_BUILD_GFLAGS_LIBRARY_DIR})
     endif()
-
     # Search quietly s/t we control the timing of the error message if not found.
     find_package(Gflags QUIET)
     if (GFLAGS_FOUND)
@@ -313,9 +254,8 @@ else (CERES_USES_MINIGLOG)
         "dependency: gflags. Searched using GFLAGS_INCLUDE_DIR_HINTS: "
         "${GFLAGS_INCLUDE_DIR_HINTS} and gflags_DIR: ${gflags_DIR}.")
     endif()
-    list(APPEND CERES_INCLUDE_DIRS ${GFLAGS_INCLUDE_DIRS})
   endif()
-endif (CERES_USES_MINIGLOG)
+endif(CERES_USES_MINIGLOG)
 
 # Import exported Ceres targets, if they have not already been imported.
 if (NOT TARGET ceres AND NOT Ceres_BINARY_DIR)
@@ -324,9 +264,6 @@ endif (NOT TARGET ceres AND NOT Ceres_BINARY_DIR)
 # Set the expected XX_LIBRARIES variable for FindPackage().
 set(CERES_LIBRARIES ceres)
 
-# Set legacy include directories variable for backwards compatibility.
-set(CERES_INCLUDES ${CERES_INCLUDE_DIRS})
-
 # Reset CMake module path to its state when this script was called.
 set(CMAKE_MODULE_PATH ${CALLERS_CMAKE_MODULE_PATH})
 

cmake/uninstall.cmake.in

@@ -29,11 +29,6 @@
 # Author: arnaudgelas@gmail.com (Arnaud Gelas)
 #         alexs.mac@gmail.com (Alex Stewart)
 
-if (COMMAND cmake_policy)
-  # Ignore empty elements in LIST() commands.
-  cmake_policy(SET CMP0007 OLD)
-endif (COMMAND cmake_policy)
-
 if (NOT EXISTS "@CMAKE_CURRENT_BINARY_DIR@/install_manifest.txt")
   message(FATAL_ERROR "Cannot find install manifest: "
                       "\"@CMAKE_CURRENT_BINARY_DIR@/install_manifest.txt\"")

docs/source/installation.rst

@@ -867,12 +867,11 @@ used:
 
 .. code-block:: cmake
 
-    cmake_minimum_required(VERSION 2.8)
+    cmake_minimum_required(VERSION 3.5)
 
     project(helloworld)
 
     find_package(Ceres REQUIRED)
-    include_directories(${CERES_INCLUDE_DIRS})
 
     # helloworld
     add_executable(helloworld helloworld.cc)
@@ -886,6 +885,13 @@ directory containing the installed ``CeresConfig.cmake`` file
 ``Ceres_DIR`` should be the path to the exported Ceres build
 directory.
 
+  .. NOTE ::
+
+     You do not need to call include_directories(${CERES_INCLUDE_DIRS})
+     as the exported Ceres CMake target already contains the definitions
+     of its public include directories which will be automatically
+     included by CMake when compiling a target that links against Ceres.
+
 Specify Ceres components
 -------------------------------------
 

internal/ceres/CMakeLists.txt

@@ -271,6 +271,80 @@ else (BUILD_SHARED_LIBS)
   target_link_libraries(ceres ${CERES_LIBRARY_DEPENDENCIES})
 endif (BUILD_SHARED_LIBS)
 
+# Add the Ceres headers to its target.
+#
+# Force the location containing the configured config.h to the front of the
+# include_directories list (by default it is appended to the back) to ensure
+# that if the user has an installed version of Ceres in the same location as one
+# of the dependencies (e.g. /usr/local) that we find the config.h we just
+# configured, not the (older) installed config.h.
+target_include_directories(ceres BEFORE PUBLIC
+  $<BUILD_INTERFACE:${Ceres_BINARY_DIR}/config>)
+target_include_directories(ceres PRIVATE ${Ceres_SOURCE_DIR}/internal)
+target_include_directories(ceres PUBLIC
+  $<BUILD_INTERFACE:${Ceres_SOURCE_DIR}/include>
+  $<INSTALL_INTERFACE:include>)
+
+# Eigen SparseQR generates various compiler warnings related to unused and
+# uninitialised local variables.  To avoid having to individually suppress these
+# warnings around the #include statments for Eigen headers across all GCC/Clang
+# versions, we tell CMake to treat Eigen headers as system headers.  This
+# results in all compiler warnings from them being suppressed.
+target_include_directories(ceres SYSTEM PUBLIC ${EIGEN_INCLUDE_DIRS})
+
+# Gather the list of public & private include locations for all enabled optional
+# dependencies to be added to the Ceres target.
+set(CERES_LIBRARY_PRIVATE_DEPENDENCIES_INCLUDE_DIRS "")
+set(CERES_LIBRARY_PUBLIC_DEPENDENCIES_INCLUDE_DIRS "")
+if (MINIGLOG)
+  # Force the miniglog headers to the front of the public include directories
+  # to protect against the case when the user has glog installed in a standard
+  # location (specifically the same as the Ceres install location) but compiled
+  # Ceres with MINIGLOG anyway.  Otherwise: "glog/logging.h" in the public Ceres
+  # headers used in client code would match the installed version of glog, not
+  # the miniglog headers, and the client application would fail to link.
+  #
+  # Note that this is an imperfect fix, as we cannot control the include
+  # directories in client projects, and they could easily invert this ordering
+  # themselves (intentionally or otherwise) and so break their build.
+  target_include_directories(ceres BEFORE PUBLIC
+    $<BUILD_INTERFACE:${Ceres_SOURCE_DIR}/internal/ceres/miniglog>
+    $<INSTALL_INTERFACE:include/ceres/internal/miniglog>)
+elseif (NOT FOUND_INSTALLED_GLOG_CMAKE_CONFIGURATION)
+  # Only append glog include directories if the glog found was not a CMake
+  # exported target that already includes them.
+  list(APPEND CERES_LIBRARY_PUBLIC_DEPENDENCIES_INCLUDE_DIRS
+    ${GLOG_INCLUDE_DIRS})
+endif()
+if (SUITESPARSE)
+  list(APPEND CERES_LIBRARY_PRIVATE_DEPENDENCIES_INCLUDE_DIRS
+    ${SUITESPARSE_INCLUDE_DIRS})
+endif()
+if (CXSPARSE)
+  list(APPEND CERES_LIBRARY_PRIVATE_DEPENDENCIES_INCLUDE_DIRS
+    ${CXSPARSE_INCLUDE_DIRS})
+endif()
+if (ACCELERATESPARSE)
+  list(APPEND CERES_LIBRARY_PRIVATE_DEPENDENCIES_INCLUDE_DIRS
+    ${AccelerateSparse_INCLUDE_DIRS})
+endif()
+if (GFLAGS AND NOT FOUND_INSTALLED_GFLAGS_CMAKE_CONFIGURATION)
+  # Only append gflags include directories if the gflags found was not a CMake
+  # exported target that already includes them.
+  list(APPEND CERES_LIBRARY_PUBLIC_DEPENDENCIES_INCLUDE_DIRS
+    ${GFLAGS_INCLUDE_DIRS})
+endif()
+# Add include locations for optional dependencies to the Ceres target without
+# duplication.
+list(REMOVE_DUPLICATES CERES_LIBRARY_PRIVATE_DEPENDENCIES_INCLUDE_DIRS)
+foreach(INC_DIR ${CERES_LIBRARY_PRIVATE_DEPENDENCIES_INCLUDE_DIRS})
+  target_include_directories(ceres PRIVATE ${INC_DIR})
+endforeach()
+list(REMOVE_DUPLICATES CERES_LIBRARY_PUBLIC_DEPENDENCIES_INCLUDE_DIRS)
+foreach(INC_DIR ${CERES_LIBRARY_PUBLIC_DEPENDENCIES_INCLUDE_DIRS})
+  target_include_directories(ceres PUBLIC ${INC_DIR})
+endforeach()
+
 install(TARGETS ceres
         EXPORT  CeresExport
         RUNTIME DESTINATION bin
@@ -289,6 +363,7 @@ if (BUILD_TESTING AND GFLAGS)
               evaluator_test_utils.cc
               numeric_diff_test_utils.cc
               test_util.cc)
+  target_include_directories(test_util PUBLIC ${Ceres_SOURCE_DIR}/internal)
 
   if (MINIGLOG)
     # When using miniglog, it is compiled into Ceres, thus Ceres becomes
@@ -302,6 +377,11 @@ if (BUILD_TESTING AND GFLAGS)
 
   macro (CERES_TEST NAME)
     add_executable(${NAME}_test ${NAME}_test.cc)
+    # Pull in local headers from the generated test directories when ceres_test()
+    # is invoked there, as well as the private headers in this directory which
+    # may be referenced without the 'ceres' path prefix.
+    target_include_directories(${NAME}_test PUBLIC ${CMAKE_CURRENT_LIST_DIR})
+    target_include_directories(${NAME}_test PUBLIC ${Ceres_SOURCE_DIR}/internal/ceres)
     target_link_libraries(${NAME}_test test_util ceres gtest)
     if (BUILD_SHARED_LIBS)
       # Define gtest-specific shared library flags for linking.
@@ -403,13 +483,18 @@ if (BUILD_TESTING AND GFLAGS)
 
 endif (BUILD_TESTING AND GFLAGS)
 
+macro(add_dependencies_to_benchmark BENCHMARK_TARGET)
+  target_link_libraries(${BENCHMARK_TARGET} ceres benchmark::benchmark)
+  target_include_directories(${BENCHMARK_TARGET} PUBLIC ${Ceres_SOURCE_DIR}/internal)
+endmacro()
+
 if (BUILD_BENCHMARKS)
   add_executable(autodiff_cost_function_benchmark autodiff_cost_function_benchmark.cc)
-  target_link_libraries(autodiff_cost_function_benchmark ceres benchmark::benchmark)
+  add_dependencies_to_benchmark(autodiff_cost_function_benchmark)
 
   add_executable(small_blas_gemv_benchmark small_blas_gemv_benchmark.cc)
-  target_link_libraries(small_blas_gemv_benchmark ceres benchmark::benchmark)
+  add_dependencies_to_benchmark(small_blas_gemv_benchmark)
 
   add_executable(small_blas_gemm_benchmark small_blas_gemm_benchmark.cc)
-  target_link_libraries(small_blas_gemm_benchmark ceres benchmark::benchmark)
+  add_dependencies_to_benchmark(small_blas_gemm_benchmark)
 endif (BUILD_BENCHMARKS)