Aggressively clean up the Protobuf CMake dependency.

README.md

@@ -286,10 +286,10 @@ After cloning Protobuf and before you build, check-out version 3.25.3:
 `git checkout v3.25.3`
 
 Please note that while all Protobuf versions newer than 3.0 should work for
-`p4c` itself, you may run into trouble with some extensions and other p4lang
+`p4c` itself, you may run into trouble with, Abseil, some extensions and other p4lang
 projects unless you install version 3.25.3.
 
-`p4c` also depends on Google Abseil library. This library is also a pre-requisite for Protobuf of any version newer than 3.21. Therefore the use of Protobuf of suitable version automatically fulfils Abseil dependency. P4C typically installs its own version of Abseil using CMake's `FetchContent` module (Abseil LTS 20240116.1 at the moment).
+`p4c` also depends on Google Abseil library. This library is also a pre-requisite for Protobuf of any version newer than 3.21. Therefore the use of Protobuf of suitable version automatically fulfills Abseil dependency. P4C typically installs its own version of Abseil using CMake's `FetchContent` module (Abseil LTS 20240116.1 at the moment).
 
 ### CMake
 p4c requires a CMake version of at least 3.16.3 or higher. On older systems, a newer version of CMake can be installed using `pip3 install --user cmake==3.16.3`. We have a CI test on Ubuntu 18.04 that uses this option, but there is no guarantee that this will lead to a successful build.

backends/dpdk/CMakeLists.txt

@@ -18,32 +18,38 @@ configure_file("${CMAKE_CURRENT_SOURCE_DIR}/version.h.cmake"
 ################ Proto
 set (DPDK_P4RUNTIME_DIR ${CMAKE_CURRENT_SOURCE_DIR}/control-plane/proto)
 set (DPDK_P4RUNTIME_INFO_PROTO ${DPDK_P4RUNTIME_DIR}/p4info.proto)
-set (DPDK_P4RUNTIME_INFO_GEN_SRCS ${CMAKE_CURRENT_BINARY_DIR}/p4/config/p4info.pb.cc)
-set (DPDK_P4RUNTIME_INFO_GEN_HDRS ${CMAKE_CURRENT_BINARY_DIR}/p4/config/p4info.pb.h)
 
 add_custom_target (dpdk_runtime_dir
   ${CMAKE_COMMAND} -E make_directory
   ${CMAKE_CURRENT_BINARY_DIR}/p4/config)
+
 # Generate source code from .proto using protoc. The output is
 # placed in the build directory inside `control-plane` directory
-add_custom_command(
-    OUTPUT ${DPDK_P4RUNTIME_INFO_GEN_SRCS} ${DPDK_P4RUNTIME_INFO_GEN_HDRS}
-  COMMAND ${Protobuf_PROTOC_EXECUTABLE}
-    --proto_path ${DPDK_P4RUNTIME_DIR}
-    --proto_path ${P4RUNTIME_STD_DIR}
-    --cpp_out ${CMAKE_CURRENT_BINARY_DIR}/p4/config
-    --python_out ${CMAKE_CURRENT_BINARY_DIR}/p4/config
-          ${DPDK_P4RUNTIME_INFO_PROTO}
-  DEPENDS ${DPDK_P4RUNTIME_INFO_PROTO} dpdk_runtime_dir
-  COMMENT "Generating protobuf files for p4info on target DPDK."
-  )
-add_dependencies(dpdk_runtime_dir controlplane)
-
-add_library(dpdk_runtime STATIC ${DPDK_P4RUNTIME_INFO_GEN_SRCS})
+add_library(dpdk_runtime OBJECT ${DPDK_P4RUNTIME_INFO_PROTO})
+target_include_directories(dpdk_runtime PUBLIC ${CMAKE_CURRENT_BINARY_DIR})
 target_link_libraries(dpdk_runtime PRIVATE controlplane)
+# Ideally we use DEPENDENCIES, but it only works in later versions (3.28).
+add_dependencies(dpdk_runtime mkP4configdir dpdk_runtime_dir)
+protobuf_generate(
+  TARGET dpdk_runtime
+  LANGUAGE cpp
+  IMPORT_DIRS ${P4RUNTIME_STD_DIR} ${DPDK_P4RUNTIME_DIR} ${Protobuf_INCLUDE_DIRS}
+  PROTOC_OUT_DIR ${CMAKE_CURRENT_BINARY_DIR}/p4/config
+  OUT_VAR DPDK_P4RUNTIME_INFO_CPP_GEN_SRCS
+  DEPENDENCIES ${DPDK_P4RUNTIME_INFO_PROTO} mkP4configdir dpdk_runtime_dir
+)
+protobuf_generate(
+  TARGET dpdk_runtime
+  LANGUAGE python
+  IMPORT_DIRS ${P4RUNTIME_STD_DIR} ${DPDK_P4RUNTIME_DIR} ${Protobuf_INCLUDE_DIRS}
+  PROTOC_OUT_DIR ${CMAKE_CURRENT_BINARY_DIR}/p4/config
+  OUT_VAR DPDK_P4RUNTIME_INFO_PY_GEN_SRCS
+  DEPENDENCIES ${DPDK_P4RUNTIME_INFO_PROTO}
+  mkP4configdir dpdk_runtime_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(${DPDK_P4RUNTIME_INFO_GEN_SRCS} {$DPDK_P4RUNTIME_INFO_GEN_HDRS} PROPERTIES COMPILE_FLAGS "-Wno-unused-parameter -Wno-array-bounds -Wno-error")
+set_source_files_properties(${DPDK_P4RUNTIME_INFO_CPP_GEN_SRCS} PROPERTIES COMPILE_FLAGS "-Wno-unused-parameter -Wno-array-bounds -Wno-error")
 
 set(P4C_DPDK_SOURCES
     ../bmv2/common/lower.cpp

backends/p4tools/modules/testgen/targets/bmv2/test/TestTemplate.cmake

@@ -46,7 +46,10 @@ function(validate_protobuf testfile testfolder)
   file(APPEND ${testfile} "for item in \${txtpbfiles[@]}\n")
   file(APPEND ${testfile} "do\n")
   file(APPEND ${testfile} "\techo \"Found \${item}\"\n")
-  file(APPEND ${testfile} "\t${PROTOC_BINARY} ${PROTOBUF_PROTOC_INCLUDES} -I${CMAKE_CURRENT_LIST_DIR}/../proto -I${P4RUNTIME_STD_DIR} -I${P4C_SOURCE_DIR}/control-plane --encode=p4testgen.TestCase p4testgen.proto < \${item} > /dev/null\n")
+  file(
+    APPEND ${testfile}
+    "\t${Protobuf_PROTOC_EXECUTABLE} --proto_path ${Protobuf_INCLUDE_DIRS} --proto_path ${CMAKE_CURRENT_LIST_DIR}/../proto --proto_path ${P4RUNTIME_STD_DIR} --proto_path ${P4C_SOURCE_DIR}/control-plane --encode=p4testgen.TestCase p4testgen.proto < \${item} > /dev/null\n"
+  )
   file(APPEND ${testfile} "done\n")
 endfunction(validate_protobuf)
 
@@ -60,7 +63,10 @@ function(validate_protobuf_ir testfile testfolder)
   file(APPEND ${testfile} "for item in \${txtpbfiles[@]}\n")
   file(APPEND ${testfile} "do\n")
   file(APPEND ${testfile} "\techo \"Found \${item}\"\n")
-  file(APPEND ${testfile} "\t${PROTOC_BINARY} ${PROTOBUF_PROTOC_INCLUDES} -I${CMAKE_CURRENT_LIST_DIR}/../proto -I${P4RUNTIME_STD_DIR} -I${P4C_SOURCE_DIR} -I${P4C_SOURCE_DIR}/control-plane --encode=p4testgen_ir.TestCase p4testgen_ir.proto < \${item} > /dev/null\n")
+  file(
+    APPEND ${testfile}
+    "\t${Protobuf_PROTOC_EXECUTABLE} --proto_path ${Protobuf_INCLUDE_DIRS} --proto_path ${CMAKE_CURRENT_LIST_DIR}/../proto --proto_path ${P4RUNTIME_STD_DIR} --proto_path ${P4C_SOURCE_DIR} --proto_path ${P4C_SOURCE_DIR}/control-plane --encode=p4testgen_ir.TestCase p4testgen_ir.proto < \${item} > /dev/null\n"
+  )
   file(APPEND ${testfile} "done\n")
 endfunction(validate_protobuf_ir)
 

cmake/Abseil.cmake

@@ -20,7 +20,7 @@ macro(p4c_obtain_abseil)
     endif()
   else()
     set(P4C_ABSEIL_VERSION "20240116.1")
-    message("Fetching Abseil version ${P4C_ABSEIL_VERSION} for P4C...")
+    message(status "Fetching Abseil version ${P4C_ABSEIL_VERSION} for P4C...")
 
     # Unity builds do not work for Abseil...
     set(CMAKE_UNITY_BUILD_PREV ${CMAKE_UNITY_BUILD})
@@ -48,7 +48,7 @@ macro(p4c_obtain_abseil)
 
     # Suppress warnings for all Abseil targets.
     get_all_targets(ABSL_BUILD_TARGETS ${absl_SOURCE_DIR})
-    foreach(target in ${ABSL_BUILD_TARGETS})
+    foreach(target ${ABSL_BUILD_TARGETS})
       if(target MATCHES "absl_*")
         # Do not suppress warnings for Abseil library targets that are aliased.
         get_target_property(target_type ${target} TYPE)
@@ -63,5 +63,5 @@ macro(p4c_obtain_abseil)
     set(FETCHCONTENT_QUIET ${FETCHCONTENT_QUIET_PREV})
   endif()
 
-  message("Done with setting up Abseil for P4C.")
+  message(status "Done with setting up Abseil for P4C.")
 endmacro(p4c_obtain_abseil)

cmake/Protobuf.cmake

@@ -1,4 +1,6 @@
 macro(p4c_obtain_protobuf)
+  set(P4C_PROTOBUF_VERSION "25.3")
+  set(P4C_PROTOC_VERSION "4.25")
   option(
     P4C_USE_PREINSTALLED_PROTOBUF
     "Look for a preinstalled version of Protobuf in the system instead of installing a prebuilt binary using FetchContent."
@@ -11,26 +13,25 @@ macro(p4c_obtain_protobuf)
       set(SAVED_CMAKE_FIND_LIBRARY_SUFFIXES ${CMAKE_FIND_LIBRARY_SUFFIXES})
       set(CMAKE_FIND_LIBRARY_SUFFIXES .a)
     endif()
-    set(protobuf_MODULE_COMPATIBLE TRUE)
-    find_package(Protobuf CONFIG)
-    if(NOT Protobuf_FOUND)
-      find_package(Protobuf REQUIRED)
+    # For MacOS, we may need to look for Protobuf in additional folders.
+    if(APPLE)
+      set(P4C_PROTOBUF_PATHS PATHS /usr/local/opt/protobuf /opt/homebrew/opt/protobuf)
     endif()
-    # Protobuf sets the protoc binary to a generator expression, which are problematic. They are
-    # problematic because they are only evaluated at build time. However, we may have scripts that
-    # depend on the actual build time during the configure phase. Hard code a custom binary instead.
-    find_program(PROTOC_BINARY protoc)
+    # We do not set a minimum version here because setting a minimum version seems broken. We
+    # recommend 4.25 as minimum.
+    find_package(Protobuf REQUIRED CONFIG ${P4C_PROTOBUF_PATHS})
+
+    # Protobuf sets the protoc binary to a generator expression "$<TARGET_FILE:protoc>",
+    # but we many not be able to use this generator expression in some text-based test scripts.
+    # The reason is that protoc is only evaluated at build time, not during generation of the test
+    # scripts. TODO: Maybe we can improve these scripts somehow?
+    find_program(Protobuf_PROTOC_EXECUTABLE protoc)
 
     if(ENABLE_PROTOBUF_STATIC)
       set(CMAKE_FIND_LIBRARY_SUFFIXES ${SAVED_CMAKE_FIND_LIBRARY_SUFFIXES})
     endif()
-    # While Protobuf_LIBRARY is already defined by find_package(Protobuf) because we set protobuf_MODULE_COMPATIBLE,
-    # it only contains the path to the protobuf shared object, not libraries Protobuf depends on, such as abseil for Protobuf >= 22.
-    # See https://github.com/p4lang/p4c/issues/4316
-    set(Protobuf_LIBRARY "protobuf::libprotobuf")
   else()
-    set(P4C_PROTOBUF_VERSION "25.3")
-    message("Fetching Protobuf version ${P4C_PROTOBUF_VERSION} for P4C...")
+    message(status "Fetching Protobuf version ${P4C_PROTOBUF_VERSION} for P4C...")
 
     # Unity builds do not work for Protobuf...
     set(CMAKE_UNITY_BUILD_PREV ${CMAKE_UNITY_BUILD})
@@ -46,6 +47,7 @@ macro(p4c_obtain_protobuf)
     set(protobuf_BUILD_PROTOC_BINARIES ON CACHE BOOL "Build libprotoc and protoc compiler.")
     # Only ever build the static library. It is not safe to link with a local dynamic version.
     set(protobuf_BUILD_SHARED_LIBS OFF CACHE BOOL "Build Shared Libraries")
+    # Exclude Protobuf from the main make install step. We only want to use it locally.
     set(protobuf_INSTALL OFF CACHE BOOL "Install Protobuf")
     set(protobuf_ABSL_PROVIDER "package" CACHE STRING "Use system-provided abseil")
     set(protobuf_BUILD_EXPORT OFF)
@@ -58,28 +60,23 @@ macro(p4c_obtain_protobuf)
       USES_TERMINAL_DOWNLOAD TRUE
       GIT_PROGRESS TRUE
     )
+    fetchcontent_makeavailable(protobuf)
 
-    # Exclude Protobuf from the main make install step. We only want to use it locally.
-    fetchcontent_makeavailable_but_exclude_install(protobuf)
-
-    # Protobuf and protoc source code may trigger warnings which we need to ignore.
-    set_target_properties(libprotobuf PROPERTIES COMPILE_FLAGS "-Wno-error")
-    set_target_properties(libprotoc PROPERTIES COMPILE_FLAGS "-Wno-error")
-
-    # Add the Protobuf directory to our module path for module builds.
-    list(APPEND CMAKE_MODULE_PATH "${protobuf_BINARY_DIR}/cmake/protobuf")
+    # Protobuf and protoc source code may trigger warnings which we ignore.
+    set_target_properties(libprotobuf-lite PROPERTIES COMPILE_FLAGS "-Wno-error -w")
+    set_target_properties(libprotobuf PROPERTIES COMPILE_FLAGS "-Wno-error -w")
+    set_target_properties(libprotoc PROPERTIES COMPILE_FLAGS "-Wno-error -w")
 
     # Set some Protobuf variables manually until we are able to call FindPackage directly.
-    set(Protobuf_FOUND TRUE)
-    set(Protobuf_LIBRARY "protobuf::libprotobuf")
-    set(Protobuf_PROTOC_LIBRARY "protobuf::libprotoc")
-    set(Protobuf_PROTOC_EXECUTABLE $<TARGET_FILE:protoc>)
-    # Protobuf sets the protoc binary to a generator expression, which are problematic. They are
-    # problematic because they are only evaluated at build time. However, we may have scripts that
-    # depend on the actual build time during the configure phase. Hard code a custom binary which we
-    # can use in addition to the generator expression.
-    # TODO: Maybe we can improve these scripts somehow?
-    set(PROTOC_BINARY ${protobuf_BINARY_DIR}/protoc)
+    # This should be possible with CMake 3.24.
+    # Protobuf sets the protoc binary to a generator expression "$<TARGET_FILE:protoc>",
+    # but we many not be able to use this generator expression in some text-based test scripts.
+    # The reason is that protoc is only evaluated at build time, not during generation of the test
+    # scripts. TODO: Maybe we can improve these scripts somehow?
+    set(Protobuf_PROTOC_EXECUTABLE ${protobuf_BINARY_DIR}/protoc)
+    include(${protobuf_SOURCE_DIR}/cmake/protobuf-generate.cmake)
+    # Protobuf does not seem to set Protobuf_INCLUDE_DIRS correctly when used as a module, but we
+    # need this variable for generating code.
     list(APPEND Protobuf_INCLUDE_DIRS "${protobuf_SOURCE_DIR}/src/")
 
     # Reset temporary variable modifications.
@@ -88,13 +85,5 @@ macro(p4c_obtain_protobuf)
     set(CMAKE_POSITION_INDEPENDENT_CODE ${CMAKE_POSITION_INDEPENDENT_CODE_PREV})
   endif()
 
-  # Protobuf does not seem to set Protobuf_INCLUDE_DIR correctly, but we need this variable for
-  # generating code with protoc. Instead, we rely on Protobuf_INCLUDE_DIRS and generate a custom
-  # utility variable for includes.
-  set(PROTOBUF_PROTOC_INCLUDES)
-  foreach(dir ${Protobuf_INCLUDE_DIRS})
-    list(APPEND PROTOBUF_PROTOC_INCLUDES "-I${dir}")
-  endforeach()
-
-  message("Done with setting up Protobuf for P4C.")
+  message(status "Done with setting up Protobuf for P4C.")
 endmacro(p4c_obtain_protobuf)