Update GoogleTest and install it via FetchContent.

p4lang · Aug 2, 2023 · 2402244 · 2402244
1 parent b1b0fde
commit 2402244
Show file tree

Hide file tree

Showing 13 changed files with 56 additions and 53 deletions.
diff --git a/.gitmodules b/.gitmodules
@@ -1,7 +1,3 @@
-[submodule "test/frameworks/gtest"]
-	path = test/frameworks/gtest
-	url = https://github.com/google/googletest.git
-	ignored = untracked
 [submodule "backends/ebpf/runtime/contrib/libbpf"]
 	path = backends/ebpf/runtime/contrib/libbpf
 	url = https://github.com/libbpf/libbpf

diff --git a/CMakeLists.txt b/CMakeLists.txt
@@ -327,7 +327,6 @@ include_directories (
   ${P4C_SOURCE_DIR}
   ${P4C_BINARY_DIR}
   )
-include_directories(SYSTEM ${P4C_SOURCE_DIR}/test/frameworks/gtest/googletest/include)
 add_definitions (-DCONFIG_PREFIX="${CMAKE_INSTALL_PREFIX}")
 add_definitions (-DCONFIG_PKGDATADIR="${CMAKE_INSTALL_PREFIX}/${P4C_ARTIFACTS_OUTPUT_DIRECTORY}")
 
@@ -346,15 +345,21 @@ configure_file ("${P4C_SOURCE_DIR}/cmake/config.h.cmake" "${P4C_BINARY_DIR}/conf
 set (IR_GENERATOR_DIRECTORY ${P4C_BINARY_DIR}/tools/ir-generator)
 set (IR_GENERATOR ${IR_GENERATOR_DIRECTORY}/irgenerator)
 
-# the order of adding subdirectories matters because of target dependencies
+# Component libraries: must be defined before being used in the back end executables.
+set (P4C_LIBRARIES controlplane midend frontend ir p4ctoolkit ir-generated)
+
+# If we enable GTest, install GoogleTest with the appropriate options.
+if (ENABLE_GTESTS)
+  include(GoogleTest)
+  p4c_obtain_googletest()
+endif ()
+
+# The order of adding subdirectories matters because of target dependencies.
 add_subdirectory (tools/driver)
 add_subdirectory (lib)
 add_subdirectory (tools/ir-generator)
 add_subdirectory (ir)
 
-# component libraries: must be defined before being used in the
-# backend executables
-set (P4C_LIBRARIES controlplane midend frontend ir p4ctoolkit ir-generated)
 
 # add extensions - before the frontends as they produce IR and extra frontend sources
 set(EXTENSION_IR_SOURCES)

diff --git a/backends/bmv2/CMakeLists.txt b/backends/bmv2/CMakeLists.txt
@@ -221,7 +221,7 @@ else()
 endif()
 
 set (GTEST_BMV2_SOURCES
-  )
-
+  gtest/load_ir_from_json.cpp
+)
 set (GTEST_SOURCES ${GTEST_SOURCES} ${GTEST_BMV2_SOURCES} PARENT_SCOPE)
 set (GTEST_LDADD ${GTEST_LDADD} bmv2backend PARENT_SCOPE)
diff --git a/backends/dpdk/CMakeLists.txt b/backends/dpdk/CMakeLists.txt
@@ -15,9 +15,6 @@
 configure_file("${CMAKE_CURRENT_SOURCE_DIR}/version.h.cmake"
   "${CMAKE_CURRENT_BINARY_DIR}/version.h" @ONLY)
 
-# For files generated by the protobuf compiler
-include_directories(${P4C_BINARY_DIR}/control-plane)
-
 ################ Proto
 set (DPDK_P4RUNTIME_DIR ${CMAKE_CURRENT_SOURCE_DIR}/control-plane/proto)
 set (DPDK_P4RUNTIME_INFO_PROTO ${DPDK_P4RUNTIME_DIR}/p4info.proto)

diff --git a/backends/p4tools/common/CMakeLists.txt b/backends/p4tools/common/CMakeLists.txt
@@ -46,7 +46,6 @@ target_include_directories(
   SYSTEM BEFORE PUBLIC ${P4TOOLS_Z3_INCLUDE_DIR}
   PUBLIC "${CMAKE_BINARY_DIR}/common"
   PUBLIC "${P4C_SOURCE_DIR}"
-  PUBLIC "${P4C_SOURCE_DIR}/test/frameworks/gtest/googletest/include"
   PUBLIC "${P4C_BINARY_DIR}"
 )
 

diff --git a/backends/p4tools/modules/testgen/CMakeLists.txt b/backends/p4tools/modules/testgen/CMakeLists.txt
@@ -138,23 +138,16 @@ add_dependencies(p4testgen linkp4testgen)
 
 if(ENABLE_GTESTS)
   add_executable(testgen-gtest ${TESTGEN_GTEST_SOURCES})
-  target_include_directories(
-    testgen-gtest
-    PRIVATE ${P4C_SOURCE_DIR}/test/frameworks/gtest/googlemock/include
-    PRIVATE ${P4C_SOURCE_DIR}/test/frameworks/gtest/googletest/include
-  )
   target_link_libraries(
     testgen-gtest
     PRIVATE testgen
     PRIVATE gtest
     ${TESTGEN_LIBS}
     ${P4C_LIBRARIES}
     ${P4C_LIB_DEPS}
-    ${CMAKE_THREAD_LIBS_INIT}
   )
 
   if(ENABLE_TESTING)
-    add_definitions("-DGTEST_HAS_PTHREAD=0")
     add_test(NAME testgen-gtest COMMAND testgen-gtest)
     set_tests_properties(testgen-gtest PROPERTIES LABELS "gtest-testgen")
   endif()

diff --git a/backends/p4tools/modules/testgen/test/gtest_utils.h b/backends/p4tools/modules/testgen/test/gtest_utils.h
@@ -1,13 +1,14 @@
 #ifndef BACKENDS_P4TOOLS_MODULES_TESTGEN_TEST_GTEST_UTILS_H_
 #define BACKENDS_P4TOOLS_MODULES_TESTGEN_TEST_GTEST_UTILS_H_
 
+#include <gtest/gtest.h>
+
 #include <optional>
 #include <string>
 
 #include "backends/p4tools/common/lib/variables.h"
 #include "frontends/common/options.h"
 #include "ir/ir.h"
-#include "test/frameworks/gtest/googletest/include/gtest/gtest.h"
 
 namespace Test {
 

diff --git a/cmake/GoogleTest.cmake b/cmake/GoogleTest.cmake
@@ -0,0 +1,25 @@
+macro(p4c_obtain_googletest)
+  # Some P4C source files include gtest files.
+  # We need this definition to guard against compilation errors.
+  add_definitions(-DP4C_GTEST_ENABLED)
+  # Print download state while setting up GTest.
+  set(FETCHCONTENT_QUIET_PREV ${FETCHCONTENT_QUIET})
+  set(FETCHCONTENT_QUIET OFF)
+  set(INSTALL_GTEST OFF CACHE BOOL "Enable installation of googletest. (Projects embedding googletest may want to turn this OFF.")
+  set(gtest_disable_pthreads ON CACHE BOOL "Disable uses of pthreads in gtest.")
+  # Fetch and build the GTest dependency.
+  FetchContent_Declare(
+    gtest
+    GIT_REPOSITORY https://github.com/google/googletest.git
+    # https://github.com/google/googletest/releases/tag/v1.14.0
+    GIT_TAG        f8d7d77c06936315286eb55f8de22cd23c188571
+    GIT_PROGRESS TRUE
+    GIT_SHALLOW TRUE
+  )
+  FetchContent_MakeAvailable(gtest)
+  set(FETCHCONTENT_QUIET ${FETCHCONTENT_QUIET_PREV})
+  message("Done with setting up GoogleTest for P4C.")
+
+  include_directories(BEFORE SYSTEM ${gtest_SOURCE_DIR}/googletest/include)
+  include_directories(BEFORE SYSTEM ${gtest_SOURCE_DIR}/googlemock/include)
+endmacro(p4c_obtain_googletest)
diff --git a/control-plane/CMakeLists.txt b/control-plane/CMakeLists.txt
@@ -97,13 +97,15 @@ target_include_directories(controlplane-gen
   SYSTEM BEFORE PUBLIC ${Protobuf_INCLUDE_DIR}
 )
 target_link_libraries (controlplane-gen PUBLIC ${PROTOBUF_LIBRARY})
-
 # Needed for the correct import of google/status.pb.cc
-include_directories (${CMAKE_CURRENT_SOURCE_DIR} ${CMAKE_CURRENT_BINARY_DIR})
+target_include_directories(
+  controlplane-gen PUBLIC ${CMAKE_CURRENT_SOURCE_DIR} ${CMAKE_CURRENT_BINARY_DIR}
+)
+
 
 # Silence various warnings as the root issue is out of our control, example https://github.com/protocolbuffers/protobuf/issues/7140
 set_source_files_properties (${P4RUNTIME_GEN_SRCS} {$P4RUNTIME_GEN_HDRS} PROPERTIES COMPILE_FLAGS "-Wno-unused-parameter -Wno-array-bounds -Wno-error")
 
-add_library (controlplane STATIC ${CONTROLPLANE_SRCS} )
-target_link_libraries (controlplane ${CMAKE_THREAD_LIBS_INIT} controlplane-gen)
-add_dependencies (controlplane mkP4configdir ir-generated controlplane-gen frontend-parser-gen)
+add_library(controlplane STATIC ${CONTROLPLANE_SRCS})
+target_link_libraries(controlplane ${CMAKE_THREAD_LIBS_INIT} controlplane-gen)
+add_dependencies(controlplane mkP4configdir ir-generated controlplane-gen frontend-parser-gen)
diff --git a/lib/json.h b/lib/json.h
@@ -22,7 +22,9 @@ limitations under the License.
 #include <type_traits>
 #include <vector>
 
+#ifdef P4C_GTEST_ENABLED
 #include "gtest/gtest_prod.h"
+#endif
 #include "lib/big_int_util.h"
 #include "lib/castable.h"
 #include "lib/cstring.h"
@@ -43,7 +45,9 @@ class IJson : public ICastable {
 };
 
 class JsonValue final : public IJson {
+#ifdef P4C_GTEST_ENABLED
     FRIEND_TEST(Util, Json);
+#endif
 
  public:
     enum Kind { String, Number, True, False, Null };

diff --git a/lib/source_file.h b/lib/source_file.h
@@ -28,7 +28,9 @@ limitations under the License.
 #include "stringref.h"
 
 // GTest
+#ifdef P4C_GTEST_ENABLED
 #include "gtest/gtest_prod.h"
+#endif
 
 namespace Test {
 class UtilSourceFile;
@@ -261,7 +263,9 @@ class Comment final : IHasDbPrint {
   This class implements a singleton pattern: there is a single instance of this class.
 */
 class InputSources final {
+#ifdef P4C_GTEST_ENABLED
     FRIEND_TEST(UtilSourceFile, InputSources);
+#endif
 
  public:
     InputSources();

diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
@@ -16,23 +16,6 @@
 # GTests
 ################################################################################
 
-# Flags needed for GTest tests.
-set (GTEST_ROOT ${P4C_SOURCE_DIR}/test/frameworks/gtest/googletest)
-
-include_directories(BEFORE ${GTEST_ROOT} ${GTEST_ROOT}/include
-                    ${P4C_SOURCE_DIR}/control-plane/PI/include)
-# For files generated by the protobuf compiler
-include_directories(${P4C_BINARY_DIR}/control-plane)
-
-# Configure GTest to disable support for SEH (which is irrelevant for p4c since
-# we don't currently support Windows) and pthreads (which prevents GTest from
-# using its internal TLS implementation, which interacts badly with libgc).
-add_definitions(-DGTEST_HAS_SEH=0)
-add_definitions(-DGTEST_HAS_PTHREAD=0)
-
-# Build the GTest library itself.
-add_library(gtest ${P4C_STATIC_BUILD} ${GTEST_ROOT}/src/gtest-all.cc)
-
 set (GTEST_UNITTEST_SOURCES
   gtest/arch_test.cpp
   gtest/bitvec_test.cpp
@@ -60,13 +43,7 @@ set (GTEST_UNITTEST_SOURCES
   gtest/source_file_test.cpp
   gtest/transforms.cpp
   gtest/stringify.cpp
-  )
-if (ENABLE_BMV2)
-  set (GTEST_UNITTEST_SOURCES ${GTEST_UNITTEST_SOURCES} gtest/load_ir_from_json.cpp)
-endif()
-set (GTEST_UNITTEST_HEADERS
-  gtest/helpers.h
-  )
+)
 
 # Combine the executable and the non-backend-specific unit tests into a single
 # unity compilation group.
@@ -92,6 +69,7 @@ include_directories(${CMAKE_CURRENT_BINARY_DIR}/test)
 configure_file(gtest/env.h.in ${CMAKE_CURRENT_BINARY_DIR}/gtest/env.h)
 add_executable (gtestp4c ${GTESTP4C_SOURCES})
 target_link_libraries (gtestp4c ${GTEST_LDADD} ${P4C_LIBRARIES} gtest ${P4C_LIB_DEPS})
+add_dependencies(gtestp4c gtest genIR frontend controlplane)
 
 # Tests
 add_test (NAME gtestp4c COMMAND gtestp4c WORKING_DIRECTORY ${P4C_BINARY_DIR})

diff --git a/test/frameworks/gtest b/test/frameworks/gtest