Protobuf: Do not require version 3 do support Protobuf 4.23.2 (23.2)

[traversaro]

🎉 New feature

GzProtobuf: Do not require version 3 do support Protobuf 4.23.2 (23.2)

Summary

The motivation is similar to gazebosim/gz-msgs#346 .

Test it

Install protobuf 23.2 and run the tests (I guess the only way to do it easily for now is to use conda-forge, see https://repology.org/project/protobuf/versions .

Checklist

• Signed all commits for DCO
• Added tests
• Added example and/or tutorial
• Updated documentation (as needed)
• Updated migration guide (as needed)
• Consider updating Python bindings (if the library has them)
• 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 and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

[codecov]

Codecov Report

Merging #2006 (17e2e70) into ign-gazebo6 (c0a7428) will decrease coverage by 0.03%.
The diff coverage is n/a.

❗ Current head 17e2e70 differs from pull request most recent head b5060e3. Consider uploading reports for the commit b5060e3 to get more accurate results

@@               Coverage Diff               @@
##           ign-gazebo6    #2006      +/-   ##
===============================================
- Coverage        65.35%   65.33%   -0.03%     
===============================================
  Files              327      327              
  Lines            26996    26996              
===============================================
- Hits             17644    17637       -7     
- Misses            9352     9359       +7     

see 2 files with indirect coverage changes

[traversaro]

There is another failure with Protobuf >= 22 :

/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_pla/include -fdebug-prefix-map=/home/conda/feedstock_root/build_artifacts/gz-sim7_1686297165359/work=/usr/local/src/conda/libgz-sim7-7.5.0 -fdebug-prefix-map=/home/conda/feedstock_root/build_artifacts/gz-sim7_1686297165359/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_pla=/usr/local/src/conda-prefix -O3 -DNDEBUG  -Wall -Wextra -Wno-long-long -Wno-unused-value -Wfloat-equal -Wshadow -Winit-self -Wswitch-default -Wmissing-include-dirs -pedantic  -std=c++17 -fPIC -fvisibility=default -msse -msse2 -mfpmath=sse -msse3 -mssse3 -msse4.1 -msse4.2 -fPIC -I/home/conda/feedstock_root/build_artifacts/gz-sim7_1686297165359/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_pla/include/uuid -I/home/conda/feedstock_root/build_artifacts/gz-sim7_1686297165359/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_pla/include -MD -MT src/systems/triggered_publisher/CMakeFiles/gz-sim7-triggered-publisher-system.dir/TriggeredPublisher.cc.o -MF src/systems/triggered_publisher/CMakeFiles/gz-sim7-triggered-publisher-system.dir/TriggeredPublisher.cc.o.d -o src/systems/triggered_publisher/CMakeFiles/gz-sim7-triggered-publisher-system.dir/TriggeredPublisher.cc.o -c /home/conda/feedstock_root/build_artifacts/gz-sim7_1686297165359/work/src/systems/triggered_publisher/TriggeredPublisher.cc
2023-06-09T08:15:20.9083563Z /home/conda/feedstock_root/build_artifacts/gz-sim7_1686297165359/work/src/systems/triggered_publisher/TriggeredPublisher.cc: In lambda function:
2023-06-09T08:15:20.9084226Z /home/conda/feedstock_root/build_artifacts/gz-sim7_1686297165359/work/src/systems/triggered_publisher/TriggeredPublisher.cc:838:72: error: expected unqualified-id before '&' token
2023-06-09T08:15:20.9084589Z   838 |                        } catch (const google::protobuf::FatalException &err)
2023-06-09T08:15:20.9084838Z       |                                                                        ^
2023-06-09T08:15:20.9085295Z /home/conda/feedstock_root/build_artifacts/gz-sim7_1686297165359/work/src/systems/triggered_publisher/TriggeredPublisher.cc:838:71: error: expected ')' before '&' token
2023-06-09T08:15:20.9085651Z   838 |                        } catch (const google::protobuf::FatalException &err)
2023-06-09T08:15:20.9085893Z       |                                ~                                      ^~
2023-06-09T08:15:20.9086071Z       |                                                                       )
2023-06-09T08:15:20.9086542Z /home/conda/feedstock_root/build_artifacts/gz-sim7_1686297165359/work/src/systems/triggered_publisher/TriggeredPublisher.cc:838:72: error: expected '{' before '&' token
2023-06-09T08:15:20.9087011Z   838 |                        } catch (const google::protobuf::FatalException &err)
2023-06-09T08:15:20.9087241Z       |                                                                        ^
2023-06-09T08:15:20.9087750Z /home/conda/feedstock_root/build_artifacts/gz-sim7_1686297165359/work/src/systems/triggered_publisher/TriggeredPublisher.cc:838:73: error: 'err' was not declared in this scope; did you mean 'erf'?
2023-06-09T08:15:20.9088108Z   838 |                        } catch (const google::protobuf::FatalException &err)
2023-06-09T08:15:20.9088354Z       |                                                                         ^~~
2023-06-09T08:15:20.9088549Z       |                                                                         erf
2023-06-09T08:15:21.1861360Z [446/462] Building CXX object src/systems/trajectory_follower/CMakeFiles/gz-sim7-trajectory-follower-system.dir/TrajectoryFollower.cc.o
2023-06-09T08:15:39.8958146Z [447/462] Building CXX object src/systems/user_commands/CMakeFiles/gz-sim7-user-commands-system.dir/UserCommands.cc.o
2023-06-09T08:15:39.8959116Z ninja: build stopped: subcommand failed.

FatalException was removed in Protobuf 22, I could not find the exact commit, but I guess it is a change similar to protocolbuffers/protobuf#11471 . So probably we can just avoid to catch that exception if protobuf >= 22 . Perhaps @azeey has some specific idea as he wrote that code.

[scpeters]

There is another failure with Protobuf >= 22 :

/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_pla/include -fdebug-prefix-map=/home/conda/feedstock_root/build_artifacts/gz-sim7_1686297165359/work=/usr/local/src/conda/libgz-sim7-7.5.0 -fdebug-prefix-map=/home/conda/feedstock_root/build_artifacts/gz-sim7_1686297165359/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_pla=/usr/local/src/conda-prefix -O3 -DNDEBUG  -Wall -Wextra -Wno-long-long -Wno-unused-value -Wfloat-equal -Wshadow -Winit-self -Wswitch-default -Wmissing-include-dirs -pedantic  -std=c++17 -fPIC -fvisibility=default -msse -msse2 -mfpmath=sse -msse3 -mssse3 -msse4.1 -msse4.2 -fPIC -I/home/conda/feedstock_root/build_artifacts/gz-sim7_1686297165359/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_pla/include/uuid -I/home/conda/feedstock_root/build_artifacts/gz-sim7_1686297165359/_h_env_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_placehold_pla/include -MD -MT src/systems/triggered_publisher/CMakeFiles/gz-sim7-triggered-publisher-system.dir/TriggeredPublisher.cc.o -MF src/systems/triggered_publisher/CMakeFiles/gz-sim7-triggered-publisher-system.dir/TriggeredPublisher.cc.o.d -o src/systems/triggered_publisher/CMakeFiles/gz-sim7-triggered-publisher-system.dir/TriggeredPublisher.cc.o -c /home/conda/feedstock_root/build_artifacts/gz-sim7_1686297165359/work/src/systems/triggered_publisher/TriggeredPublisher.cc
2023-06-09T08:15:20.9083563Z /home/conda/feedstock_root/build_artifacts/gz-sim7_1686297165359/work/src/systems/triggered_publisher/TriggeredPublisher.cc: In lambda function:
2023-06-09T08:15:20.9084226Z /home/conda/feedstock_root/build_artifacts/gz-sim7_1686297165359/work/src/systems/triggered_publisher/TriggeredPublisher.cc:838:72: error: expected unqualified-id before '&' token
2023-06-09T08:15:20.9084589Z   838 |                        } catch (const google::protobuf::FatalException &err)
2023-06-09T08:15:20.9084838Z       |                                                                        ^
2023-06-09T08:15:20.9085295Z /home/conda/feedstock_root/build_artifacts/gz-sim7_1686297165359/work/src/systems/triggered_publisher/TriggeredPublisher.cc:838:71: error: expected ')' before '&' token
2023-06-09T08:15:20.9085651Z   838 |                        } catch (const google::protobuf::FatalException &err)
2023-06-09T08:15:20.9085893Z       |                                ~                                      ^~
2023-06-09T08:15:20.9086071Z       |                                                                       )
2023-06-09T08:15:20.9086542Z /home/conda/feedstock_root/build_artifacts/gz-sim7_1686297165359/work/src/systems/triggered_publisher/TriggeredPublisher.cc:838:72: error: expected '{' before '&' token
2023-06-09T08:15:20.9087011Z   838 |                        } catch (const google::protobuf::FatalException &err)
2023-06-09T08:15:20.9087241Z       |                                                                        ^
2023-06-09T08:15:20.9087750Z /home/conda/feedstock_root/build_artifacts/gz-sim7_1686297165359/work/src/systems/triggered_publisher/TriggeredPublisher.cc:838:73: error: 'err' was not declared in this scope; did you mean 'erf'?
2023-06-09T08:15:20.9088108Z   838 |                        } catch (const google::protobuf::FatalException &err)
2023-06-09T08:15:20.9088354Z       |                                                                         ^~~
2023-06-09T08:15:20.9088549Z       |                                                                         erf
2023-06-09T08:15:21.1861360Z [446/462] Building CXX object src/systems/trajectory_follower/CMakeFiles/gz-sim7-trajectory-follower-system.dir/TrajectoryFollower.cc.o
2023-06-09T08:15:39.8958146Z [447/462] Building CXX object src/systems/user_commands/CMakeFiles/gz-sim7-user-commands-system.dir/UserCommands.cc.o
2023-06-09T08:15:39.8959116Z ninja: build stopped: subcommand failed.

FatalException was removed in Protobuf 22, I could not find the exact commit, but I guess it is a change similar to protocolbuffers/protobuf#11471 . So probably we can just avoid to catch that exception if protobuf >= 22 . Perhaps @azeey has some specific idea as he wrote that code.

I believe it was removed in protocolbuffers/protobuf@a9f1ea6#diff-1978da6171b419e33e933316518e3d91f0f01526777d27ec8ef9c00451e761e3, and I think we can just remove that catch statement to fix the build

[scpeters]

I believe it was removed in protocolbuffers/protobuf@a9f1ea6#diff-1978da6171b419e33e933316518e3d91f0f01526777d27ec8ef9c00451e761e3, and I think we can just remove that catch statement to fix the build

I'm attempting this in b5060e3

cc @azeey for your thoughts

[azeey]

I believe it was removed in protocolbuffers/protobuf@a9f1ea6#diff-1978da6171b419e33e933316518e3d91f0f01526777d27ec8ef9c00451e761e3, and I think we can just remove that catch statement to fix the build

I'm attempting this in b5060e3

cc @azeey for your thoughts

I think the conditions that cause protobuf to have fatal errors are checked in

gz-sim/src/systems/triggered_publisher/TriggeredPublisher.cc

Line 225 in 3a36842

const auto *matcherDesc = _matcher.GetDescriptor();

before calling MessageDifferencer::Compare and MessageDifferencer::CompareWithFields, so your fix should be safe.

[traversaro]

Thanks a lot @scpeters and @azeey !