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

FindUUID: Always define UUID::UUID on Apple platforms #128

Merged
merged 2 commits into from
Dec 15, 2020

Conversation

traversaro
Copy link
Contributor

On Apple platforms, the headers of libuuid and its symbols are provided by the OS SDK, so no further linking is necessary.

Fix #127

Signed-off-by: Silvio Traversaro silvio.traversaro@iit.it

@traversaro traversaro requested a review from mxgrey as a code owner November 15, 2020 22:27
@github-actions github-actions bot added 🏢 edifice Ignition Edifice 🏰 citadel Ignition Citadel 📜 blueprint Ignition Blueprint 🔮 dome Ignition Dome labels Nov 15, 2020
traversaro added a commit to traversaro/libignition-cmake0-feedstock that referenced this pull request Nov 15, 2020
@scpeters
Copy link
Member

@scpeters
Copy link
Member

this looks reasonable to me; I'd just like to test it with a larger set of packages before approving

@traversaro
Copy link
Contributor Author

this looks reasonable to me; I'd just like to test it with a larger set of packages before approving

Thanks, I don't have currently a macOS box to test, so any additional test is welcome.

@chapulina chapulina added the macOS macOS support label Nov 16, 2020
@chapulina chapulina requested a review from scpeters November 16, 2020 18:53
@traversaro
Copy link
Contributor Author

traversaro commented Nov 17, 2020

I actually was able to propagate the change in conda and test it in https://github.com/robotology/robotology-superbuild/pull/513/https://github.com/robotology/robotology-superbuild/pull/513/checks?check_run_id=1414508924, and I found another problem: the target should only be defined if no target with the same name is already defined, otherwise if you call twice find_package(UUID) as it happes when you call find_package(gazebo) thanks to dependencies, you will get the error:

2020-11-17T20:31:22.6139260Z CMake Error at /usr/local/miniconda/envs/test/share/cmake/ignition-cmake2/cmake2/FindUUID.cmake:61 (add_library):
2020-11-17T20:31:22.6140410Z   add_library cannot create imported target "UUID::UUID" because another
2020-11-17T20:31:22.6141100Z   target with the same name already exists.

I fixed the problem in the latest version of the PR.

On Apple platforms, the headers of libuuid and its symbols
are provided by the OS SDK, so no further linking is necessary.

Fix gazebosim#127

Signed-off-by: Silvio Traversaro <silvio.traversaro@iit.it>
@scpeters
Copy link
Member

sorry for the delayed review; this looks good. I suppose we can stop depending on the ossp-uuid formula in osrf/homebrew-simulation as well. I'll open a ticket for that

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📜 blueprint Ignition Blueprint 🏰 citadel Ignition Citadel 🔮 dome Ignition Dome 🏢 edifice Ignition Edifice macOS macOS support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Ignition projects on macOS mix inclusion of system's uuid/uuid.h and linking to homebrew's ossp-uuid
3 participants