Use Protobuf_IMPORT_DIRS instead of PROTOBUF_IMPORT_DIRS for compatibility with Protobuf CMake config

[traversaro]

🦟 Bug fix

Fixes the compilation of Ignition Gazebo on Windows or in any case on a platform in which Protobuf installs its CMake Config files.

Summary

Ignition Gazebo uses the PROTOBUF_GENERATE_CPP CMake function to generate C++ code from the .proto files.
Depending on the system and the protobuf version installed, this function can be defined in two possible places:

• If a CMake config file for Protobuf is installed, the FindIgnProtobuf prefers it over the CMake's FindProtobuf (see https://github.com/ignitionrobotics/ign-cmake/blob/ignition-cmake2_2.6.2/cmake/FindIgnProtobuf.cmake#L29), and so the function defined in the protobuf-module.cmake is used https://github.com/protocolbuffers/protobuf/blob/v3.15.6/cmake/protobuf-module.cmake.in#L4 . This function considers in input the Protobuf_IMPORT_DIRS variable.
• If a CMake config file for Protobuf is not installed (as it happens for Ubuntu 20.04 debian packages), the the CMake's FindProtobuf is used, and the PROTOBUF_GENERATE_CPP function used is the one defined there: https://github.com/Kitware/CMake/blob/v3.16.9/Modules/FindProtobuf.cmake#L245 . This function considers in input the Protobuf_IMPORT_DIRS and PROTOBUF_IMPORT_DIRS variables.

As currently ignition-gazebo only sets the PROTOBUF_IMPORT_DIRS variable, if a Protobuf CMake config is used, the generation of messages fails with error:

2021-03-27T23:20:08.8623834Z -- Build files have been written to: D:/bld/libignition-gazebo4_1616885912413/work/build
2021-03-27T23:20:09.2594171Z [1/408] cmd.exe /C "cd /D %SRC_DIR%\build\src\msgs && %PREFIX%\Library\bin\protoc.exe --cpp_out D:/bld/libignition-gazebo4_1616885912413/work/build/src/msgs -I D:/bld/libignition-gazebo4_1616885912413/work/src/msgs D:/bld/libignition-gazebo4_1616885912413/work/src/msgs/peer_info.proto"
2021-03-27T23:20:09.2597900Z FAILED: src/msgs/peer_info.pb.h src/msgs/peer_info.pb.cc 
2021-03-27T23:20:09.2605328Z cmd.exe /C "cd /D %SRC_DIR%\build\src\msgs && %PREFIX%\Library\bin\protoc.exe --cpp_out D:/bld/libignition-gazebo4_1616885912413/work/build/src/msgs -I D:/bld/libignition-gazebo4_1616885912413/work/src/msgs D:/bld/libignition-gazebo4_1616885912413/work/src/msgs/peer_info.proto"
2021-03-27T23:20:09.2606754Z ignition/msgs/header.proto: File not found.
2021-03-27T23:20:09.2613215Z peer_info.proto:22:1: Import "ignition/msgs/header.proto" was not found or had errors.
2021-03-27T23:20:09.2614568Z peer_info.proto:28:3: "ignition.msgs.Header" is not defined.
2021-03-27T23:20:09.2616338Z [2/408] cmd.exe /C "cd /D %SRC_DIR%\build\src\msgs && %PREFIX%\Library\bin\protoc.exe --cpp_out D:/bld/libignition-gazebo4_1616885912413/work/build/src/msgs -I D:/bld/libignition-gazebo4_1616885912413/work/src/msgs D:/bld/libignition-gazebo4_1616885912413/work/src/msgs/peer_control.proto"
2021-03-27T23:20:09.2618072Z FAILED: src/msgs/peer_control.pb.h src/msgs/peer_control.pb.cc 
2021-03-27T23:20:09.2619552Z cmd.exe /C "cd /D %SRC_DIR%\build\src\msgs && %PREFIX%\Library\bin\protoc.exe --cpp_out D:/bld/libignition-gazebo4_1616885912413/work/build/src/msgs -I D:/bld/libignition-gazebo4_1616885912413/work/src/msgs D:/bld/libignition-gazebo4_1616885912413/work/src/msgs/peer_control.proto"
2021-03-27T23:20:09.2621084Z ignition/msgs/header.proto: File not found.
2021-03-27T23:20:09.2625509Z peer_control.proto:22:1: Import "ignition/msgs/header.proto" was not found or had errors.
2021-03-27T23:20:09.2627093Z peer_control.proto:30:3: "ignition.msgs.Header" is not defined.
2021-03-27T23:20:09.2628926Z [3/408] cmd.exe /C "cd /D %SRC_DIR%\build\src\msgs && %PREFIX%\Library\bin\protoc.exe --cpp_out D:/bld/libignition-gazebo4_1616885912413/work/build/src/msgs -I D:/bld/libignition-gazebo4_1616885912413/work/src/msgs D:/bld/libignition-gazebo4_1616885912413/work/src/msgs/simulation_step.proto"
2021-03-27T23:20:09.2630539Z FAILED: src/msgs/simulation_step.pb.h src/msgs/simulation_step.pb.cc 
2021-03-27T23:20:09.2638659Z cmd.exe /C "cd /D %SRC_DIR%\build\src\msgs && %PREFIX%\Library\bin\protoc.exe --cpp_out D:/bld/libignition-gazebo4_1616885912413/work/build/src/msgs -I D:/bld/libignition-gazebo4_1616885912413/work/src/msgs D:/bld/libignition-gazebo4_1616885912413/work/src/msgs/simulation_step.proto"
2021-03-27T23:20:09.2639864Z ignition/msgs/world_stats.proto: File not found.
2021-03-27T23:20:09.2640359Z ignition/msgs/entity.proto: File not found.

(error taken from conda-forge/libignition-gazebo-feedstock#3).

To avoid this errors, I switched the Ignition Gazebo to set the Protobuf_IMPORT_DIRS variable that will work fine regardless of whether a CMake config file for ProtoBuf is used or not.

However, as the use of Protobuf_IMPORT_DIRS (instead of PROTOBUF_IMPORT_DIRS) was only introduced in CMake 3.6 (see https://github.com/Kitware/CMake/blob/v3.20.0/Modules/FindProtobuf.cmake#L13), I also bumped the minimum required version to 3.10.2 (for consistency with ignition-cmake2). CMake 3.10.2 is available in apt in Ubuntu 18.04 (see https://repology.org/project/cmake/versions).

Checklist

• Signed all commits for DCO
• Added tests
• Updated documentation (as needed)
• Updated migration guide (as needed)
• codecheck passed (See contributing)
• All tests passed (See test coverage)
• While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge

[traversaro]

(I will fix the DCO soon).

[traversaro]

(I will fix the DCO soon).

Done.

[traversaro]

Added tests

I did not add tests, I am not sure if they are needed for something like that, probably a relevant coverage could be done by setting up Windows CI, but that probably calls for a separate endeavor.

[traversaro]

While waiting for a review on your PR, please help review another open pull request to support the maintainers

I reviewed gazebosim/gz-gui#202, even if it is probably an easy one. :D

[chapulina]

Thanks! The bump in CMake version shouldn't have negative effects, as pointed out in the PR description, upstream libraries already require it.