Build with -Werror on CI

We used to have the Jenkins error parser that made the job fail if there were any warnings, but now we have nothing and warnings can slip through. This is a little worse as it makes the build fail immediately rather than letting it run the tests, but it's better than nothing.
realm · Apr 29, 2024 · 76179df · 76179df
1 parent 431ffe6
commit 76179df
Show file tree

Hide file tree

Showing 11 changed files with 34 additions and 20 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -20,6 +20,7 @@
 ### Internals
 * Follow on to ([PR #7300](https://github.com/realm/realm-core/pull/7300)) to allow SDKs to construct a fake user for testing SyncManager::get_user -> App::create_fake_user_for_testing ([PR #7632](https://github.com/realm/realm-core/pull/7632))
 * Fix build-apple-device.sh, broken in [#7603](https://github.com/realm/realm-core/pull/7603) ([PR #7640](https://github.com/realm/realm-core/pull/7640)).
+* Build with -Werror on CI to ensure that new warnings don't slip in. ([PR #7646](https://github.com/realm/realm-core/pull/7646))
 
 ----------------------------------------------
 

diff --git a/evergreen/config.yml b/evergreen/config.yml
@@ -172,6 +172,7 @@ functions:
           fi
 
           set_cmake_var realm_vars REALM_NO_TESTS BOOL ${no_tests|Off}
+          set_cmake_var realm_vars CMAKE_COMPILE_WARNING_AS_ERROR BOOL On
 
           echo "Running cmake with these vars:"
           cat cmake_vars/*.txt | tee cmake_vars.txt

diff --git a/src/external/IntelRDFPMathLib20U2/CMakeLists.txt b/src/external/IntelRDFPMathLib20U2/CMakeLists.txt
@@ -32,4 +32,6 @@ if(HAVE-Wunused-but-set-variable)
     target_compile_options(Bid PRIVATE -Wno-unused-but-set-variable)
 endif()
 
-set_target_properties(Bid PROPERTIES CXX_VISIBILITY_PRESET hidden)
+set_target_properties(Bid PROPERTIES
+    CXX_VISIBILITY_PRESET hidden
+    COMPILE_WARNING_AS_ERROR Off)
diff --git a/src/external/bson/CMakeLists.txt b/src/external/bson/CMakeLists.txt
@@ -53,3 +53,7 @@ if (CMAKE_SYSTEM_NAME MATCHES "Darwin")
 endif()
 target_include_directories(Bson PUBLIC .. ${PROJECT_BINARY_DIR}/src/external)
 target_compile_definitions(Bson INTERFACE BSON_STATIC)
+
+set_target_properties(Bson PROPERTIES
+    CXX_VISIBILITY_PRESET hidden
+    COMPILE_WARNING_AS_ERROR Off)
diff --git a/src/external/bson/bson-types.h b/src/external/bson/bson-types.h
@@ -389,7 +389,7 @@ typedef struct {
 BSON_ALIGNED_BEGIN (BSON_ALIGN_OF_PTR)
 typedef struct {
    uint32_t type;
-   /*< private >*/
+   /* < private > */
 } bson_reader_t BSON_ALIGNED_END (BSON_ALIGN_OF_PTR);
 
 

diff --git a/src/realm/object-store/object_store.cpp b/src/realm/object-store/object_store.cpp
@@ -114,7 +114,7 @@ std::optional<CollectionType> process_collection(const Property& property)
     else if (is_dictionary(property.type)) {
         return CollectionType::Dictionary;
     }
-    return {};
+    return std::nullopt;
 }
 
 ColKey add_column(Group& group, Table& table, Property const& property)

diff --git a/src/realm/object-store/sync/app.hpp b/src/realm/object-store/sync/app.hpp
@@ -190,7 +190,7 @@ class App : public std::enable_shared_from_this<App>,
     /// @param access_token The access token of the user
     /// @param refresh_token The refresh token of the user
     std::shared_ptr<User> create_fake_user_for_testing(const std::string& user_id, const std::string& access_token,
-                                                       const std::string& refresh_token);
+                                                       const std::string& refresh_token) REQUIRES(!m_user_mutex);
 
     // MARK: - Provider Clients
 

diff --git a/src/realm/object-store/sync/impl/app_metadata.cpp b/src/realm/object-store/sync/impl/app_metadata.cpp
@@ -272,16 +272,6 @@ void migrate_to_v7(std::shared_ptr<Realm> old_realm, std::shared_ptr<Realm> real
     }
 }
 
-std::shared_ptr<Realm> try_get_realm(const RealmConfig& config)
-{
-    try {
-        return Realm::get_shared_realm(config);
-    }
-    catch (const InvalidDatabase&) {
-        return nullptr;
-    }
-}
-
 std::shared_ptr<Realm> open_realm(RealmConfig& config, const app::AppConfig& app_config)
 {
     bool should_encrypt = app_config.metadata_mode == app::AppConfig::MetadataMode::Encryption;
@@ -296,6 +286,15 @@ std::shared_ptr<Realm> open_realm(RealmConfig& config, const app::AppConfig& app
     }
 
 #if REALM_PLATFORM_APPLE
+    auto try_get_realm = [&]() -> std::shared_ptr<Realm> {
+        try {
+            return Realm::get_shared_realm(config);
+        }
+        catch (const InvalidDatabase&) {
+            return nullptr;
+        }
+    };
+
     // This logic is all a giant race condition once we have multi-process sync.
     // Wrapping it all (including the keychain accesses) in DB::call_with_lock()
     // might suffice.
@@ -306,7 +305,7 @@ std::shared_ptr<Realm> open_realm(RealmConfig& config, const app::AppConfig& app
     auto key = keychain::get_existing_metadata_realm_key(app_config.app_id, app_config.security_access_group);
     if (key) {
         config.encryption_key = *key;
-        if (auto realm = try_get_realm(config))
+        if (auto realm = try_get_realm())
             return realm;
     }
 
@@ -315,7 +314,7 @@ std::shared_ptr<Realm> open_realm(RealmConfig& config, const app::AppConfig& app
     // from a previous run being unable to access the keychain.
     if (util::File::exists(config.path)) {
         config.encryption_key.clear();
-        if (auto realm = try_get_realm(config))
+        if (auto realm = try_get_realm())
             return realm;
 
         // We weren't able to open the existing file with either the stored key
@@ -330,7 +329,7 @@ std::shared_ptr<Realm> open_realm(RealmConfig& config, const app::AppConfig& app
         key = keychain::create_new_metadata_realm_key(app_config.app_id, app_config.security_access_group);
     if (key)
         config.encryption_key = std::move(*key);
-    return try_get_realm(config);
+    return try_get_realm();
 #else  // REALM_PLATFORM_APPLE
     REALM_UNREACHABLE();
 #endif // REALM_PLATFORM_APPLE

diff --git a/test/object-store/CMakeLists.txt b/test/object-store/CMakeLists.txt
@@ -227,6 +227,7 @@ if(NOT APPLE AND NOT EMSCRIPTEN AND NOT WINDOWS_STORE AND NOT ANDROID)
         set(libuv_target uv_a)
     endif()
 
+    set_target_properties(${libuv_target} PROPERTIES COMPILE_WARNING_AS_ERROR Off)
     target_link_libraries(ObjectStoreTestLib ${libuv_target})
     target_compile_definitions(ObjectStoreTestLib PRIVATE TEST_SCHEDULER_UV=1)
 

diff --git a/test/object-store/benchmarks/main.cpp b/test/object-store/benchmarks/main.cpp
@@ -45,9 +45,15 @@ int main(int argc, char** argv)
     SetCurrentDirectoryA(path);
 #else
     char executable[PATH_MAX];
-    (void)realpath(argv[0], executable);
+    if (!realpath(argv[0], executable)) {
+        fprintf(stderr, "Failed to resolve path: '%s'", executable);
+        return 1;
+    }
     const char* directory = dirname(executable);
-    chdir(directory);
+    if (chdir(directory)) {
+        fprintf(stderr, "Failed to change directory");
+        return 1;
+    }
 #endif
 
 #if TEST_SCHEDULER_UV

diff --git a/test/object-store/sync/client_reset.cpp b/test/object-store/sync/client_reset.cpp
@@ -3091,7 +3091,7 @@ TEMPLATE_TEST_CASE("client reset collections of links", "[sync][pbs][client rese
                              {Remove{dest_pk_1}, Remove{dest_pk_2}}, {dest_pk_3, dest_pk_4});
         }
     }
-    else if constexpr (test_type_is_set) {
+    if constexpr (test_type_is_set) {
         SECTION("remote adds two links to objects which are both removed by local") {
             reset_collection({RemoveObject("dest", dest_pk_4), RemoveObject("dest", dest_pk_5),
                               CreateObject("dest", 6), Add{6}, Remove{dest_pk_1}},