Protobuf version requirement not set and package finding is broken

[ingomueller-net]

CMakeLists.txt does currently not require a particular version in its calls to find_package(Protobuf), so if some version of that library is installed in the system, CMake configures happily but then compilation fails if an incompatible version is found. Those calls should, thus, ensure that only compatible versions are accepted.

One problem is that FindProtobuf.cmake is broken (see this issue I just filed). If no suitable version is found, find_package will report so but still create targets, such that the fall back of downloading a compatible version fails because the targets it wants to create already exist. The only way I see to fix this is force CONFIG mode, which doesn't have that problem but, according to the comment, requires CMake version 3.27. (If we, instead, wait for FindProtobuf.cmake to be fixed, we'd get that at the earliest in CMake version 3.31, so that would't make things better.)

Finally, we should decide which version of Protobuf we require, store that once in some global variable, and use that everywhere. I know for sure that version 3.21 is not enough because, since #126, we use ErrorCollector::RecordError, which only exists since version 3.22. I am not sure, though, if that version fully works.

[ingomueller-net]

@mortbopet: you may be interested in that.

[mortbopet]

Definitely a concern. As of now, my project is relying on building protobuf as part of the project, in large part due to the reasons you outline... If one uses the substrait-cpp-provided protobuf.cmake, there's technically a fixed version, since it's pointing to a tag.

This is obviously insufficient, since the top-level CMakeLists by default attempts to find_package(Protobuf) first, and falls back to building it internally.

[ingomueller-net]

Yeah, I see, if no Protobuf installation is found, then the version from FetchContent is used. One possible solution is to invert the order: use FetchContent by default and only fall back to find_package if a command line option like SUBSTRAIT_CPP_USE_SYSTEM_PROTOBUF is enabled.