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 · 76e6643 · 76e6643
1 parent 27549a2
commit 76e6643
Show file tree

Hide file tree

Showing 15 changed files with 40 additions and 26 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -22,6 +22,7 @@
 * 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)).
 * Added a CAPI interface for SDKs to bring their own managed users with core's app services turned off. ([PR #7615](https://github.com/realm/realm-core/pull/7615)).
 * Bump the minimum deployment targets on Apple platforms to the minimums supported by Xcode 15 and clean up now unused availability checks. ([PR #7648](https://github.com/realm/realm-core/pull/7648)).
+* 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-macros.h b/src/external/bson/bson-macros.h
@@ -173,8 +173,8 @@
 #endif
 #else
 #if defined(_MSC_VER)
-#define BSON_ALIGNED_BEGIN(_N) __declspec (align (BSON_ALIGN_OF_PTR))
-#define BSON_ALIGNED_END(_N)
+#define BSON_ALIGNED_BEGIN(_N) _Pragma("warning(push)") _Pragma("warning(disable: 4359)") __declspec (align (BSON_ALIGN_OF_PTR))
+#define BSON_ALIGNED_END(_N) _Pragma("warning(pop)")
 #else
 #define BSON_ALIGNED_BEGIN(_N)
 #define BSON_ALIGNED_END(_N) \

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/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/src/realm/object-store/sync/sync_manager.cpp b/src/realm/object-store/sync/sync_manager.cpp
@@ -142,7 +142,7 @@ void SyncManager::do_make_logger()
     REALM_ASSERT(m_logger_ptr);
 }
 
-const std::shared_ptr<util::Logger>& SyncManager::get_logger() const
+std::shared_ptr<util::Logger> SyncManager::get_logger() const
 {
     util::CheckedLockGuard lock(m_mutex);
     return m_logger_ptr;

diff --git a/src/realm/object-store/sync/sync_manager.hpp b/src/realm/object-store/sync/sync_manager.hpp
@@ -138,7 +138,7 @@ class SyncManager : public std::enable_shared_from_this<SyncManager> {
     }
 
     // Return the cached logger
-    const std::shared_ptr<util::Logger>& get_logger() const REQUIRES(!m_mutex);
+    std::shared_ptr<util::Logger> get_logger() const REQUIRES(!m_mutex);
 
     struct OnlyForTesting {
         friend class TestHelper;

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}},

diff --git a/test/object-store/util/test_file.cpp b/test/object-store/util/test_file.cpp
@@ -250,7 +250,7 @@ std::string SyncServer::url_for_realm(StringData realm_name) const
 
 int SyncServer::port() const
 {
-    return m_server.listen_endpoint().port();
+    return static_cast<int>(m_server.listen_endpoint().port());
 }
 
 struct WaitForSessionState {

diff --git a/test/test_encrypted_file_mapping.cpp b/test/test_encrypted_file_mapping.cpp
@@ -332,10 +332,10 @@ static void check_attach_and_read(const char* key, const std::string& path, size
         REALM_ASSERT_3(foo->where().equal(pk_col, util::format("name %1", num_entries - 1).c_str()).count(), ==, 1);
     }
     catch (const std::exception& e) {
-        size_t fs = File::get_size_static(path);
+        auto fs = File::get_size_static(path);
         util::format(std::cout, "Error for num_entries %1 with page_size of %2 on file of size %3\n%4", num_entries,
                      page_size(), fs, e.what());
-        throw e;
+        throw;
     }
 }