Skip to content

Commit

Permalink
Add CMake install rules for gtest libraries and headers
Browse files Browse the repository at this point in the history
  • Loading branch information
@nocnokneo
nocnokneo committed Nov 23, 2015
1 parent 786564f commit 98d988d
Showing 1 changed file with 8 additions and 0 deletions.
8 changes: 8 additions & 0 deletions googletest/CMakeLists.txt
View file
Expand Up @@ -91,6 +91,14 @@ cxx_library(gtest "${cxx_strict}" src/gtest-all.cc)
cxx_library(gtest_main "${cxx_strict}" src/gtest_main.cc)
target_link_libraries(gtest_main gtest)

########################################################################
#
# Install rules
install(TARGETS gtest gtest_main
DESTINATION lib)
install(DIRECTORY ${gtest_SOURCE_DIR}/include/gtest
DESTINATION include)

########################################################################
#
# Samples on how to link user tests with gtest or gtest_main.
Expand Down

4 comments on commit 98d988d

@byllyfish
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My cmake project uses googletest via add_subdirectory(external/googletest/googletest). I'm probably not the only one who does this.

After updating to the latest googletest, my project's make install target is installing googletest files into /usr/include and /usr/lib. I do not want the gtest files to be installed with my project; I doubt my customers do either.

Is there a reason the install code was added and is turned on by default? Could install be made optional and default to off?

Thanks.

@nocnokneo
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bundling the build system of another project into your project this way is a simple but fragile approach. Any number of valid upstream changes in the googletest CMake project could break your project. It could have defined target name that collided with a target name in your project for example.

Adding an install target is also completely valid for GTest to do. The lack of an install target until now was a bug that contributed to the rise of the add_subdirectory approach. For example: http://stackoverflow.com/questions/9689183

An install target that is disabled by default is bizarre. I've never needed to opt-in for make install to work. But I don't see why an option that is enabled by default couldn't be added.

It will cost you a few more lines of CMake code, but the more robust way to pull in GTest is with the ExternalProject module. Either with a superbuild like:

option(USE_SUPERBUILD "Fetch/build/install dependencies before building MyProject" ON)

if(USE_SUPERBUILD)
  project(MyProject-Superbuild NONE)
  include(ExternalProject)
  ExternalProject_Add(GTest SOURCE_DIR external/googletest/googletest)
  ExternalProject_Get_Property(GTest INSTALL_DIR)
  ExternalProject_Add(MyProject
    DEPENDS GTest
    SOURCE_DIR ${PROJECT_SOURCE_DIR}
    BINARY_DIR ${CMAKE_CURRENT_BINARY_DIR}/MyProject-build
    CMAKE_ARGS -DUSE_SUPERBUILD=OFF -DGTEST_ROOT:PATH=${INSTALL_DIR}
  )
  return()
endif()

project(MyProject)
find_package(GTest)
# ...

Or with an IMPORTED target like:

ExternalProject_Add(GTest SOURCE_DIR external/googletest/googletest)
ExternalProject_Get_Property(GTest INSTALL_DIR)
add_library(GTest::gtest IMPORTED STATIC GLOBAL)
set_target_properties(GTest::gtest PROPERTIES
  IMPORTED_LOCATION ${INSTALL_DIR}/lib/libgtest.a
  INTERFACE_INCLUDE_DIRECTORIES ${INSTALL_DIR}/include
)
add_executable(foo main.cpp)
target_link_libraries(foo GTest::gtest)

When I get a chance, I'll submit a follow up patch that installs a GTestConfig.cmake with IMPORTED targets so that the superbuild+find_package approach automatically gets the same convenience of doing target_link_libraries(foo GTest::gtest).

@snikulov
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nocnokneo Sorry for been late, but is this statement no longer true?

https://github.com/google/googletest/blob/master/googletest/Makefile.am#L305

Why cmake adding install rules, while makefiles still have install target disabled?

@nocnokneo
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can't speak for the GTest team, but I can give you some background.

The opinion of the GTest team is that GTest should always be built by the consuming project so that GTest can be built with the exact same compiler, compiler flags, libraries, etc.

The case I made for adding install rules to the CMake build was that "super builds" are a common way to structure CMake-based projects which satisfies the need to compile GTest with all the same compile flags. But super builds are much simpler to use when the external projects like GTest have proper install rules setup (the super build uses the install rules to install external projects into a staging directory within the build directory)

Please sign in to comment.