Build and test both C++ loggers where possible (#595)

* Rename FIREBASE_BINARY_DIR to FIREBASE_INSTALL_DIR

  Make this consistent with the outer superbuild and also make the
  association with CMAKE_INSTALL_PREFIX more obvious.

* Build and test log_stdio separate from the rest of util

  This is in preparation for adding a test for log_apple

* Build and test log_apple under CMake

  Also add notes about how FIRLogger's debug mode can break this test

* Refactor log_apple to cut down duplicate switch statements

  There's also a slight reduction in final binary size.

8 files changed, 123 insertions, 48 deletions

diff --git a/.gitignore b/.gitignore
index 61fc41b..10ada7a 100644
--- a/.gitignore
+++ b/.gitignore
@@ -33,6 +33,9 @@ DerivedData
 *.hmap
 *.ipa
 
+# IntelliJ
+.idea
+
 # Vim
 *.swo
 *.swp
@@ -56,5 +59,5 @@ Podfile.lock
 *.xcworkspace
 
 # CMake
-.downloads
-.idea/
+Debug
+Release
diff --git a/Firestore/CMakeLists.txt b/Firestore/CMakeLists.txt
index ba9998d..cffd015 100644
--- a/Firestore/CMakeLists.txt
+++ b/Firestore/CMakeLists.txt
@@ -16,7 +16,14 @@ cmake_minimum_required(VERSION 2.8.11)
 project(firestore)
 
 set(FIREBASE_SOURCE_DIR "${CMAKE_CURRENT_LIST_DIR}/..")
-set(FIREBASE_BINARY_DIR "${CMAKE_CURRENT_BINARY_DIR}/..")
+
+# CMAKE_INSTALL_PREFIX should be passed in to this build so that it can find
+# outputs of the superbuild. This is handled automatically if run via the
+# superbuild (i.e. by invoking cmake on the directory above this).
+#
+# If you want to use this project directly in e.g. CLion, make sure you
+# configure this.
+set(FIREBASE_INSTALL_DIR "${CMAKE_INSTALL_PREFIX}")
 
 list(INSERT CMAKE_MODULE_PATH 0 ${FIREBASE_SOURCE_DIR}/cmake)
 include(utils)
@@ -36,5 +43,18 @@ set(CMAKE_CXX_EXTENSIONS OFF)
 # Fully qualified imports, project wide
 include_directories("${FIREBASE_SOURCE_DIR}")
 
+if(APPLE)
+  # CMake has no special support for Objective-C as a distinct language but enabling modules and
+  # other clang extensions would apply even to regular C++ sources which is nonportable. Keep these
+  # flags separate to avoid misuse.
+  set(
+    OBJC_FLAGS
+    -fobjc-arc
+    -fmodules
+    -fno-autolink
+    -F${FIREBASE_INSTALL_DIR}/Frameworks
+  )
+endif(APPLE)
+
 enable_testing()
 add_subdirectory(core)
diff --git a/Firestore/core/src/firebase/firestore/util/CMakeLists.txt b/Firestore/core/src/firebase/firestore/util/CMakeLists.txt
index ebb2301..d70397d 100644
--- a/Firestore/core/src/firebase/firestore/util/CMakeLists.txt
+++ b/Firestore/core/src/firebase/firestore/util/CMakeLists.txt
@@ -15,6 +15,47 @@
 add_library(
   firebase_firestore_util
   autoid.cc
-  log_stdio.cc
   secure_random_arc4random.cc
 )
+
+# log_stdio can be built and tested everywhere
+add_library(
+  firebase_firestore_util_log_stdio
+  log_stdio.cc
+)
+
+# log_apple can only built and tested on apple plaforms
+if(APPLE)
+  add_library(
+    firebase_firestore_util_log_apple
+    log_apple.mm
+  )
+  target_compile_options(
+    firebase_firestore_util_log_apple
+    PRIVATE
+    ${OBJC_FLAGS}
+  )
+  target_link_libraries(
+    firebase_firestore_util_log_apple
+    PUBLIC
+    FirebaseCore
+  )
+endif(APPLE)
+
+# Export a dependency on the correct logging library for this platform. All
+# buildable libraries are built and tested but only the best fit is exported.
+if(APPLE)
+  target_link_libraries(
+    firebase_firestore_util
+    PUBLIC
+    firebase_firestore_util_log_apple
+  )
+
+else(NOT APPLE)
+  target_link_libraries(
+    firebase_firestore_util
+    PUBLIC
+    firebase_firestore_util_log_stdio
+  )
+
+endif(APPLE)
diff --git a/Firestore/core/src/firebase/firestore/util/log_apple.mm b/Firestore/core/src/firebase/firestore/util/log_apple.mm
index 7cd834b..afa087f 100644
--- a/Firestore/core/src/firebase/firestore/util/log_apple.mm
+++ b/Firestore/core/src/firebase/firestore/util/log_apple.mm
@@ -16,47 +16,50 @@
 
 #include "Firestore/core/src/firebase/firestore/util/log.h"
 
-#include <string>
-
 #import <FirebaseCore/FIRLogger.h>
+#import <Foundation/Foundation.h>
 
-#import "Firestore/Source/API/FIRFirestore+Internal.h"
+#include <string>
 
 namespace firebase {
 namespace firestore {
 namespace util {
 
-static NSString* FormatString(const char* format) {
+namespace {
+
+// Translates a C format string to the equivalent NSString without making a
+// copy.
+NSString* FormatString(const char* format) {
   return [[NSString alloc] initWithBytesNoCopy:(void*)format
                                         length:strlen(format)
                                       encoding:NSUTF8StringEncoding
                                   freeWhenDone:NO];
 }
 
-void LogSetLevel(LogLevel level) {
+// Translates a C++ LogLevel to the equivalent Objective-C FIRLoggerLevel
+FIRLoggerLevel ToFIRLoggerLevel(LogLevel level) {
   switch (level) {
-    case kLogLevelVerbose:
-      FIRSetLoggerLevel(FIRLoggerLevelMax);
-      break;
+    case kLogLevelVerbose:  // fall through
     case kLogLevelDebug:
-      FIRSetLoggerLevel(FIRLoggerLevelDebug);
-      break;
+      return FIRLoggerLevelDebug;
     case kLogLevelInfo:
-      FIRSetLoggerLevel(FIRLoggerLevelInfo);
-      break;
+      return FIRLoggerLevelInfo;
     case kLogLevelWarning:
-      FIRSetLoggerLevel(FIRLoggerLevelWarning);
-      break;
+      return FIRLoggerLevelWarning;
     case kLogLevelError:
-      FIRSetLoggerLevel(FIRLoggerLevelError);
-      break;
+      return FIRLoggerLevelError;
     default:
       // Unsupported log level. FIRSetLoggerLevel will deal with it.
-      FIRSetLoggerLevel((FIRLoggerLevel)-1);
-      break;
+      return static_cast<FIRLoggerLevel>(-1);
   }
 }
 
+}  // namespace
+
+void LogSetLevel(LogLevel level) {
+  FIRSetLoggerLevel(ToFIRLoggerLevel(level));
+}
+
 LogLevel LogGetLevel() {
   // We return the true log level. True log level is what the SDK used to
   // determine whether to log instead of what parameter is used in the last call
@@ -111,29 +114,8 @@ void LogError(const char* format, ...) {
 }
 
 void LogMessageV(LogLevel log_level, const char* format, va_list args) {
-  switch (log_level) {
-    case kLogLevelVerbose:  // fall through
-    case kLogLevelDebug:
-      FIRLogBasic(FIRLoggerLevelDebug, kFIRLoggerFirestore, @"I-FST000001",
-                  FormatString(format), args);
-      break;
-    case kLogLevelInfo:
-      FIRLogBasic(FIRLoggerLevelInfo, kFIRLoggerFirestore, @"I-FST000001",
-                  FormatString(format), args);
-      break;
-    case kLogLevelWarning:
-      FIRLogBasic(FIRLoggerLevelWarning, kFIRLoggerFirestore, @"I-FST000001",
-                  FormatString(format), args);
-      break;
-    case kLogLevelError:
-      FIRLogBasic(FIRLoggerLevelError, kFIRLoggerFirestore, @"I-FST000001",
-                  FormatString(format), args);
-      break;
-    default:
-      FIRLogBasic(FIRLoggerLevelError, kFIRLoggerFirestore, @"I-FST000001",
-                  FormatString(format), args);
-      break;
-  }
+  FIRLogBasic(ToFIRLoggerLevel(log_level), kFIRLoggerFirestore, @"I-FST000001",
+              FormatString(format), args);
 }
 
 void LogMessage(LogLevel log_level, const char* format, ...) {
diff --git a/Firestore/core/test/firebase/firestore/util/CMakeLists.txt b/Firestore/core/test/firebase/firestore/util/CMakeLists.txt
index ae2c3b0..42c4dcc 100644
--- a/Firestore/core/test/firebase/firestore/util/CMakeLists.txt
+++ b/Firestore/core/test/firebase/firestore/util/CMakeLists.txt
@@ -15,10 +15,29 @@
 cc_test(
   firebase_firestore_util_test
   autoid_test.cc
-  log_test.cc
   secure_random_test.cc
 )
 target_link_libraries(
   firebase_firestore_util_test
   firebase_firestore_util
 )
+
+if(APPLE)
+  cc_test(
+    firebase_firestore_util_log_apple_test
+    log_test.cc
+  )
+  target_link_libraries(
+    firebase_firestore_util_log_apple_test
+    firebase_firestore_util_log_apple
+  )
+endif(APPLE)
+
+cc_test(
+  firebase_firestore_util_log_stdio_test
+  log_test.cc
+)
+target_link_libraries(
+  firebase_firestore_util_log_stdio_test
+  firebase_firestore_util_log_stdio
+)
diff --git a/Firestore/core/test/firebase/firestore/util/log_test.cc b/Firestore/core/test/firebase/firestore/util/log_test.cc
index 09b2c08..46cbc4e 100644
--- a/Firestore/core/test/firebase/firestore/util/log_test.cc
+++ b/Firestore/core/test/firebase/firestore/util/log_test.cc
@@ -22,6 +22,16 @@ namespace firebase {
 namespace firestore {
 namespace util {
 
+// When running against the log_apple.mm implementation (backed by FIRLogger)
+// this test can fail if debug_mode gets persisted in the user defaults. Check
+// for defaults getting in your way with
+//
+//   defaults read firebase_firestore_util_log_apple_test
+//
+// You can fix it with:
+//
+//   defaults write firebase_firestore_util_log_apple_test \
+//       /google/firebase/debug_mode NO
 TEST(Log, SetAndGet) {
   LogSetLevel(kLogLevelVerbose);
 
diff --git a/cmake/FindFirebaseCore.cmake b/cmake/FindFirebaseCore.cmake
index 6e68dfb..eec29dd 100644
--- a/cmake/FindFirebaseCore.cmake
+++ b/cmake/FindFirebaseCore.cmake
@@ -15,7 +15,7 @@
 find_library(
   FIREBASECORE_LIBRARY
   FirebaseCore
-  PATHS ${FIREBASE_BINARY_DIR}/Frameworks
+  PATHS ${FIREBASE_INSTALL_DIR}/Frameworks
 )
 
 include(FindPackageHandleStandardArgs)
diff --git a/cmake/FindLevelDB.cmake b/cmake/FindLevelDB.cmake
index ae32a17..386a298 100644
--- 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}/third_party/leveldb/src/leveldb)
+set(binary_dir ${FIREBASE_INSTALL_DIR}/third_party/leveldb/src/leveldb)
 
 find_path(
   LEVELDB_INCLUDE_DIR leveldb/db.h