Aggressively clean up the Protobuf CMake dependency.

[fruffy]

Now that we are mandating Protobuf 3.25 try to simplify the use of extra variables and workarounds for our Protobuf dependency.

• Use protobuf_generate instead our own custom command. This way we can rely on the canonical definitions instead of having to maintain our own generators.
• Do not use ${Protobuf_LIBRARY}, instead use the aliased versions protobuf::
• Do not use PROTOBUF_PROTOC_INCLUDES, instead use Protobuf_INCLUDE_DIRS only.
• Also clean up some unnecessary include directories in the top-level CMakeLists. Try to simplify things.

With CMake 3.24 we may also be able to override find_package with FetchContent: https://cmake.org/cmake/help/latest/module/FetchContent.html

Fixes #4477.

[fruffy]

@jkhsjdhjs We are trying to refresh our Protobuf dependency and simplify it but I know you had some trouble with setting the right variables in Arch Protobuf. If we mandate a recent version do these build settings work for you? Or do we still need to keep the compatibility variables?

[jkhsjdhjs]

@fruffy Yep, it doesn't build for me with the changes of this PR:

CMake Error at cmake/Protobuf.cmake:14 (find_package):
  Could not find a configuration file for package "Protobuf" that is
  compatible with requested version "25.3".

  The following configuration files were considered but not accepted:

    /usr/lib/cmake/protobuf/protobuf-config.cmake, version: 25.3.0
    /lib/cmake/protobuf/protobuf-config.cmake, version: 25.3.0

Call Stack (most recent call first):
  CMakeLists.txt:185 (p4c_obtain_protobuf)

It works if I remove the 25.3 from the find_package() call.

However, I then get these errors:

CMake Error at control-plane/CMakeLists.txt:53 (add_custom_command):
  Error evaluating generator expression:

    $<TARGET_FILE:protoc>

  No target "protoc"


CMake Error at backends/dpdk/CMakeLists.txt:29 (add_custom_command):
  Error evaluating generator expression:

    $<TARGET_FILE:protoc>

  No target "protoc"


CMake Error at control-plane/CMakeLists.txt:53 (add_custom_command):
  Error evaluating generator expression:

    $<TARGET_FILE:protoc>

  No target "protoc"


CMake Error at backends/dpdk/CMakeLists.txt:29 (add_custom_command):
  Error evaluating generator expression:

    $<TARGET_FILE:protoc>

  No target "protoc"

[fruffy]

Thanks for checking this! Maybe using 25.3.0 as the required version works? Or just not using CONFIG. There is some reason why that particular build is considered but not accepted...

[jkhsjdhjs]

Not using CONFIG doesn't work due to cmake's find_protobuf still not being compatible with recent protobuf versions:

CMake Error at /usr/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:230 (message):
  Could NOT find Protobuf: Found unsuitable version "4.25.3", but required is
  at least "25.3" (found /usr/lib/libprotobuf.so)
Call Stack (most recent call first):
  /usr/share/cmake/Modules/FindPackageHandleStandardArgs.cmake:598 (_FPHSA_FAILURE_MESSAGE)
  /usr/share/cmake/Modules/FindProtobuf.cmake:749 (FIND_PACKAGE_HANDLE_STANDARD_ARGS)
  cmake/Protobuf.cmake:14 (find_package)
  CMakeLists.txt:185 (p4c_obtain_protobuf)

Using 25.3.0 with CONFIG works, but I suggest not requiring a specific version here, as with P4C_USE_PREINSTALLED_PROTOBUF you'll want to use whatever version is available. Requiring a minimum version doesn't seem to work, maybe due to https://cmake.org/cmake/help/latest/command/find_package.html#config-mode-version-selection

[fruffy]

@jkhsjdhjs I tested this locally and on MacOS, and I believe things should now work. Took me a lot of trial and error.

[jkhsjdhjs]

Took me a lot of trial and error.

Yes, I can see that :D

I just tested a build with the changes on latest Arch Linux using system protobuf and abseil and it works fine. LGTM 👍

[fruffy]

Took me a lot of trial and error.

Yes, I can see that :D

I just tested a build with the changes on latest Arch Linux using system protobuf and abseil and it works fine. LGTM 👍

Great! Yeah, a lot of this was me banging my head against the wall trying to figure out why the MacOS build can not find Protobuf. Only to realize that it requires path hints.

[vlstill]

Just two things I've noticed. (I got little lost in all the changes so I did not fully review it, but it looks reasonable overall).

[vlstill]

I am no familiar with protobuf and how it should be used in CMake. Nor am I familiar with the P4C control plane. But I don't see any problems with the code.

[fruffy]

I am no familiar with protobuf and how it should be used in CMake. Nor am I familiar with the P4C control plane. But I don't see any problems with the code.

fwiw I have been testing this PR in downstream dependencies for months now.