From ceb99d0d0a191045584d51dc55e810205bdd5edc Mon Sep 17 00:00:00 2001 From: Vimanyu Date: Wed, 14 Jul 2021 16:35:12 -0700 Subject: [PATCH 1/9] changing compiler option for exceptions to private --- app/CMakeLists.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/CMakeLists.txt b/app/CMakeLists.txt index 0a72876367..b73fb0bd12 100644 --- a/app/CMakeLists.txt +++ b/app/CMakeLists.txt @@ -328,9 +328,9 @@ set_property(TARGET firebase_app PROPERTY FOLDER "Firebase Cpp") # Disable exceptions in std if (MSVC) - target_compile_options(firebase_app PUBLIC /EHs-c-) + target_compile_options(firebase_app PRIVATE /EHs-c-) else() - target_compile_options(firebase_app PUBLIC -fno-exceptions) + target_compile_options(firebase_app PRIVATE -fno-exceptions) endif() target_include_directories(firebase_app From beca0cbad16ad0c1070c6d1c98bb6a1bc54afc30 Mon Sep 17 00:00:00 2001 From: Vimanyu Date: Tue, 20 Jul 2021 15:49:54 -0700 Subject: [PATCH 2/9] fix unit tests now that exceptions are enabled. --- app/tests/thread_test.cc | 35 +++++++++++++++++++++++------------ 1 file changed, 23 insertions(+), 12 deletions(-) diff --git a/app/tests/thread_test.cc b/app/tests/thread_test.cc index bf6237bb40..049ea530f3 100644 --- a/app/tests/thread_test.cc +++ b/app/tests/thread_test.cc @@ -20,6 +20,17 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" +#define EXPECT_SYSTEM_ERROR(statement, error_code) \ + EXPECT_THROW({\ + try { \ + GTEST_SUPPRESS_UNREACHABLE_CODE_WARNING_BELOW_(statement); \ + } \ + catch(const std::system_error &exception) {\ + EXPECT_EQ(error_code, exception.code());\ + throw;\ + }\ + }, std::system_error) + namespace { using ::testing::Eq; @@ -107,66 +118,66 @@ TEST(ThreadDeathTest, MovingIntoRunningThreadShouldAbort) { } TEST(ThreadDeathTest, JoinEmptyThreadShouldAbort) { - ASSERT_DEATH( + EXPECT_SYSTEM_ERROR( { firebase::Thread thread; thread.Join(); }, - ""); + std::errc::invalid_argument); } TEST(ThreadDeathTest, JoinThreadMultipleTimesShouldAbort) { - ASSERT_DEATH( + EXPECT_SYSTEM_ERROR( { firebase::Thread thread([] {}); thread.Join(); thread.Join(); }, - ""); + std::errc::invalid_argument); } TEST(ThreadDeathTest, JoinDetachedThreadShouldAbort) { - ASSERT_DEATH( + EXPECT_SYSTEM_ERROR( { firebase::Thread thread([] {}); thread.Detach(); thread.Join(); }, - ""); + std::errc::invalid_argument); } TEST(ThreadDeathTest, DetachJoinedThreadShouldAbort) { - ASSERT_DEATH( + EXPECT_SYSTEM_ERROR( { firebase::Thread thread([] {}); thread.Join(); thread.Detach(); }, - ""); + std::errc::invalid_argument); } TEST(ThreadDeathTest, DetachEmptyThreadShouldAbort) { - ASSERT_DEATH( + EXPECT_SYSTEM_ERROR( { firebase::Thread thread; thread.Detach(); }, - ""); + std::errc::invalid_argument); } TEST(ThreadDeathTest, DetachThreadMultipleTimesShouldAbort) { - ASSERT_DEATH( + EXPECT_SYSTEM_ERROR( { firebase::Thread thread([] {}); thread.Detach(); thread.Detach(); }, - ""); + std::errc::invalid_argument); } TEST(ThreadDeathTest, WhenJoinableThreadIsDestructedShouldAbort) { From c3db71c934b51a90e0c2c3782615454f68b393c7 Mon Sep 17 00:00:00 2001 From: Vimanyu Date: Wed, 21 Jul 2021 15:37:47 -0700 Subject: [PATCH 3/9] using a better name for macro --- app/tests/thread_test.cc | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/app/tests/thread_test.cc b/app/tests/thread_test.cc index 049ea530f3..5beb34d656 100644 --- a/app/tests/thread_test.cc +++ b/app/tests/thread_test.cc @@ -20,7 +20,7 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -#define EXPECT_SYSTEM_ERROR(statement, error_code) \ +#define EXPECT_THROW_ERROR_CODE(statement, error_code) \ EXPECT_THROW({\ try { \ GTEST_SUPPRESS_UNREACHABLE_CODE_WARNING_BELOW_(statement); \ @@ -118,7 +118,7 @@ TEST(ThreadDeathTest, MovingIntoRunningThreadShouldAbort) { } TEST(ThreadDeathTest, JoinEmptyThreadShouldAbort) { - EXPECT_SYSTEM_ERROR( + EXPECT_THROW_ERROR_CODE( { firebase::Thread thread; thread.Join(); @@ -127,7 +127,7 @@ TEST(ThreadDeathTest, JoinEmptyThreadShouldAbort) { } TEST(ThreadDeathTest, JoinThreadMultipleTimesShouldAbort) { - EXPECT_SYSTEM_ERROR( + EXPECT_THROW_ERROR_CODE( { firebase::Thread thread([] {}); @@ -138,7 +138,7 @@ TEST(ThreadDeathTest, JoinThreadMultipleTimesShouldAbort) { } TEST(ThreadDeathTest, JoinDetachedThreadShouldAbort) { - EXPECT_SYSTEM_ERROR( + EXPECT_THROW_ERROR_CODE( { firebase::Thread thread([] {}); @@ -149,7 +149,7 @@ TEST(ThreadDeathTest, JoinDetachedThreadShouldAbort) { } TEST(ThreadDeathTest, DetachJoinedThreadShouldAbort) { - EXPECT_SYSTEM_ERROR( + EXPECT_THROW_ERROR_CODE( { firebase::Thread thread([] {}); @@ -160,7 +160,7 @@ TEST(ThreadDeathTest, DetachJoinedThreadShouldAbort) { } TEST(ThreadDeathTest, DetachEmptyThreadShouldAbort) { - EXPECT_SYSTEM_ERROR( + EXPECT_THROW_ERROR_CODE( { firebase::Thread thread; @@ -170,7 +170,7 @@ TEST(ThreadDeathTest, DetachEmptyThreadShouldAbort) { } TEST(ThreadDeathTest, DetachThreadMultipleTimesShouldAbort) { - EXPECT_SYSTEM_ERROR( + EXPECT_THROW_ERROR_CODE( { firebase::Thread thread([] {}); From c926d5b383bcf0ae20dba4682f0821e5076bef5e Mon Sep 17 00:00:00 2001 From: Vimanyu Date: Thu, 22 Jul 2021 09:49:52 -0700 Subject: [PATCH 4/9] using old cmake on windows builds --- .github/workflows/integration_tests.yml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/.github/workflows/integration_tests.yml b/.github/workflows/integration_tests.yml index 8a2a17cd6b..c94beca9ca 100644 --- a/.github/workflows/integration_tests.yml +++ b/.github/workflows/integration_tests.yml @@ -237,6 +237,13 @@ jobs: - name: Add msbuild to PATH (Windows) if: startsWith(matrix.os, 'windows') uses: microsoft/setup-msbuild@v1.0.2 + # TODO: remove this after we fix issues with our SDK + # build and cmake 3.21 on Windows. + - name: Setup cmake + if: startsWith(matrix.os, 'windows') + uses: jwlawson/actions-setup-cmake@v1.9 + with: + cmake-version: '3.20.5' - name: Cache vcpkg C++ dependencies id: cache_vcpkg uses: actions/cache@v2 @@ -395,6 +402,13 @@ jobs: - name: Add msbuild to PATH (Windows) if: startsWith(matrix.os, 'windows') uses: microsoft/setup-msbuild@v1.0.2 + # TODO: remove this after we fix issues with our SDK + # build and cmake 3.21 on Windows. + - name: Setup cmake + if: startsWith(matrix.os, 'windows') + uses: jwlawson/actions-setup-cmake@v1.9 + with: + cmake-version: '3.20.5' - name: Cache NDK id: cache_ndk uses: actions/cache@v2 From a1959de2536b3be402bb8de28c7a31e4340903db Mon Sep 17 00:00:00 2001 From: Vimanyu Date: Thu, 22 Jul 2021 12:47:03 -0700 Subject: [PATCH 5/9] no exceptions compiler settings on app updated --- release_build_files/readme.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/release_build_files/readme.md b/release_build_files/readme.md index 3322e2c49f..3e0a877cb5 100644 --- a/release_build_files/readme.md +++ b/release_build_files/readme.md @@ -574,6 +574,8 @@ code. the overload that takes a `std::function` argument instead. - Firestore: `FieldValue::Increment` functions are no longer guarded by the `INTERNAL_EXPERIMENTAL` macro. + - General: Compiler setting of "no exceptions" on app is PRIVATE now + and will not affect other targets in CMake builds. ### 8.2.0 - Changes From 6352a708c041a34b7fba82bb3860fc5a0000dfef Mon Sep 17 00:00:00 2001 From: Vimanyu Date: Thu, 22 Jul 2021 14:40:32 -0700 Subject: [PATCH 6/9] no need to pin cmake version --- .github/workflows/integration_tests.yml | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/.github/workflows/integration_tests.yml b/.github/workflows/integration_tests.yml index f50185c990..acb00cdd74 100644 --- a/.github/workflows/integration_tests.yml +++ b/.github/workflows/integration_tests.yml @@ -249,13 +249,6 @@ jobs: - name: Add msbuild to PATH (Windows) if: startsWith(matrix.os, 'windows') uses: microsoft/setup-msbuild@v1.0.2 - # TODO: remove this after we fix issues with our SDK - # build and cmake 3.21 on Windows. - - name: Setup cmake - if: startsWith(matrix.os, 'windows') - uses: jwlawson/actions-setup-cmake@v1.9 - with: - cmake-version: '3.20.5' - name: Cache vcpkg C++ dependencies id: cache_vcpkg uses: actions/cache@v2 @@ -414,13 +407,6 @@ jobs: - name: Add msbuild to PATH (Windows) if: startsWith(matrix.os, 'windows') uses: microsoft/setup-msbuild@v1.0.2 - # TODO: remove this after we fix issues with our SDK - # build and cmake 3.21 on Windows. - - name: Setup cmake - if: startsWith(matrix.os, 'windows') - uses: jwlawson/actions-setup-cmake@v1.9 - with: - cmake-version: '3.20.5' - name: Cache NDK id: cache_ndk uses: actions/cache@v2 From 9b657b8094c352dc26c8337a8ba1d5187deb477e Mon Sep 17 00:00:00 2001 From: Vimanyu Date: Mon, 26 Jul 2021 16:49:51 -0700 Subject: [PATCH 7/9] fixed formatting --- app/tests/thread_test.cc | 19 ++++++++++--------- 1 file changed, 10 insertions(+), 9 deletions(-) diff --git a/app/tests/thread_test.cc b/app/tests/thread_test.cc index 5beb34d656..b822d67229 100644 --- a/app/tests/thread_test.cc +++ b/app/tests/thread_test.cc @@ -20,16 +20,17 @@ #include "gmock/gmock.h" #include "gtest/gtest.h" -#define EXPECT_THROW_ERROR_CODE(statement, error_code) \ - EXPECT_THROW({\ - try { \ +#define EXPECT_THROW_ERROR_CODE(statement, error_code) \ + EXPECT_THROW( \ + { \ + try { \ GTEST_SUPPRESS_UNREACHABLE_CODE_WARNING_BELOW_(statement); \ - } \ - catch(const std::system_error &exception) {\ - EXPECT_EQ(error_code, exception.code());\ - throw;\ - }\ - }, std::system_error) + } catch (const std::system_error& exception) { \ + EXPECT_EQ(error_code, exception.code()); \ + throw; \ + } \ + }, \ + std::system_error) namespace { From 7e9e4b31acf0126b79a5fe758350e4c1e57db09a Mon Sep 17 00:00:00 2001 From: Vimanyu Date: Mon, 26 Jul 2021 17:51:08 -0700 Subject: [PATCH 8/9] reordered release notes --- release_build_files/readme.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/release_build_files/readme.md b/release_build_files/readme.md index e781a8e892..da48d614e8 100644 --- a/release_build_files/readme.md +++ b/release_build_files/readme.md @@ -575,13 +575,13 @@ code. iOS and tvOS. The following products are currently included for tvOS: Auth, Database, Firestore, Functions, Installations, Messaging, Remote Config, Storage. + - General: Compiler setting of "no exceptions" on app is PRIVATE now + and will not affect other targets in CMake builds. - Firestore: Removed the deprecated `Firestore::RunTransaction(TransactionFunction*)` function. Please use the overload that takes a `std::function` argument instead. - Firestore: `FieldValue::Increment` functions are no longer guarded by the `INTERNAL_EXPERIMENTAL` macro. - - General: Compiler setting of "no exceptions" on app is PRIVATE now - and will not affect other targets in CMake builds. - Firestore: added more validation of invalid input. - Firestore: added an `is_valid` method to the public API classes that can be in an invalid state. From c95c5cfc5842377d393db8008b485c9e84571e0b Mon Sep 17 00:00:00 2001 From: Vimanyu Date: Mon, 26 Jul 2021 18:16:13 -0700 Subject: [PATCH 9/9] fixed release notes --- release_build_files/readme.md | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/release_build_files/readme.md b/release_build_files/readme.md index da48d614e8..0a94ff2a80 100644 --- a/release_build_files/readme.md +++ b/release_build_files/readme.md @@ -569,14 +569,15 @@ code. ### 8.3.0 - Changes - - General: This release adds tvOS C++ libraries that wrap the community - supported Firebase tvOS SDK. `libs/tvos` contains tvOS specific - libraries and the `xcframeworks` directory now includes support for both - iOS and tvOS. The following products are currently included for tvOS: - Auth, Database, Firestore, Functions, Installations, Messaging, - Remote Config, Storage. - - General: Compiler setting of "no exceptions" on app is PRIVATE now - and will not affect other targets in CMake builds. + - General: This release adds tvOS C++ libraries that wrap the + community-supported Firebase tvOS SDK. `libs/tvos` contains + tvOS-specific libraries, and the `xcframeworks` directory now + includes support for both iOS and tvOS. The following products are + currently included for tvOS: Auth, Database, Firestore, Functions, + Installations, Messaging, Remote Config, Storage. + - General: When building from source, the compiler setting of + "no exceptions" on app is PRIVATE now and will not affect any other + targets in the build. - Firestore: Removed the deprecated `Firestore::RunTransaction(TransactionFunction*)` function. Please use the overload that takes a `std::function` argument instead.