From f4e49ca569b059827c6b154a0dddc373fec99ae7 Mon Sep 17 00:00:00 2001 From: Marek Gilbert Date: Sat, 6 Jan 2018 10:19:08 -0800 Subject: [PATCH 1/3] Add abseil as a subdirectory of Firestore This saves having to redefine all the libraries that abseil defines as imported libraries. --- CMakeLists.txt | 1 - Firestore/CMakeLists.txt | 1 + .../Firestore.xcodeproj/project.pbxproj | 2 ++ .../third_party/abseil-cpp/CMakeLists.txt | 8 ++++- cmake/external/abseil-cpp.cmake | 35 ------------------- cmake/external/firestore.cmake | 2 +- 6 files changed, 11 insertions(+), 38 deletions(-) delete mode 100644 cmake/external/abseil-cpp.cmake diff --git a/CMakeLists.txt b/CMakeLists.txt index 52a4376fd8f..edcd611e029 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -40,5 +40,4 @@ include(external/FirebaseCore) include(external/googletest) include(external/leveldb) -include(external/abseil-cpp) include(external/firestore) diff --git a/Firestore/CMakeLists.txt b/Firestore/CMakeLists.txt index cffd0151422..9b90815da18 100644 --- a/Firestore/CMakeLists.txt +++ b/Firestore/CMakeLists.txt @@ -57,4 +57,5 @@ if(APPLE) endif(APPLE) enable_testing() +add_subdirectory(third_party/abseil-cpp EXCLUDE_FROM_ALL) add_subdirectory(core) diff --git a/Firestore/Example/Firestore.xcodeproj/project.pbxproj b/Firestore/Example/Firestore.xcodeproj/project.pbxproj index faed0b89db8..c4fa7dd50c6 100644 --- a/Firestore/Example/Firestore.xcodeproj/project.pbxproj +++ b/Firestore/Example/Firestore.xcodeproj/project.pbxproj @@ -1454,6 +1454,7 @@ HEADER_SEARCH_PATHS = ( "$(inherited)", "\"${PODS_ROOT}/../../..\"", + "\"${PODS_ROOT}/../../../Firestore/third_party/abseil-cpp\"", "\"${PODS_ROOT}/leveldb-library/include\"", "\"${PODS_ROOT}/GoogleTest/googletest/include\"", ); @@ -1486,6 +1487,7 @@ HEADER_SEARCH_PATHS = ( "$(inherited)", "\"${PODS_ROOT}/../../..\"", + "\"${PODS_ROOT}/../../../Firestore/third_party/abseil-cpp\"", "\"${PODS_ROOT}/leveldb-library/include\"", "\"${PODS_ROOT}/GoogleTest/googletest/include\"", ); diff --git a/Firestore/third_party/abseil-cpp/CMakeLists.txt b/Firestore/third_party/abseil-cpp/CMakeLists.txt index 1c53a72e34c..b25a0065e86 100644 --- a/Firestore/third_party/abseil-cpp/CMakeLists.txt +++ b/Firestore/third_party/abseil-cpp/CMakeLists.txt @@ -53,7 +53,13 @@ set(CMAKE_CXX_FLAGS "${CMAKE_CXX_WARNING_VLA} ${CMAKE_CXX_FLAGS} ") ## pthread find_package(Threads REQUIRED) -find_package(GTest REQUIRED) +# commented: used only for standalone test +#add_subdirectory(cctz) +#add_subdirectory(googletest) + +## check targets +check_target(GTest::GTest) +check_target(GTest::Main) # -fexceptions set(ABSL_EXCEPTIONS_FLAG "${CMAKE_CXX_EXCEPTIONS}") diff --git a/cmake/external/abseil-cpp.cmake b/cmake/external/abseil-cpp.cmake deleted file mode 100644 index d29b7d5c05b..00000000000 --- a/cmake/external/abseil-cpp.cmake +++ /dev/null @@ -1,35 +0,0 @@ -# Copyright 2017 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. - -include(ExternalProject) - -set(source_dir ${PROJECT_SOURCE_DIR}/Firestore/third_party/abseil-cpp) -set(binary_dir ${PROJECT_BINARY_DIR}/Firestore/third_party/abseil-cpp) - -ExternalProject_Add( - abseil-cpp - DEPENDS googletest - - PREFIX "${binary_dir}" - SOURCE_DIR "${source_dir}" - BINARY_DIR "${binary_dir}" - - INSTALL_DIR "${FIREBASE_INSTALL_DIR}" - INSTALL_COMMAND "" - TEST_BEFORE_INSTALL ON - - CMAKE_ARGS - -DCMAKE_BUILD_TYPE:STRING=${CMAKE_BUILD_TYPE} - -DCMAKE_INSTALL_PREFIX:PATH= -) diff --git a/cmake/external/firestore.cmake b/cmake/external/firestore.cmake index 61f79f31b5b..1abb629c36d 100644 --- a/cmake/external/firestore.cmake +++ b/cmake/external/firestore.cmake @@ -19,7 +19,7 @@ set(binary_dir ${PROJECT_BINARY_DIR}/Firestore) ExternalProject_Add( Firestore - DEPENDS abseil-cpp FirebaseCore googletest leveldb + DEPENDS FirebaseCore googletest leveldb # Lay the binary directory out as if this were a subproject. This makes it # possible to build and test in it directly. From 4d39184258bbcc43325b3abbfaf711a811f5999f Mon Sep 17 00:00:00 2001 From: Marek Gilbert Date: Sat, 6 Jan 2018 10:23:07 -0800 Subject: [PATCH 2/3] Rename firebase_firesture_util_log_* targets Cut the log out of the name to reflect that these will get more components besides just logging. --- .../core/src/firebase/firestore/util/CMakeLists.txt | 12 ++++++------ .../core/test/firebase/firestore/util/CMakeLists.txt | 12 ++++++------ 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/Firestore/core/src/firebase/firestore/util/CMakeLists.txt b/Firestore/core/src/firebase/firestore/util/CMakeLists.txt index d70397d5ca2..aaa2a76ee57 100644 --- a/Firestore/core/src/firebase/firestore/util/CMakeLists.txt +++ b/Firestore/core/src/firebase/firestore/util/CMakeLists.txt @@ -20,23 +20,23 @@ add_library( # log_stdio can be built and tested everywhere add_library( - firebase_firestore_util_log_stdio + firebase_firestore_util_stdio log_stdio.cc ) # log_apple can only built and tested on apple plaforms if(APPLE) add_library( - firebase_firestore_util_log_apple + firebase_firestore_util_apple log_apple.mm ) target_compile_options( - firebase_firestore_util_log_apple + firebase_firestore_util_apple PRIVATE ${OBJC_FLAGS} ) target_link_libraries( - firebase_firestore_util_log_apple + firebase_firestore_util_apple PUBLIC FirebaseCore ) @@ -48,14 +48,14 @@ if(APPLE) target_link_libraries( firebase_firestore_util PUBLIC - firebase_firestore_util_log_apple + firebase_firestore_util_apple ) else(NOT APPLE) target_link_libraries( firebase_firestore_util PUBLIC - firebase_firestore_util_log_stdio + firebase_firestore_util_stdio ) endif(APPLE) diff --git a/Firestore/core/test/firebase/firestore/util/CMakeLists.txt b/Firestore/core/test/firebase/firestore/util/CMakeLists.txt index 42c4dcce9ab..25028644748 100644 --- a/Firestore/core/test/firebase/firestore/util/CMakeLists.txt +++ b/Firestore/core/test/firebase/firestore/util/CMakeLists.txt @@ -24,20 +24,20 @@ target_link_libraries( if(APPLE) cc_test( - firebase_firestore_util_log_apple_test + firebase_firestore_util_apple_test log_test.cc ) target_link_libraries( - firebase_firestore_util_log_apple_test - firebase_firestore_util_log_apple + firebase_firestore_util_apple_test + firebase_firestore_util_apple ) endif(APPLE) cc_test( - firebase_firestore_util_log_stdio_test + firebase_firestore_util_stdio_test log_test.cc ) target_link_libraries( - firebase_firestore_util_log_stdio_test - firebase_firestore_util_log_stdio + firebase_firestore_util_stdio_test + firebase_firestore_util_stdio ) From 8e3379b30616258c4cd00136f6d955bb75024ad0 Mon Sep 17 00:00:00 2001 From: Marek Gilbert Date: Fri, 5 Jan 2018 21:10:24 -0800 Subject: [PATCH 3/3] Port StringPrintf from //base. Prefer this to approaches based on variadic templates. While the variadic template mechanisms are strictly safer, they result in binary bloat we can't afford. This is essentially the same StringPrintf previously open sourced as a part of protobuf, though updated for C++11 which saves a copy and a temporary buffer on the heap. --- .../Firestore.xcodeproj/project.pbxproj | 4 + .../firebase/firestore/util/CMakeLists.txt | 29 ++++- .../firebase/firestore/util/string_printf.cc | 101 ++++++++++++++++++ .../firebase/firestore/util/string_printf.h | 48 +++++++++ .../firebase/firestore/util/CMakeLists.txt | 1 + .../firestore/util/string_printf_test.cc | 78 ++++++++++++++ 6 files changed, 257 insertions(+), 4 deletions(-) create mode 100644 Firestore/core/src/firebase/firestore/util/string_printf.cc create mode 100644 Firestore/core/src/firebase/firestore/util/string_printf.h create mode 100644 Firestore/core/test/firebase/firestore/util/string_printf_test.cc diff --git a/Firestore/Example/Firestore.xcodeproj/project.pbxproj b/Firestore/Example/Firestore.xcodeproj/project.pbxproj index c4fa7dd50c6..a8ad7997ced 100644 --- a/Firestore/Example/Firestore.xcodeproj/project.pbxproj +++ b/Firestore/Example/Firestore.xcodeproj/project.pbxproj @@ -24,6 +24,7 @@ /* Begin PBXBuildFile section */ 3B843E4C1F3A182900548890 /* remote_store_spec_test.json in Resources */ = {isa = PBXBuildFile; fileRef = 3B843E4A1F3930A400548890 /* remote_store_spec_test.json */; }; + 5436F32420008FAD006E51E3 /* string_printf_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 5436F32320008FAD006E51E3 /* string_printf_test.cc */; }; 54740A571FC914BA00713A1A /* secure_random_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 54740A531FC913E500713A1A /* secure_random_test.cc */; }; 54740A581FC914F000713A1A /* autoid_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 54740A521FC913E500713A1A /* autoid_test.cc */; }; 54764FAB1FAA0C320085E60A /* string_util_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 54764FAA1FAA0C320085E60A /* string_util_test.cc */; }; @@ -186,6 +187,7 @@ 3B843E4A1F3930A400548890 /* remote_store_spec_test.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = remote_store_spec_test.json; sourceTree = ""; }; 42491D7DC8C8CD245CC22B93 /* Pods-SwiftBuildTest.debug.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-SwiftBuildTest.debug.xcconfig"; path = "Pods/Target Support Files/Pods-SwiftBuildTest/Pods-SwiftBuildTest.debug.xcconfig"; sourceTree = ""; }; 4EBC5F5ABE1FD097EFE5E224 /* Pods-Firestore_Example.release.xcconfig */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = text.xcconfig; name = "Pods-Firestore_Example.release.xcconfig"; path = "Pods/Target Support Files/Pods-Firestore_Example/Pods-Firestore_Example.release.xcconfig"; sourceTree = ""; }; + 5436F32320008FAD006E51E3 /* string_printf_test.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = string_printf_test.cc; path = ../../core/test/firebase/firestore/util/string_printf_test.cc; sourceTree = ""; }; 54740A521FC913E500713A1A /* autoid_test.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = autoid_test.cc; path = ../../core/test/firebase/firestore/util/autoid_test.cc; sourceTree = ""; }; 54740A531FC913E500713A1A /* secure_random_test.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = secure_random_test.cc; path = ../../core/test/firebase/firestore/util/secure_random_test.cc; sourceTree = ""; }; 54764FAA1FAA0C320085E60A /* string_util_test.cc */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.cpp; name = string_util_test.cc; path = ../../Port/string_util_test.cc; sourceTree = ""; }; @@ -376,6 +378,7 @@ 54740A521FC913E500713A1A /* autoid_test.cc */, 54C2294E1FECABAE007D065B /* log_test.cc */, 54740A531FC913E500713A1A /* secure_random_test.cc */, + 5436F32320008FAD006E51E3 /* string_printf_test.cc */, ); name = util; sourceTree = ""; @@ -1186,6 +1189,7 @@ DE51B1F41F0D491B0013853F /* FSTRemoteEventTests.m in Sources */, 54E928241F33953300C1953E /* FSTEventAccumulator.m in Sources */, DE51B1D11F0D48CD0013853F /* FSTTargetIDGeneratorTests.m in Sources */, + 5436F32420008FAD006E51E3 /* string_printf_test.cc in Sources */, DE51B1EF1F0D49140013853F /* FSTDocumentTests.m in Sources */, DE51B1DC1F0D490D0013853F /* FSTLocalSerializerTests.m in Sources */, DE51B1E71F0D490D0013853F /* FSTRemoteDocumentChangeBufferTests.m in Sources */, diff --git a/Firestore/core/src/firebase/firestore/util/CMakeLists.txt b/Firestore/core/src/firebase/firestore/util/CMakeLists.txt index aaa2a76ee57..a2da3b489d7 100644 --- a/Firestore/core/src/firebase/firestore/util/CMakeLists.txt +++ b/Firestore/core/src/firebase/firestore/util/CMakeLists.txt @@ -12,19 +12,33 @@ # See the License for the specific language governing permissions and # limitations under the License. +# firebase_firestore_util is the interface of this module. The rest of the +# libraries in here are an implementation detail of making this a +# mutli-platform build. + add_library( - firebase_firestore_util - autoid.cc + firebase_firestore_util_base secure_random_arc4random.cc + string_printf.cc +) +target_link_libraries( + firebase_firestore_util_base + PRIVATE + absl_base ) -# log_stdio can be built and tested everywhere +# stdio-dependent bits can be built and tested everywhere add_library( firebase_firestore_util_stdio log_stdio.cc ) +target_link_libraries( + firebase_firestore_util_stdio + PUBLIC + firebase_firestore_util_base +) -# log_apple can only built and tested on apple plaforms +# apple-dependent bits can only built and tested on apple plaforms if(APPLE) add_library( firebase_firestore_util_apple @@ -42,6 +56,11 @@ if(APPLE) ) endif(APPLE) +add_library( + firebase_firestore_util + autoid.cc +) + # 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) @@ -49,6 +68,7 @@ if(APPLE) firebase_firestore_util PUBLIC firebase_firestore_util_apple + firebase_firestore_util_base ) else(NOT APPLE) @@ -56,6 +76,7 @@ else(NOT APPLE) firebase_firestore_util PUBLIC firebase_firestore_util_stdio + firebase_firestore_util_base ) endif(APPLE) diff --git a/Firestore/core/src/firebase/firestore/util/string_printf.cc b/Firestore/core/src/firebase/firestore/util/string_printf.cc new file mode 100644 index 00000000000..60cc5644f14 --- /dev/null +++ b/Firestore/core/src/firebase/firestore/util/string_printf.cc @@ -0,0 +1,101 @@ +/* + * 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. + */ + +#include "Firestore/core/src/firebase/firestore/util/string_printf.h" + +#include + +namespace firebase { +namespace firestore { +namespace util { + +void StringAppendV(std::string* dst, const char* format, va_list ap) { + // First try with a small fixed size buffer + static const int kSpaceLength = 1024; + char space[kSpaceLength]; + + // It's possible for methods that use a va_list to invalidate + // the data in it upon use. The fix is to make a copy + // of the structure before using it and use that copy instead. + va_list backup_ap; + va_copy(backup_ap, ap); + int result = vsnprintf(space, kSpaceLength, format, backup_ap); + va_end(backup_ap); + + if (result < kSpaceLength) { + if (result >= 0) { + // Normal case -- everything fit. + dst->append(space, result); + return; + } + +#ifdef _MSC_VER + // Error or MSVC running out of space. MSVC 8.0 and higher + // can be asked about space needed with the special idiom below: + va_copy(backup_ap, ap); + result = vsnprintf(nullptr, 0, format, backup_ap); + va_end(backup_ap); +#endif + + if (result < 0) { + // Just an error. + return; + } + } + + // Increase the buffer size to the size requested by vsnprintf, + // plus one for the closing \0. + size_t initial_size = dst->size(); + size_t target_size = initial_size + result; + + dst->resize(target_size + 1); + char* buf = &(*dst)[initial_size]; + int buf_remain = result + 1; + + // Restore the va_list before we use it again + va_copy(backup_ap, ap); + result = vsnprintf(buf, buf_remain, format, backup_ap); + va_end(backup_ap); + + if (result >= 0 && result < buf_remain) { + // It fit and vsnprintf copied in directly. Resize down one to + // remove the trailing \0. + dst->resize(target_size); + } else { + // Didn't fit. Leave the original string unchanged. + dst->resize(initial_size); + } +} + +std::string StringPrintf(const char* format, ...) { + va_list ap; + va_start(ap, format); + std::string result; + StringAppendV(&result, format, ap); + va_end(ap); + return result; +} + +void StringAppendF(std::string* dst, const char* format, ...) { + va_list ap; + va_start(ap, format); + StringAppendV(dst, format, ap); + va_end(ap); +} + +} // namespace util +} // namespace firestore +} // namespace firebase diff --git a/Firestore/core/src/firebase/firestore/util/string_printf.h b/Firestore/core/src/firebase/firestore/util/string_printf.h new file mode 100644 index 00000000000..9e2b9c0054d --- /dev/null +++ b/Firestore/core/src/firebase/firestore/util/string_printf.h @@ -0,0 +1,48 @@ +/* + * 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. + */ + +#ifndef FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_UTIL_STRING_PRINTF_H_ +#define FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_UTIL_STRING_PRINTF_H_ + +#include + +#include + +#include + +namespace firebase { +namespace firestore { +namespace util { + +/** Return a C++ string. */ +std::string StringPrintf(const char* format, ...) + ABSL_PRINTF_ATTRIBUTE(1, 2); + +/** Append result to a supplied string. */ +void StringAppendF(std::string* dst, const char* format, ...) + ABSL_PRINTF_ATTRIBUTE(2, 3); + +/** + * Lower-level routine that takes a va_list and appends to a specified + * string. All other routines are just convenience wrappers around it. + */ +void StringAppendV(std::string* dst, const char* format, va_list ap); + +} // namespace util +} // namespace firestore +} // namespace firebase + +#endif // FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_UTIL_STRING_FORMAT_H_ diff --git a/Firestore/core/test/firebase/firestore/util/CMakeLists.txt b/Firestore/core/test/firebase/firestore/util/CMakeLists.txt index 25028644748..5e107156a4f 100644 --- a/Firestore/core/test/firebase/firestore/util/CMakeLists.txt +++ b/Firestore/core/test/firebase/firestore/util/CMakeLists.txt @@ -16,6 +16,7 @@ cc_test( firebase_firestore_util_test autoid_test.cc secure_random_test.cc + string_printf_test.cc ) target_link_libraries( firebase_firestore_util_test diff --git a/Firestore/core/test/firebase/firestore/util/string_printf_test.cc b/Firestore/core/test/firebase/firestore/util/string_printf_test.cc new file mode 100644 index 00000000000..76f7cde5c33 --- /dev/null +++ b/Firestore/core/test/firebase/firestore/util/string_printf_test.cc @@ -0,0 +1,78 @@ +/* + * Copyright 2017 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. + */ + +#include "Firestore/core/src/firebase/firestore/util/string_printf.h" + +#include + +namespace firebase { +namespace firestore { +namespace util { + +TEST(StringPrintf, Empty) { + EXPECT_EQ("", StringPrintf("")); + EXPECT_EQ("", StringPrintf("%s", std::string().c_str())); + EXPECT_EQ("", StringPrintf("%s", "")); +} + +TEST(StringAppendFTest, Empty) { + std::string value("Hello"); + const char* empty = ""; + StringAppendF(&value, "%s", empty); + EXPECT_EQ("Hello", value); +} + +TEST(StringAppendFTest, EmptyString) { + std::string value("Hello"); + StringAppendF(&value, "%s", ""); + EXPECT_EQ("Hello", value); +} + +TEST(StringAppendFTest, String) { + std::string value("Hello"); + StringAppendF(&value, " %s", "World"); + EXPECT_EQ("Hello World", value); +} + +TEST(StringAppendFTest, Int) { + std::string value("Hello"); + StringAppendF(&value, " %d", 123); + EXPECT_EQ("Hello 123", value); +} + +TEST(StringPrintf, DontOverwriteErrno) { + // Check that errno isn't overwritten unless we're printing + // something significantly larger than what people are normally + // printing in their badly written PLOG() statements. + errno = ECHILD; + std::string value = StringPrintf("Hello, %s!", "World"); + EXPECT_EQ(ECHILD, errno); +} + +TEST(StringPrintf, LargeBuf) { + // Check that the large buffer is handled correctly. + int n = 2048; + char* buf = new char[n + 1]; + memset(buf, ' ', n); + buf[n] = 0; + std::string value = StringPrintf("%s", buf); + EXPECT_EQ(buf, value); + delete[] buf; +} + +} // namespace util +} // namespace firestore +} // namespace firebase