From 412e759a19117974cca18e86ea44742ae0dec659 Mon Sep 17 00:00:00 2001 From: Greg Soltis Date: Mon, 1 Jan 2018 17:43:54 -0800 Subject: Expose library version, move it out of options (#588) * slight cleanup * Use -D defines for versions * Undo FIROptionsTest change * Drop failed macro attempt * Add correct version to podspec * Add newline * Shuffle files around * Bring back log change * Fix change * Fix space --- .gitignore | 1 + 1 file changed, 1 insertion(+) (limited to '.gitignore') diff --git a/.gitignore b/.gitignore index 93e3974..61fc41b 100644 --- a/.gitignore +++ b/.gitignore @@ -57,3 +57,4 @@ Podfile.lock # CMake .downloads +.idea/ -- cgit v1.2.3 From 5a6155f6a38a2d5515667406a5926c39753627e5 Mon Sep 17 00:00:00 2001 From: Gil Date: Wed, 3 Jan 2018 07:00:54 -0800 Subject: 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. --- .gitignore | 7 ++- Firestore/CMakeLists.txt | 22 +++++++- .../src/firebase/firestore/util/CMakeLists.txt | 43 ++++++++++++++- .../core/src/firebase/firestore/util/log_apple.mm | 64 ++++++++-------------- .../test/firebase/firestore/util/CMakeLists.txt | 21 ++++++- .../core/test/firebase/firestore/util/log_test.cc | 10 ++++ cmake/FindFirebaseCore.cmake | 2 +- cmake/FindLevelDB.cmake | 2 +- 8 files changed, 123 insertions(+), 48 deletions(-) (limited to '.gitignore') 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 - #import +#import -#import "Firestore/Source/API/FIRFirestore+Internal.h" +#include 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(-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 -- cgit v1.2.3