Update to latest Abseil, remove custom status matchers

This also ensures that `BUILD_TESTING`, `BINEXPORT_BUILD_TESTING` and `BINDIFF_BUILD_TESTING` do the right thing. PiperOrigin-RevId: 662074134 Change-Id: I34f26a4cc2eed8f88ce9d23b418f0eb5c573e117
google · Aug 12, 2024 · 7dfbaf4 · 7dfbaf4
1 parent 633f472
commit 7dfbaf4
Show file tree

Hide file tree

Showing 9 changed files with 65 additions and 162 deletions.
diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -41,10 +41,9 @@ include(Util)
 include(BinExportOptions)
 include(BinExportDeps)
 
-include(CTest)
 if(BUILD_TESTING AND BINEXPORT_BUILD_TESTING)
+  include(CTest)
   include(GoogleTest)
-  enable_testing()
 endif()
 
 # Make Google-style includes work
@@ -169,12 +168,12 @@ if(BUILD_TESTING AND BINEXPORT_BUILD_TESTING)
     util/format_test.cc
     util/process_test.cc
     util/status_macros_test.cc
-    util/status_matchers.h
     util/timer_test.cc
   )
   target_link_libraries(binexport_shared_test PUBLIC
     binexport_test_base
     binexport_shared
+    absl::status_matchers
   )
   gtest_discover_tests(binexport_shared_test)
 endif()

diff --git a/cmake/BinExportDeps.cmake b/cmake/BinExportDeps.cmake
@@ -22,24 +22,11 @@ endfunction()
 
 include(FetchContent)
 
-# Abseil
-FetchContent_Declare(absl
-  URL https://github.com/abseil/abseil-cpp/archive/1343b6d0d634397c33220e90fc469738f56b07c1.zip  # 2024-05-24
-  URL_HASH SHA256=9e08e607b57abc1dad24d8b6ff1e8bc87ae028b860f7b3bc7755a360dc44a069
-)
-set(ABSL_CXX_STANDARD ${CMAKE_CXX_STANDARD} CACHE STRING "" FORCE)
-set(ABSL_PROPAGATE_CXX_STD ON CACHE BOOL "" FORCE)
-set(ABSL_USE_EXTERNAL_GOOGLETEST ON CACHE BOOL "" FORCE)
-FetchContent_MakeAvailable(absl)
-# Protobuf's utf8_range fork insists on this being set
-set(ABSL_ROOT_DIR "${absl_SOURCE_DIR}" CACHE STRING "" FORCE)
-binexport_check_target(absl::core_headers)
-
 if(BUILD_TESTING AND BINEXPORT_BUILD_TESTING)
   # Googletest (needs to come after Abseil due to C++ standard propagation)
   FetchContent_Declare(googletest
-    URL https://github.com/google/googletest/archive/b3a9ba2b8e975550799838332803d468797ae2e1.zip  # 2023-12-04
-    URL_HASH SHA256=93b52882cff4e8fb344070106facee0df5e61be91b1948a69c8581b3bcbe0c61
+    URL https://github.com/google/googletest/archive/5bcb2d78a16edd7110e72ef694d229815aa29542.zip  # 2024-07-24
+    URL_HASH SHA256=55d80e3e4b3ae63c4b377895babc2ecd17834aedfc0e6cb8aedb5c7adb97defd
   )
   FetchContent_MakeAvailable(googletest)
   binexport_check_target(gtest)
@@ -61,10 +48,30 @@ if(BUILD_TESTING AND BINEXPORT_BUILD_TESTING)
   endif()
 endif()
 
+# Abseil
+FetchContent_Declare(absl
+  URL https://github.com/abseil/abseil-cpp/archive/7e5c339b1aa790ae03cc614a8d7626d5b4831891.zip  # 2024-07-25 (RC2)
+  URL_HASH SHA256=2ad33d08a720fa3a67ec12bd8cf9846da7ed53045163d69047856cc671a0cbe5
+)
+set(ABSL_CXX_STANDARD ${CMAKE_CXX_STANDARD} CACHE STRING "" FORCE)
+set(ABSL_PROPAGATE_CXX_STD ON CACHE BOOL "" FORCE)
+set(ABSL_USE_EXTERNAL_GOOGLETEST ON CACHE BOOL "" FORCE)
+set(ABSL_FIND_GOOGLETEST OFF CACHE BOOL "" FORCE)
+if(BUILD_TESTING AND BINEXPORT_BUILD_TESTING)
+  # Need this for absl::status_matchers to be available
+  set(ABSL_BUILD_TESTING ON CACHE BOOL "" FORCE)
+endif()
+FetchContent_MakeAvailable(absl)
+binexport_check_target(absl::core_headers)
+if(BUILD_TESTING AND BINEXPORT_BUILD_TESTING)
+  # Fix a bug in Abseil's CMake build files
+  target_link_libraries(absl_no_destructor_test PUBLIC gmock)
+endif()
+
 # Protocol Buffers
 FetchContent_Declare(protobuf
   GIT_REPOSITORY https://github.com/protocolbuffers/protobuf.git
-  GIT_TAG        v27.0 # 2024-05-23 (must be a branch for GIT_SHALLOW to work)
+  GIT_TAG        v27.2 # 2024-05-25 (must be a branch for GIT_SHALLOW to work)
   GIT_SUBMODULES third_party/jsoncpp
   GIT_SHALLOW    TRUE
 )
@@ -138,3 +145,6 @@ find_package(Git)
 if(BINEXPORT_ENABLE_IDAPRO)
   find_package(IdaSdk REQUIRED)
 endif()
+
+#set(BUILD_TESTING ${BINEXPORT_SAVE_BUILD_TESTING})
+
diff --git a/cmake/BinExportOptions.cmake b/cmake/BinExportOptions.cmake
@@ -12,8 +12,17 @@
 # See the License for the specific language governing permissions and
 # limitations under the License.
 
+if(CMAKE_SOURCE_DIR STREQUAL CMAKE_CURRENT_SOURCE_DIR)
+  set(BINEXPORT_TOP_LEVEL TRUE)
+endif()
+
+# If unset, force a default value for this standard option
+if(NOT BUILD_TESTING)
+  set(BUILD_TESTING OFF)
+endif()
+
 option(BINEXPORT_BUILD_TESTING
-       "If ON, this will build all of BinExport's own tests" ON)
+  "If ON, this will build all of BinExport's own tests" ${BINEXPORT_TOP_LEVEL})
 option(BINEXPORT_BUILD_BENCHMARK
        "If this and BINEXPORT_BUILD_TESTING is ON, build benchmark tests" OFF)
 

diff --git a/util/filesystem_test.cc b/util/filesystem_test.cc
@@ -19,12 +19,14 @@
 
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include "third_party/absl/status/status_matchers.h"
 #include "third_party/absl/strings/str_cat.h"
-#include "third_party/zynamics/binexport/util/status_matchers.h"
+#include "third_party/zynamics/binexport/util/status_macros.h"
 
 namespace security::binexport {
 namespace {
 
+using ::absl_testing::IsOk;
 using ::testing::IsEmpty;
 using ::testing::IsFalse;
 using ::testing::IsTrue;
@@ -100,9 +102,9 @@ TEST(FileSystemTest, CreateAndRemoveDirectories) {
                           GetOrCreateTempDirectory("test"));
 
   const std::string test_path = JoinPath(temp_dir, "sub", "dir", "s2");
-  EXPECT_THAT(CreateDirectories(test_path).ok(), IsTrue());
+  EXPECT_THAT(CreateDirectories(test_path), IsOk());
 
-  EXPECT_THAT(RemoveAll(test_path).ok(), IsTrue());
+  EXPECT_THAT(RemoveAll(test_path), IsOk());
   EXPECT_THAT(IsDirectory(test_path), IsFalse());
 }
 
@@ -117,7 +119,7 @@ TEST(FileSystemTest, LinkingFiles) {
     ASSERT_THAT(out.good(), IsTrue());
   }
 
-  EXPECT_THAT(CreateOrUpdateLinkWithFallback(target, link_path).ok(), IsTrue());
+  EXPECT_THAT(CreateOrUpdateLinkWithFallback(target, link_path), IsOk());
 }
 
 }  // namespace

diff --git a/util/idb_export_test.cc b/util/idb_export_test.cc
@@ -20,13 +20,14 @@
 
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include "third_party/absl/status/status_matchers.h"
 #include "third_party/zynamics/binexport/util/filesystem.h"
-#include "third_party/zynamics/binexport/util/status_matchers.h"
+#include "third_party/zynamics/binexport/util/status_macros.h"
 
 namespace security::binexport {
 namespace {
 
-using ::not_absl::IsOk;
+using ::absl_testing::IsOk;
 using ::testing::AnyOf;
 using ::testing::IsFalse;
 using ::testing::Not;

diff --git a/util/process_test.cc b/util/process_test.cc
@@ -19,33 +19,33 @@
 
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
+#include "third_party/absl/status/status_matchers.h"
 #include "third_party/absl/strings/str_cat.h"
-#include "third_party/zynamics/binexport/util/status_matchers.h"
+#include "third_party/zynamics/binexport/util/status_macros.h"
 
 namespace security::binexport {
 namespace {
 
-using ::not_absl::IsOk;
+using ::absl_testing::IsOk;
 using ::testing::Eq;
 using ::testing::IsEmpty;
-using ::testing::IsFalse;
 using ::testing::Not;
 
 TEST(UtilityTest, SpawnProcessEmptyArgs) {
   auto exit_code_or = SpawnProcessAndWait({});
-  EXPECT_THAT(exit_code_or.ok(), IsFalse());
+  EXPECT_THAT(exit_code_or, Not(IsOk()));
   EXPECT_THAT(exit_code_or.status().message(), Not(IsEmpty()));
 }
 
 TEST(UtilityTest, SpawnProcessNonExistingWait) {
   auto exit_code_or = SpawnProcessAndWait({"not.an.executable"});
-  EXPECT_THAT(exit_code_or.ok(), IsFalse());
+  EXPECT_THAT(exit_code_or, Not(IsOk()));
   EXPECT_THAT(exit_code_or.status().message(), Not(IsEmpty()));
 }
 
 TEST(UtilityTest, SpawnProcessNonExistingNoWait) {
   auto status = SpawnProcess({"not.an.executable"});
-  EXPECT_THAT(status.ok(), IsFalse());
+  EXPECT_THAT(status, Not(IsOk()));
   EXPECT_THAT(status.message(), Not(IsEmpty()));
 }
 

diff --git a/util/status_macros.h b/util/status_macros.h
@@ -45,4 +45,13 @@
   }                                                    \
   lhs = std::move(statusor).value();
 
+#define NA_ASSERT_OK_AND_ASSIGN(lhs, rexpr) \
+  NA_ASSERT_OK_AND_ASSIGN_IMPL(             \
+      NA_MACROS_IMPL_CONCAT(_sapi_statusor, __LINE__), lhs, rexpr)
+
+#define NA_ASSERT_OK_AND_ASSIGN_IMPL(statusor, lhs, rexpr) \
+  auto statusor = (rexpr);                                 \
+  ASSERT_THAT(statusor.status(), ::absl_testing::IsOk());  \
+  lhs = std::move(statusor).value();
+
 #endif  // UTIL_STATUS_MACROS_H_
diff --git a/util/status_macros_test.cc b/util/status_macros_test.cc
@@ -20,13 +20,15 @@
 #include "gtest/gtest.h"
 #include "third_party/absl/memory/memory.h"
 #include "third_party/absl/status/status.h"
+#include "third_party/absl/status/status_matchers.h"
 #include "third_party/absl/status/statusor.h"
 #include "third_party/absl/strings/str_cat.h"
-#include "third_party/zynamics/binexport/util/status_matchers.h"
 
 namespace not_absl {
 namespace {
 
+using ::absl_testing::StatusIs;
+
 TEST(ReturnIfError, ReturnsOnErrorStatus) {
   auto func = []() -> absl::Status {
     NA_RETURN_IF_ERROR(absl::OkStatus());

diff --git a/util/status_matchers.h b/util/status_matchers.h