Let Travis run for CMake test and lint.sh

.travis.yml

@@ -13,6 +13,8 @@ before_install:
   - bundle exec pod install --project-directory=Example --repo-update
   - bundle exec pod install --project-directory=Firestore/Example --no-repo-update
   - brew install clang-format
+  - brew install cmake
+  - brew install go # Somehow the build for Abseil requires this.
   - echo "$TRAVIS_COMMIT_RANGE"
   - echo "$TRAVIS_PULL_REQUEST"
   - |
@@ -35,6 +37,10 @@ script:
     if [ $SKIP_FIREBASE != 1 ]; then
       ./test.sh
     fi
+  - |
+    if [ $SKIP_FIRESTORE != 1 ]; then
+      ./scripts/lint.sh # Google C++ style compliance
+    fi
   - |
     if [ $SKIP_FIRESTORE != 1 ]; then
       ./Firestore/test.sh

Firestore/core/src/firebase/firestore/model/CMakeLists.txt

@@ -22,6 +22,8 @@ cc_library(
     field_path.h
     field_value.cc
     field_value.h
+    resource_path.cc
+    resource_path.h
     timestamp.cc
     timestamp.h
   DEPENDS

Firestore/core/src/firebase/firestore/model/base_path.h

@@ -166,7 +166,7 @@ class BasePath {
   }
   BasePath(std::initializer_list<std::string> list) : segments_{list} {
   }
-  BasePath(SegmentsT&& segments) : segments_{std::move(segments)} {
+  explicit BasePath(SegmentsT&& segments) : segments_{std::move(segments)} {
   }
 
  private:

Firestore/core/src/firebase/firestore/model/field_path.cc

@@ -51,7 +51,7 @@ bool IsValidIdentifier(const std::string& segment) {
       (first < 'A' || first > 'Z')) {
     return false;
   }
-  for (int i = 1; i != segment.size(); ++i) {
+  for (size_t i = 1; i != segment.size(); ++i) {
     const unsigned char c = segment[i];
     if (c != '_' && (c < 'a' || c > 'z') && (c < 'A' || c > 'Z') &&
         (c < '0' || c > '9')) {
@@ -93,7 +93,7 @@ FieldPath FieldPath::FromServerFormat(const absl::string_view path) {
 
   // Inside backticks, dots are treated literally.
   bool inside_backticks = false;
-  int i = 0;
+  size_t i = 0;
   while (i < path.size()) {
     const char c = path[i];
     // std::string (and string_view) may contain embedded nulls. For full

Firestore/core/src/firebase/firestore/model/field_path.h

@@ -79,7 +79,7 @@ class FieldPath : public impl::BasePath<FieldPath> {
   }
 
  private:
-  FieldPath(SegmentsT&& segments) : BasePath{std::move(segments)} {
+  explicit FieldPath(SegmentsT&& segments) : BasePath{std::move(segments)} {
   }
 
   // So that methods of base can construct FieldPath using the private

Firestore/core/src/firebase/firestore/model/field_value.h

@@ -69,7 +69,7 @@ class FieldValue {
     // position instead, see the doc comment above.
   };
 
-  FieldValue() : tag_(Type::Null) {
+  FieldValue() {
   }
 
   // Do not inline these ctor/dtor below, which contain call to non-trivial
@@ -123,7 +123,7 @@ class FieldValue {
    */
   void SwitchTo(const Type type);
 
-  Type tag_;
+  Type tag_ = Type::Null;
   union {
     // There is no null type as tag_ alone is enough for Null FieldValue.
     bool boolean_value_;

Firestore/core/src/firebase/firestore/model/resource_path.cc

@@ -18,6 +18,7 @@
 
 #include <algorithm>
 #include <utility>
+#include <vector>
 
 #include "Firestore/core/src/firebase/firestore/util/firebase_assert.h"
 #include "absl/strings/str_join.h"

Firestore/core/src/firebase/firestore/model/resource_path.h

@@ -19,6 +19,7 @@
 
 #include <initializer_list>
 #include <string>
+#include <utility>
 
 #include "Firestore/core/src/firebase/firestore/model/base_path.h"
 #include "absl/strings/string_view.h"
@@ -69,7 +70,7 @@ class ResourcePath : public impl::BasePath<ResourcePath> {
   }
 
  private:
-  ResourcePath(SegmentsT&& segments) : BasePath{std::move(segments)} {
+  explicit ResourcePath(SegmentsT&& segments) : BasePath{std::move(segments)} {
   }
 
   // So that methods of base can construct ResourcePath using the private
@@ -81,4 +82,4 @@ class ResourcePath : public impl::BasePath<ResourcePath> {
 }  // namespace firestore
 }  // namespace firebase
 
-#endif
+#endif  // FIRESTORE_CORE_SRC_FIREBASE_FIRESTORE_MODEL_RESOURCE_PATH_H_

Firestore/core/test/firebase/firestore/model/field_path_test.cc

@@ -29,18 +29,18 @@ namespace model {
 TEST(FieldPath, Constructors) {
   const FieldPath empty_path;
   EXPECT_TRUE(empty_path.empty());
-  EXPECT_EQ(0, empty_path.size());
+  EXPECT_EQ(0u, empty_path.size());
   EXPECT_TRUE(empty_path.begin() == empty_path.end());
 
   const FieldPath path_from_list = {"rooms", "Eros", "messages"};
   EXPECT_FALSE(path_from_list.empty());
-  EXPECT_EQ(3, path_from_list.size());
+  EXPECT_EQ(3u, path_from_list.size());
   EXPECT_TRUE(path_from_list.begin() + 3 == path_from_list.end());
 
   std::vector<std::string> segments{"rooms", "Eros", "messages"};
   const FieldPath path_from_segments{segments.begin(), segments.end()};
   EXPECT_FALSE(path_from_segments.empty());
-  EXPECT_EQ(3, path_from_segments.size());
+  EXPECT_EQ(3u, path_from_segments.size());
   EXPECT_TRUE(path_from_segments.begin() + 3 == path_from_segments.end());
 
   FieldPath copied = path_from_list;
@@ -168,13 +168,13 @@ TEST(FieldPath, IsPrefixOf) {
 
 TEST(FieldPath, AccessFailures) {
   const FieldPath path;
-  ASSERT_DEATH_IF_SUPPORTED(path.first_segment(), "");
-  ASSERT_DEATH_IF_SUPPORTED(path.last_segment(), "");
-  ASSERT_DEATH_IF_SUPPORTED(path[0], "");
-  ASSERT_DEATH_IF_SUPPORTED(path[1], "");
-  ASSERT_DEATH_IF_SUPPORTED(path.PopFirst(), "");
-  ASSERT_DEATH_IF_SUPPORTED(path.PopFirst(2), "");
-  ASSERT_DEATH_IF_SUPPORTED(path.PopLast(), "");
+  ASSERT_ANY_THROW(path.first_segment());
+  ASSERT_ANY_THROW(path.last_segment());
+  ASSERT_ANY_THROW(path[0]);
+  ASSERT_ANY_THROW(path[1]);
+  ASSERT_ANY_THROW(path.PopFirst());
+  ASSERT_ANY_THROW(path.PopFirst(2));
+  ASSERT_ANY_THROW(path.PopLast());
 }
 
 TEST(FieldPath, Parsing) {
@@ -201,7 +201,7 @@ TEST(FieldPath, Parsing) {
 
   const auto path_with_dot = FieldPath::FromServerFormat(R"(foo\.bar)");
   EXPECT_EQ(path_with_dot.CanonicalString(), "`foo.bar`");
-  EXPECT_EQ(path_with_dot.size(), 1);
+  EXPECT_EQ(path_with_dot.size(), 1u);
 }
 
 // This is a special case in C++: std::string may contain embedded nulls. To
@@ -213,22 +213,22 @@ TEST(FieldPath, ParseEmbeddedNull) {
   str += ".bar";
 
   const auto path = FieldPath::FromServerFormat(str);
-  EXPECT_EQ(path.size(), 1);
+  EXPECT_EQ(path.size(), 1u);
   EXPECT_EQ(path.CanonicalString(), "foo");
 }
 
 TEST(FieldPath, ParseFailures) {
-  ASSERT_DEATH_IF_SUPPORTED(FieldPath::FromServerFormat(""), "");
-  ASSERT_DEATH_IF_SUPPORTED(FieldPath::FromServerFormat("."), "");
-  ASSERT_DEATH_IF_SUPPORTED(FieldPath::FromServerFormat(".."), "");
-  ASSERT_DEATH_IF_SUPPORTED(FieldPath::FromServerFormat("foo."), "");
-  ASSERT_DEATH_IF_SUPPORTED(FieldPath::FromServerFormat(".bar"), "");
-  ASSERT_DEATH_IF_SUPPORTED(FieldPath::FromServerFormat("foo..bar"), "");
-  ASSERT_DEATH_IF_SUPPORTED(FieldPath::FromServerFormat(R"(foo\)"), "");
-  ASSERT_DEATH_IF_SUPPORTED(FieldPath::FromServerFormat(R"(foo.\)"), "");
-  ASSERT_DEATH_IF_SUPPORTED(FieldPath::FromServerFormat("foo`"), "");
-  ASSERT_DEATH_IF_SUPPORTED(FieldPath::FromServerFormat("foo```"), "");
-  ASSERT_DEATH_IF_SUPPORTED(FieldPath::FromServerFormat("`foo"), "");
+  ASSERT_ANY_THROW(FieldPath::FromServerFormat(""));
+  ASSERT_ANY_THROW(FieldPath::FromServerFormat("."));
+  ASSERT_ANY_THROW(FieldPath::FromServerFormat(".."));
+  ASSERT_ANY_THROW(FieldPath::FromServerFormat("foo."));
+  ASSERT_ANY_THROW(FieldPath::FromServerFormat(".bar"));
+  ASSERT_ANY_THROW(FieldPath::FromServerFormat("foo..bar"));
+  ASSERT_ANY_THROW(FieldPath::FromServerFormat(R"(foo\)"));
+  ASSERT_ANY_THROW(FieldPath::FromServerFormat(R"(foo.\)"));
+  ASSERT_ANY_THROW(FieldPath::FromServerFormat("foo`"));
+  ASSERT_ANY_THROW(FieldPath::FromServerFormat("foo```"));
+  ASSERT_ANY_THROW(FieldPath::FromServerFormat("`foo"));
 }
 
 TEST(FieldPath, CanonicalStringOfSubstring) {

Firestore/core/test/firebase/firestore/model/resource_path_test.cc

@@ -29,18 +29,18 @@ namespace model {
 TEST(ResourcePath, Constructor) {
   const ResourcePath empty_path;
   EXPECT_TRUE(empty_path.empty());
-  EXPECT_EQ(0, empty_path.size());
+  EXPECT_EQ(0u, empty_path.size());
   EXPECT_TRUE(empty_path.begin() == empty_path.end());
 
   const ResourcePath path_from_list{{"rooms", "Eros", "messages"}};
   EXPECT_FALSE(path_from_list.empty());
-  EXPECT_EQ(3, path_from_list.size());
+  EXPECT_EQ(3u, path_from_list.size());
   EXPECT_TRUE(path_from_list.begin() + 3 == path_from_list.end());
 
   std::vector<std::string> segments{"rooms", "Eros", "messages"};
   const ResourcePath path_from_segments{segments.begin(), segments.end()};
   EXPECT_FALSE(path_from_segments.empty());
-  EXPECT_EQ(3, path_from_segments.size());
+  EXPECT_EQ(3u, path_from_segments.size());
   EXPECT_TRUE(path_from_segments.begin() + 3 == path_from_segments.end());
 
   ResourcePath copied = path_from_list;
@@ -96,8 +96,8 @@ TEST(ResourcePath, Parsing) {
 }
 
 TEST(ResourcePath, ParseFailures) {
-  ASSERT_DEATH_IF_SUPPORTED(ResourcePath::Parse("//"), "");
-  ASSERT_DEATH_IF_SUPPORTED(ResourcePath::Parse("foo//bar"), "");
+  ASSERT_ANY_THROW(ResourcePath::Parse("//"));
+  ASSERT_ANY_THROW(ResourcePath::Parse("foo//bar"));
 }
 
 }  // namespace model

Firestore/test.sh

@@ -38,6 +38,23 @@ test_iOS() {
     | xcpretty
 }
 
+test_CMake() {
+  echo "cpu core: $(sysctl -n hw.ncpu)"
+  echo "set cmake build" && \
+    mkdir build && \
+    cd build && \
+    cmake .. || \
+    exit 1
+
+  echo "initial cmake build" && \
+    make -j $(sysctl -n hw.ncpu) all || \
+    exit 2
+
+  echo "test Firestore cmake build" && \
+    cd Firestore && \
+    make test
+}
+
 test_iOS; RESULT=$?
 if [[ $RESULT == 65 ]]; then
   echo "xcodebuild exited with 65, retrying"
@@ -46,4 +63,8 @@ if [[ $RESULT == 65 ]]; then
   test_iOS; RESULT=$?
 fi
 
-exit $RESULT
+if [ $RESULT != 0 ]; then exit $RESULT; fi
+
+test_CMake; RESULT=$?
+
+if [ $RESULT != 0 ]; then exit $RESULT; fi

cmake/FindNanopb.cmake

@@ -10,7 +10,7 @@ find_path(
 find_library(
   NANOPB_LIBRARY
   NAMES protobuf-nanopb protobuf-nanopbd
-  HINTS ${BINARY_DIR}/src/nanopb-build
+  HINTS ${BINARY_DIR}/src/nanopb
 )
 
 find_package_handle_standard_args(

cmake/external/nanopb.cmake

@@ -55,9 +55,14 @@ ExternalProject_Add(
   # nanopb relies on $PATH for the location of protoc. cmake makes it difficult
   # to adjust the path, so we'll just patch the build files with the exact
   # location of protoc.
+  #
+  # NB: cmake sometimes runs the patch command multiple times in the same src
+  # dir, so we need to make sure this is idempotent. (eg 'make && make clean &&
+  # make')
   PATCH_COMMAND
-    perl -i -pe s,protoc,${NANOPB_PROTOC_BIN},g
-        ./CMakeLists.txt ./generator/proto/Makefile
+    grep ${NANOPB_PROTOC_BIN} ./generator/proto/Makefile
+      || perl -i -pe s,protoc,${NANOPB_PROTOC_BIN},g
+           ./CMakeLists.txt ./generator/proto/Makefile
 
   UPDATE_COMMAND ""
   INSTALL_COMMAND ""