Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Set protobuf_MODULE_COMPATIBLE before any find_package call #957

Merged
merged 1 commit into from
Aug 5, 2021

Conversation

traversaro
Copy link
Contributor

@traversaro traversaro commented Aug 5, 2021

🦟 Bug fix

Fixes #956

Summary

In some environments where Protobuf is compiled using CMake (so not in debian-based distributions, as there protobuf is compiled via autotools), it is necessary to set the protobuf_MODULE_COMPATIBLE variable before any call to find_package, as any call to find_package could call transitively find_package(protobuf).

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 traversaro requested a review from chapulina as a code owner August 5, 2021 14:41
@github-actions github-actions bot added the 🏢 edifice Ignition Edifice label Aug 5, 2021
Copy link
Contributor

@chapulina chapulina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, @traversaro ! I'll see if I can retarget at ign-gazebo3 so we can get the fix from Citadel onwards.

@chapulina chapulina changed the base branch from ign-gazebo5 to ign-gazebo3 August 5, 2021 17:22
@chapulina chapulina added 🏰 citadel Ignition Citadel and removed 🏢 edifice Ignition Edifice labels Aug 5, 2021
@chapulina
Copy link
Contributor

Rebased to Citadel, just waiting for CI.

I'll also cross-reference #501, which originally added protobuf_MODULE_COMPATIBLE to Edifice, and hasn't been backported because it includes other breaking changes.

@azeey azeey removed their request for review August 5, 2021 17:29
@codecov
Copy link

codecov bot commented Aug 5, 2021

Codecov Report

Merging #957 (9d2d7e9) into ign-gazebo3 (a83f56f) will not change coverage.
The diff coverage is n/a.

❗ Current head 9d2d7e9 differs from pull request most recent head 8650a5c. Consider uploading reports for the commit 8650a5c to get more accurate results
Impacted file tree graph

@@             Coverage Diff              @@
##           ign-gazebo3     #957   +/-   ##
============================================
  Coverage        77.90%   77.90%           
============================================
  Files              221      221           
  Lines            12678    12678           
============================================
  Hits              9877     9877           
  Misses            2801     2801           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a83f56f...8650a5c. Read the comment docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏰 citadel Ignition Citadel
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unknown CMake command "PROTOBUF_GENERATE_CPP".
2 participants