From 251d2d0c521c781276eea3748d92fc92e5dbceab Mon Sep 17 00:00:00 2001 From: Luke Yeager Date: Fri, 1 Sep 2017 23:20:16 -0700 Subject: [PATCH] cmake: stop including files from the install directory Summary: Here is the buggy behavior which this change fixes: * On the first configure with CMake, a system-wide benchmark installation is not found, so we use the version in `third_party/` ([see here](https://github.com/caffe2/caffe2/blob/v0.8.1/cmake/Dependencies.cmake#L98-L100)) * On installation, the benchmark sub-project installs its headers to `CMAKE_INSTALL_PREFIX` ([see here](https://github.com/google/benchmark/blob/4bf28e611b/src/CMakeLists.txt#L41-L44)) * On a rebuild, CMake searches the system again for a benchmark installation (see https://github.com/caffe2/caffe2/issues/916 for details on why the first search is not cached) * CMake includes `CMAKE_INSTALL_PREFIX` when searching the system ([docs](https://cmake.org/cmake/help/v3.0/variable/CMAKE_SYSTEM_PREFIX_PATH.html)) * Voila, a "system" installation of benchmark is found at `CMAKE_INSTALL_PREFIX` * On a rebuild, `-isystem $CMAKE_INSTALL_PREFIX/include` is added to every build target ([see here](https://github.com/caffe2/caffe2/blob/v0.8.1/cmake/Dependencies.cmake#L97)). e.g: cd /caffe2/build/caffe2/binaries && ccache /usr/bin/c++ -I/caffe2/build -isystem /caffe2/third_party/googletest/googletest/include -isystem /caffe2/install/include -isystem /usr/include/opencv -isystem /caffe2/third_party/eigen -isystem /usr/include/python2.7 -isystem /usr/lib/python2.7/dist-packages/numpy/core/include -isystem /caffe2/third_party/pybind11/include -isystem /usr/local/cuda/include -isystem /caffe2/third_party/cub -I/caffe2 -I/caffe2/build_host_protoc/include -fopenmp -std=c++11 -O2 -fPIC -Wno-narrowing -O3 -DNDEBUG -o CMakeFiles/split_db.dir/split_db.cc.o -c /caffe2/caffe2/binaries/split_db.cc This causes two issues: 1. Since the headers and libraries at `CMAKE_INSTALL_PREFIX` have a later timestamp than the built files, an unnecessary rebuild is triggered 2. Out-dated headers from the install directory are used during compilation, which can lead to strange build errors (which can usually be fixed by `rm -rf`'ing the install directory) Possible solutions: * Stop searching the system for an install of benchmark, and always use the version in `third_party/` * Cache the initial result of the system-wide search for benchmark, so we don't accidentally pick up the installed version later * Hack CMake to stop looking for headers and libraries in the installation directory This PR is an implementation of the first solution. Feel free to close this and fix the issue in another way if you like. Closes https://github.com/caffe2/caffe2/pull/1112 Differential Revision: D5761750 Pulled By: Yangqing fbshipit-source-id: 2240088994ffafdb6eedb3626d898b505a4ba564 --- CMakeLists.txt | 3 +++ cmake/Modules/FindBenchmark.cmake | 14 ++++++++------ 2 files changed, 11 insertions(+), 6 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 95f2482d012..9c2e1feacff 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,5 +1,8 @@ cmake_minimum_required(VERSION 2.8.12 FATAL_ERROR) +message(STATUS "Setting CMAKE_FIND_NO_INSTALL_PREFIX") +set(CMAKE_FIND_NO_INSTALL_PREFIX TRUE) + project(Caffe2 CXX C) # TODO(bwasti): versioning diff --git a/cmake/Modules/FindBenchmark.cmake b/cmake/Modules/FindBenchmark.cmake index 8d68cb36f21..e6420a5eced 100644 --- a/cmake/Modules/FindBenchmark.cmake +++ b/cmake/Modules/FindBenchmark.cmake @@ -4,13 +4,15 @@ # Benchmark_LIBRARIES - libraries needed to use benchmark find_path(Benchmark_INCLUDE_DIR - NAMES benchmark/benchmark.h - DOC "The directory where benchmark includes reside" + NAMES benchmark/benchmark.h + NO_SYSTEM_ENVIRONMENT_PATH + DOC "The directory where benchmark includes reside" ) find_library(Benchmark_LIBRARY - NAMES benchmark - DOC "The benchmark library" + NAMES benchmark + NO_SYSTEM_ENVIRONMENT_PATH + DOC "The benchmark library" ) set(Benchmark_INCLUDE_DIRS ${Benchmark_INCLUDE_DIR}) @@ -18,8 +20,8 @@ set(Benchmark_LIBRARIES ${Benchmark_LIBRARY}) include(FindPackageHandleStandardArgs) find_package_handle_standard_args(Benchmark - FOUND_VAR Benchmark_FOUND - REQUIRED_VARS Benchmark_INCLUDE_DIR Benchmark_LIBRARY + FOUND_VAR Benchmark_FOUND + REQUIRED_VARS Benchmark_INCLUDE_DIR Benchmark_LIBRARY ) mark_as_advanced(Benchmark_FOUND)