Firestore CMake cleanup (firebase#1510)

* Pull out sanitizer options into a separate file * Reorder packages * Remove the extra "external/$target" directory from PREFIX Instead of BINARY_DIR/external/foo/src/foo the contents will be in BINARY_DIR/src/foo. * Make libprotobuf-nanopb targets line up with nanopb's CMakeLists.txt * Fix CMake style
gorillatech · Jul 11, 2018 · 7fc953f · 7fc953f
1 parent e1ace70
commit 7fc953f
Show file tree

Hide file tree

Showing 18 changed files with 98 additions and 85 deletions.
diff --git a/Firestore/CMakeLists.txt b/Firestore/CMakeLists.txt
@@ -15,48 +15,6 @@
 cmake_minimum_required(VERSION 2.8.11)
 project(firestore C CXX)
 
-option(WITH_ASAN "Build with Address Sanitizer" OFF)
-# TODO(varconst): msan
-# Memory sanitizer is more complicated:
-# - it requires all dependencies to be compiled with msan enabled (see
-#   https://github.com/google/sanitizers/wiki/MemorySanitizerLibcxxHowTo);
-# - AppleClang doesn't support it.
-option(WITH_TSAN "Build with Thread Sanitizer (mutually exculsive with other sanitizers)" OFF)
-option(WITH_UBSAN "Build with Undefined Behavior sanitizer" OFF)
-
-macro(add_to_compile_and_link_flags flag)
-  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${flag}")
-  set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${flag}")
-endmacro()
-
-if(CMAKE_CXX_COMPILER_ID MATCHES "Clang" OR CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
-  if(WITH_ASAN)
-    add_to_compile_and_link_flags("-fsanitize=address")
-  endif()
-
-  if(WITH_TSAN)
-    if(WITH_ASAN OR WITH_UBSAN)
-      message(FATAL_ERROR "Cannot combine TSan with other santizers")
-    endif()
-    add_to_compile_and_link_flags("-fsanitize=thread")
-  endif()
-
-  if(WITH_UBSAN)
-    add_to_compile_and_link_flags("-fsanitize=undefined")
-  endif()
-
-  if (WITH_ASAN OR WITH_TSAN OR WITH_UBSAN)
-    # Recommended to "get nicer stack traces in error messages"
-    # TODO(varconst): double-check that TSan actually needs this flag (it's
-    # explicitly recommended in the docs for ASan and UBSan)
-    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-omit-frame-pointer")
-  endif()
-else()
-  if(WITH_ASAN OR WITH_TSAN OR WITH_UBSAN)
-    message(FATAL_ERROR "Only Clang and GCC support sanitizers")
-  endif()
-endif()
-
 set(FIREBASE_SOURCE_DIR ${CMAKE_CURRENT_LIST_DIR}/..)
 
 # CMAKE_INSTALL_PREFIX should be passed in to this build so that it can find
@@ -75,19 +33,21 @@ set(FIREBASE_INSTALL_DIR ${CMAKE_INSTALL_PREFIX})
 set(FIREBASE_BINARY_DIR ${PROJECT_BINARY_DIR}/..)
 
 list(INSERT CMAKE_MODULE_PATH 0 ${FIREBASE_SOURCE_DIR}/cmake)
+include(SanitizerOptions)
 include(utils)
 
-find_package(GTest REQUIRED)
-find_package(GMock REQUIRED)
-find_package(LevelDB REQUIRED)
-find_package(Nanopb REQUIRED)
-find_package(Protobuf REQUIRED)
-find_package(GRPC REQUIRED)
 
+# External packages
 if(APPLE)
-  find_package(GoogleUtilities REQUIRED)
   find_package(FirebaseCore REQUIRED)
+  find_package(GoogleUtilities REQUIRED)
 endif()
+find_package(GMock REQUIRED)
+find_package(GRPC REQUIRED)
+find_package(GTest REQUIRED)
+find_package(LevelDB REQUIRED)
+find_package(Nanopb REQUIRED)
+find_package(Protobuf REQUIRED)
 
 enable_testing()
 add_subdirectory(third_party/abseil-cpp)

diff --git a/Firestore/Protos/CMakeLists.txt b/Firestore/Protos/CMakeLists.txt
@@ -50,19 +50,19 @@ cc_library(
     nanopb/google/type/latlng.pb.c
     nanopb/google/type/latlng.nanopb.h
   DEPENDS
-    nanopb
+    protobuf-nanopb
 )
 
 target_compile_definitions(
-  firebase_firestore_protos_nanopb PUBLIC
-    -DPB_FIELD_16BIT
+  firebase_firestore_protos_nanopb
+  PUBLIC -DPB_FIELD_16BIT
 )
 
 # TODO(rsgowman): this may be worth moving into cc_library, possibly via an
 # INCLUDE_DIRS or similar.
 target_include_directories(
-  firebase_firestore_protos_nanopb PUBLIC
-    ${FIREBASE_SOURCE_DIR}/Firestore/Protos/nanopb
+  firebase_firestore_protos_nanopb
+  PUBLIC ${FIREBASE_SOURCE_DIR}/Firestore/Protos/nanopb
 )
 
 # libprotobuf based generated protos. Expected only to be used in test (as
@@ -102,10 +102,10 @@ cc_library(
 
 target_include_directories(
   firebase_firestore_protos_libprotobuf PUBLIC
-    ${FIREBASE_SOURCE_DIR}/Firestore/Protos/cpp
+  ${FIREBASE_SOURCE_DIR}/Firestore/Protos/cpp
 )
 
 set_target_properties(
-  firebase_firestore_protos_libprotobuf PROPERTIES
-    COMPILE_FLAGS "-Wno-unused-parameter"
+  firebase_firestore_protos_libprotobuf
+  PROPERTIES COMPILE_FLAGS "-Wno-unused-parameter"
 )
diff --git a/Firestore/core/src/firebase/firestore/local/CMakeLists.txt b/Firestore/core/src/firebase/firestore/local/CMakeLists.txt
@@ -29,5 +29,5 @@ cc_library(
     firebase_firestore_protos_nanopb
     firebase_firestore_remote
     firebase_firestore_util
-    nanopb
+    protobuf-nanopb
 )
diff --git a/Firestore/core/src/firebase/firestore/nanopb/CMakeLists.txt b/Firestore/core/src/firebase/firestore/nanopb/CMakeLists.txt
@@ -23,5 +23,5 @@ cc_library(
   DEPENDS
     firebase_firestore_util
     firebase_firestore_protos_nanopb
-    nanopb
+    protobuf-nanopb
 )
diff --git a/Firestore/core/src/firebase/firestore/remote/CMakeLists.txt b/Firestore/core/src/firebase/firestore/remote/CMakeLists.txt
@@ -27,5 +27,5 @@ cc_library(
     firebase_firestore_protos_nanopb
     firebase_firestore_util
     grpc::grpc
-    nanopb
+    protobuf-nanopb
 )
diff --git a/cmake/FindGRPC.cmake b/cmake/FindGRPC.cmake
@@ -15,8 +15,6 @@
 include(FindPackageHandleStandardArgs)
 include(FindZLIB)
 
-set(BINARY_DIR ${FIREBASE_BINARY_DIR}/external/grpc)
-
 ## ZLIB
 
 # the grpc ExternalProject already figures out if zlib should be built or
@@ -25,7 +23,7 @@ set(BINARY_DIR ${FIREBASE_BINARY_DIR}/external/grpc)
 find_library(
   ZLIB_LIBRARY
   NAMES z
-  HINTS ${BINARY_DIR}/src/grpc-build/third_party/zlib
+  HINTS ${FIREBASE_BINARY_DIR}/src/grpc-build/third_party/zlib
 )
 
 # If found above, the standard package will honor the ZLIB_LIBRARY variable.
@@ -36,19 +34,19 @@ find_package(ZLIB REQUIRED)
 
 find_path(
   OPENSSL_INCLUDE_DIR openssl/ssl.h
-  HINTS ${BINARY_DIR}/src/grpc/third_party/boringssl/include
+  HINTS ${FIREBASE_BINARY_DIR}/src/grpc/third_party/boringssl/include
 )
 
 find_library(
   OPENSSL_SSL_LIBRARY
   NAMES ssl
-  HINTS ${BINARY_DIR}/src/grpc-build/third_party/boringssl/ssl
+  HINTS ${FIREBASE_BINARY_DIR}/src/grpc-build/third_party/boringssl/ssl
 )
 
 find_library(
   OPENSSL_CRYPTO_LIBRARY
   NAMES crypto
-  HINTS ${BINARY_DIR}/src/grpc-build/third_party/boringssl/crypto
+  HINTS ${FIREBASE_BINARY_DIR}/src/grpc-build/third_party/boringssl/crypto
 )
 
 find_package(OpenSSL REQUIRED)
@@ -69,7 +67,7 @@ find_path(
   HINTS
     $ENV{GRPC_ROOT}/include
     ${GRPC_ROOT}/include
-    ${BINARY_DIR}/src/grpc/include
+    ${FIREBASE_BINARY_DIR}/src/grpc/include
 )
 
 find_library(
@@ -78,7 +76,7 @@ find_library(
   HINTS
     $ENV{GRPC_ROOT}/lib
     ${GRPC_ROOT}/lib
-    ${BINARY_DIR}/src/grpc-build
+    ${FIREBASE_BINARY_DIR}/src/grpc-build
 )
 
 find_library(
@@ -87,7 +85,7 @@ find_library(
   HINTS
     $ENV{GRPC_ROOT}/lib
     ${GRPC_ROOT}/lib
-    ${BINARY_DIR}/src/grpc-build
+    ${FIREBASE_BINARY_DIR}/src/grpc-build
 )
 
 find_package_handle_standard_args(

diff --git a/cmake/FindLevelDB.cmake b/cmake/FindLevelDB.cmake
@@ -12,7 +12,7 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
-set(binary_dir ${FIREBASE_BINARY_DIR}/external/leveldb/src/leveldb)
+set(binary_dir ${FIREBASE_BINARY_DIR}/src/leveldb)
 
 find_path(
   LEVELDB_INCLUDE_DIR leveldb/db.h

diff --git a/cmake/FindNanopb.cmake b/cmake/FindNanopb.cmake
@@ -14,8 +14,6 @@
 
 include(FindPackageHandleStandardArgs)
 
-set(BINARY_DIR ${FIREBASE_BINARY_DIR}/external/nanopb)
-
 find_path(
   NANOPB_INCLUDE_DIR pb.h
 )
@@ -35,10 +33,10 @@ find_package_handle_standard_args(
 if(NANOPB_FOUND)
   set(NANOPB_INCLUDE_DIRS ${NANOPB_INCLUDE_DIR})
 
-  if (NOT TARGET nanopb)
-    add_library(nanopb UNKNOWN IMPORTED)
+  if (NOT TARGET protobuf-nanopb)
+    add_library(protobuf-nanopb UNKNOWN IMPORTED)
     set_target_properties(
-      nanopb PROPERTIES
+      protobuf-nanopb PROPERTIES
       INTERFACE_INCLUDE_DIRECTORIES ${NANOPB_INCLUDE_DIRS}
       IMPORTED_LOCATION ${NANOPB_LIBRARY}
     )

diff --git a/cmake/SanitizerOptions.cmake b/cmake/SanitizerOptions.cmake
@@ -0,0 +1,57 @@
+# Copyright 2018 Google
+#
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#
+#      http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+# Adds Sanitzer options to the current build.
+
+option(WITH_ASAN "Build with Address Sanitizer" OFF)
+# TODO(varconst): msan
+# Memory sanitizer is more complicated:
+# - it requires all dependencies to be compiled with msan enabled (see
+#   https://github.com/google/sanitizers/wiki/MemorySanitizerLibcxxHowTo);
+# - AppleClang doesn't support it.
+option(WITH_TSAN "Build with Thread Sanitizer (mutually exculsive with other sanitizers)" OFF)
+option(WITH_UBSAN "Build with Undefined Behavior sanitizer" OFF)
+
+macro(add_to_compile_and_link_flags flag)
+  set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} ${flag}")
+  set(CMAKE_EXE_LINKER_FLAGS "${CMAKE_EXE_LINKER_FLAGS} ${flag}")
+endmacro()
+
+if(CMAKE_CXX_COMPILER_ID MATCHES "Clang" OR CMAKE_CXX_COMPILER_ID STREQUAL "GNU")
+  if(WITH_ASAN)
+    add_to_compile_and_link_flags("-fsanitize=address")
+  endif()
+
+  if(WITH_TSAN)
+    if(WITH_ASAN OR WITH_UBSAN)
+      message(FATAL_ERROR "Cannot combine TSan with other santizers")
+    endif()
+    add_to_compile_and_link_flags("-fsanitize=thread")
+  endif()
+
+  if(WITH_UBSAN)
+    add_to_compile_and_link_flags("-fsanitize=undefined")
+  endif()
+
+  if (WITH_ASAN OR WITH_TSAN OR WITH_UBSAN)
+    # Recommended to "get nicer stack traces in error messages"
+    # TODO(varconst): double-check that TSan actually needs this flag (it's
+    # explicitly recommended in the docs for ASan and UBSan)
+    set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -fno-omit-frame-pointer")
+  endif()
+else()
+  if(WITH_ASAN OR WITH_TSAN OR WITH_UBSAN)
+    message(FATAL_ERROR "Only Clang and GCC support sanitizers")
+  endif()
+endif()
diff --git a/cmake/external/c-ares.cmake b/cmake/external/c-ares.cmake
@@ -25,7 +25,7 @@ ExternalProject_Add(
   URL https://github.com/c-ares/c-ares/archive/cares-1_14_0.tar.gz
   URL_HASH SHA256=62dd12f0557918f89ad6f5b759f0bf4727174ae9979499f5452c02be38d9d3e8
 
-  PREFIX ${PROJECT_BINARY_DIR}/external/cares
+  PREFIX ${PROJECT_BINARY_DIR}
 
   CMAKE_ARGS
     -DCMAKE_BUILD_TYPE:STRING=${CMAKE_BUILD_TYPE}

diff --git a/cmake/external/firestore.cmake b/cmake/external/firestore.cmake
@@ -38,7 +38,7 @@ ExternalProject_Add(
 
   # Lay the binary directory out as if this were a subproject. This makes it
   # possible to build and test in it directly.
-  PREFIX ${PROJECT_BINARY_DIR}/external/Firestore
+  PREFIX ${PROJECT_BINARY_DIR}
   SOURCE_DIR ${PROJECT_SOURCE_DIR}/Firestore
   BINARY_DIR ${PROJECT_BINARY_DIR}/Firestore
 

diff --git a/cmake/external/googletest.cmake b/cmake/external/googletest.cmake
@@ -26,7 +26,7 @@ ExternalProject_Add(
   URL https://github.com/google/googletest/archive/release-1.8.0.tar.gz
   URL_HASH SHA256=58a6f4277ca2bc8565222b3bbd58a177609e9c488e8a72649359ba51450db7d8
 
-  PREFIX ${PROJECT_BINARY_DIR}/external/googletest
+  PREFIX ${PROJECT_BINARY_DIR}
 
   CMAKE_CACHE_ARGS
     -DCMAKE_INSTALL_PREFIX:STRING=${FIREBASE_INSTALL_DIR}

diff --git a/cmake/external/grpc.cmake b/cmake/external/grpc.cmake
@@ -121,7 +121,7 @@ ExternalProject_Add(
 
   ${GRPC_GIT}
 
-  PREFIX ${PROJECT_BINARY_DIR}/external/grpc
+  PREFIX ${PROJECT_BINARY_DIR}
 
   CMAKE_ARGS
     ${CMAKE_ARGS}

diff --git a/cmake/external/leveldb.cmake b/cmake/external/leveldb.cmake
@@ -55,7 +55,7 @@ ExternalProject_Add(
   URL https://github.com/google/leveldb/archive/v1.20.tar.gz
   URL_HASH SHA256=f5abe8b5b209c2f36560b75f32ce61412f39a2922f7045ae764a2c23335b6664
 
-  PREFIX ${FIREBASE_BINARY_DIR}/external/leveldb
+  PREFIX ${FIREBASE_BINARY_DIR}
 
   # LevelDB's configuration is done in the Makefile
   CONFIGURE_COMMAND ""

diff --git a/cmake/external/nanopb.cmake b/cmake/external/nanopb.cmake
@@ -30,7 +30,7 @@ ExternalProject_Add(
   URL https://github.com/nanopb/nanopb/archive/nanopb-0.3.9.1.tar.gz
   URL_HASH SHA256=67460d0c0ad331ef4d5369ad337056d0cd2f900c94887628d287eb56c69324bc
 
-  PREFIX ${PROJECT_BINARY_DIR}/external/nanopb
+  PREFIX ${PROJECT_BINARY_DIR}
 
   CMAKE_ARGS
     -DCMAKE_BUILD_TYPE:STRING=${CMAKE_BUILD_TYPE}

diff --git a/cmake/external/protobuf.cmake b/cmake/external/protobuf.cmake
@@ -67,7 +67,7 @@ if(CMAKE_VERSION VERSION_LESS "3.7")
   list(
     APPEND configure
     ${cmake_args}
-    "${PROJECT_BINARY_DIR}/external/protobuf/src/protobuf/cmake"
+    "${PROJECT_BINARY_DIR}/src/protobuf/cmake"
   )
 
   # Compose BUILD_COMMAND and INSTALL_COMMAND
@@ -104,7 +104,7 @@ ExternalProject_Add(
   URL https://github.com/google/protobuf/archive/v3.5.1.1.tar.gz
   URL_HASH SHA256=56b5d9e1ab2bf4f5736c4cfba9f4981fbc6976246721e7ded5602fbaee6d6869
 
-  PREFIX ${PROJECT_BINARY_DIR}/external/protobuf
+  PREFIX ${PROJECT_BINARY_DIR}
   INSTALL_DIR ${FIREBASE_INSTALL_DIR}
 
   ${commands}

diff --git a/cmake/external/zlib.cmake b/cmake/external/zlib.cmake
@@ -33,7 +33,7 @@ ExternalProject_Add(
   URL https://github.com/madler/zlib/archive/v1.2.11.tar.gz
   URL_HASH SHA256=629380c90a77b964d896ed37163f5c3a34f6e6d897311f1df2a7016355c45eff
 
-  PREFIX ${PROJECT_BINARY_DIR}/external/zlib
+  PREFIX ${PROJECT_BINARY_DIR}
 
   CMAKE_ARGS
     -DCMAKE_BUILD_TYPE:STRING=${CMAKE_BUILD_TYPE}

diff --git a/cmake/xcodebuild.cmake b/cmake/xcodebuild.cmake
@@ -63,7 +63,7 @@ function(xcodebuild framework)
     ${framework}
     DEPENDS ${xcb_DEPENDS}
 
-    PREFIX ${PROJECT_BINARY_DIR}/external/${framework}
+    PREFIX ${PROJECT_BINARY_DIR}
 
     # The source directory doesn't actually matter
     SOURCE_DIR ${PROJECT_SOURCE_DIR}