Fix build with system Protobuf (p4lang#4321)

* Fix build with system Protobuf

This sets the flag `protobuf_MODULE_COMPATIBLE` in the Protobuf cmake
file, so the Protobuf files expose some variables required by the build,
such as `Protobuf_PROTOC_EXECUTABLE`.

Furthermore, `Protobuf_LIBRARY` is defined manually as
`protobuf::libprotobuf`, because it is only holds the path to the
Protobuf library, not the abseil library Protobuf depends on since
version 22.

Fix p4lang#4316

* Use `Protobuf_INCLUDE_DIRS` with protoc

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.

---------

Co-authored-by: Fabian Ruffy <[email protected]>

backends/dpdk/CMakeLists.txt

@@ -41,7 +41,7 @@ add_dependencies(dpdk_runtime_dir controlplane)
 
 add_library(dpdk_runtime STATIC ${DPDK_P4RUNTIME_INFO_GEN_SRCS})
 target_include_directories(dpdk_runtime
-  SYSTEM BEFORE PUBLIC ${Protobuf_INCLUDE_DIR}
+  SYSTEM BEFORE PUBLIC ${Protobuf_INCLUDE_DIRS}
 )
 
 # Silence various warnings as the root issue is out of our control, example https://github.com/protocolbuffers/protobuf/issues/7140

backends/p4tools/modules/testgen/targets/bmv2/CMakeLists.txt

@@ -58,6 +58,6 @@ set(
 
 set(
   TESTGEN_INCLUDES ${TESTGEN_INCLUDES}
-  SYSTEM BEFORE PUBLIC ${Protobuf_INCLUDE_DIR}
+  SYSTEM BEFORE PUBLIC ${Protobuf_INCLUDE_DIRS}
   PARENT_SCOPE
 )

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

@@ -46,7 +46,7 @@ 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} -I${Protobuf_INCLUDE_DIR} -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${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} "done\n")
 endfunction(validate_protobuf)
 
@@ -60,7 +60,7 @@ 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} -I${Protobuf_INCLUDE_DIR} -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${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} "done\n")
 endfunction(validate_protobuf_ir)
 

cmake/Protobuf.cmake

@@ -11,19 +11,23 @@ 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)
     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.
+    # 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)
 
     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()
     # Google introduced another breaking change with protobuf 22.x by adding abseil as a new
     # dependency. https://protobuf.dev/news/2022-08-03/#abseil-dep We do not want abseil, so we stay
@@ -71,19 +75,27 @@ macro(p4c_obtain_protobuf)
     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.
+    # 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)
-    set(Protobuf_INCLUDE_DIR "${protobuf_SOURCE_DIR}/src/")
+    list(APPEND Protobuf_INCLUDE_DIRS "${protobuf_SOURCE_DIR}/src/")
 
     # Reset temporary variable modifications.
     set(CMAKE_UNITY_BUILD ${CMAKE_UNITY_BUILD_PREV})
     set(FETCHCONTENT_QUIET ${FETCHCONTENT_QUIET_PREV})
     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.")
 endmacro(p4c_obtain_protobuf)

control-plane/CMakeLists.txt

@@ -52,7 +52,7 @@ add_custom_target (mkP4configdir
   ${CMAKE_COMMAND} -E make_directory ${CMAKE_BINARY_DIR}/p4/config)
 add_custom_command(OUTPUT ${P4RUNTIME_GEN_SRCS} ${P4RUNTIME_GEN_HDRS}
   COMMAND ${Protobuf_PROTOC_EXECUTABLE}
-          -I${P4RUNTIME_STD_DIR} -I${CMAKE_CURRENT_SOURCE_DIR} -I${Protobuf_INCLUDE_DIR}
+          -I${P4RUNTIME_STD_DIR} -I${CMAKE_CURRENT_SOURCE_DIR} ${PROTOBUF_PROTOC_INCLUDES}
           --cpp_out ${CMAKE_CURRENT_BINARY_DIR}
           --python_out ${CMAKE_CURRENT_BINARY_DIR}
           --experimental_allow_proto3_optional
@@ -94,7 +94,7 @@ set (CONTROLPLANE_HDRS
 
 add_library(controlplane-gen OBJECT ${P4RUNTIME_GEN_SRCS})
 target_include_directories(controlplane-gen
-  SYSTEM BEFORE PUBLIC ${Protobuf_INCLUDE_DIR}
+  SYSTEM BEFORE PUBLIC ${Protobuf_INCLUDE_DIRS}
 )
 target_link_libraries (controlplane-gen PUBLIC ${Protobuf_LIBRARY})
 # Needed for the correct import of google/status.pb.cc